From 8a05c0cee88679516b7ba2114283ba52c9e9af67 Mon Sep 17 00:00:00 2001
From: Ilya Etingof <etingof@gmail.com>
Date: Wed, 27 Sep 2017 15:42:13 +0200
Subject: [PATCH] Report /dev/disk/by-path on inspection followup

This is the followup patch for
commit d0a53149f82a3587515a4371f0f4cad8570dc715) fixing
issues with the unit tests not addressed initially.

Change-Id: I7889bf908bcb64b79bf303c6ae356fd3f4e94a83
---
 ironic_python_agent/hardware.py               |  6 ++---
 .../tests/unit/test_hardware.py               | 27 +++++++++----------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 377d9d70e..f050f0654 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -130,9 +130,9 @@ def list_all_block_devices(block_type='disk'):
             by_path_mapping[devname] = path
 
     except OSError as e:
-        LOG.warning("Path %(path)s is inaccessible, skipping by-path "
-                    "block devices reporting "
-                    "Error: %(error)s", {'path': disk_by_path_dir, 'error': e})
+        LOG.warning("Path %(path)s is inaccessible, /dev/disk/by-path/* "
+                    "version of block device name is unavailable "
+                    "Cause: %(error)s", {'path': disk_by_path_dir, 'error': e})
 
     columns = ['KNAME', 'MODEL', 'SIZE', 'ROTA', 'TYPE']
     report = utils.execute('lsblk', '-Pbdi', '-o{}'.format(','.join(columns)),
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index c399024fe..0aa547b33 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -646,7 +646,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_get_os_install_device(self, mocked_execute, mock_cached_node,
                                    mocked_listdir, mocked_readlink):
-        mocked_readlink.return_value = '/dev/sda'
+        mocked_readlink.return_value = '../../sda'
         mocked_listdir.return_value = ['1:0:0:0']
         mock_cached_node.return_value = None
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
@@ -664,7 +664,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                          mock_cached_node,
                                          mocked_listdir, mocked_readlink):
         """Fail to find device >=4GB w/o root device hints"""
-        mocked_readlink.return_value = '/dev/sda'
+        mocked_readlink.return_value = '../../sda'
         mocked_listdir.return_value = ['1:0:0:0']
         mock_cached_node.return_value = None
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE_SMALL, '')
@@ -747,9 +747,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             self._get_os_install_device_root_device_hints(
                 {'rotational': value}, '/dev/sdb')
 
-    # TODO(etingof): enable this test once the patch below is merged
-    # https://review.openstack.org/#/c/500524/
-    def skip_test_get_os_install_device_root_device_hints_by_path(self):
+    def test_get_os_install_device_root_device_hints_by_path(self):
         self._get_os_install_device_root_device_hints(
             {'by_path': '/dev/disk/by-path/1:0:0:0'}, '/dev/sdb')
 
@@ -937,10 +935,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                    mocked_dev_vendor, mock_listdir,
                                    mock_readlink):
         by_path_map = {
-            '/dev/disk/by-path/1:0:0:0': '/dev/sda',
-            '/dev/disk/by-path/1:0:0:1': '/dev/sdb',
-            '/dev/disk/by-path/1:0:0:2': '/dev/sdc',
-            '/dev/disk/by-path/1:0:0:3': '/dev/sdd',
+            '/dev/disk/by-path/1:0:0:0': '../../dev/sda',
+            '/dev/disk/by-path/1:0:0:1': '../../dev/sdb',
+            '/dev/disk/by-path/1:0:0:2': '../../dev/sdc',
+            # pretend that the by-path link to ../../dev/sdd is missing
         }
         mock_readlink.side_effect = lambda x, m=by_path_map: m[x]
         mock_listdir.return_value = [os.path.basename(x)
@@ -976,8 +974,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                  size=1765517033472,
                                  rotational=False,
                                  vendor='Super Vendor',
-                                 hctl='1:0:0:0',
-                                 by_path='/dev/disk/by-path/1:0:0:3'),
+                                 hctl='1:0:0:0'),
         ]
 
         self.assertEqual(4, len(devices))
@@ -992,7 +989,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mock_listdir.assert_has_calls(expected_calls)
 
         expected_calls = [mock.call('/dev/disk/by-path/1:0:0:%d' % dev)
-                          for dev in range(4)]
+                          for dev in range(3)]
         mock_readlink.assert_has_calls(expected_calls)
 
     @mock.patch.object(os, 'readlink', autospec=True)
@@ -1004,7 +1001,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                            mocked_dev_vendor, mocked_listdir,
                                            mocked_readlink):
         # test compatibility with pyudev < 0.18
-        mocked_readlink.return_value = '/dev/sda'
+        mocked_readlink.return_value = '../../sda'
         mocked_listdir.return_value = ['1:0:0:0']
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
         mocked_udev.side_effect = OSError()
@@ -1039,7 +1036,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     def test_list_all_block_device_with_udev(self, mocked_execute, mocked_udev,
                                              mocked_dev_vendor, mocked_listdir,
                                              mocked_readlink):
-        mocked_readlink.return_value = '/dev/sda'
+        mocked_readlink.return_value = '../../sda'
         mocked_listdir.return_value = ['1:0:0:0']
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
         mocked_udev.side_effect = iter([
@@ -1817,7 +1814,7 @@ class TestModuleFunctions(base.IronicAgentTest):
     def test_list_all_block_devices_success(self, mocked_fromdevfile,
                                             mocked_udev, mocked_readlink,
                                             mocked_execute):
-        mocked_readlink.return_value = '/dev/sda'
+        mocked_readlink.return_value = '../../sda'
         mocked_fromdevfile.return_value = {}
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE_SMALL, '')
         result = hardware.list_all_block_devices()