From 6956b0619e26a9fc6bd482f4eed1901cfd4e3dd5 Mon Sep 17 00:00:00 2001 From: Paresh Sao Date: Wed, 6 Dec 2023 09:03:52 +0000 Subject: [PATCH] Fixes Raid creation in iLO6 and other BMC with latest schema This commit removes 'VolumeType' which param has long been deprecated in DMTF Redfish schema, also removes 'Encrypted' param as per discussion, and places 'Drives' inside 'Links' as per the new DMTF schema. Closes-Bug: 2045645 Change-Id: I91d2decab19e352ca3560227d17acfaa1a1dca94 --- ironic/drivers/modules/redfish/raid.py | 24 +-- .../unit/drivers/modules/redfish/test_raid.py | 193 +++++++++--------- ...sh-fix-raid-creation-f437066b1301c032.yaml | 6 + 3 files changed, 111 insertions(+), 112 deletions(-) create mode 100644 releasenotes/notes/redfish-fix-raid-creation-f437066b1301c032.yaml diff --git a/ironic/drivers/modules/redfish/raid.py b/ironic/drivers/modules/redfish/raid.py index b0a1358527..550b0f538f 100644 --- a/ironic/drivers/modules/redfish/raid.py +++ b/ironic/drivers/modules/redfish/raid.py @@ -43,7 +43,6 @@ RAID_LEVELS = { 'min_disks': 1, 'max_disks': 1000, 'type': 'simple', - 'volume_type': 'NonRedundant', 'raid_type': 'RAID0', 'overhead': 0 }, @@ -51,7 +50,6 @@ RAID_LEVELS = { 'min_disks': 2, 'max_disks': 2, 'type': 'simple', - 'volume_type': 'Mirrored', 'raid_type': 'RAID1', 'overhead': 1 }, @@ -59,7 +57,6 @@ RAID_LEVELS = { 'min_disks': 3, 'max_disks': 1000, 'type': 'simple', - 'volume_type': 'StripedWithParity', 'raid_type': 'RAID5', 'overhead': 1 }, @@ -67,25 +64,21 @@ RAID_LEVELS = { 'min_disks': 4, 'max_disks': 1000, 'type': 'simple', - 'volume_type': 'StripedWithParity', 'raid_type': 'RAID6', 'overhead': 2 }, '1+0': { 'type': 'spanned', - 'volume_type': 'SpannedMirrors', 'raid_type': 'RAID10', 'span_type': '1' }, '5+0': { 'type': 'spanned', - 'volume_type': 'SpannedStripesWithParity', 'raid_type': 'RAID50', 'span_type': '5' }, '6+0': { 'type': 'spanned', - 'volume_type': 'SpannedStripesWithParity', 'raid_type': 'RAID60', 'span_type': '6' } @@ -618,13 +611,15 @@ def _drive_path(storage, drive_id): def _construct_volume_payload( node, storage, raid_controller, physical_disks, raid_level, size_bytes, disk_name=None, span_length=None, span_depth=None): - payload = {'Encrypted': False, - 'VolumeType': RAID_LEVELS[raid_level]['volume_type'], - 'RAIDType': RAID_LEVELS[raid_level]['raid_type'], - 'CapacityBytes': size_bytes} + payload = { + 'RAIDType': RAID_LEVELS[raid_level]['raid_type'], + 'CapacityBytes': size_bytes + } if physical_disks: - payload['Drives'] = [{"@odata.id": _drive_path(storage, d)} for d in - physical_disks] + payload['Links'] = { + "Drives": [{"@odata.id": _drive_path(storage, d)} for d in + physical_disks] + } if disk_name: payload['Name'] = disk_name LOG.debug('Payload for RAID logical disk creation on node %(node_uuid)s: ' @@ -1152,8 +1147,7 @@ class RedfishRAID(base.RAIDInterface): controller_id = storage.identity iter_volumes = iter(storage.volumes.get_members()) for volume in iter_volumes: - if (volume.raid_type or volume.volume_type not in - [None, sushy.VOLUME_TYPE_RAW_DEVICE]): + if volume.raid_type: if controller_id not in vols_to_delete: vols_to_delete[controller_id] = [] apply_time = self._get_apply_time( diff --git a/ironic/tests/unit/drivers/modules/redfish/test_raid.py b/ironic/tests/unit/drivers/modules/redfish/test_raid.py index 44319933db..af90694256 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_raid.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_raid.py @@ -47,12 +47,11 @@ def _mock_drive(identity, block_size_bytes=None, capacity_bytes=None, ) -def _mock_volume(identity, volume_type=None, raid_type=None, +def _mock_volume(identity, raid_type=None, capacity_bytes=units.Gi, volume_name=None): volume = mock.MagicMock( _path='/redfish/v1/Systems/1/Storage/1/Volumes/' + identity, identity=identity, - volume_type=volume_type, raid_type=raid_type, capacity_bytes=capacity_bytes ) @@ -254,15 +253,15 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3} + ] + } } self.mock_storage.volumes.create.assert_called_once_with( expected_payload, apply_time=None @@ -318,15 +317,15 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3} - ], + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3} + ] + }, 'Name': 'test-volume' } self.mock_storage.volumes.create.assert_called_once_with( @@ -389,15 +388,15 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3} + ] + } } self.mock_storage.volumes.create.assert_called_once_with( expected_payload, apply_time=sushy.APPLY_TIME_ON_RESET) @@ -462,25 +461,25 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload1 = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id5}, - {'@odata.id': pre + self.drive_id6}, - {'@odata.id': pre + self.drive_id7} - ] + '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, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2} + ] + } } self.assertEqual( self.mock_storage.volumes.create.call_count, 2) @@ -552,14 +551,14 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'Mirrored', 'RAIDType': 'RAID1', 'CapacityBytes': 536870912000, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2} + ] + } } expected_raid_configs = { 'operation': 'create', @@ -625,15 +624,15 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3} + ] + } } self.mock_storage.volumes.create.assert_called_once_with( expected_payload, apply_time=None @@ -701,10 +700,10 @@ class RedfishRAIDTestCase(db_base.DbTestCase): created_volumes = [ _mock_volume( '1', raid_type=sushy.RAIDType.RAID10, - capacity_bytes=50 * units.Gi, volume_name='root_volume'), + capacity_bytes=50 * units.Gi), _mock_volume( '2', raid_type=sushy.RAIDType.RAID5, - capacity_bytes=100 * units.Gi, volume_name='data_volume')] + capacity_bytes=100 * units.Gi)] # Called after volumes created self.mock_storage.volumes.get_members.return_value = created_volumes mock_get_system.return_value.storage.get_members.return_value = [ @@ -716,28 +715,28 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload1 = { - 'Encrypted': False, - 'VolumeType': 'SpannedMirrors', 'RAIDType': 'RAID10', 'CapacityBytes': 53687091200, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3}, - {'@odata.id': pre + self.drive_id4} - ], + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3}, + {'@odata.id': pre + self.drive_id4} + ] + }, 'Name': 'root_volume' } expected_payload2 = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id2}, - {'@odata.id': pre + self.drive_id3}, - {'@odata.id': pre + self.drive_id4} - ], + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id2}, + {'@odata.id': pre + self.drive_id3}, + {'@odata.id': pre + self.drive_id4} + ] + }, 'Name': 'data_volume' } self.assertEqual( @@ -752,12 +751,12 @@ class RedfishRAIDTestCase(db_base.DbTestCase): self.assertEqual( [{'controller': 'RAID controller 1', 'id': '1', - 'name': 'root_volume', + 'name': 'Volume 1', 'raid_level': '1+0', 'size_gb': 50}, {'controller': 'RAID controller 1', 'id': '2', - 'name': 'data_volume', + 'name': 'Volume 2', 'raid_level': '5', 'size_gb': 100}], task.node.raid_config['logical_disks']) @@ -842,23 +841,23 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload1 = { - 'Encrypted': False, - 'VolumeType': 'Mirrored', 'RAIDType': 'RAID1', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2} + ] + } } expected_payload2 = { - 'Encrypted': False, - 'VolumeType': 'NonRedundant', 'RAIDType': 'RAID0', 'CapacityBytes': 536870912000, - 'Drives': [ - {'@odata.id': pre + self.drive_id3} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id3} + ] + } } self.assertEqual( self.mock_storage.volumes.create.call_count, 2) @@ -961,25 +960,25 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.driver.raid.create_configuration(task) pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload1 = { - 'Encrypted': False, - 'VolumeType': 'StripedWithParity', 'RAIDType': 'RAID5', 'CapacityBytes': 107374182400, - 'Drives': [ - {'@odata.id': pre + self.drive_id5}, - {'@odata.id': pre + self.drive_id6}, - {'@odata.id': pre + self.drive_id7} - ] + '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, - 'Drives': [ - {'@odata.id': pre + self.drive_id1}, - {'@odata.id': pre + self.drive_id2} - ] + 'Links': { + 'Drives': [ + {'@odata.id': pre + self.drive_id1}, + {'@odata.id': pre + self.drive_id2} + ] + } } self.assertEqual( self.mock_storage.volumes.create.call_count, 2) @@ -1020,7 +1019,7 @@ class RedfishRAIDTestCase(db_base.DbTestCase): mock_volumes = [] for i in ["1", "2"]: mock_volumes.append(_mock_volume( - i, volume_type='Mirrored', raid_type=sushy.RAIDType.RAID1)) + i, raid_type=sushy.RAIDType.RAID1)) op_apply_time_support = mock.MagicMock() op_apply_time_support.mapped_supported_values = [ sushy.APPLY_TIME_IMMEDIATE, sushy.APPLY_TIME_ON_RESET] @@ -1074,7 +1073,7 @@ class RedfishRAIDTestCase(db_base.DbTestCase): mock_volumes = [] for i in ["1", "2"]: mock_volumes.append(_mock_volume( - i, volume_type='Mirrored', raid_type=sushy.RAIDType.RAID1)) + i, raid_type=sushy.RAIDType.RAID1)) op_apply_time_support = mock.MagicMock() op_apply_time_support.mapped_supported_values = [ sushy.APPLY_TIME_ON_RESET] @@ -1129,11 +1128,11 @@ class RedfishRAIDTestCase(db_base.DbTestCase): capacity_bytes = 53739520000 pre = '/redfish/v1/Systems/1/Storage/1/Drives/' expected_payload = { - 'Encrypted': False, - 'VolumeType': 'Mirrored', 'RAIDType': 'RAID1', 'CapacityBytes': capacity_bytes, - 'Drives': [{'@odata.id': pre + drive_id}] + 'Links': { + 'Drives': [{'@odata.id': pre + drive_id}] + } } with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: diff --git a/releasenotes/notes/redfish-fix-raid-creation-f437066b1301c032.yaml b/releasenotes/notes/redfish-fix-raid-creation-f437066b1301c032.yaml new file mode 100644 index 0000000000..ae131ec6d2 --- /dev/null +++ b/releasenotes/notes/redfish-fix-raid-creation-f437066b1301c032.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes Raid creation issue in iLO6 and other BMC with latest schema by + removing 'VolumeType', 'Encrypted' and changing placement of 'Drives' + to inside 'Links'.