Secure boot support for pxe_ilo driver

This patch implements secure boot support for pxe_ilo driver.

Implements: blueprint uefi-secure-boot-pxe-ilo
Change-Id: Ibb19693f37ea692595169c1530bcec806909a8e6

ironic/drivers/modules/ilo/deploy.py

@@ -368,6 +368,24 @@ def _update_secure_boot_mode(task, mode):
                  {'mode': mode, 'node': task.node.uuid})
 
 
+def _disable_secure_boot_if_supported(task):
+    """Disables secure boot on node, does not throw if its not supported.
+
+    :param task: a TaskManager instance containing the node to act on.
+    :raises: IloOperationError, if some operation on iLO failed.
+    """
+    try:
+        _update_secure_boot_mode(task, False)
+    # We need to handle IloOperationNotSupported exception so that if
+    # the user has incorrectly specified the Node capability
+    # 'secure_boot' to a node that does not have that capability and
+    # attempted deploy. Handling this exception here, will help the
+    # user to tear down such a Node.
+    except exception.IloOperationNotSupported:
+        LOG.warn(_LW('Secure boot mode is not supported for node %s'),
+                 task.node.uuid)
+
+
 class IloVirtualMediaIscsiDeploy(base.DeployInterface):
 
     def get_properties(self):
@@ -453,16 +471,7 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface):
         :returns: deploy state DELETED.
         """
         manager_utils.node_power_action(task, states.POWER_OFF)
-        try:
+        _disable_secure_boot_if_supported(task)
-            _update_secure_boot_mode(task, False)
-        # We need to handle IloOperationNotSupported exception so that if
-        # the user has incorrectly specified the Node capability
-        # 'secure_boot' to a node that does not have that capability and
-        # attempted deploy. Handling this exception here, will help the
-        # user to tear down such a Node.
-        except exception.IloOperationNotSupported:
-            LOG.warn(_LW('Secure boot mode is not supported for node %s'),
-                     task.node.uuid)
         return states.DELETED
 
     def prepare(self, task):
@@ -532,16 +541,7 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
         :returns: states.DELETED
         """
         manager_utils.node_power_action(task, states.POWER_OFF)
-        try:
+        _disable_secure_boot_if_supported(task)
-            _update_secure_boot_mode(task, False)
-        # We need to handle IloOperationNotSupported exception so that if
-        # User had incorrectly specified the Node capability 'secure_boot'
-        # to a node that do not have such capability and attempted deploy.
-        # Handling this exception here, will help user to tear down such a
-        # Node.
-        except exception.IloOperationNotSupported:
-            LOG.warn(_LW('Secure boot mode is not supported for node %s'),
-                     task.node.uuid)
         return states.DELETED
 
     def prepare(self, task):
