diff options
author | Zuul <zuul@review.opendev.org> | 2022-03-15 18:05:29 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2022-03-15 18:05:29 +0000 |
commit | f08f70134d24a23355511e9c8feac9987f2e62e4 (patch) | |
tree | ddb0cf7bad86a326910ccfdbff8c9ff2bbd8cd6a | |
parent | db981d07a470256dc7fe6b6947595e152207ca34 (diff) | |
parent | c5f7f18bcb2e1876b0712c691996346620ffc920 (diff) | |
download | ironic-python-agent-f08f70134d24a23355511e9c8feac9987f2e62e4.tar.gz |
Merge "Improve efficiency of storage cleaning in mixed media envs"
-rw-r--r-- | ironic_python_agent/hardware.py | 91 | ||||
-rw-r--r-- | ironic_python_agent/tests/unit/test_hardware.py | 41 | ||||
-rw-r--r-- | releasenotes/notes/add-erase-devices-express-1df107c75f2b3627.yaml | 8 |
3 files changed, 124 insertions, 16 deletions
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 62e39f1d..54de8d72 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -1349,34 +1349,45 @@ class GenericHardwareManager(HardwareManager): LOG.error(msg) raise errors.IncompatibleHardwareMethodError(msg) - def erase_devices_metadata(self, node, ports): - """Attempt to erase the disk devices metadata. - - :param node: Ironic node object - :param ports: list of Ironic port objects - :raises BlockDeviceEraseError: when there's an error erasing the - block device - """ + def _list_erasable_devices(self): block_devices = self.list_block_devices(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. block_devices.sort(key=lambda dev: dev.name, reverse=True) - erase_errors = {} + erasable_devices = [] for dev in block_devices: if self._is_virtual_media_device(dev): - LOG.info("Skipping metadata erase of virtual media device %s", + LOG.info("Skipping erasure of virtual media device %s", dev.name) continue if self._is_linux_raid_member(dev): - LOG.info("Skipping metadata erase of RAID member device %s", + LOG.info("Skipping erasure of RAID member device %s", dev.name) continue if self._is_read_only_device(dev): - LOG.info("Skipping metadata erase of read-only device %s", + LOG.info("Skipping erasure of read-only device %s", dev.name) continue + erasable_devices.append(dev) + + return erasable_devices + + def erase_devices_metadata(self, node, ports): + """Attempt to erase the disk devices metadata. + :param node: Ironic node object + :param ports: list of Ironic port objects + :raises BlockDeviceEraseError: when there's an error erasing the + block device + """ + block_devices = self.list_block_devices(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. + block_devices.sort(key=lambda dev: dev.name, reverse=True) + erase_errors = {} + for dev in self._list_erasable_devices(): try: disk_utils.destroy_disk_metadata(dev.name, node['uuid']) except processutils.ProcessExecutionError as e: @@ -1390,6 +1401,55 @@ class GenericHardwareManager(HardwareManager): for k, v in erase_errors.items()])) raise errors.BlockDeviceEraseError(excpt_msg) + def erase_devices_express(self, node, ports): + """Attempt to perform time-optimised disk erasure: + + for NVMe devices, perform NVMe Secure Erase if supported. For other + devices, perform metadata erasure + + :param node: Ironic node object + :param ports: list of Ironic port objects + :raises BlockDeviceEraseError: when there's an error erasing the + block device + """ + erase_errors = {} + info = node.get('driver_internal_info', {}) + if not self._list_erasable_devices: + LOG.debug("No erasable devices have been found.") + return + for dev in self._list_erasable_devices(): + try: + if self._is_nvme(dev): + execute_nvme_erase = info.get( + 'agent_enable_nvme_secure_erase', True) + if execute_nvme_erase and self._nvme_erase(dev): + continue + except errors.BlockDeviceEraseError as e: + LOG.error('Failed to securely erase device "%(dev)s". ' + 'Error: %(error)s, falling back to metadata ' + 'clean', {'dev': dev.name, 'error': e}) + secure_erase_error = e + try: + disk_utils.destroy_disk_metadata(dev.name, node['uuid']) + except processutils.ProcessExecutionError as e: + LOG.error('Failed to erase the metadata on device ' + '"%(dev)s". Error: %(error)s', + {'dev': dev.name, 'error': e}) + if secure_erase_error: + erase_errors[dev.name] = ( + "Secure erase failed: %s. " + "Fallback to metadata erase also failed: %s.", + secure_erase_error, e) + else: + erase_errors[dev.name] = e + + if erase_errors: + excpt_msg = ('Failed to conduct an express erase on ' + 'the device(s): %s' % '\n'.join('"%s": %s' % item + for item in + erase_errors.items())) + raise errors.BlockDeviceEraseError(excpt_msg) + def _find_pstore_mount_point(self): """Find the pstore mount point by scanning /proc/mounts. @@ -1943,6 +2003,13 @@ class GenericHardwareManager(HardwareManager): 'abortable': True }, { + 'step': 'erase_devices_express', + 'priority': 0, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, + { 'step': 'erase_pstore', 'priority': 0, 'interface': 'deploy', diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 60820727..0db92354 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -137,6 +137,13 @@ class TestGenericHardwareManager(base.IronicAgentTest): 'abortable': True }, { + 'step': 'erase_devices_express', + 'priority': 0, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, + { 'step': 'erase_pstore', 'priority': 0, 'interface': 'deploy', @@ -2103,6 +2110,32 @@ class TestGenericHardwareManager(base.IronicAgentTest): ]) @mock.patch.object(il_utils, 'execute', autospec=True) + @mock.patch.object(disk_utils, 'destroy_disk_metadata', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, + '_nvme_erase', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, + '_list_erasable_devices', autospec=True) + def test_erase_devices_express( + self, mock_list_erasable_devices, mock_nvme_erase, + mock_destroy_disk_metadata, mock_execute): + block_devices = [ + hardware.BlockDevice('/dev/sda', 'sata', 65535, False), + hardware.BlockDevice('/dev/md0', 'raid-device', 32767, False), + hardware.BlockDevice('/dev/nvme0n1', 'nvme', 32767, False), + hardware.BlockDevice('/dev/nvme1n1', 'nvme', 32767, False) + ] + mock_list_erasable_devices.return_value = list(block_devices) + + self.hardware.erase_devices_express(self.node, []) + self.assertEqual([mock.call(self.hardware, block_devices[2]), + mock.call(self.hardware, block_devices[3])], + mock_nvme_erase.call_args_list) + 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.patch.object(il_utils, 'execute', autospec=True) @mock.patch.object(hardware.GenericHardwareManager, '_is_virtual_media_device', autospec=True) @mock.patch.object(hardware.GenericHardwareManager, @@ -2135,8 +2168,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock.call('/dev/sda', self.node['uuid']), mock.call('/dev/md0', self.node['uuid'])], mock_metadata.call_args_list) - mock_list_devs.assert_called_once_with(self.hardware, - include_partitions=True) + mock_list_devs.assert_called_with(self.hardware, + include_partitions=True) self.assertEqual([mock.call(self.hardware, block_devices[0]), mock.call(self.hardware, block_devices[1]), mock.call(self.hardware, block_devices[4]), @@ -2183,8 +2216,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual([mock.call('/dev/sdb', self.node['uuid']), mock.call('/dev/sda', self.node['uuid'])], mock_metadata.call_args_list) - mock_list_devs.assert_called_once_with(self.hardware, - include_partitions=True) + mock_list_devs.assert_called_with(self.hardware, + include_partitions=True) self.assertEqual([mock.call(self.hardware, block_devices[1]), mock.call(self.hardware, block_devices[0])], mock__is_vmedia.call_args_list) diff --git a/releasenotes/notes/add-erase-devices-express-1df107c75f2b3627.yaml b/releasenotes/notes/add-erase-devices-express-1df107c75f2b3627.yaml new file mode 100644 index 00000000..790d1189 --- /dev/null +++ b/releasenotes/notes/add-erase-devices-express-1df107c75f2b3627.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Adds support for express cleaning mode where hardware-assisted, fast and + secure data erasure is performed on NVMe devices that support it, + while other devices fall back to erase_devices_metadata. + The goal of this feature is to enable express node cleaning in + environments with hybrid storage configuration (e.g. NVMe + HDD). |