Create RAIDs with volume name

Use 'volume_name' field from 'target_raid_config' to create logical
disks if it is present
Do not allow two logical disks to have the same volume name

Change-Id: If3e4e9f8698ec3e0cb49717f8ed2087d2ba03f2c

ironic_python_agent/hardware.py

@@ -2603,6 +2603,7 @@ class GenericHardwareManager(HardwareManager):
                    "two logical disks")
             raid_errors.append(msg)
 
+        volume_names = []
         # All disks need to be flagged for Software RAID
         for logical_disk in logical_disks:
             if logical_disk.get('controller') != 'software':
@@ -2610,6 +2611,15 @@ class GenericHardwareManager(HardwareManager):
                        "disks to have 'controller'='software'")
                 raid_errors.append(msg)
 
+            volume_name = logical_disk.get('volume_name')
+            if volume_name is not None:
+                if volume_name in volume_names:
+                    msg = ("Duplicate software RAID device name %s "
+                           "detected" % volume_name)
+                    raid_errors.append(msg)
+                else:
+                    volume_names.append(volume_name)
+
             physical_disks = logical_disk.get('physical_disks')
             if physical_disks is not None:
                 if (not isinstance(physical_disks, list)

ironic_python_agent/raid_utils.py

@@ -223,13 +223,19 @@ def create_raid_device(index, logical_disk):
     # The schema check allows '1+0', but mdadm knows it as '10'.
     if raid_level == '1+0':
         raid_level = '10'
+    volume_name = logical_disk.get('volume_name')
     try:
-        LOG.debug("Creating md device %(dev)s on %(comp)s",
-                  {'dev': md_device, 'comp': component_devices})
+        if volume_name is None:
+            volume_name = md_device
+        LOG.debug("Creating md device %(dev)s with name %(name)s"
+                  "on %(comp)s",
+                  {'dev': md_device, 'name': volume_name,
+                   'comp': component_devices})
         utils.execute('mdadm', '--create', md_device, '--force',
                       '--run', '--metadata=1', '--level', raid_level,
-                      '--raid-devices', len(component_devices),
-                      *component_devices)
+                      '--name', volume_name, '--raid-devices',
+                      len(component_devices), *component_devices)
+
     except processutils.ProcessExecutionError as e:
         msg = "Failed to create md device {} on {}: {}".format(
             md_device, ' '.join(component_devices), e)

ironic_python_agent/tests/unit/test_hardware.py

@@ -3096,11 +3096,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
 
         self.assertEqual(raid_config, result)
 
@@ -3201,11 +3201,13 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 3,
-                      '/dev/sda1', '/dev/sdb1', '/dev/sdc1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 3, '/dev/sda1', '/dev/sdb1',
+                      '/dev/sdc1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '5', '--raid-devices', 3,
-                      '/dev/sda2', '/dev/sdb2', '/dev/sdc2')])
+                      '--metadata=1', '--level', '5', '--name', '/dev/md1',
+                      '--raid-devices', 3, '/dev/sda2', '/dev/sdb2',
+                      '/dev/sdc2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3317,11 +3319,13 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 4,
-                      '/dev/sda1', '/dev/sdb1', '/dev/sdc1', '/dev/sdd1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 4, '/dev/sda1', '/dev/sdb1',
+                      '/dev/sdc1', '/dev/sdd1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '6', '--raid-devices', 4,
-                      '/dev/sda2', '/dev/sdb2', '/dev/sdc2', '/dev/sdd2')])
+                      '--metadata=1', '--level', '6', '--name', '/dev/md1',
+                      '--raid-devices', 4, '/dev/sda2', '/dev/sdb2',
+                      '/dev/sdc2', '/dev/sdd2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3398,11 +3402,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3485,11 +3489,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3567,11 +3571,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3651,11 +3655,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '0', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '1', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
@@ -3744,11 +3748,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/sda1', '/dev/sdb1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/sda1', '/dev/sdb1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/sda2', '/dev/sdb2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/sda2', '/dev/sdb2')])
         self.assertEqual(raid_config, result)
 
         self.assertEqual(2, mock_list_parts.call_count)
