diff --git a/ironic/conf/drac.py b/ironic/conf/drac.py index 83e60f3e6e..0770815678 100644 --- a/ironic/conf/drac.py +++ b/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')) ] diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index 4bb41c1f67..eaf2fbad33 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/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): diff --git a/ironic/drivers/modules/redfish/raid.py b/ironic/drivers/modules/redfish/raid.py index f726d9d57a..9d0a947127 100644 --- a/ironic/drivers/modules/redfish/raid.py +++ b/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. diff --git a/ironic/tests/unit/drivers/modules/drac/test_raid.py b/ironic/tests/unit/drivers/modules/drac/test_raid.py index 84ac06af66..3a5c725544 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_raid.py +++ b/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() diff --git a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py index 87fb792dfc..e0aaf92ace 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py +++ b/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', diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py index cd1b6bb3e5..9db2f5ee9a 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mocks.py +++ b/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 diff --git a/releasenotes/notes/idrac-redfish-raid-convert-from-nonraid-e9b5bbac89c71537.yaml b/releasenotes/notes/idrac-redfish-raid-convert-from-nonraid-e9b5bbac89c71537.yaml new file mode 100644 index 0000000000..5a92c35a9e --- /dev/null +++ b/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.