Merge "Add erase_devices_metadata cleaning step"

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 [
+        return []
-            {
-                'step': 'erase_devices',
-                'priority': 10,
-                'interface': 'deploy',
-                'reboot_requested': False,
-                'abortable': True
-            }
-        ]
 
     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

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', ''

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...).