Enable skipping disks for cleaning

Introduce a field skip_block_devices in properties - this is a list of dictionaries
Create a helper function list_block_devices_check_skip_list
Update tests of erase_devices_express to use node when calling _list_erasable_devices
Add tests covering various options of the skip list definition
Use the helper function in get_os_install_device when node is cached

Story: 2009914

Change-Id: I3bdad3cca8acb3e0a69ebb218216e8c8419e9d65

doc/source/admin/hardware_managers.rst

@@ -110,6 +110,14 @@ Cleaning safeguards
 The stock hardware manager contains a number of safeguards to prevent
 unsafe conditions from occuring.
 
+Devices Skip List
+~~~~~~~~~~~~~~~~~
+
+A list of devices that Ironic does not touch during the cleaning process
+can be specified in the node properties field under
+``skip_block_devices``. This should be a list of dictionaries
+containing hints to identify the drives.
+
 Shared Disk Cluster Filesystems
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

ironic_python_agent/hardware.py

@@ -862,6 +862,18 @@ class HardwareManager(object, metaclass=abc.ABCMeta):
         """
         raise errors.IncompatibleHardwareMethodError
 
+    def list_block_devices_check_skip_list(self, node,
+                                           include_partitions=False):
+        """List physical block devices without the ones listed in
+
+        properties/skip_block_devices list
+
+        :param node: A node used to check the skip list
+        :param include_partitions: If to include partitions
+        :return: A list of BlockDevices
+        """
+        raise errors.IncompatibleHardwareMethodError
+
     def get_memory(self):
         raise errors.IncompatibleHardwareMethodError
 
@@ -931,7 +943,7 @@ class HardwareManager(object, metaclass=abc.ABCMeta):
         :return: a dictionary in the form {device.name: erasure output}
         """
         erase_results = {}
-        block_devices = self.list_block_devices()
+        block_devices = self.list_block_devices_check_skip_list(node)
         if not len(block_devices):
             return {}
 
@@ -1378,6 +1390,36 @@ class GenericHardwareManager(HardwareManager):
             )
         return block_devices
 
+    def list_block_devices_check_skip_list(self, node,
+                                           include_partitions=False):
+        block_devices = self.list_block_devices(
+            include_partitions=include_partitions)
+        properties = node.get('properties', {})
+        skip_list_hints = properties.get("skip_block_devices", [])
+        if skip_list_hints is not None:
+            skip_list = None
+            serialized_devs = [dev.serialize() for dev in block_devices]
+            for hint in skip_list_hints:
+                found_devs = il_utils.find_devices_by_hints(serialized_devs,
+                                                            hint)
+                excluded_devs = {dev['name'] for dev in found_devs}
+                skipped_devices = None
+                if skip_list is None:
+                    skip_list = excluded_devs
+                    skipped_devices = excluded_devs
+                else:
+                    skipped_devices = excluded_devs.difference(skip_list)
+                    skip_list = skip_list.union(excluded_devs)
+                if skipped_devices is not None and len(skipped_devices) > 0:
+                    for d in skipped_devices:
+                        LOG.warning("Skipping device %(device)s "
+                                    "using hint %(hint)s",
+                                    {'device': d, 'hint': hint})
+            if skip_list is not None:
+                block_devices = [d for d in block_devices
+                                 if d.name not in skip_list]
+        return block_devices
+
     def get_os_install_device(self, permit_refresh=False):
         cached_node = get_cached_node()
         root_device_hints = None
@@ -1392,8 +1434,10 @@ class GenericHardwareManager(HardwareManager):
                     or cached_node['properties'].get('root_device'))
             LOG.debug('Looking for a device matching root hints %s',
                       root_device_hints)
-
+            block_devices = self.list_block_devices_check_skip_list(
-        block_devices = self.list_block_devices()
+                cached_node)
+        else:
+            block_devices = self.list_block_devices()
         if not root_device_hints:
             dev_name = utils.guess_root_disk(block_devices).name
         else:
@@ -1515,8 +1559,9 @@ class GenericHardwareManager(HardwareManager):
         LOG.error(msg)
         raise errors.IncompatibleHardwareMethodError(msg)
 
-    def _list_erasable_devices(self):
+    def _list_erasable_devices(self, node):
-        block_devices = self.list_block_devices(include_partitions=True)
+        block_devices = self.list_block_devices_check_skip_list(
+            node, include_partitions=True)
         # NOTE(coreywright): Reverse sort by device name so a partition (eg
         # sda1) is processed before it disappears when its associated disk (eg
         # sda) has its partition table erased and the kernel notified.
