From e4659c94cfffae88c8cc7879cd7130c8ed24e757 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Sun, 17 Nov 2019 13:27:36 -0800 Subject: [PATCH] RAID 5/6 Looks like adding RAID 5/6 support may be easier than most could immagine. The code, as written appears to be safe and logical creating a RAID 5 or RAID 6 volume. Not that we can really test this in CI, but it seems only validation code needs to be changed to loosen the constraint. Change-Id: Ib891b3c97f0bfb02af3b59581a451c4b25e03b85 --- ironic_python_agent/hardware.py | 14 +- .../tests/unit/test_hardware.py | 248 ++++++++++++++++++ .../raid5-6-support-0807597c3633a26c.yaml | 7 + 3 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/raid5-6-support-0807597c3633a26c.yaml diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 412f61105..79f7ac8f6 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -52,7 +52,7 @@ UNIT_CONVERTER.define('MB = 1048576 bytes') _MEMORY_ID_RE = re.compile(r'^memory(:\d+)?$') NODE = None -SUPPORTED_SOFTWARE_RAID_LEVELS = frozenset(['0', '1', '1+0']) +SUPPORTED_SOFTWARE_RAID_LEVELS = frozenset(['0', '1', '1+0', '5', '6']) def _get_device_info(dev, devclass, field): @@ -1822,7 +1822,17 @@ class GenericHardwareManager(HardwareManager): msg = ("Software RAID configuration does not support " "RAID level %s" % current_level) raid_errors.append(msg) - + physical_device_count = len(self.list_block_devices()) + if current_level == '5' and physical_device_count < 3: + msg = ("Software RAID configuration is not possible for " + "RAID level 5 with only %s block devices found." + % physical_device_count) + raid_errors.append(msg) + if current_level == '6' and physical_device_count < 4: + msg = ("Software RAID configuration is not possible for " + "RAID level 6 with only %s block devices found." + % physical_device_count) + raid_errors.append(msg) if raid_errors: error = ('Could not validate Software RAID config for %(node)s: ' '%(errors)s') % {'node': node['uuid'], diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 169baa39d..1caa0d935 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2772,6 +2772,178 @@ class TestGenericHardwareManager(base.IronicAgentTest): '/dev/sda2', '/dev/sdb2')]) self.assertEqual(raid_config, result) + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_raid_5(self, mocked_execute): + node = self.node + raid_config = { + "logical_disks": [ + { + "size_gb": "10", + "raid_level": "1", + "controller": "software", + }, + { + "size_gb": "MAX", + "raid_level": "5", + "controller": "software", + }, + ] + } + node['target_raid_config'] = raid_config + device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True) + device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True) + device3 = hardware.BlockDevice('/dev/sdc', 'sdc', 107374182400, True) + self.hardware.list_block_devices = mock.Mock() + self.hardware.list_block_devices.return_value = [device1, device2, + device3] + + mocked_execute.side_effect = [ + None, # mklabel sda + ('42', None), # sgdisk -F sda + None, # mklabel sdb + ('42', None), # sgdisk -F sdb + None, # mklabel sdc + ('42', None), # sgdisk -F sdc + None, None, # parted + partx sda + None, None, # parted + partx sdb + None, None, # parted + partx sdc + None, None, # parted + partx sda + None, None, # parted + partx sdb + None, None, # parted + partx sdc + None, None # mdadms + ] + + result = self.hardware.create_configuration(node, []) + + mocked_execute.assert_has_calls([ + mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sda'), + mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sdb'), + mock.call('parted', '/dev/sdc', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sdc'), + mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sda', check_exit_code=False), + mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sdb', check_exit_code=False), + mock.call('parted', '/dev/sdc', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sdc', check_exit_code=False), + mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sda', check_exit_code=False), + mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sdb', check_exit_code=False), + mock.call('parted', '/dev/sdc', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sdc', check_exit_code=False), + mock.call('mdadm', '--create', '/dev/md0', '--force', '--run', + '--metadata=1', '--level', '1', '--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')]) + self.assertEqual(raid_config, result) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_raid_6(self, mocked_execute): + node = self.node + raid_config = { + "logical_disks": [ + { + "size_gb": "10", + "raid_level": "1", + "controller": "software", + }, + { + "size_gb": "MAX", + "raid_level": "6", + "controller": "software", + }, + ] + } + node['target_raid_config'] = raid_config + device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True) + device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True) + device3 = hardware.BlockDevice('/dev/sdc', 'sdc', 107374182400, True) + device4 = hardware.BlockDevice('/dev/sdd', 'sdd', 107374182400, True) + self.hardware.list_block_devices = mock.Mock() + self.hardware.list_block_devices.return_value = [device1, device2, + device3, device4] + + mocked_execute.side_effect = [ + None, # mklabel sda + ('42', None), # sgdisk -F sda + None, # mklabel sdb + ('42', None), # sgdisk -F sdb + None, # mklabel sdc + ('42', None), # sgdisk -F sdc + None, # mklabel sdd + ('42', None), # sgdisk -F sdd + None, None, # parted + partx sda + None, None, # parted + partx sdb + None, None, # parted + partx sdc + None, None, # parted + partx sdd + None, None, # parted + partx sda + None, None, # parted + partx sdb + None, None, # parted + partx sdc + None, None, # parted + partx sdd + None, None # mdadms + ] + + result = self.hardware.create_configuration(node, []) + + mocked_execute.assert_has_calls([ + mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sda'), + mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sdb'), + mock.call('parted', '/dev/sdc', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sdc'), + mock.call('parted', '/dev/sdd', '-s', '--', 'mklabel', + 'msdos'), + mock.call('sgdisk', '-F', '/dev/sdd'), + mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sda', check_exit_code=False), + mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sdb', check_exit_code=False), + mock.call('parted', '/dev/sdc', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sdc', check_exit_code=False), + mock.call('parted', '/dev/sdd', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '42s', '10GiB'), + mock.call('partx', '-u', '/dev/sdd', check_exit_code=False), + mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sda', check_exit_code=False), + mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sdb', check_exit_code=False), + mock.call('parted', '/dev/sdc', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sdc', check_exit_code=False), + mock.call('parted', '/dev/sdd', '-s', '-a', 'optimal', '--', + 'mkpart', 'primary', '10GiB', '-1'), + mock.call('partx', '-u', '/dev/sdd', check_exit_code=False), + mock.call('mdadm', '--create', '/dev/md0', '--force', '--run', + '--metadata=1', '--level', '1', '--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')]) + self.assertEqual(raid_config, result) + @mock.patch.object(utils, 'execute', autospec=True) def test_create_configuration_no_max(self, mocked_execute): node = self.node @@ -2941,8 +3113,10 @@ class TestGenericHardwareManager(base.IronicAgentTest): 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.assertRaises(errors.SoftwareRAIDError, self.hardware.create_configuration, self.node, []) @@ -2974,6 +3148,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): [device1, device2], [device1, device2], [device1, device2], + [device1, device2], + [device1, device2], + [device1, device2], [device1, device2]] # partition table creation @@ -3009,6 +3186,74 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.hardware.create_configuration, self.node, []) + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_device_handling_failures_raid5( + self, mocked_execute): + raid_config = { + "logical_disks": [ + { + "size_gb": "100", + "raid_level": "1", + "controller": "software", + }, + { + "size_gb": "MAX", + "raid_level": "5", + "controller": "software", + }, + ] + } + self.node['target_raid_config'] = raid_config + device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True) + device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True) + self.hardware.list_block_devices = mock.Mock() + self.hardware.list_block_devices.side_effect = [ + [device1, device2], + [device1, device2]] + + # validation configuration explicitly fails before any action + error_regex = ("Software RAID configuration is not possible for " + "RAID level 5 with only 2 block devices found.") + # Execute is actually called for listing_block_devices + self.assertFalse(mocked_execute.called) + self.assertRaisesRegex(errors.SoftwareRAIDError, error_regex, + self.hardware.create_configuration, + self.node, []) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_device_handling_failures_raid6( + self, mocked_execute): + raid_config = { + "logical_disks": [ + { + "size_gb": "100", + "raid_level": "1", + "controller": "software", + }, + { + "size_gb": "MAX", + "raid_level": "6", + "controller": "software", + }, + ] + } + self.node['target_raid_config'] = raid_config + device1 = hardware.BlockDevice('/dev/sda', 'sda', 107374182400, True) + device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 107374182400, True) + device3 = hardware.BlockDevice('/dev/sdc', 'sdc', 107374182400, True) + self.hardware.list_block_devices = mock.Mock() + self.hardware.list_block_devices.side_effect = [ + [device1, device2, device3], + [device1, device2, device3]] + # pre-creation validation fails as insufficent number of devices found + error_regex = ("Software RAID configuration is not possible for " + "RAID level 6 with only 3 block devices found.") + # Execute is actually called for listing_block_devices + self.assertFalse(mocked_execute.called) + self.assertRaisesRegex(errors.SoftwareRAIDError, error_regex, + self.hardware.create_configuration, + self.node, []) + def test_create_configuration_empty_target_raid_config(self): self.node['target_raid_config'] = {} result = self.hardware.create_configuration(self.node, []) @@ -3107,6 +3352,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): [device1, device2], [device1, device2], [device1, device2], + [device1, device2], + [device1, device2], + [device1, device2], [device1, device2]] # partition table creation diff --git a/releasenotes/notes/raid5-6-support-0807597c3633a26c.yaml b/releasenotes/notes/raid5-6-support-0807597c3633a26c.yaml new file mode 100644 index 000000000..3f142f301 --- /dev/null +++ b/releasenotes/notes/raid5-6-support-0807597c3633a26c.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds support to allow selection of RAID ``5`` and RAID ``6`` + protection levels for software RAID support. This may only be + the secondary volume, as these volume types of software RAID + volumes cannot be used to directly boot an operating system.