Fix RAID steps for non-RAID in idrac-redfish

When creating configuration convert eligible non-RAID drives
to RAID mode and recalculate volume sizes when necessary
after conversion changes available disk size.

Story: 2009160
Task: 43144

Depends-On: https://review.opendev.org/c/x/sushy-oem-idrac/+/805655
Change-Id: I60a9bc3997740f2c0e6e003a260a213020d3dded

ironic/conf/drac.py

@@ -44,7 +44,12 @@ opts = [
                default=600,
                min=1,
                help=_('Maximum time (in seconds) to wait for factory reset of '
-                      'BIOS settings to complete.'))
+                      'BIOS settings to complete.')),
+    cfg.IntOpt('raid_job_timeout',
+               default=300,
+               min=1,
+               help=_('Maximum time (in seconds) to wait for RAID job to '
+                      'complete'))
 
 ]
 

ironic/drivers/modules/drac/raid.py

@@ -37,10 +37,12 @@ from ironic.drivers.modules.drac import common as drac_common
 from ironic.drivers.modules.drac import job as drac_job
 from ironic.drivers.modules.drac import utils as drac_utils
 from ironic.drivers.modules.redfish import raid as redfish_raid
+from ironic.drivers.modules.redfish import utils as redfish_utils
 
 drac_exceptions = importutils.try_import('dracclient.exceptions')
 drac_constants = importutils.try_import('dracclient.constants')
 sushy = importutils.try_import('sushy')
+sushy_oem_idrac = importutils.try_import('sushy_oem_idrac')
 
 LOG = logging.getLogger(__name__)
 
@@ -1288,6 +1290,135 @@ class DracRedfishRAID(redfish_raid.RedfishRAID):
     def _validate_vendor(self, task):
         pass  # for now assume idrac-redfish is used with iDRAC BMC, thus pass
 
