Merge "Only check for partitions on devices that are part of software RAID"

ironic_python_agent/hardware.py

@@ -1528,21 +1528,21 @@ class GenericHardwareManager(HardwareManager):
         # Log the validated target_raid_configuration.
         LOG.debug("Target Software RAID configuration: %s", raid_config)
 
-        # Make sure there are no partitions yet (or left behind).
-        block_devices = self.list_block_devices()
-        block_devices_partitions = self.list_block_devices(
-            include_partitions=True)
-        # TODO(dtantsur): limit this validation to only the discs involved the
-        # software RAID.
-        if len(block_devices) != len(block_devices_partitions):
-            partitions = ' '.join(
-                partition.name for partition in block_devices_partitions)
-            msg = "Partitions detected during RAID config: {}". format(
-                  partitions)
-            raise errors.SoftwareRAIDError(msg)
-
         block_devices, logical_disks = raid_utils.get_block_devices_for_raid(
-            block_devices, logical_disks)
+            self.list_block_devices(), logical_disks)
+        # Make sure there are no partitions yet (or left behind).
+        with_parts = []
+        for dev_name in block_devices:
+            try:
+                if disk_utils.list_partitions(dev_name):
+                    with_parts.append(dev_name)
+            except processutils.ProcessExecutionError:
+                # Presumably no partitions (or no partition table)
+                continue
+        if with_parts:
+            msg = ("Partitions detected on devices %s during RAID config"
+                   % ', '.join(with_parts))
+            raise errors.SoftwareRAIDError(msg)
 
         parted_start_dict = {}
         # Create an MBR partition table on each disk.

ironic_python_agent/tests/unit/test_hardware.py

@@ -2779,8 +2779,9 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                           self.hardware.validate_configuration,
                           self.node, [])
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration(self, mocked_execute):
+    def test_create_configuration(self, mocked_execute, mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -2801,6 +2802,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True)
         self.hardware.list_block_devices = mock.Mock()
         self.hardware.list_block_devices.return_value = [device1, device2]
+        mock_list_parts.side_effect = [
+            [],
+            processutils.ProcessExecutionError
+        ]
 
         mocked_execute.side_effect = [
             None,  # mklabel sda
@@ -2843,8 +2848,16 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
+        self.assertEqual(2, mock_list_parts.call_count)
+        mock_list_parts.assert_has_calls([
+            mock.call(x) for x in ['/dev/sda', '/dev/sdb']
+        ])
+
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_raid_5(self, mocked_execute):
+    def test_create_configuration_raid_5(self, mocked_execute,
+                                         mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -2922,8 +2935,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2', '/dev/sdc2')])
         self.assertEqual(raid_config, result)
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_raid_6(self, mocked_execute):
+    def test_create_configuration_raid_6(self, mocked_execute,
+                                         mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -3015,8 +3031,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2', '/dev/sdc2', '/dev/sdd2')])
         self.assertEqual(raid_config, result)
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_no_max(self, mocked_execute):
+    def test_create_configuration_no_max(self, mocked_execute,
+                                         mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -3078,8 +3097,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_max_is_first_logical(self, mocked_execute):
+    def test_create_configuration_max_is_first_logical(self, mocked_execute,
+                                                       mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -3141,8 +3163,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_with_hints(self, mocked_execute):
+    def test_create_configuration_with_hints(self, mocked_execute,
+                                             mock_list_parts):
         node = self.node
         raid_config = {
             "logical_disks": [
@@ -3216,6 +3241,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
+        self.assertEqual(2, mock_list_parts.call_count)
+        mock_list_parts.assert_has_calls([
+            mock.call(x) for x in ['/dev/sda', '/dev/sdb']
+        ])
+
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_create_configuration_invalid_raid_config(self, mocked_execute):
         raid_config = {
@@ -3291,8 +3321,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                    self.hardware.create_configuration,
                                    self.node, [])
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_partitions_detected(self, mocked_execute):
+    def test_create_configuration_partitions_detected(self, mocked_execute,
+                                                      mock_list_parts):
         raid_config = {
             "logical_disks": [
                 {
@@ -3307,23 +3339,28 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                 },
             ]
         }
+        mock_list_parts.side_effect = [
+            [],
+            [{'partition_name': '/dev/sdb1'}],
+        ]
         self.node['target_raid_config'] = raid_config
         device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True)
         device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True)
-        partition1 = hardware.BlockDevice('/dev/sdb1', 'sdb1', 268435456, True)
         self.hardware.list_block_devices = mock.Mock()
-        self.hardware.list_block_devices.side_effect = [
-            [device1, device2],  # pre-flight validation call
-            [device1, device2],
-            [device1, device2, partition1]]
+        self.hardware.list_block_devices.return_value = [
+            device1, device2
+        ]
 
         self.assertRaises(errors.SoftwareRAIDError,
                           self.hardware.create_configuration,
                           self.node, [])
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_create_configuration_device_handling_failures(self,
-                                                           mocked_execute):
+                                                           mocked_execute,
+                                                           mock_list_parts):
         raid_config = {
             "logical_disks": [
                 {
@@ -3386,9 +3423,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                                self.hardware.create_configuration,
                                self.node, [])
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_create_configuration_device_handling_failures_raid5(
-            self, mocked_execute):
+            self, mocked_execute, mock_list_parts):
         raid_config = {
             "logical_disks": [
                 {
@@ -3459,8 +3498,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         result = self.hardware.create_configuration(self.node, [])
         self.assertEqual(result, {})
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_with_nvme(self, mocked_execute):
+    def test_create_configuration_with_nvme(self, mocked_execute,
+                                            mock_list_parts):
         raid_config = {
             "logical_disks": [
                 {
@@ -3524,8 +3566,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       '/dev/nvme0n1p2', '/dev/nvme1n1p2')])
         self.assertEqual(raid_config, result)
 
+    @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
+                       return_value=[])
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test_create_configuration_failure_with_nvme(self, mocked_execute):
+    def test_create_configuration_failure_with_nvme(self, mocked_execute,
+                                                    mock_list_parts):
         raid_config = {
             "logical_disks": [
                 {