diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 1248eb047..bb5cf5176 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -19,7 +19,7 @@ import os import shlex import time - +from ironic_lib import disk_utils import netifaces from oslo_concurrency import processutils from oslo_config import cfg @@ -399,15 +399,7 @@ class HardwareManager(object): dict as defined above """ - return [ - { - 'step': 'erase_devices', - 'priority': 10, - 'interface': 'deploy', - 'reboot_requested': False, - 'abortable': True - } - ] + return [] def get_version(self): """Get a name and version for this hardware manager. @@ -437,7 +429,8 @@ class HardwareManager(object): class GenericHardwareManager(HardwareManager): HARDWARE_MANAGER_NAME = 'generic_hardware_manager' - HARDWARE_MANAGER_VERSION = '1.0' + # 1.1 - Added new clean step called erase_devices_metadata + HARDWARE_MANAGER_VERSION = '1.1' def __init__(self): self.sys_path = '/sys' @@ -753,6 +746,35 @@ 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 + """ + block_devices = self.list_block_devices() + erase_errors = {} + for dev in block_devices: + if self._is_virtual_media_device(dev): + LOG.info("Skipping the erase of virtual media device %s", + dev.name) + continue + + 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}) + erase_errors[dev.name] = e + + if erase_errors: + excpt_msg = ('Failed to erase the metadata on the device(s): %s' % + '; '.join(['"%s": %s' % (k, v) + for k, v in erase_errors.items()])) + raise errors.BlockDeviceEraseError(excpt_msg) + def _shred_block_device(self, node, block_device): """Erase a block device using shred. @@ -897,6 +919,24 @@ class GenericHardwareManager(HardwareManager): return out.strip() + def get_clean_steps(self, node, ports): + return [ + { + 'step': 'erase_devices', + 'priority': 10, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, + { + 'step': 'erase_devices_metadata', + 'priority': 99, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + } + ] + def _compare_extensions(ext1, ext2): mgr1 = ext1.obj diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 9beb48e0e..d18a838de 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -15,6 +15,7 @@ import os import time +from ironic_lib import disk_utils import mock import netifaces from oslo_concurrency import processutils @@ -284,6 +285,26 @@ class TestGenericHardwareManager(test_base.BaseTestCase): CONF.clear_override('disk_wait_attempts') CONF.clear_override('disk_wait_delay') + def test_get_clean_steps(self): + expected_clean_steps = [ + { + 'step': 'erase_devices', + 'priority': 10, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, + { + 'step': 'erase_devices_metadata', + 'priority': 99, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + } + ] + clean_steps = self.hardware.get_clean_steps(self.node, []) + self.assertEqual(expected_clean_steps, clean_steps) + @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @mock.patch('os.path.exists') @@ -1194,6 +1215,64 @@ class TestGenericHardwareManager(test_base.BaseTestCase): test_security_erase_option( self, False, '--security-erase') + @mock.patch.object(hardware.GenericHardwareManager, + '_is_virtual_media_device', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, + 'list_block_devices', autospec=True) + @mock.patch.object(disk_utils, 'destroy_disk_metadata', autospec=True) + def test_erase_devices_metadata( + self, mock_metadata, mock_list_devs, mock__is_vmedia): + block_devices = [ + hardware.BlockDevice('/dev/sr0', 'vmedia', 12345, True), + hardware.BlockDevice('/dev/sda', 'small', 65535, False), + ] + mock_list_devs.return_value = block_devices + mock__is_vmedia.side_effect = (True, False) + + self.hardware.erase_devices_metadata(self.node, []) + mock_metadata.assert_called_once_with( + '/dev/sda', self.node['uuid']) + mock_list_devs.assert_called_once_with(mock.ANY) + mock__is_vmedia.assert_has_calls([ + mock.call(mock.ANY, block_devices[0]), + mock.call(mock.ANY, block_devices[1]) + ]) + + @mock.patch.object(hardware.GenericHardwareManager, + '_is_virtual_media_device', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, + 'list_block_devices', autospec=True) + @mock.patch.object(disk_utils, 'destroy_disk_metadata', autospec=True) + def test_erase_devices_metadata_error( + self, mock_metadata, mock_list_devs, mock__is_vmedia): + block_devices = [ + hardware.BlockDevice('/dev/sda', 'small', 65535, False), + hardware.BlockDevice('/dev/sdb', 'big', 10737418240, True), + ] + mock__is_vmedia.return_value = False + mock_list_devs.return_value = block_devices + # Simulate /dev/sda failing and /dev/sdb succeeding + error_output = 'Booo00000ooommmmm' + mock_metadata.side_effect = ( + processutils.ProcessExecutionError(error_output), + None, + ) + + self.assertRaisesRegex(errors.BlockDeviceEraseError, error_output, + self.hardware.erase_devices_metadata, + self.node, []) + # Assert all devices are erased independent if one of them + # failed previously + mock_metadata.assert_has_calls([ + mock.call('/dev/sda', self.node['uuid']), + mock.call('/dev/sdb', self.node['uuid']), + ]) + mock_list_devs.assert_called_once_with(mock.ANY) + mock__is_vmedia.assert_has_calls([ + mock.call(mock.ANY, block_devices[0]), + mock.call(mock.ANY, block_devices[1]) + ]) + @mock.patch.object(utils, 'execute') def test_get_bmc_address(self, mocked_execute): mocked_execute.return_value = '192.1.2.3\n', '' diff --git a/releasenotes/notes/erase-device-metadata-clean-step-31b4a615c0ff7f18.yaml b/releasenotes/notes/erase-device-metadata-clean-step-31b4a615c0ff7f18.yaml new file mode 100644 index 000000000..11ec17b34 --- /dev/null +++ b/releasenotes/notes/erase-device-metadata-clean-step-31b4a615c0ff7f18.yaml @@ -0,0 +1,6 @@ +--- +features: + - Add a new cleaning step called "erase_devices_metadata" to the generic + hardware manager which is responsible for destroying the metadata + on the disk devices (partition tables, signatures, file-system + identifications, etc...).