Fix drac implementation of set_boot_device

The drac implementation of set_boot_device was not recognizing when
it did not actually need to change the boot order, which caused it to
try running a config job when there was nothing to do.

This adds a check for that condition, and exits early if there is
nothing to do.

Closes-Bug: 1454259
Change-Id: I48971c79ba3d745056ff3d3157fde08b3d566f94

2 files changed, 42 insertions, 2 deletions

diff --git a/ironic/drivers/modules/drac/management.py b/ironic/drivers/modules/drac/management.py
index 3451147d2..51c27adce 100644
--- a/ironic/drivers/modules/drac/management.py
+++ b/ironic/drivers/modules/drac/management.py
@@ -224,6 +224,13 @@ class DracManagement(base.ManagementInterface):
         :raises: DracPendingConfigJobExists on an error when creating the job.
 
         """
+
+        # If we are already booting from the right device, do nothing.
+        if self.get_boot_device(task) == {'boot_device': device,
+                                          'persistent': persistent}:
+            LOG.debug('DRAC already set to boot from %s', device)
+            return
+
         # Check for an existing configuration job
         _check_for_config_job(task.node)
 
diff --git a/ironic/tests/drivers/drac/test_management.py b/ironic/tests/drivers/drac/test_management.py
index ea1051de3..efa4d85ec 100644
--- a/ironic/tests/drivers/drac/test_management.py
+++ b/ironic/tests/drivers/drac/test_management.py
@@ -229,11 +229,16 @@ class DracManagementTestCase(db_base.DbTestCase):
             mock.ANY, resource_uris.DCIM_BootSourceSetting,
             filter_query=mock.ANY)
 
+    @mock.patch.object(drac_mgmt.DracManagement, 'get_boot_device',
+                       spec_set=True, autospec=True)
     @mock.patch.object(drac_mgmt, '_check_for_config_job', spec_set=True,
                        autospec=True)
     @mock.patch.object(drac_mgmt, '_create_config_job', spec_set=True,
                        autospec=True)
-    def test_set_boot_device(self, mock_ccj, mock_cfcj, mock_client_pywsman):
+    def test_set_boot_device(self, mock_ccj, mock_cfcj, mock_gbd,
+                             mock_client_pywsman):
+        mock_gbd.return_value = {'boot_device': boot_devices.PXE,
+                                 'persistent': True}
         result_xml_enum = test_utils.build_soap_xml([{'InstanceID': 'NIC'}],
                                       resource_uris.DCIM_BootSourceSetting)
         result_xml_invk = test_utils.build_soap_xml(
@@ -258,15 +263,20 @@ class DracManagementTestCase(db_base.DbTestCase):
             resource_uris.DCIM_BootConfigSetting,
             'ChangeBootOrderByInstanceID',
             None)
+        mock_gbd.assert_called_once_with(self.driver, task)
         mock_cfcj.assert_called_once_with(self.node)
         mock_ccj.assert_called_once_with(self.node)
 
+    @mock.patch.object(drac_mgmt.DracManagement, 'get_boot_device',
+                       spec_set=True, autospec=True)
     @mock.patch.object(drac_mgmt, '_check_for_config_job', spec_set=True,
                        autospec=True)
     @mock.patch.object(drac_mgmt, '_create_config_job', spec_set=True,
                        autospec=True)
-    def test_set_boot_device_fail(self, mock_ccj, mock_cfcj,
+    def test_set_boot_device_fail(self, mock_ccj, mock_cfcj, mock_gbd,
                                   mock_client_pywsman):
+        mock_gbd.return_value = {'boot_device': boot_devices.PXE,
+                                 'persistent': True}
         result_xml_enum = test_utils.build_soap_xml([{'InstanceID': 'NIC'}],
                                       resource_uris.DCIM_BootSourceSetting)
         result_xml_invk = test_utils.build_soap_xml(
@@ -291,15 +301,21 @@ class DracManagementTestCase(db_base.DbTestCase):
             resource_uris.DCIM_BootConfigSetting,
             'ChangeBootOrderByInstanceID',
             None)
+        mock_gbd.assert_called_once_with(self.driver, task)
         mock_cfcj.assert_called_once_with(self.node)
         self.assertFalse(mock_ccj.called)
 
+    @mock.patch.object(drac_mgmt.DracManagement, 'get_boot_device',
+                       spec_set=True, autospec=True)
     @mock.patch.object(drac_client.Client, 'wsman_enumerate', spec_set=True,
                        autospec=True)
     @mock.patch.object(drac_mgmt, '_check_for_config_job', spec_set=True,
                        autospec=True)
     def test_set_boot_device_client_error(self, mock_cfcj, mock_we,
+                                          mock_gbd,
                                           mock_client_pywsman):
+        mock_gbd.return_value = {'boot_device': boot_devices.PXE,
+                                 'persistent': True}
         mock_we.side_effect = exception.DracClientError('E_FAKE')
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
@@ -307,10 +323,27 @@ class DracManagementTestCase(db_base.DbTestCase):
             self.assertRaises(exception.DracClientError,
                               self.driver.set_boot_device, task,
                               boot_devices.PXE)
+        mock_gbd.assert_called_once_with(self.driver, task)
         mock_we.assert_called_once_with(
             mock.ANY, resource_uris.DCIM_BootSourceSetting,
             filter_query=mock.ANY)
 
+    @mock.patch.object(drac_mgmt.DracManagement, 'get_boot_device',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(drac_mgmt, '_check_for_config_job', spec_set=True,
+                       autospec=True)
+    def test_set_boot_device_noop(self, mock_cfcj, mock_gbd,
+                                  mock_client_pywsman):
+        mock_gbd.return_value = {'boot_device': boot_devices.PXE,
+                                 'persistent': False}
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node = self.node
+            result = self.driver.set_boot_device(task, boot_devices.PXE)
+        self.assertIsNone(result)
+        mock_gbd.assert_called_once_with(self.driver, task)
+        self.assertFalse(mock_cfcj.called)
+
     def test_get_sensors_data(self, mock_client_pywsman):
         self.assertRaises(NotImplementedError,
                           self.driver.get_sensors_data, self.task)