@@ -661,7 +661,7 @@ class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
         :raises: IloOperationError, if some operation on iLO failed.
         :raises: InvalidParameterValue, if some information is invalid.
         """
-        ilo_common.update_boot_mode(task)
+        _prepare_node_for_deploy(task)
 
         # Check if 'boot_option' is compatible with 'boot_mode' and image.
         # Whole disk image deploy is not supported in UEFI boot mode if
@@ -687,6 +687,19 @@ class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
         manager_utils.node_set_boot_device(task, boot_devices.PXE)
         return super(IloPXEDeploy, self).deploy(task)
 
+    @task_manager.require_exclusive_lock
+    def tear_down(self, task):
+        """Tear down a previous deployment on the task's node.
+
+        :param task: a TaskManager instance.
+        :returns: states.DELETED
+        """
+        # Powering off the Node before disabling secure boot. If the node is
+        # is in POST, disable secure boot will fail.
+        manager_utils.node_power_action(task, states.POWER_OFF)
+        _disable_secure_boot_if_supported(task)
+        return super(IloPXEDeploy, self).tear_down(task)
+
 
 class IloConsoleInterface(ipmitool.IPMIShellinaboxConsole):
     """A ConsoleInterface that uses ipmitool and shellinabox."""
@@ -718,9 +731,40 @@ class IloPXEVendorPassthru(iscsi_deploy.VendorPassthru):
 
     @base.passthru(['POST'])
     def pass_deploy_info(self, task, **kwargs):
-        manager_utils.node_set_boot_device(task, boot_devices.PXE, True)
+        LOG.debug('Pass deploy info for the deployment on node %s',
+                  task.node.uuid)
+        manager_utils.node_set_boot_device(task, boot_devices.PXE,
+                                           persistent=True)
+        # Set boot mode
+        ilo_common.update_boot_mode(task)
+        # Need to enable secure boot, if being requested
+        _update_secure_boot_mode(task, True)
+
         super(IloPXEVendorPassthru, self).pass_deploy_info(task, **kwargs)
 
+    @task_manager.require_exclusive_lock
+    def continue_deploy(self, task, **kwargs):
+        """Method invoked when deployed with the IPA ramdisk.
+
+        This method is invoked during a heartbeat from an agent when
+        the node is in wait-call-back state. This deploys the image on
+        the node and then configures the node to boot according to the
+        desired boot option (netboot or localboot).
+
+        :param task: a TaskManager object containing the node.
+        :param kwargs: the kwargs passed from the heartbeat method.
+        :raises: InstanceDeployFailure, if it encounters some error during
+            the deploy.
+        :raises: IloOperationError, if some operation on iLO failed.
+        """
+        LOG.debug('Continuing the deployment on node %s', task.node.uuid)
+        # Set boot mode
+        ilo_common.update_boot_mode(task)
+        # Need to enable secure boot, if being requested
+        _update_secure_boot_mode(task, True)
+
+        super(IloPXEVendorPassthru, self).continue_deploy(task, **kwargs)
+
 
 class VendorPassthru(agent_base_vendor.BaseAgentVendor):
     """Vendor-specific interfaces for iLO deploy drivers."""

ironic/tests/drivers/ilo/test_deploy.py

@@ -1395,24 +1395,24 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
                        autospec=True)
-    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
+    @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
     def test_prepare(self,
-                     update_boot_mode_mock,
+                     prepare_node_mock,
                      pxe_prepare_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.properties['capabilities'] = 'boot_mode:uefi'
             task.driver.deploy.prepare(task)
-            update_boot_mode_mock.assert_called_once_with(task)
+            prepare_node_mock.assert_called_once_with(task)
             pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
                        autospec=True)
-    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
+    @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
     def test_prepare_uefi_whole_disk_image_fail(self,
-                                                update_boot_mode_mock,
+                                                prepare_node_for_deploy_mock,
                                                 pxe_prepare_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
@@ -1420,7 +1420,7 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
             task.node.driver_internal_info['is_whole_disk_image'] = True
             self.assertRaises(exception.InvalidParameterValue,
                               task.driver.deploy.prepare, task)
-            update_boot_mode_mock.assert_called_once_with(task)
+            prepare_node_for_deploy_mock.assert_called_once_with(task)
             self.assertFalse(pxe_prepare_mock.called)
 
     @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'deploy', spec_set=True,
@@ -1435,6 +1435,48 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
             set_persistent_mock.assert_called_with(task, boot_devices.PXE)
             pxe_deploy_mock.assert_called_once_with(mock.ANY, task)
 
+    @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'tear_down',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_deploy, '_update_secure_boot_mode', autospec=True)
+    @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,
+                       autospec=True)
+    def test_tear_down(self, node_power_action_mock,
+                       update_secure_boot_mode_mock, pxe_tear_down_mock):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            pxe_tear_down_mock.return_value = states.DELETED
+            returned_state = task.driver.deploy.tear_down(task)
+            node_power_action_mock.assert_called_once_with(task,
+                                                           states.POWER_OFF)
+            update_secure_boot_mode_mock.assert_called_once_with(task, False)
+            pxe_tear_down_mock.assert_called_once_with(mock.ANY, task)
+            self.assertEqual(states.DELETED, returned_state)
+
+    @mock.patch.object(ilo_deploy.LOG, 'warn', spec_set=True, autospec=True)
+    @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'tear_down',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_deploy, 'exception', spec_set=True, autospec=True)
+    @mock.patch.object(ilo_deploy, '_update_secure_boot_mode',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,
+                       autospec=True)
+    def test_tear_down_handle_exception(self, node_power_action_mock,
+                                        update_secure_boot_mode_mock,
+                                        exception_mock, pxe_tear_down_mock,
+                                        mock_log):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            pxe_tear_down_mock.return_value = states.DELETED
+            exception_mock.IloOperationNotSupported = Exception
+            update_secure_boot_mode_mock.side_effect = Exception
+            returned_state = task.driver.deploy.tear_down(task)
+            update_secure_boot_mode_mock.assert_called_once_with(task, False)
+            pxe_tear_down_mock.assert_called_once_with(mock.ANY, task)
+            node_power_action_mock.assert_called_once_with(task,
+                                                           states.POWER_OFF)
+            self.assertTrue(mock_log.called)
+            self.assertEqual(states.DELETED, returned_state)
+
 
 class IloPXEVendorPassthruTestCase(db_base.DbTestCase):
 
@@ -1463,9 +1505,15 @@ class IloPXEVendorPassthruTestCase(db_base.DbTestCase):
 
     @mock.patch.object(iscsi_deploy.VendorPassthru, 'pass_deploy_info',
                        spec_set=True, autospec=True)
+    @mock.patch.object(ilo_deploy, '_update_secure_boot_mode', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
+                       autospec=True)
     @mock.patch.object(manager_utils, 'node_set_boot_device', spec_set=True,
                        autospec=True)
     def test_vendorpassthru_pass_deploy_info(self, set_boot_device_mock,
+                                             func_update_boot_mode,
+                                             func_update_secure_boot_mode,
                                              pxe_vendorpassthru_mock):
         kwargs = {'address': '123456'}
         with task_manager.acquire(self.context, self.node.uuid,
@@ -1474,7 +1522,28 @@ class IloPXEVendorPassthruTestCase(db_base.DbTestCase):
             task.node.target_provision_state = states.ACTIVE
             task.driver.vendor.pass_deploy_info(task, **kwargs)
             set_boot_device_mock.assert_called_with(task, boot_devices.PXE,
-                                                    True)
+                                                    persistent=True)
+            func_update_boot_mode.assert_called_once_with(task)
+            func_update_secure_boot_mode.assert_called_once_with(task, True)
+            pxe_vendorpassthru_mock.assert_called_once_with(
+                mock.ANY, task, **kwargs)
+
+    @mock.patch.object(iscsi_deploy.VendorPassthru, 'continue_deploy',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_deploy, '_update_secure_boot_mode', autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)
+    def test_vendorpassthru_continue_deploy(self,
+                                            func_update_boot_mode,
+                                            func_update_secure_boot_mode,
+                                            pxe_vendorpassthru_mock):
+        kwargs = {'address': '123456'}
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.provision_state = states.DEPLOYWAIT
+            task.node.target_provision_state = states.ACTIVE
+            task.driver.vendor.continue_deploy(task, **kwargs)
+            func_update_boot_mode.assert_called_once_with(task)
+            func_update_secure_boot_mode.assert_called_once_with(task, True)
             pxe_vendorpassthru_mock.assert_called_once_with(
                 mock.ANY, task, **kwargs)