From daa20b01d1522cd2748abb024b526dcb99562396 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Mon, 15 Aug 2022 16:00:17 +0000 Subject: [PATCH] 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 | 10 ++ ironic_python_agent/raid_utils.py | 14 ++- .../tests/unit/test_hardware.py | 102 +++++++++++------- .../tests/unit/test_raid_utils.py | 30 +++++- ...ids_with_volume_name-93e0bb59ef210fe4.yaml | 5 + 5 files changed, 117 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/create_raids_with_volume_name-93e0bb59ef210fe4.yaml diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 53344eb6c..c6740cc69 100644 --- a/ironic_python_agent/hardware.py +++ b/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) diff --git a/ironic_python_agent/raid_utils.py b/ironic_python_agent/raid_utils.py index 0a53eec5e..3d5f260fe 100644 --- a/ironic_python_agent/raid_utils.py +++ b/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) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index d3393cb4d..29026ca76 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/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 diff --git a/ironic_python_agent/tests/unit/test_raid_utils.py b/ironic_python_agent/tests/unit/test_raid_utils.py index 5b8577e27..6624304bf 100644 --- a/ironic_python_agent/tests/unit/test_raid_utils.py +++ b/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) ] diff --git a/releasenotes/notes/create_raids_with_volume_name-93e0bb59ef210fe4.yaml b/releasenotes/notes/create_raids_with_volume_name-93e0bb59ef210fe4.yaml new file mode 100644 index 000000000..64a9f7c60 --- /dev/null +++ b/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.