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.

Change-Id: I9d295135e6059a47cb541ae1218b11c3d56f85e1
Story: 2007711
Task: 39844

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
+
+    # 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[

ironic/tests/unit/drivers/modules/drac/test_raid.py

@@ -15,6 +15,7 @@
 Test class for DRAC RAID interface
 """
 
+from collections import defaultdict
 from unittest import mock
 
 from dracclient import constants
@@ -284,6 +285,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 +417,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 +1094,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 +1107,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 +1128,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 +1849,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 +1857,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 +1886,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 +1916,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_validate_job_queue, mock_list_raid_settings,
-            mock_get_drac_client):
+            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 +1951,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 +1998,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)

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)

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 <https://storyboard.openstack.org/#!/story/2007711>`_
+    for more details