@@ -4092,11 +4096,13 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                       delay_on_retry=True),
             mock.call('udevadm', 'settle'),
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 2,
-                      '/dev/nvme0n1p1', '/dev/nvme1n1p1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 2, '/dev/nvme0n1p1',
+                      '/dev/nvme1n1p1'),
             mock.call('mdadm', '--create', '/dev/md1', '--force', '--run',
-                      '--metadata=1', '--level', '0', '--raid-devices', 2,
-                      '/dev/nvme0n1p2', '/dev/nvme1n1p2')])
+                      '--metadata=1', '--level', '0', '--name', '/dev/md1',
+                      '--raid-devices', 2, '/dev/nvme0n1p2', '/dev/nvme1n1p2')
+        ])
         self.assertEqual(raid_config, result)
 
     @mock.patch.object(disk_utils, 'list_partitions', autospec=True,
@@ -4506,6 +4512,30 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                           self.hardware.validate_configuration,
                           raid_config, self.node)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
+    def test_validate_configuration_invalid_duplicate_volume_name(
+            self, mocked_execute):
+        raid_config = {
+            "logical_disks": [
+                {
+                    "size_gb": "100",
+                    "raid_level": "1",
+                    "controller": "software",
+                    "volume_name": "thedisk"
+                },
+                {
+                    "size_gb": "MAX",
+                    "raid_level": "0",
+                    "controller": "software",
+                    "volume_name": "thedisk"
+                },
+            ]
+        }
+        mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '')
+        self.assertRaises(errors.SoftwareRAIDError,
+                          self.hardware.validate_configuration,
+                          raid_config, self.node)
+
     @mock.patch.object(il_utils, 'execute', autospec=True)
     def test_get_system_vendor_info(self, mocked_execute):
         mocked_execute.return_value = hws.LSHW_JSON_OUTPUT_V1

ironic_python_agent/tests/unit/test_raid_utils.py

@@ -57,8 +57,29 @@ class TestRaidUtils(base.IronicAgentTest):
 
         mock_execute.assert_called_once_with(
             'mdadm', '--create', '/dev/md0', '--force', '--run',
-            '--metadata=1', '--level', '1', '--raid-devices', 3,
-            '/dev/sda1', '/dev/sdb1', '/dev/sdc1')
+            '--metadata=1', '--level', '1', '--name', '/dev/md0',
+            '--raid-devices', 3, '/dev/sda1', '/dev/sdb1', '/dev/sdc1')
+
+    @mock.patch.object(raid_utils, '_get_actual_component_devices',
+                       autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_create_raid_device_with_volume_name(self, mock_execute,
+                                                 mocked_components):
+        logical_disk = {
+            "block_devices": ['/dev/sda', '/dev/sdb', '/dev/sdc'],
+            "raid_level": "1",
+            "volume_name": "diskname"
+        }
+        mocked_components.return_value = ['/dev/sda1',
+                                          '/dev/sdb1',
+                                          '/dev/sdc1']
+
+        raid_utils.create_raid_device(0, logical_disk)
+
+        mock_execute.assert_called_once_with(
+            'mdadm', '--create', '/dev/md0', '--force', '--run',
+            '--metadata=1', '--level', '1', '--name', 'diskname',
+            '--raid-devices', 3, '/dev/sda1', '/dev/sdb1', '/dev/sdc1')
 
     @mock.patch.object(raid_utils, '_get_actual_component_devices',
                        autospec=True)
@@ -76,8 +97,9 @@ class TestRaidUtils(base.IronicAgentTest):
 
         expected_calls = [
             mock.call('mdadm', '--create', '/dev/md0', '--force', '--run',
-                      '--metadata=1', '--level', '1', '--raid-devices', 3,
-                      '/dev/sda1', '/dev/sdb1', '/dev/sdc1'),
+                      '--metadata=1', '--level', '1', '--name', '/dev/md0',
+                      '--raid-devices', 3, '/dev/sda1', '/dev/sdb1',
+                      '/dev/sdc1'),
             mock.call('mdadm', '--add', '/dev/md0', '/dev/sdb1',
                       attempts=3, delay_on_retry=True)
         ]

releasenotes/notes/create_raids_with_volume_name-93e0bb59ef210fe4.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Software RAID devices are built with the `--name` option
+    followed by volume name if it is defined in target raid config
+    and an internal ID otherwise.