+    def pre_create_configuration(self, task, logical_disks_to_create):
+        """Perform required actions before creating config.
+
+        Converts any physical disks of selected controllers to RAID mode
+        if in non-RAID mode.
+
+        :param task: a TaskManager instance containing the node to act on.
+        :param logical_disks_to_create: list of logical disks to create.
+        :returns: updated list of logical disks to create
+        """
+        system = redfish_utils.get_system(task.node)
+        controller_to_disks = {}
+        for logical_disk in logical_disks_to_create:
+            storage, controller = DracRedfishRAID._get_storage_controller(
+                system, logical_disk.get('controller'))
+            controller_to_disks[controller] = []
+            for drive in storage.drives:
+                if drive.identity in logical_disk.get('physical_disks'):
+                    controller_to_disks[controller].append(drive)
+
+        converted = DracRedfishRAID._change_physical_disk_state(
+            system,
+            sushy_oem_idrac.PHYSICAL_DISK_STATE_MODE_RAID,
+            controller_to_disks)
+
+        if converted:
+            # Recalculate sizes as disks size changes after conversion
+            return DracRedfishRAID._get_revalidated_logical_disks(
+                task.node, system, logical_disks_to_create)
+        else:
+            return logical_disks_to_create
+
+    @staticmethod
+    def _get_storage_controller(system, identity):
+        """Finds storage and controller by identity
+
+        :param system: Redfish system
+        :param identity: identity of controller to find
+        :returns: Storage and its controller
+        """
+        for storage in system.storage.get_members():
+            if storage.identity == identity:
+                controller = (storage.storage_controllers[0]
+                              if storage.storage_controllers else None)
+                if controller:
+                    return storage, controller
+
+        raise exception.IronicException(
+            (_("Couldn't find storage by '%(identity)s'"),
+             {'identity': identity}))
+
+    @staticmethod
+    def _change_physical_disk_state(system, mode, controller_to_disks=None):
+        """Changes physical disk state and waits for it to complete
+
+        :param system: Redfish system
+        :param mode: sushy_oem_idrac.PHYSICAL_DISK_STATE_MODE_RAID or
+            sushy_oem_idrac.PHYSICAL_DISK_STATE_MODE_NONRAID
+        :controller_to_disks: dictionary of controllers and their
+            drives. Optional. If not provided, then converting all
+            eligible drives on system.
+        :returns: True if any drive got converted, otherwise False
+        """
+        oem_sys = system.get_oem_extension('Dell')
+        try:
+            task_mons = oem_sys.change_physical_disk_state(
+                mode, controller_to_disks)
+        except AttributeError as ae:
+            # For backported version where libraries could be too old
+            LOG.warning('Failed to find method to convert drives to RAID '
+                        'mode. Possibly because `sushy-oem-idrac` is too old. '
+                        'Without newer `sushy-oem-idrac` RAID configuration '
+                        'will fail if selected physical disks are in non-RAID '
+                        'mode. To avoid that update `sushy-oem-idrac`. '
+                        'Error: %(err)s', {'err': ae})
+            return False
+
+        for task_mon in task_mons:
+            # All jobs should be real-time, because all RAID controllers
+            # that offer physical disk mode conversion support real-time
+            # task execution. Note that BOSS does not offer disk mode
+            # conversion nor support real-time task execution.
+            if task_mon.check_is_processing:
+                task_mon.wait(CONF.drac.raid_job_timeout)
+
+        return bool(task_mons)
+
+    @staticmethod
+    def _get_revalidated_logical_disks(
+            node, system, logical_disks_to_create):
+        """Revalidates calculated volume size after RAID mode conversion
+
+        :param node: an Ironic node
+        :param system: Redfish system
+        :param logical_disks_to_create:
+        :returns: Revalidated logical disk list. If no changes in size,
+            same as input `logical_disks_to_create`
+        """
+        new_physical_disks, disk_to_controller =\
+            redfish_raid.get_physical_disks(node)
+        free_space_bytes = {}
+        for disk in new_physical_disks:
+            free_space_bytes[disk] = disk.capacity_bytes
+
+        new_processed_volumes = []
+        for logical_disk in logical_disks_to_create:
+            selected_disks = [disk for disk in new_physical_disks
+                              if disk.identity
+                              in logical_disk['physical_disks']]
+
+            spans_count = redfish_raid._calculate_spans(
+                logical_disk['raid_level'], len(selected_disks))
+            new_max_vol_size_bytes = redfish_raid._max_volume_size_bytes(
+                logical_disk['raid_level'], selected_disks, free_space_bytes,
+                spans_count=spans_count)
+            if logical_disk['size_bytes'] > new_max_vol_size_bytes:
+                logical_disk['size_bytes'] = new_max_vol_size_bytes
+                LOG.info("Logical size does not match so calculating volume "
+                         "properties for current logical_disk")
+                redfish_raid._calculate_volume_props(
+                    logical_disk, new_physical_disks, free_space_bytes,
+                    disk_to_controller)
+                new_processed_volumes.append(logical_disk)
+
+        if new_processed_volumes:
+            return new_processed_volumes
+
+        return logical_disks_to_create
+
 
 class DracWSManRAID(base.RAIDInterface):
 

ironic/drivers/modules/redfish/raid.py

@@ -1,4 +1,5 @@
 # Copyright 2021 DMTF. All rights reserved.
+# Copyright (c) 2021 Dell Inc. or its subsidiaries.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
@@ -814,7 +815,8 @@ class RedfishRAID(base.RAIDInterface):
         logical_disks_to_create = _filter_logical_disks(
             logical_disks, create_root_volume, create_nonroot_volumes)
 
-        self.pre_create_configuration(task, logical_disks_to_create)
+        logical_disks_to_create = self.pre_create_configuration(
+            task, logical_disks_to_create)
 
         reboot_required = False
         raid_configs = list()
@@ -961,8 +963,9 @@ class RedfishRAID(base.RAIDInterface):
 
         :param task: a TaskManager instance containing the node to act on.
         :param logical_disks_to_create: list of logical disks to create.
