diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 9e60c46c61..4e7c283466 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/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: - _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) + _disable_secure_boot_if_supported(task) 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: - _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) + _disable_secure_boot_if_supported(task) 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.""" diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index 3a4c630a48..a28af51b11 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/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)