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