Merge "DRAC: Fix a failure to create virtual disk" into stable/ussuri

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[

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)

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