+        :returns: updated list of logical disks to create.
         """
-        pass
+        return logical_disks_to_create
 
     def post_create_configuration(self, task, raid_configs, return_state=None):
         """Perform post create_configuration action to commit the config.

ironic/tests/unit/drivers/modules/drac/test_raid.py

@@ -26,6 +26,7 @@ import tenacity
 from ironic.common import exception
 from ironic.common import states
 from ironic.conductor import task_manager
+from ironic.conf import CONF
 from ironic.drivers import base
 from ironic.drivers.modules.drac import common as drac_common
 from ironic.drivers.modules.drac import job as drac_job
@@ -37,6 +38,7 @@ from ironic.tests.unit.drivers.modules.drac import utils as test_utils
 from ironic.tests.unit.objects import utils as obj_utils
 
 sushy = importutils.try_import('sushy')
+sushy_oem_idrac = importutils.try_import('sushy_oem_idrac')
 
 INFO_DICT = test_utils.INFO_DICT
 
@@ -2360,3 +2362,91 @@ class DracRedfishRAIDTestCase(test_utils.BaseDracTest):
         task = mock.Mock(node=self.node, context=self.context)
         self.node.properties['vendor'] = 'Dell Inc.'
         self.raid.validate(task)
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_pre_create_configuration(self, mock_get_system):
+        mock_task_mon1 = mock.Mock(check_is_processing=True)
+        mock_task_mon2 = mock.Mock(check_is_processing=False)
+        fake_oem_system = mock.Mock()
+        fake_oem_system.change_physical_disk_state.return_value = [
+            mock_task_mon1, mock_task_mon2]
+        fake_system = mock.Mock()
+        fake_system.get_oem_extension.return_value = fake_oem_system
+
+        mock_drive1 = mock.Mock(
+            identity='Disk.Bay.0:Enclosure.Internal.0-1:RAID.Integrated.1-1')
+        mock_drive2 = mock.Mock(
+            identity='Disk.Bay.1:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+            capacity_bytes=599550590976)  # mocked size in RAID mode
+        mock_drive3 = mock.Mock(
+            identity='Disk.Direct.0-0:AHCI.Slot.2-1')
+
+        mock_controller1 = mock.Mock()
+        mock_storage1 = mock.Mock(storage_controllers=[mock_controller1],
+                                  drives=[mock_drive1, mock_drive2],
+                                  identity='RAID.Integrated.1-1')
+        mock_controller2 = mock.Mock()
+        mock_storage2 = mock.Mock(storage_controllers=[mock_controller2],
+                                  drives=[mock_drive3],
+                                  identity='AHCI.Slot.2-1')
+
+        fake_system.storage.get_members.return_value = [
+            mock_storage1, mock_storage2]
+
+        mock_get_system.return_value = fake_system
+        task = mock.Mock(node=self.node, context=self.context)
+
+        logical_disks_to_create = [{
+            'raid_level': '0',
+            'size_bytes': 600087461888,  # before RAID conversion
+            'physical_disks': [
+                'Disk.Bay.1:Enclosure.Internal.0-1:RAID.Integrated.1-1'],
+            'span_depth': 1,
+            'span_length': 1.0,
+            'controller': 'RAID.Integrated.1-1'}]
+
+        result = self.raid.pre_create_configuration(
+            task, logical_disks_to_create)
+
+        self.assertEqual(
+            [{'controller': 'RAID.Integrated.1-1',
+              'physical_disks': [
+                  'Disk.Bay.1:Enclosure.Internal.0-1:RAID.Integrated.1-1'],
+              'raid_level': '0',
+              'size_bytes': 599550590976,  # recalculated after RAID conversion
+              'span_depth': 1,
+              'span_length': 1.0}], result)
+        fake_oem_system.change_physical_disk_state.assert_called_once_with(
+            sushy_oem_idrac.PHYSICAL_DISK_STATE_MODE_RAID,
+            {mock_controller1: [mock_drive2]})
+        mock_task_mon1.wait.assert_called_once_with(CONF.drac.raid_job_timeout)
+        mock_task_mon2.wait.assert_not_called()
+
+    def test__get_storage_controller_invalid_identity(self):
+        fake_system = mock.Mock()
+
+        mock_storage1 = mock.Mock(storage_controllers=[mock.Mock()],
+                                  identity='RAID.Integrated.1-1')
+        mock_storage2 = mock.Mock(storage_controllers=[mock.Mock()],
+                                  identity='AHCI.Slot.2-1')
+
+        fake_system.storage.get_members.return_value = [
+            mock_storage1, mock_storage2]
+
+        self.assertRaises(
+            exception.IronicException,
+            drac_raid.DracRedfishRAID._get_storage_controller,
+            fake_system, 'NonExisting')
+
+    @mock.patch.object(drac_raid.LOG, 'warning', autospec=True)
+    def test__change_physical_disk_state_attribute_error(self, mock_log):
+        fake_oem_system = mock.Mock(spec=[])
+        fake_system = mock.Mock()
+        fake_system.get_oem_extension.return_value = fake_oem_system
+
+        result = drac_raid.DracRedfishRAID._change_physical_disk_state(
+            fake_system, sushy_oem_idrac.PHYSICAL_DISK_STATE_MODE_RAID
+        )
+
+        self.assertEqual(False, result)
+        mock_log.assert_called_once()

ironic/tests/unit/drivers/third_party_driver_mock_specs.py

@@ -1,5 +1,6 @@
 # Copyright 2015 Intel Corporation
 # All Rights Reserved.
+# Copyright (c) 2021 Dell Inc. or its subsidiaries.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
@@ -46,6 +47,12 @@ DRACCLIENT_CONSTANTS_RAID_STATUS_MOD_SPEC = (
     'raid'
 )
 
+# sushy_oem_idrac
+SUSHY_OEM_IDRAC_MOD_SPEC = (
+    'PHYSICAL_DISK_STATE_MODE_RAID',
+    'PHYSICAL_DISK_STATE_MODE_NONRAID',
+)
+
 # proliantutils
 PROLIANTUTILS_SPEC = (
     'exception',

ironic/tests/unit/drivers/third_party_driver_mocks.py

@@ -1,5 +1,6 @@
 # Copyright 2014 Hewlett-Packard Development Company, L.P.
 # All Rights Reserved.
+# Copyright (c) 2021 Dell Inc. or its subsidiaries.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
@@ -27,6 +28,7 @@ Current list of mocked libraries:
 - scciclient
 - python-dracclient
 - python-ibmcclient
+- sushy_oem_idrac
 """
 
 import importlib
@@ -122,6 +124,17 @@ if not dracclient:
     if 'ironic.drivers.modules.drac' in sys.modules:
         importlib.reload(sys.modules['ironic.drivers.modules.drac'])
 
+sushy_oem_idrac = importutils.try_import('sushy_oem_idrac')
+if not sushy_oem_idrac:
+    raidmode = mock.sentinel.PHYSICAL_DISK_STATE_MODE_RAID
+    nonraidmode = mock.sentinel.PHYSICAL_DISK_STATE_MODE_NONRAID
+    sushy_oem_idrac = mock.MagicMock(
+        spec_set=mock_specs.SUSHY_OEM_IDRAC_MOD_SPEC,
+        PHYSICAL_DISK_STATE_MODE_RAID=raidmode,
+        PHYSICAL_DISK_STATE_MODE_NONRAID=nonraidmode
+    )
+
+    sys.modules['sushy_oem_idrac'] = sushy_oem_idrac
 
 # attempt to load the external 'pysnmp' library, which is required by
 # the optional drivers.modules.snmp module

releasenotes/notes/idrac-redfish-raid-convert-from-nonraid-e9b5bbac89c71537.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes ``idrac-redfish`` RAID interface in ``create_configuration`` clean
+    step and ``apply_configuration`` deploy step when there are drives in
+    non-RAID mode. With this fix, non-RAID drives are converted to RAID mode
+    before creating virtual disks.