From 0212337bd564763eecb8f207ca31820497e27c26 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Mon, 25 Jul 2022 08:11:26 +0000 Subject: [PATCH] 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 | 8 + ironic_python_agent/hardware.py | 59 ++++- .../tests/unit/test_hardware.py | 201 +++++++++++++++++- ...nable-skipping-disks-0c4c8b72231715a1.yaml | 6 + 4 files changed, 265 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/enable-skipping-disks-0c4c8b72231715a1.yaml diff --git a/doc/source/admin/hardware_managers.rst b/doc/source/admin/hardware_managers.rst index e51093f0b..ddc7dacf3 100644 --- a/doc/source/admin/hardware_managers.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index b3f035dc2..291b9b44a 100644 --- a/ironic_python_agent/hardware.py +++ b/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() + block_devices = self.list_block_devices_check_skip_list( + 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): - block_devices = self.list_block_devices(include_partitions=True) + def _list_erasable_devices(self, node): + 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): diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 213490505..54e84f79b 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/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') ]) diff --git a/releasenotes/notes/enable-skipping-disks-0c4c8b72231715a1.yaml b/releasenotes/notes/enable-skipping-disks-0c4c8b72231715a1.yaml new file mode 100644 index 000000000..bd1e0f855 --- /dev/null +++ b/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``.