@@ -1552,7 +1597,7 @@ class GenericHardwareManager(HardwareManager):
                  of an environmental misconfiguration.
         """
         erase_errors = {}
-        for dev in self._list_erasable_devices():
+        for dev in self._list_erasable_devices(node):
             safety_check_block_device(node, dev.name)
             try:
                 disk_utils.destroy_disk_metadata(dev.name, node['uuid'])
@@ -1587,7 +1632,7 @@ class GenericHardwareManager(HardwareManager):
         if not self._list_erasable_devices:
             LOG.debug("No erasable devices have been found.")
             return
-        for dev in self._list_erasable_devices():
+        for dev in self._list_erasable_devices(node):
             safety_check_block_device(node, dev.name)
             try:
                 if self._is_nvme(dev):

ironic_python_agent/tests/unit/test_hardware.py

@@ -1232,6 +1232,103 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual(1, mock_cached_node.call_count)
         mock_dev.assert_called_once_with()
 
+    @mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
+    @mock.patch.object(hardware, 'get_cached_node', autospec=True)
+    def test_get_os_install_device_skip_list_non_exist(
+            self, mock_cached_node, mock_dev):
+        mock_cached_node.return_value = {
+            'instance_info': {},
+            'properties': {
+                'skip_block_devices': [
+                    {'vendor': 'vendor that does not exist'}
+                ]
+            },
+            'uuid': 'node1'
+        }
+        mock_dev.return_value = [
+            hardware.BlockDevice(name='/dev/sda',
+                                 model='TinyUSB Drive',
+                                 size=3116853504,
+                                 rotational=False,
+                                 vendor='vendor0',
+                                 wwn='wwn0',
+                                 serial='serial0'),
+            hardware.BlockDevice(name='/dev/sdb',
+                                 model='Another Model',
+                                 size=10737418240,
+                                 rotational=False,
+                                 vendor='vendor1',
+                                 wwn='fake-wwn',
+                                 serial='fake-serial'),
+        ]
+        self.assertEqual('/dev/sdb',
+                         self.hardware.get_os_install_device())
+        mock_cached_node.assert_called_once_with()
+        mock_dev.assert_called_once_with()
+
+    @mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
+    @mock.patch.object(hardware, 'get_cached_node', autospec=True)
+    def test_get_os_install_device_complete_skip_list(
+            self, mock_cached_node, mock_dev):
+        mock_cached_node.return_value = {
+            'instance_info': {},
+            'properties': {
+                'skip_block_devices': [{'vendor': 'basic vendor'}]
+            }
+        }
+        mock_dev.return_value = [
+            hardware.BlockDevice(name='/dev/sda',
+                                 model='TinyUSB Drive',
+                                 size=3116853504,
+                                 rotational=False,
+                                 vendor='basic vendor',
+                                 wwn='wwn0',
+                                 serial='serial0'),
+            hardware.BlockDevice(name='/dev/sdb',
+                                 model='Another Model',
+                                 size=10737418240,
+                                 rotational=False,
+                                 vendor='basic vendor',
+                                 wwn='fake-wwn',
+                                 serial='fake-serial'),
+        ]
+        self.assertRaises(errors.DeviceNotFound,
+                          self.hardware.get_os_install_device)
+        mock_cached_node.assert_called_once_with()
+        mock_dev.assert_called_once_with()
+
+    @mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
+    @mock.patch.object(hardware, 'get_cached_node', autospec=True)
+    def test_get_os_install_device_root_device_hints_skip_list(
+            self, mock_cached_node, mock_dev):
+        mock_cached_node.return_value = {
+            'instance_info': {},
+            'properties': {
+                'root_device': {'wwn': 'fake-wwn'},
+                'skip_block_devices': [{'vendor': 'fake-vendor'}]
+            }
+        }
+        mock_dev.return_value = [
+            hardware.BlockDevice(name='/dev/sda',
+                                 model='TinyUSB Drive',
+                                 size=3116853504,
+                                 rotational=False,
+                                 vendor='Super Vendor',
+                                 wwn='wwn0',
+                                 serial='serial0'),
+            hardware.BlockDevice(name='/dev/sdb',
+                                 model='Another Model',
+                                 size=10737418240,
+                                 rotational=False,
+                                 vendor='fake-vendor',
+                                 wwn='fake-wwn',
+                                 serial='fake-serial'),
+        ]
+        self.assertRaises(errors.DeviceNotFound,
+                          self.hardware.get_os_install_device)
+        mock_cached_node.assert_called_once_with()
+        mock_dev.assert_called_once_with()
+
     def test__get_device_info(self):
         fileobj = mock.mock_open(read_data='fake-vendor')
         with mock.patch(
@@ -1445,6 +1542,104 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                                  ignore_raid=True)],
                          list_mock.call_args_list)
 
+    @mock.patch.object(hardware.GenericHardwareManager,
+                       'list_block_devices', autospec=True)
+    def test_list_block_devices_check_skip_list_with_skip_list(self,
+                                                               mock_list_devs):
+        mock_list_devs.return_value = [
+            hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
+            hardware.BlockDevice('/dev/hdaa', 'small', 65535, False),
+        ]
+        device = hardware.BlockDevice('/dev/hdaa', 'small', 65535, False)
+        node = self.node
+
+        node['properties'] = {
+            'skip_block_devices': [{
+                'name': '/dev/sdj'
+            }]
+        }
+
+        returned_devices = self.hardware.list_block_devices_check_skip_list(
+            node)
+
+        self.assertEqual([device], returned_devices)
+
+        mock_list_devs.assert_called_once_with(self.hardware,
+                                               include_partitions=False)
+
+    @mock.patch.object(hardware.GenericHardwareManager,
+                       'list_block_devices', autospec=True)
+    def test_list_block_devices_check_skip_list_no_skip_list(self,
+                                                             mock_list_devs):
+        devices = [
+            hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
+            hardware.BlockDevice('/dev/hdaa', 'small', 65535, False),
+        ]
+
+        mock_list_devs.return_value = devices
+
+        returned_devices = self.hardware.list_block_devices_check_skip_list(
+            self.node)
+
+        self.assertEqual(devices, returned_devices)
+
+        mock_list_devs.assert_called_once_with(self.hardware,
+                                               include_partitions=False)
+
+    @mock.patch.object(hardware.GenericHardwareManager,
+                       'list_block_devices', autospec=True)
+    def test_list_block_devices_check_skip_list_with_skip_list_non_exist(
+            self, mock_list_devs):
+        devices = [
+            hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
+            hardware.BlockDevice('/dev/hdaa', 'small', 65535, False),
+        ]
+
+        node = self.node
+
+        node['properties'] = {
+            'skip_block_devices': [{
+                'name': '/this/device/does/not/exist'
+            }]
+        }
+
+        mock_list_devs.return_value = devices
+
+        returned_devices = self.hardware.list_block_devices_check_skip_list(
+            self.node)
+
+        self.assertEqual(devices, returned_devices)
+
+        mock_list_devs.assert_called_once_with(self.hardware,
+                                               include_partitions=False)
+
+    @mock.patch.object(hardware.GenericHardwareManager,
+                       'list_block_devices', autospec=True)
+    def test_list_block_devices_check_skip_list_with_complete_skip_list(
+            self, mock_list_devs):
+        mock_list_devs.return_value = [
+            hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
+            hardware.BlockDevice('/dev/hdaa', 'small', 65535, False),
+        ]
+
+        node = self.node
+
+        node['properties'] = {
+            'skip_block_devices': [{
+                'name': '/dev/sdj'
+            }, {
+                'name': '/dev/hdaa'
+            }]
+        }
+
+        returned_devices = self.hardware.list_block_devices_check_skip_list(
+            self.node)
+
+        self.assertEqual([], returned_devices)
+
+        mock_list_devs.assert_called_once_with(self.hardware,
+                                               include_partitions=False)
+
     @mock.patch.object(hardware, 'get_multipath_status', lambda *_: True)
     @mock.patch.object(os, 'readlink', autospec=True)
     @mock.patch.object(os, 'listdir', autospec=True)
@@ -2511,7 +2706,8 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual([mock.call('/dev/sda', self.node['uuid']),
                          mock.call('/dev/md0', self.node['uuid'])],
                          mock_destroy_disk_metadata.call_args_list)
-        mock_list_erasable_devices.assert_called_with(self.hardware)
+        mock_list_erasable_devices.assert_called_with(self.hardware,
+                                                      self.node)
         mock_safety_check.assert_has_calls([
             mock.call(self.node, '/dev/sda'),
             mock.call(self.node, '/dev/md0'),
@@ -2544,7 +2740,8 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                           self.hardware.erase_devices_express, self.node, [])
         mock_nvme_erase.assert_not_called()
         mock_destroy_disk_metadata.assert_not_called()
-        mock_list_erasable_devices.assert_called_with(self.hardware)
+        mock_list_erasable_devices.assert_called_with(self.hardware,
+                                                      self.node)
         mock_safety_check.assert_has_calls([
             mock.call(self.node, '/dev/sda')
         ])

releasenotes/notes/enable-skipping-disks-0c4c8b72231715a1.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Users can specify a list of devices to be skipped during the cleaning
+    process or be chosen as the root device in property field
+    ``skip_block_devices``.