Merge "DRAC: Fix a bug for job creation when only required" into stable/train13.0.1

3 files changed, 60 insertions, 1 deletions

diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py
index 4ed40e62e..bcddee510 100644
--- a/ironic/drivers/modules/drac/raid.py
+++ b/ironic/drivers/modules/drac/raid.py
@@ -788,6 +788,10 @@ def _commit_to_controllers(node, controllers, substep="completed"):
     if 'raid_config_job_ids' not in driver_internal_info:
         driver_internal_info['raid_config_job_ids'] = []
 
+    # remove controller which does not require configuration job
+    controllers = [controller for controller in controllers
+                   if controller['is_commit_required']]
+
     all_realtime = True
     optional = drac_constants.RebootRequired.optional
     for controller in controllers:
@@ -798,7 +802,6 @@ def _commit_to_controllers(node, controllers, substep="completed"):
         # controller without real time support. In that case the reboot
         # is triggered when the configuration is committed to the last
         # controller.
-
         realtime = controller['is_reboot_required'] == optional
         all_realtime = all_realtime and realtime
         if controller == controllers[-1]:
@@ -960,6 +963,8 @@ class DracWSManRAID(base.RAIDInterface):
             controller['raid_controller'] = logical_disk['controller']
             controller['is_reboot_required'] = controller_cap[
                 'is_reboot_required']
+            controller['is_commit_required'] = controller_cap[
+                'is_commit_required']
             if controller not in controllers:
                 controllers.append(controller)
 
@@ -1176,6 +1181,8 @@ class DracWSManRAID(base.RAIDInterface):
                 controller["raid_controller"] = cntrl.id
                 controller["is_reboot_required"] = controller_cap[
                     "is_reboot_required"]
+                controller["is_commit_required"] = controller_cap[
+                    "is_commit_required"]
                 controllers.append(controller)
         return controllers
 
diff --git a/ironic/tests/unit/drivers/modules/drac/test_raid.py b/ironic/tests/unit/drivers/modules/drac/test_raid.py
index f609858d5..d0246f382 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_raid.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_raid.py
@@ -323,6 +323,50 @@ class DracManageVirtualDisksTestCase(test_utils.BaseDracTest):
             exception.DracOperationError, drac_raid.commit_config, self.node,
             'controller1')
 
+    @mock.patch.object(drac_raid, 'commit_config', spec_set=True,
+                       autospec=True)
+    def test__commit_to_controllers_with_config_job(self, mock_commit_config,
+                                                    mock_get_drac_client):
+        controllers = [{'is_reboot_required': 'true',
+                        'is_commit_required': True,
+                        'raid_controller': 'AHCI.Slot.3-1'}]
+        substep = "delete_foreign_config"
+
+        mock_client = mock.Mock()
+        mock_get_drac_client.return_value = mock_client
+        mock_commit_config.return_value = "42"
+        drac_raid._commit_to_controllers(self.node,
+                                         controllers=controllers,
+                                         substep=substep)
+
+        self.assertEqual(1, mock_commit_config.call_count)
+        self.assertEqual(['42'],
+                         self.node.driver_internal_info['raid_config_job_ids'])
+        self.assertEqual(substep,
+                         self.node.driver_internal_info['raid_config_substep'])
+
+    @mock.patch.object(drac_raid, 'commit_config', spec_set=True,
+                       autospec=True)
+    def test__commit_to_controllers_without_config_job(
+            self, mock_commit_config, mock_get_drac_client):
+        controllers = [{'is_reboot_required': 'true',
+                        'is_commit_required': False,
+                        'raid_controller': 'AHCI.Slot.3-1'}]
+        substep = "delete_foreign_config"
+
+        mock_client = mock.Mock()
+        mock_get_drac_client.return_value = mock_client
+        mock_commit_config.return_value = None
+        drac_raid._commit_to_controllers(self.node,
+                                         controllers=controllers,
+                                         substep=substep)
+
+        self.assertEqual(0, mock_commit_config.call_count)
+        self.assertEqual([],
+                         self.node.driver_internal_info['raid_config_job_ids'])
+        self.assertEqual(substep,
+                         self.node.driver_internal_info['raid_config_substep'])
+
     def test_abandon_config(self, mock_get_drac_client):
         mock_client = mock.Mock()
         mock_get_drac_client.return_value = mock_client
diff --git a/releasenotes/notes/fix-commit-to-controller-d26f083ac388a65e.yaml b/releasenotes/notes/fix-commit-to-controller-d26f083ac388a65e.yaml
new file mode 100644
index 000000000..7a37efd32
--- /dev/null
+++ b/releasenotes/notes/fix-commit-to-controller-d26f083ac388a65e.yaml
@@ -0,0 +1,8 @@
+fixes:
+  - |
+    Fixes a bug in the ``idrac`` hardware type where configuration job
+    for RAID ``delete_configuration`` cleaning step gets created even
+    when there are no virtual disks or hotspares/dedicated hotspares
+    present on any controller.
+    See bug `2006562 https://storyboard.openstack.org/#!/story/2006562`
+    for details.