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

Now that an operator can pick the devices that participate in RAID,
it no longer makes sense to verify all devices.

Change-Id: Id5d8d539183f0db4ba3c4132ce6bc9919f9cd1ea
Story: #2006369

ironic_python_agent/hardware.py

@@ -1527,21 +1527,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": [
                 {