From c3c6cda65ac71ed384b34a4eaf2d32627970b0ff Mon Sep 17 00:00:00 2001 From: Rachit7194 Date: Tue, 26 May 2020 03:57:31 -0400 Subject: [PATCH] DRAC: Fix a failure to create virtual disk PERC H740P controllers supports RAID mode and Enhanced HBA mode. When the controller is in Enhanced HBA, it creates single disk RAID0 virtual disks of NON-RAID physical disks. Hence the user's request for VD creation with supported RAID fails due to no available physical disk. This patch checks the Perc H740P controllers whether it supports enhanced HBA mode and if controllers are found in enhanced HBA mode it converts back to RAID mode. Conflicts: ironic/tests/unit/drivers/modules/drac/test_raid.py Change-Id: I9d295135e6059a47cb541ae1218b11c3d56f85e1 Story: 2007711 Task: 39844 (cherry picked from commit 325f28043468472c2f97f659c66817192aba39f7) --- ironic/drivers/modules/drac/raid.py | 165 +++++++++++++++- .../unit/drivers/modules/drac/test_raid.py | 177 +++++++++++++++++- .../tests/unit/drivers/modules/drac/utils.py | 5 + ...rac-add-ehba-support-10b90c92b8865364.yaml | 15 ++ 4 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/idrac-add-ehba-support-10b90c92b8865364.yaml diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index d4624e269f..4b6cfd6017 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/ironic/drivers/modules/drac/raid.py @@ -42,6 +42,11 @@ LOG = logging.getLogger(__name__) METRICS = metrics_utils.get_metrics_logger(__name__) +_CURRENT_RAID_CONTROLLER_MODE = "RAIDCurrentControllerMode" +_REQUESTED_RAID_CONTROLLER_MODE = "RAIDRequestedControllerMode" +_EHBA_MODE = "Enhanced HBA" +_RAID_MODE = "RAID" + RAID_LEVELS = { '0': { 'min_disks': 1, @@ -310,6 +315,71 @@ def clear_foreign_config(node, raid_controller): raise exception.DracOperationError(error=exc) +def set_raid_settings(node, controller_fqdd, settings): + """Sets the RAID configuration + + It sets the pending_value parameter for each of the attributes + passed in. For the values to be applied, a config job must + be created. + + :param node: an ironic node object. + :param controller_fqdd: the ID of the RAID controller. + :param settings: a dictionary containing the proposed values, with + each key being the name of attribute and the value + being the proposed value. + :returns: a dictionary containing: + - The is_commit_required key with a boolean value indicating + whether a config job must be created for the values to be + applied. + - The is_reboot_required key with a RebootRequired enumerated + value indicating whether the server must be rebooted for the + values to be applied. Possible values are true and false. + :raises: DRACOperationFailed on error reported back by the DRAC + interface + """ + try: + + drac_job.validate_job_queue(node) + + client = drac_common.get_drac_client(node) + return client.set_raid_settings(controller_fqdd, settings) + except drac_exceptions.BaseClientException as exc: + LOG.error('DRAC driver failed to set raid settings ' + 'on %(raid_controller_fqdd)s ' + 'for node %(node_uuid)s. ' + 'Reason: %(error)s.', + {'raid_controller_fqdd': controller_fqdd, + 'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) + + +def list_raid_settings(node): + """List the RAID configuration settings + + :param node: an ironic node object. + :returns: a dictionary with the RAID settings using InstanceID as the + key. The attributes are RAIDEnumerableAttribute, + RAIDStringAttribute and RAIDIntegerAttribute objects. + :raises: DRACOperationFailed on error reported back by the DRAC + interface + """ + try: + + drac_job.validate_job_queue(node) + + client = drac_common.get_drac_client(node) + return client.list_raid_settings() + except drac_exceptions.BaseClientException as exc: + LOG.error('DRAC driver failed to list raid settings' + 'on %(raid_controller_fqdd)s ' + 'for node %(node_uuid)s. ' + 'Reason: %(error)s.', + {'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) + + def change_physical_disk_state(node, mode=None, controllers_to_physical_disk_ids=None): """Convert disks RAID status @@ -874,6 +944,36 @@ def _validate_volume_size(node, logical_disks): return logical_disks +def _switch_to_raid_mode(node, controller_fqdd): + """Convert the controller mode from Enhanced HBA to RAID mode + + :param node: an ironic node object + :param controller_fqdd: the ID of the RAID controller. + :returns: a dictionary containing + - The raid_controller key with a ID of the + RAID controller value. + - The is_commit_required needed key with a + boolean value indicating whether a config job must be created + for the values to be applied. + - The is_reboot_required key with a RebootRequired enumerated + value indicating whether the server must be rebooted to + switch the controller mode to RAID. + """ + # wait for pending jobs to complete + drac_job.wait_for_job_completion(node) + + raid_attr = "{}:{}".format(controller_fqdd, + _REQUESTED_RAID_CONTROLLER_MODE) + settings = {raid_attr: _RAID_MODE} + settings_results = set_raid_settings( + node, controller_fqdd, settings) + controller = { + 'raid_controller': controller_fqdd, + 'is_reboot_required': settings_results['is_reboot_required'], + 'is_commit_required': settings_results['is_commit_required']} + return controller + + def _commit_to_controllers(node, controllers, substep="completed"): """Commit changes to RAID controllers on the node. @@ -918,8 +1018,17 @@ def _commit_to_controllers(node, controllers, substep="completed"): driver_internal_info['raid_config_job_ids'] = [] optional = drac_constants.RebootRequired.optional - all_realtime = all(cntlr['is_reboot_required'] == optional - for cntlr in controllers) + + # all realtime controllers + all_realtime = all( + (cntlr['is_reboot_required'] == optional) + and not(cntlr.get('is_ehba_mode')) + for cntlr in controllers) + + # check any controller with ehba mode + any_ehba_controllers = any( + cntrl.get('is_ehba_mode') is True for cntrl in controllers) + raid_config_job_ids = [] raid_config_parameters = [] if all_realtime: @@ -931,6 +1040,35 @@ def _commit_to_controllers(node, controllers, substep="completed"): raid_config_job_ids=raid_config_job_ids, raid_config_parameters=raid_config_parameters) + elif any_ehba_controllers: + commit_to_ehba_controllers = [] + for controller in controllers: + if controller.get('is_ehba_mode'): + job_details = _create_config_job( + node, controller=controller['raid_controller'], + reboot=False, realtime=True, + raid_config_job_ids=raid_config_job_ids, + raid_config_parameters=raid_config_parameters) + + ehba_controller = _switch_to_raid_mode( + node, controller['raid_controller']) + commit_to_ehba_controllers.append( + ehba_controller['raid_controller']) + else: + job_details = _create_config_job( + node, controller=controller['raid_controller'], + reboot=False, realtime=False, + raid_config_job_ids=raid_config_job_ids, + raid_config_parameters=raid_config_parameters) + + for controller in commit_to_ehba_controllers: + LOG.debug("Create job with Reboot to apply configuration " + "changes for ehba controllers") + job_details = _create_config_job( + node, controller=controller, + reboot=(controller == commit_to_ehba_controllers[-1]), + realtime=False, raid_config_job_ids=raid_config_job_ids, + raid_config_parameters=raid_config_parameters) else: for controller in controllers: mix_controller = controller['raid_controller'] @@ -996,6 +1134,23 @@ def _create_virtual_disks(task, node): return _commit_to_controllers(node, controllers) +def _controller_in_hba_mode(raid_settings, controller_fqdd): + controller_mode = raid_settings.get( + '{}:{}'.format(controller_fqdd, _CURRENT_RAID_CONTROLLER_MODE)) + + return _EHBA_MODE in controller_mode.current_value + + +def _controller_supports_ehba_mode(settings, controller_fqdd): + raid_cntrl_attr = "{}:{}".format(controller_fqdd, + _CURRENT_RAID_CONTROLLER_MODE) + current_cntrl_mode = settings.get(raid_cntrl_attr) + if not current_cntrl_mode: + return False + else: + return _EHBA_MODE in current_cntrl_mode.possible_values + + def _get_disk_free_size_mb(disk, pending_delete): """Return the size of free space on the disk in MB. @@ -1363,9 +1518,15 @@ class DracWSManRAID(base.RAIDInterface): node = task.node controllers = list() drac_raid_controllers = list_raid_controllers(node) + drac_raid_settings = list_raid_settings(node) for cntrl in drac_raid_controllers: if _is_raid_controller(node, cntrl.id, drac_raid_controllers): controller = dict() + if _controller_supports_ehba_mode( + drac_raid_settings, + cntrl.id) and _controller_in_hba_mode( + drac_raid_settings, cntrl.id): + controller['is_ehba_mode'] = True controller_cap = _reset_raid_config(node, cntrl.id) controller["raid_controller"] = cntrl.id controller["is_reboot_required"] = controller_cap[ diff --git a/ironic/tests/unit/drivers/modules/drac/test_raid.py b/ironic/tests/unit/drivers/modules/drac/test_raid.py index 1d2e3c0fbb..48f38a09b5 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_raid.py +++ b/ironic/tests/unit/drivers/modules/drac/test_raid.py @@ -15,9 +15,11 @@ Test class for DRAC RAID interface """ +from collections import defaultdict +from unittest import mock + from dracclient import constants from dracclient import exceptions as drac_exceptions -import mock from ironic.common import exception from ironic.common import states @@ -284,6 +286,33 @@ class DracManageVirtualDisksTestCase(test_utils.BaseDracTest): exception.DracOperationError, drac_raid.clear_foreign_config, self.node, 'RAID.Integrated.1-1') + @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, + autospec=True) + def test_set_raid_settings(self, mock_validate_job_queue, + mock_get_drac_client): + mock_client = mock.Mock() + mock_get_drac_client.return_value = mock_client + controller_fqdd = "RAID.Integrated.1-1" + raid_cntrl_attr = "RAID.Integrated.1-1:RAIDRequestedControllerMode" + raid_settings = {raid_cntrl_attr: 'RAID'} + drac_raid.set_raid_settings(self.node, controller_fqdd, raid_settings) + + mock_validate_job_queue.assert_called_once_with( + self.node) + mock_client.set_raid_settings.assert_called_once_with( + controller_fqdd, raid_settings) + + @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, + autospec=True) + def test_list_raid_settings(self, mock_validate_job_queue, + mock_get_drac_client): + mock_client = mock.Mock() + mock_get_drac_client.return_value = mock_client + drac_raid.list_raid_settings(self.node) + mock_validate_job_queue.assert_called_once_with( + self.node) + mock_client.list_raid_settings.assert_called_once_with() + @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, autospec=True) def test_change_physical_disk_state(self, @@ -389,6 +418,7 @@ class DracManageVirtualDisksTestCase(test_utils.BaseDracTest): mock_get_drac_client): controllers = [{'is_reboot_required': 'true', 'is_commit_required': True, + 'is_ehba_mode': False, 'raid_controller': 'AHCI.Slot.3-1'}] substep = "delete_foreign_config" @@ -1065,6 +1095,7 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): autospec=True) @mock.patch.object(drac_raid, '_reset_raid_config', autospec=True) @mock.patch.object(drac_raid, 'list_virtual_disks', autospec=True) + @mock.patch.object(drac_raid, 'list_raid_settings', autospec=True) @mock.patch.object(drac_raid, 'list_physical_disks', autospec=True) @mock.patch.object(drac_raid, 'change_physical_disk_state', spec_set=True, autospec=True) @@ -1077,6 +1108,7 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): mock_validate_job_queue, mock_change_physical_disk_state, mock_list_physical_disks, + mock_list_raid_settings, mock_list_virtual_disks, mock__reset_raid_config, mock_get_drac_client): @@ -1097,6 +1129,18 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): 'supports_realtime': True} raid_controller = test_utils.make_raid_controller( raid_controller_dict) + + raid_attr = "RAID.Integrated.1-1:RAIDCurrentControllerMode" + raid_controller_config = { + 'id': 'RAID.Integrated.1-1:RAIDCurrentControllerMode', + 'current_value': ['RAID'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']} + raid_cntrl_settings = { + raid_attr: test_utils.create_raid_setting(raid_controller_config)} + + mock_list_raid_settings.return_value = raid_cntrl_settings mock_list_physical_disks.return_value = physical_disks mock_commit_config.side_effect = ['12'] mock_client.list_raid_controllers.return_value = [raid_controller] @@ -1806,6 +1850,7 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): autospec=True) @mock.patch.object(drac_raid, '_reset_raid_config', autospec=True) @mock.patch.object(drac_raid, 'list_raid_controllers', autospec=True) + @mock.patch.object(drac_raid, 'list_raid_settings', autospec=True) @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, autospec=True) @mock.patch.object(drac_raid, 'commit_config', spec_set=True, @@ -1813,11 +1858,23 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): def _test_delete_configuration(self, expected_state, mock_commit_config, mock_validate_job_queue, + mock_list_raid_settings, mock_list_raid_controllers, mock__reset_raid_config, mock_get_drac_client): mock_client = mock.Mock() mock_get_drac_client.return_value = mock_client + raid_attr = "RAID.Integrated.1-1:RAIDCurrentControllerMode" + raid_controller_config = { + 'id': 'RAID.Integrated.1-1:RAIDCurrentControllerMode', + 'current_value': ['RAID'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']} + + raid_cntrl_settings = { + raid_attr: test_utils.create_raid_setting(raid_controller_config)} + raid_controller_dict = { 'id': 'RAID.Integrated.1-1', 'description': 'Integrated RAID Controller 1', @@ -1830,6 +1887,7 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): mock_list_raid_controllers.return_value = [ test_utils.make_raid_controller(raid_controller_dict)] + mock_list_raid_settings.return_value = raid_cntrl_settings mock_commit_config.return_value = '42' mock__reset_raid_config.return_value = { 'is_reboot_required': constants.RebootRequired.optional, @@ -1859,16 +1917,17 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): @mock.patch.object(drac_common, 'get_drac_client', spec_set=True, autospec=True) @mock.patch.object(drac_raid, 'list_raid_controllers', autospec=True) + @mock.patch.object(drac_raid, 'list_raid_settings', autospec=True) @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, autospec=True) @mock.patch.object(drac_raid, 'commit_config', spec_set=True, autospec=True) @mock.patch.object(drac_raid, '_reset_raid_config', spec_set=True, autospec=True) - def test_delete_configuration_with_non_realtime_controller( + def test_delete_configuration_with_mix_realtime_controller_in_raid_mode( self, mock__reset_raid_config, mock_commit_config, - mock_validate_job_queue, mock_list_raid_controllers, - mock_get_drac_client): + mock_validate_job_queue, mock_list_raid_settings, + mock_list_raid_controllers, mock_get_drac_client): mock_client = mock.Mock() mock_get_drac_client.return_value = mock_client expected_raid_config_params = ['AHCI.Slot.3-1', 'RAID.Integrated.1-1'] @@ -1893,6 +1952,25 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): test_utils.make_raid_controller(controller) for controller in mix_controllers] + raid_controller_config = [ + {'id': 'AHCI.Slot.3-1:RAIDCurrentControllerMode', + 'current_value': ['RAID'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']}, + {'id': 'RAID.Integrated.1-1:RAIDCurrentControllerMode', + 'current_value': ['RAID'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']}] + + raid_settings = defaultdict() + for sett in raid_controller_config: + raid_settings[sett.get('id')] = test_utils.create_raid_setting( + sett) + + mock_list_raid_settings.return_value = raid_settings + mock_commit_config.side_effect = ['42', '12'] mock__reset_raid_config.side_effect = [{ 'is_reboot_required': constants.RebootRequired.optional, @@ -1921,6 +1999,97 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest): self.assertEqual(['42', '12'], self.node.driver_internal_info['raid_config_job_ids']) + @mock.patch.object(drac_common, 'get_drac_client', spec_set=True, + autospec=True) + @mock.patch.object(drac_raid, 'list_raid_controllers', autospec=True) + @mock.patch.object(drac_raid, 'list_raid_settings', autospec=True) + @mock.patch.object(drac_job, 'list_unfinished_jobs', autospec=True) + @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True, + autospec=True) + @mock.patch.object(drac_raid, 'set_raid_settings', autospec=True) + @mock.patch.object(drac_raid, 'commit_config', spec_set=True, + autospec=True) + @mock.patch.object(drac_raid, '_reset_raid_config', spec_set=True, + autospec=True) + def test_delete_configuration_with_mix_realtime_controller_in_ehba_mode( + self, mock__reset_raid_config, mock_commit_config, + mock_set_raid_settings, mock_validate_job_queue, + mock_list_unfinished_jobs, mock_list_raid_settings, + mock_list_raid_controllers, mock_get_drac_client): + mock_client = mock.Mock() + mock_get_drac_client.return_value = mock_client + expected_raid_config_params = ['RAID.Integrated.1-1', 'AHCI.Slot.3-1'] + mix_controllers = [{'id': 'RAID.Integrated.1-1', + 'description': 'Integrated RAID Controller 1', + 'manufacturer': 'DELL', + 'model': 'PERC H740 Mini', + 'primary_status': 'unknown', + 'firmware_version': '50.5.0-1750', + 'bus': '3C', + 'supports_realtime': True}, + {'id': 'AHCI.Slot.3-1', + 'description': 'AHCI controller in slot 3', + 'manufacturer': 'DELL', + 'model': 'BOSS-S1', + 'primary_status': 'unknown', + 'firmware_version': '2.5.13.3016', + 'bus': '5E', + 'supports_realtime': False}] + + mock_list_raid_controllers.return_value = [ + test_utils.make_raid_controller(controller) for + controller in mix_controllers] + raid_controller_config = [ + {'id': 'RAID.Integrated.1-1:RAIDCurrentControllerMode', + 'current_value': ['Enhanced HBA'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']}, + {'id': 'AHCI.Slot.3-1:RAIDCurrentControllerMode', + 'current_value': ['RAID'], + 'read_only': True, + 'name': 'RAIDCurrentControllerMode', + 'possible_values': ['RAID', 'Enhanced HBA']}] + + raid_settings = defaultdict() + for sett in raid_controller_config: + raid_settings[sett.get('id')] = test_utils.create_raid_setting( + sett) + + mock_list_raid_settings.return_value = raid_settings + mock_list_unfinished_jobs.return_value = [] + mock_commit_config.side_effect = ['42', '12', '13'] + mock__reset_raid_config.side_effect = [{ + 'is_reboot_required': constants.RebootRequired.optional, + 'is_commit_required': True + }, { + 'is_reboot_required': constants.RebootRequired.true, + 'is_commit_required': True + }] + mock_set_raid_settings.return_value = { + 'is_reboot_required': constants.RebootRequired.true, + 'is_commit_required': True} + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + return_value = task.driver.raid.delete_configuration(task) + mock_commit_config.assert_has_calls( + [mock.call(mock.ANY, raid_controller='RAID.Integrated.1-1', + reboot=False, realtime=True), + mock.call(mock.ANY, raid_controller='AHCI.Slot.3-1', + reboot=False, realtime=False), + mock.call(mock.ANY, raid_controller='RAID.Integrated.1-1', + reboot=True, realtime=False)], + any_order=True) + + self.assertEqual(states.CLEANWAIT, return_value) + self.node.refresh() + self.assertEqual(expected_raid_config_params, + self.node.driver_internal_info[ + 'raid_config_parameters']) + self.assertEqual(['42', '12', '13'], + self.node.driver_internal_info['raid_config_job_ids']) + @mock.patch.object(drac_common, 'get_drac_client', spec_set=True, autospec=True) @mock.patch.object(drac_raid, 'list_raid_controllers', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/drac/utils.py b/ironic/tests/unit/drivers/modules/drac/utils.py index 44b92e77b0..bc248b2377 100644 --- a/ironic/tests/unit/drivers/modules/drac/utils.py +++ b/ironic/tests/unit/drivers/modules/drac/utils.py @@ -96,3 +96,8 @@ def make_physical_disk(physical_disk_dict): tuple_class = dracclient_raid.PhysicalDisk if dracclient_raid else None return dict_to_namedtuple(values=physical_disk_dict, tuple_class=tuple_class) + + +def create_raid_setting(raid_settings_dict): + """Returns the raid configuration tuple object""" + return dict_to_namedtuple(values=raid_settings_dict) diff --git a/releasenotes/notes/idrac-add-ehba-support-10b90c92b8865364.yaml b/releasenotes/notes/idrac-add-ehba-support-10b90c92b8865364.yaml new file mode 100644 index 0000000000..baf41b3e1e --- /dev/null +++ b/releasenotes/notes/idrac-add-ehba-support-10b90c92b8865364.yaml @@ -0,0 +1,15 @@ +fixes: + - | + Fixes the virtual disks creation by changing PERC H740P controller + mode from `Enhanced HBA` to `RAID` in delete_configuration clean + step. + PERC H740P controllers supports RAID mode and Enhanced HBA mode. + When the controller is in Enhanced HBA, it creates single disk + RAID0 virtual disks of NON-RAID physical disks. + Hence the request for VD creation with supported RAID + fails due to no available physical disk. + This patch converts the PERC H740P RAID controllers to RAID mode + if enhanced HBA mode found enabled + See bug + `bug 2007711 `_ + for more details