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

@@ -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'],

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

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.