From c41ed8414d6559535026f513c7e5e8a35824f549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aija=20Jaunt=C4=93va?= Date: Wed, 9 Jun 2021 04:57:24 -0400 Subject: [PATCH] Fix Redfish RAID interface_type physical disk hint Map between sushy's protocol and RAID config schema's interface_type constants. Not supporting `scsi` from RAID schema as Redfish does not support it in Protocol property. Change-Id: Ic042f0a87ff0723f313adb3888c24c5963624182 --- ironic/common/raid.py | 10 ++ ironic/drivers/modules/redfish/raid.py | 27 +++- .../unit/drivers/modules/redfish/test_raid.py | 123 +++++++++++++++++- .../drivers/third_party_driver_mock_specs.py | 3 + .../unit/drivers/third_party_driver_mocks.py | 3 + ...-raid-interface-type-4b3566b637cc2301.yaml | 5 + 6 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/fix-redfish-raid-interface-type-4b3566b637cc2301.yaml diff --git a/ironic/common/raid.py b/ironic/common/raid.py index ab60bd603d..fc4eb58e7b 100644 --- a/ironic/common/raid.py +++ b/ironic/common/raid.py @@ -22,6 +22,16 @@ from ironic.common.i18n import _ from ironic.common import utils +SATA = 'sata' +"Serial AT Attachment" + +SCSI = 'scsi' +"Small Computer System Interface" + +SAS = 'sas' +"Serial Attached SCSI" + + def _check_and_return_root_volumes(raid_config): """Returns root logical disks after validating RAID config. diff --git a/ironic/drivers/modules/redfish/raid.py b/ironic/drivers/modules/redfish/raid.py index d856eabe45..7a056935ac 100644 --- a/ironic/drivers/modules/redfish/raid.py +++ b/ironic/drivers/modules/redfish/raid.py @@ -22,6 +22,7 @@ from oslo_utils import units from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import raid from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils @@ -90,6 +91,12 @@ RAID_LEVELS = { sushy = importutils.try_import('sushy') +if sushy: + PROTOCOL_MAP = { + sushy.PROTOCOL_TYPE_SAS: raid.SAS, + sushy.PROTOCOL_TYPE_SATA: raid.SATA + } + def convert_drive_units(logical_disks, node): """Convert size in logical_disks from gb to bytes""" @@ -393,7 +400,7 @@ def _assign_disks_to_volume(logical_disks, physical_disks_by_type, continue if ('interface_type' in logical_disk and logical_disk['interface_type'].lower() - != protocol.lower()): + != PROTOCOL_MAP[protocol].lower()): continue # filter out disks without free disk space @@ -715,6 +722,24 @@ class RedfishRAID(base.RAIDInterface): self._validate_vendor(task) super(RedfishRAID, self).validate(task) + def validate_raid_config(self, task, raid_config): + """Validates the given RAID configuration. + + :param task: A TaskManager instance. + :param raid_config: The RAID configuration to validate. + :raises: InvalidParameterValue, if the RAID configuration is invalid. + """ + + super(RedfishRAID, self).validate_raid_config(task, raid_config) + + # Check if any interface_type is scsi that is not supported by Redfish + scsi_disks = ([x for x in raid_config['logical_disks'] + if x.get('interface_type') == raid.SCSI]) + + if len(scsi_disks) > 0: + raise exception.InvalidParameterValue( + _('interface type `scsi` not supported by Redfish RAID')) + @base.deploy_step(priority=0, argsinfo=base.RAID_APPLY_CONFIGURATION_ARGSINFO) def apply_configuration(self, task, raid_config, create_root_volume=True, diff --git a/ironic/tests/unit/drivers/modules/redfish/test_raid.py b/ironic/tests/unit/drivers/modules/redfish/test_raid.py index c246bdb82d..6645adf43d 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_raid.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_raid.py @@ -89,11 +89,13 @@ class RedfishRAIDTestCase(db_base.DbTestCase): self.drive_id4]: mock_drives.append(_mock_drive( identity=i, block_size_bytes=512, capacity_bytes=899527000000, - media_type='HDD', name='Drive', protocol='SAS')) + media_type='HDD', name='Drive', + protocol='Serial Attached SCSI')) for i in [self.drive_id5, self.drive_id6, self.drive_id7]: mock_drives.append(_mock_drive( identity=i, block_size_bytes=512, capacity_bytes=479559942144, - media_type='SSD', name='Solid State Drive', protocol='SATA')) + media_type='SSD', name='Solid State Drive', + protocol='Serial AT Attachment')) self.mock_storage.drives = mock_drives mock_identifier = mock.Mock() mock_identifier.durable_name = '345C59DBD970859C' @@ -767,6 +769,79 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) """ + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', + spec_set=True, autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'get_async_step_return_state', + autospec=True) + @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) + def test_create_config_interface_type( + self, + mock_set_async_step_flags, + mock_get_async_step_return_state, + mock_node_power_action, + mock_build_agent_options, + mock_prepare_ramdisk, + mock_get_system): + + target_raid_config = { + 'logical_disks': [ + { + 'size_gb': 100, + 'raid_level': '5', + 'is_root_volume': True, + 'interface_type': 'sata' + }, + { + 'size_gb': 500, + 'raid_level': '1', + 'interface_type': 'sas' + } + ] + } + mock_get_system.return_value.storage.get_members.return_value = [ + self.mock_storage] + self.node.target_raid_config = target_raid_config + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.raid.create_configuration(task) + pre = '/redfish/v1/Systems/1/Storage/1/Drives/' + expected_payload1 = { + 'Encrypted': False, + 'VolumeType': 'StripedWithParity', + 'RAIDType': 'RAID5', + 'CapacityBytes': 107374182400, + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id5}, + {'@odata.id': pre + self.drive_id6}, + {'@odata.id': pre + self.drive_id7} + ] + } + } + expected_payload2 = { + 'Encrypted': False, + 'VolumeType': 'Mirrored', + 'RAIDType': 'RAID1', + 'CapacityBytes': 536870912000, + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2} + ] + } + } + self.assertEqual( + self.mock_storage.volumes.create.call_count, 2) + self.mock_storage.volumes.create.assert_any_call( + expected_payload1, apply_time=None + ) + self.mock_storage.volumes.create.assert_any_call( + expected_payload2, apply_time=None + ) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @@ -900,6 +975,50 @@ class RedfishRAIDTestCase(db_base.DbTestCase): "with vendor Dell.Inc.", task.driver.raid.validate, task) + def test_validate_raid_config(self, mock_get_system): + raid_config = { + 'logical_disks': [ + { + 'size_gb': 500, + 'raid_level': '1+0', + 'interface_type': 'sata' + }, + { + 'size_gb': 100, + 'raid_level': '5', + 'is_root_volume': True, + } + ] + } + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.raid.validate_raid_config(task, raid_config) + + def test_validate_raid_config_scsi(self, mock_get_system): + raid_config = { + 'logical_disks': [ + { + 'size_gb': 500, + 'raid_level': '1+0', + 'interface_type': 'sata' + }, + { + 'size_gb': 100, + 'raid_level': '5', + 'is_root_volume': True, + 'interface_type': 'scsi' + } + ] + } + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.assertRaisesRegex( + exception.InvalidParameterValue, + "interface type `scsi` not supported by Redfish RAID", + task.driver.raid.validate_raid_config, task, raid_config) + def test_get_physical_disks(self, mock_get_system): nonraid_controller = mock.Mock() nonraid_controller.raid_types = [] 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 fa3cc373e9..43c89f206b 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py +++ b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py @@ -150,6 +150,9 @@ SUSHY_SPEC = ( 'PROCESSOR_ARCH_ARM', 'PROCESSOR_ARCH_MIPS', 'PROCESSOR_ARCH_OEM', + 'PROTOCOL_TYPE_iSCSI', + 'PROTOCOL_TYPE_SAS', + 'PROTOCOL_TYPE_SATA', 'STATE_ENABLED', 'STATE_DISABLED', 'STATE_ABSENT', diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py index 8274dca060..3c5e2cd7ab 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mocks.py +++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py @@ -212,6 +212,9 @@ if not sushy: PROCESSOR_ARCH_ARM='ARM', PROCESSOR_ARCH_MIPS='MIPS', PROCESSOR_ARCH_OEM='OEM-defined', + PROTOCOL_TYPE_iSCSI='Internet SCSI', + PROTOCOL_TYPE_SAS='Serial Attached SCSI', + PROTOCOL_TYPE_SATA='Serial AT Attachment', STATE_ENABLED='enabled', STATE_DISABLED='disabled', STATE_ABSENT='absent', diff --git a/releasenotes/notes/fix-redfish-raid-interface-type-4b3566b637cc2301.yaml b/releasenotes/notes/fix-redfish-raid-interface-type-4b3566b637cc2301.yaml new file mode 100644 index 0000000000..3b12f127b2 --- /dev/null +++ b/releasenotes/notes/fix-redfish-raid-interface-type-4b3566b637cc2301.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes configuring Redfish RAID using ``interface_type`` when error "failed + to find matching physical disks for all logical disks" occurs.