Follow-up on refactor DRAC management interface

This is a follow-up on "DRAC: switch to python-dracclient on management
interface" with change-id: I297514d14ad71289f21e390ac60c2d2a89850f15.

Change-Id: I2e1ae8f712ab30ea438cbf68651dd50a8323d19d

ironic/drivers/modules/drac/management.py

@@ -98,26 +98,33 @@ def set_boot_device(node, device, persistent=False):
     drac_job.validate_job_queue(node)
 
     client = drac_common.get_drac_client(node)
-    drac_boot_devices = client.list_boot_devices()
 
-    current_boot_device = _get_boot_device(node, drac_boot_devices)
-    # If we are already booting from the right device, do nothing.
-    if current_boot_device == {'boot_device': device,
-                               'persistent': persistent}:
-        LOG.debug('DRAC already set to boot from %s', device)
-        return
+    try:
+        drac_boot_devices = client.list_boot_devices()
 
-    drac_boot_device = next(drac_device.id for drac_device
-                            in drac_boot_devices[PERSISTENT_BOOT_MODE]
-                            if _BOOT_DEVICES_MAP[device] in drac_device.id)
+        current_boot_device = _get_boot_device(node, drac_boot_devices)
+        # If we are already booting from the right device, do nothing.
+        if current_boot_device == {'boot_device': device,
+                                   'persistent': persistent}:
+            LOG.debug('DRAC already set to boot from %s', device)
+            return
 
-    if persistent:
-        boot_list = PERSISTENT_BOOT_MODE
-    else:
-        boot_list = NON_PERSISTENT_BOOT_MODE
+        drac_boot_device = next(drac_device.id for drac_device
+                                in drac_boot_devices[PERSISTENT_BOOT_MODE]
+                                if _BOOT_DEVICES_MAP[device] in drac_device.id)
 
-    client.change_boot_device_order(boot_list, drac_boot_device)
-    client.commit_pending_bios_changes()
+        if persistent:
+            boot_list = PERSISTENT_BOOT_MODE
+        else:
+            boot_list = NON_PERSISTENT_BOOT_MODE
+
+        client.change_boot_device_order(boot_list, drac_boot_device)
+        client.commit_pending_bios_changes()
+    except drac_exceptions.BaseClientException as exc:
+        LOG.error(_LE('DRAC driver failed to change boot device order for '
+                      'node %(node_uuid)s. Reason: %(error)s.'),
+                  {'node_uuid': node.uuid, 'error': exc})
+        raise exception.DracOperationError(error=exc)
 
 
 # TODO(ifarkas): delete this during BIOS vendor_passthru refactor
@@ -246,11 +253,11 @@ class DracManagement(base.ManagementInterface):
         """
         node = task.node
 
-        if ('drac_boot_device' in node.driver_internal_info and
-                node.driver_internal_info['drac_boot_device'] is not None):
-            return node.driver_internal_info['drac_boot_device']
-        else:
-            return _get_boot_device(node)
+        boot_device = node.driver_internal_info.get('drac_boot_device')
+        if boot_device is not None:
+            return boot_device
+
+        return _get_boot_device(node)
 
     @task_manager.require_exclusive_lock
     def set_boot_device(self, task, device, persistent=False):

ironic/drivers/modules/drac/power.py

@@ -66,9 +66,8 @@ def _get_power_state(node):
 def _commit_boot_list_change(node):
     driver_internal_info = node.driver_internal_info
 
-    if ('drac_boot_device' in driver_internal_info and
-            driver_internal_info['drac_boot_device'] is not None):
-        boot_device = driver_internal_info['drac_boot_device']
+    boot_device = node.driver_internal_info.get('drac_boot_device')
+    if boot_device is not None:
         drac_management.set_boot_device(node, boot_device['boot_device'],
                                         boot_device['persistent'])
 
@@ -86,6 +85,14 @@ def _set_power_state(node, power_state):
     :raises: DracOperationError on an error from python-dracclient
     """
 
+    # NOTE(ifarkas): DRAC interface doesn't allow changing the boot device
+    #                multiple times in a row without a reboot. This is
+    #                because a change need to be committed via a
+    #                configuration job, and further configuration jobs
+    #                cannot be created until the previous one is processed
+    #                at the next boot. As a workaround, it is saved to
+    #                driver_internal_info during set_boot_device and committing
+    #                it here.
     _commit_boot_list_change(node)
 
     client = drac_common.get_drac_client(node)

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

@@ -133,6 +133,7 @@ class DracManagementInternalMethodsTestCase(db_base.DbTestCase):
         boot_device = drac_mgmt.set_boot_device(
             self.node, ironic.common.boot_devices.PXE, persistent=False)
 
+        mock_validate_job_queue.assert_called_once_with(self.node)
         mock_client.change_boot_device_order.assert_called_once_with(
             'OneTime', 'BIOS.Setup.1-1#BootSeq#NIC.Embedded.1-1-1')
         mock_client.commit_pending_bios_changes.assert_called_once_with()
@@ -159,6 +160,7 @@ class DracManagementInternalMethodsTestCase(db_base.DbTestCase):
         boot_device = drac_mgmt.set_boot_device(
             self.node, ironic.common.boot_devices.PXE, persistent=True)
 
+        mock_validate_job_queue.assert_called_once_with(self.node)
         self.assertEqual(0, mock_client.change_boot_device_order.call_count)
         self.assertEqual(0, mock_client.commit_pending_bios_changes.call_count)
 
@@ -251,9 +253,11 @@ class DracManagementTestCase(db_base.DbTestCase):
             expected_boot_device = {
                 'boot_device': ironic.common.boot_devices.DISK,
                 'persistent': True}
-            self.assertEqual(
-                task.node.driver_internal_info['drac_boot_device'],
-                expected_boot_device)
+
+        self.node.refresh()
+        self.assertEqual(
+            self.node.driver_internal_info['drac_boot_device'],
+            expected_boot_device)
 
     def test_set_boot_device_fail(self, mock_get_drac_client):
         with task_manager.acquire(self.context, self.node.uuid,