diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 83abb59409..aa6da57f75 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -325,24 +325,6 @@ def prepare_node_for_deploy(task): task.node.save() -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: - ilo_common.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.warning('Secure boot mode is not supported for node %s', - task.node.uuid) - - class IloVirtualMediaBoot(base.BootInterface): capabilities = ['iscsi_volume_boot', 'ramdisk_boot'] @@ -541,7 +523,7 @@ class IloVirtualMediaBoot(base.BootInterface): # Set boot mode ilo_common.update_boot_mode(task) # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) + boot_mode_utils.configure_secure_boot_if_needed(task) @METRICS.timer('IloVirtualMediaBoot.clean_up_instance') def clean_up_instance(self, task): @@ -558,7 +540,7 @@ class IloVirtualMediaBoot(base.BootInterface): """ LOG.debug("Cleaning up the instance.") manager_utils.node_power_action(task, states.POWER_OFF) - disable_secure_boot_if_supported(task) + boot_mode_utils.deconfigure_secure_boot_if_needed(task) if (deploy_utils.is_iscsi_boot(task) and task.node.driver_internal_info.get('ilo_uefi_iscsi_boot')): @@ -673,15 +655,14 @@ class IloPXEBoot(pxe.PXEBoot): :returns: None :raises: IloOperationError, if some operation on iLO failed. """ - # Set boot mode ilo_common.update_boot_mode(task) - # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) boot_mode = boot_mode_utils.get_boot_mode(task.node) if deploy_utils.is_iscsi_boot(task) and boot_mode == 'uefi': + # Need to enable secure boot, if being requested + boot_mode_utils.configure_secure_boot_if_needed(task) # Need to set 'ilo_uefi_iscsi_boot' param for clean up driver_internal_info = task.node.driver_internal_info driver_internal_info['ilo_uefi_iscsi_boot'] = True @@ -711,11 +692,11 @@ class IloPXEBoot(pxe.PXEBoot): :raises: IloOperationError, if some operation on iLO failed. """ manager_utils.node_power_action(task, states.POWER_OFF) - disable_secure_boot_if_supported(task) driver_internal_info = task.node.driver_internal_info if (deploy_utils.is_iscsi_boot(task) - and task.node.driver_internal_info.get('ilo_uefi_iscsi_boot')): + and task.node.driver_internal_info.get('ilo_uefi_iscsi_boot')): + boot_mode_utils.deconfigure_secure_boot_if_needed(task) # It will clear iSCSI info from iLO in case of booting from # volume in UEFI boot mode task.driver.management.clear_iscsi_boot_target(task) @@ -774,12 +755,11 @@ class IloiPXEBoot(ipxe.iPXEBoot): # Set boot mode ilo_common.update_boot_mode(task) - # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) - boot_mode = boot_mode_utils.get_boot_mode(task.node) if deploy_utils.is_iscsi_boot(task) and boot_mode == 'uefi': + # Need to enable secure boot, if being requested + boot_mode_utils.configure_secure_boot_if_needed(task) # Need to set 'ilo_uefi_iscsi_boot' param for clean up driver_internal_info = task.node.driver_internal_info driver_internal_info['ilo_uefi_iscsi_boot'] = True @@ -809,11 +789,11 @@ class IloiPXEBoot(ipxe.iPXEBoot): :raises: IloOperationError, if some operation on iLO failed. """ manager_utils.node_power_action(task, states.POWER_OFF) - disable_secure_boot_if_supported(task) driver_internal_info = task.node.driver_internal_info if (deploy_utils.is_iscsi_boot(task) - and task.node.driver_internal_info.get('ilo_uefi_iscsi_boot')): + and task.node.driver_internal_info.get('ilo_uefi_iscsi_boot')): + boot_mode_utils.deconfigure_secure_boot_if_needed(task) # It will clear iSCSI info from iLO in case of booting from # volume in UEFI boot mode task.driver.management.clear_iscsi_boot_target(task) @@ -1144,7 +1124,7 @@ class IloUefiHttpsBoot(base.BootInterface): "%(device)s", {'node': task.node.uuid, 'device': boot_devices.DISK}) # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) + boot_mode_utils.configure_secure_boot_if_needed(task) return params = {} @@ -1158,7 +1138,7 @@ class IloUefiHttpsBoot(base.BootInterface): manager_utils.node_set_boot_device(task, boot_devices.DISK, persistent=True) # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) + boot_mode_utils.configure_secure_boot_if_needed(task) return params.update(root_uuid=root_uuid) @@ -1173,7 +1153,7 @@ class IloUefiHttpsBoot(base.BootInterface): node.save() # Need to enable secure boot, if being requested - ilo_common.update_secure_boot_mode(task, True) + boot_mode_utils.configure_secure_boot_if_needed(task) ilo_common.setup_uefi_https(task, iso_ref, persistent=True) LOG.debug("Node %(node)s is set to boot from UEFIHTTP " @@ -1193,7 +1173,7 @@ class IloUefiHttpsBoot(base.BootInterface): "%(node)s", {'node': task.node.uuid}) image_utils.cleanup_iso_image(task) - disable_secure_boot_if_supported(task) + boot_mode_utils.deconfigure_secure_boot_if_needed(task) @METRICS.timer('IloUefiHttpsBoot.validate_rescue') def validate_rescue(self, task): diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 202aa25108..689edb2e01 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -37,7 +37,6 @@ from ironic.common import utils from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers.modules import boot_mode_utils -from ironic.drivers.modules import deploy_utils ilo_client = importutils.try_import('proliantutils.ilo.client') ilo_error = importutils.try_import('proliantutils.exception') @@ -830,28 +829,6 @@ def set_secure_boot_mode(task, flag): error=ilo_exception) -def update_secure_boot_mode(task, mode): - """Changes secure boot mode for next boot on the node. - - This method changes secure boot mode on the node for next boot. It changes - the secure boot mode setting on node only if the deploy has requested for - the secure boot. - During deploy, this method is used to enable secure boot on the node by - passing 'mode' as 'True'. - During teardown, this method is used to disable secure boot on the node by - passing 'mode' as 'False'. - - :param task: a TaskManager instance containing the node to act on. - :param mode: Boolean value requesting the next state for secure boot - :raises: IloOperationNotSupported, if operation is not supported on iLO - :raises: IloOperationError, if some operation on iLO failed. - """ - if deploy_utils.is_secure_boot_requested(task.node): - set_secure_boot_mode(task, mode) - LOG.info('Changed secure boot to %(mode)s for node %(node)s', - {'mode': mode, 'node': task.node.uuid}) - - def remove_single_or_list_of_files(file_location): """Removes (deletes) the file or list of files. diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 5e27b17fd3..9787b2057d 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -774,6 +774,38 @@ class IloManagement(base.ManagementInterface): """ return ilo_common.get_current_boot_mode(task.node) + def get_secure_boot_state(self, task): + """Get the current secure boot state for the node. + + :param task: A task from TaskManager. + :raises: MissingParameterValue if a required parameter is missing + :raises: IloOperationError on an error from IloClient library. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the hardware + :returns: Boolean + """ + try: + return ilo_common.get_secure_boot_mode(task) + except ilo_error.IloOperationNotSupported: + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_secure_boot_state') + + def set_secure_boot_state(self, task, state): + """Set the current secure boot state for the node. + + :param task: A task from TaskManager. + :param state: A new state as a boolean. + :raises: MissingParameterValue if a required parameter is missing + :raises: IloOperationError on an error from IloClient library. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the hardware + """ + try: + ilo_common.set_secure_boot_mode(task, state) + except ilo_error.IloOperationNotSupported: + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='set_secure_boot_state') + class Ilo5Management(IloManagement): diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index 0fcc7263f0..60ab2fa2e4 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -1049,8 +1049,7 @@ class IRMCVirtualMediaBoot(base.BootInterface, IRMCVolumeBootMixIn): self._configure_vmedia_boot(task, root_uuid_or_disk_id) # Enable secure boot, if being requested - if deploy_utils.is_secure_boot_requested(node): - irmc_common.set_secure_boot_mode(node, enable=True) + boot_mode_utils.configure_secure_boot_if_needed(task) @METRICS.timer('IRMCVirtualMediaBoot.clean_up_instance') def clean_up_instance(self, task): @@ -1068,8 +1067,7 @@ class IRMCVirtualMediaBoot(base.BootInterface, IRMCVolumeBootMixIn): return # Disable secure boot, if enabled secure boot - if deploy_utils.is_secure_boot_requested(task.node): - irmc_common.set_secure_boot_mode(task.node, enable=False) + boot_mode_utils.deconfigure_secure_boot_if_needed(task) _remove_share_file(_get_iso_name(task.node, label='boot')) driver_internal_info = task.node.driver_internal_info @@ -1130,39 +1128,3 @@ class IRMCPXEBoot(pxe.PXEBoot): irmc_management.backup_bios_config(task) super(IRMCPXEBoot, self).prepare_ramdisk(task, ramdisk_params) - - @METRICS.timer('IRMCPXEBoot.prepare_instance') - def prepare_instance(self, task): - """Prepares the boot of instance. - - This method prepares the boot of the instance after reading - relevant information from the node's instance_info. In case of netboot, - it updates the dhcp entries and switches the PXE config. In case of - localboot, it cleans up the PXE config. - - :param task: a task from TaskManager. - :returns: None - :raises: IRMCOperationError, if some operation on iRMC failed. - """ - - super(IRMCPXEBoot, self).prepare_instance(task) - node = task.node - if deploy_utils.is_secure_boot_requested(node): - irmc_common.set_secure_boot_mode(node, enable=True) - - @METRICS.timer('IRMCPXEBoot.clean_up_instance') - def clean_up_instance(self, task): - """Cleans up the boot of instance. - - This method cleans up the environment that was setup for booting - the instance. It unlinks the instance kernel/ramdisk in node's - directory in tftproot and removes the PXE config. - - :param task: a task from TaskManager. - :raises: IRMCOperationError, if some operation on iRMC failed. - :returns: None - """ - node = task.node - if deploy_utils.is_secure_boot_requested(node): - irmc_common.set_secure_boot_mode(node, enable=False) - super(IRMCPXEBoot, self).clean_up_instance(task) diff --git a/ironic/drivers/modules/irmc/common.py b/ironic/drivers/modules/irmc/common.py index 8f81f8cec0..0027bab974 100644 --- a/ironic/drivers/modules/irmc/common.py +++ b/ironic/drivers/modules/irmc/common.py @@ -198,6 +198,27 @@ def get_irmc_report(node): client_timeout=driver_info['irmc_client_timeout']) +def get_secure_boot_mode(node): + """Get the current secure boot mode. + + :param node: An ironic node object. + :raises: UnsupportedDriverExtension if secure boot is not present. + :raises: IRMCOperationError if the operation fails. + """ + driver_info = parse_driver_info(node) + + try: + return elcm.get_secure_boot_mode(driver_info) + except elcm.SecureBootConfigNotFound: + raise exception.UnsupportedDriverExtension( + driver=node.driver, extension='get_secure_boot_state') + except scci.SCCIError as irmc_exception: + LOG.error("Failed to get secure boot for node %s", node.uuid) + raise exception.IRMCOperationError( + operation=_("getting secure boot mode"), + error=irmc_exception) + + def set_secure_boot_mode(node, enable): """Enable or disable UEFI Secure Boot diff --git a/ironic/drivers/modules/irmc/management.py b/ironic/drivers/modules/irmc/management.py index f938df2931..99a719a150 100644 --- a/ironic/drivers/modules/irmc/management.py +++ b/ironic/drivers/modules/irmc/management.py @@ -372,3 +372,35 @@ class IRMCManagement(ipmitool.IPMIManagement): except exception.IRMCOperationError as e: raise exception.NodeCleaningFailure(node=task.node.uuid, reason=e) + + def get_secure_boot_state(self, task): + """Get the current secure boot state for the node. + + NOTE: Not all drivers support this method. Older hardware + may not implement that. + + :param task: A task from TaskManager. + :raises: MissingParameterValue if a required parameter is missing + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the driver or the hardware + :returns: Boolean + """ + return irmc_common.get_secure_boot_mode(task.node) + + def set_secure_boot_state(self, task, state): + """Set the current secure boot state for the node. + + NOTE: Not all drivers support this method. Older hardware + may not implement that. + + :param task: A task from TaskManager. + :param state: A new state as a boolean. + :raises: MissingParameterValue if a required parameter is missing + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if secure boot is + not supported by the driver or the hardware + """ + return irmc_common.set_secure_boot_mode(task.node, state) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 8d57d4fb11..9554614746 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -754,8 +754,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) @mock.patch.object(ilo_common, 'cleanup_vmedia_boot', spec_set=True, @@ -778,7 +778,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): driver_internal_info = task.node.driver_internal_info node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) + update_secure_boot_mode_mock.assert_called_once_with(task) def test_clean_up_instance_deleting(self): self.node.provisioning_state = states.DELETING @@ -792,8 +792,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): spec_set=True, autospec=True) @mock.patch.object(ilo_management.IloManagement, 'clear_iscsi_boot_target', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume( @@ -812,7 +812,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): states.POWER_OFF) clear_iscsi_boot_target_mock.assert_called_once_with(mock.ANY, task) - update_secure_boot_mode_mock.assert_called_once_with(task, False) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -822,8 +822,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume_bios( @@ -837,7 +837,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): cleanup_vmedia_mock.assert_called_once_with(task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) + update_secure_boot_mode_mock.assert_called_once_with(task) @mock.patch.object(ilo_common, 'cleanup_vmedia_boot', spec_set=True, autospec=True) @@ -857,8 +857,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + 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, @@ -881,7 +881,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): boot_devices.DISK, persistent=True) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -895,8 +895,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True, autospec=True) @mock.patch.object(ilo_boot.IloVirtualMediaBoot, @@ -921,7 +921,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): configure_vmedia_mock.assert_called_once_with( mock.ANY, task, "12312642-09d3-467f-8e09-12385826a123") update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -937,8 +937,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + spec_set=True, autospec=True) def test_prepare_instance_boot_from_volume( self, update_secure_boot_mode_mock, update_boot_mode_mock, set_boot_device_mock, @@ -958,7 +958,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): set_boot_device_mock.assert_called_once_with( task, boot_devices.ISCSIBOOT, persistent=True) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertTrue(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -995,8 +995,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + spec_set=True, autospec=True) def test_prepare_instance_boot_ramdisk(self, update_secure_boot_mode_mock, update_boot_mode_mock, set_boot_device_mock, @@ -1020,7 +1020,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): set_boot_device_mock.assert_called_once_with( task, boot_devices.CDROM, persistent=True) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) def test_validate_rescue(self): driver_info = self.node.driver_info @@ -1069,48 +1069,40 @@ class IloPXEBootTestCase(test_common.BaseIloTest): @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', spec_set=True, autospec=True) def test_clean_up_instance(self, pxe_cleanup_mock, node_power_mock, - update_secure_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: is_iscsi_boot_mock.return_value = False task.driver.boot.clean_up_instance(task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) pxe_cleanup_mock.assert_called_once_with(mock.ANY, task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume_bios( - self, pxe_cleanup_mock, node_power_mock, - update_secure_boot_mode_mock, is_iscsi_boot_mock): + self, pxe_cleanup_mock, node_power_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: is_iscsi_boot_mock.return_value = True task.driver.boot.clean_up_instance(task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) pxe_cleanup_mock.assert_called_once_with(mock.ANY, task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) @mock.patch.object(ilo_management.IloManagement, 'clear_iscsi_boot_target', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume(self, node_power_mock, @@ -1128,7 +1120,7 @@ class IloPXEBootTestCase(test_common.BaseIloTest): clear_iscsi_boot_target_mock.assert_called_once_with(mock.ANY, task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1136,15 +1128,12 @@ class IloPXEBootTestCase(test_common.BaseIloTest): spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, '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(pxe.PXEBoot, 'prepare_instance', spec_set=True, autospec=True) def test_prepare_instance(self, pxe_prepare_instance_mock, update_boot_mode_mock, - update_secure_boot_mode_mock, get_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -1153,7 +1142,6 @@ class IloPXEBootTestCase(test_common.BaseIloTest): get_boot_mode_mock.return_value = 'uefi' task.driver.boot.prepare_instance(task) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) pxe_prepare_instance_mock.assert_called_once_with(mock.ANY, task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1162,15 +1150,12 @@ class IloPXEBootTestCase(test_common.BaseIloTest): spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, '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(pxe.PXEBoot, 'prepare_instance', spec_set=True, autospec=True) def test_prepare_instance_bios(self, pxe_prepare_instance_mock, update_boot_mode_mock, - update_secure_boot_mode_mock, get_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -1179,7 +1164,6 @@ class IloPXEBootTestCase(test_common.BaseIloTest): get_boot_mode_mock.return_value = 'bios' task.driver.boot.prepare_instance(task) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) pxe_prepare_instance_mock.assert_called_once_with(mock.ANY, task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1194,8 +1178,8 @@ class IloPXEBootTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + spec_set=True, autospec=True) def test_prepare_instance_boot_from_volume( self, update_secure_boot_mode_mock, update_boot_mode_mock, set_boot_device_mock, @@ -1210,7 +1194,7 @@ class IloPXEBootTestCase(test_common.BaseIloTest): set_boot_device_mock.assert_called_once_with( task, boot_devices.ISCSIBOOT, persistent=True) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertTrue(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1252,48 +1236,40 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) @mock.patch.object(ipxe.iPXEBoot, 'clean_up_instance', spec_set=True, autospec=True) def test_clean_up_instance(self, pxe_cleanup_mock, node_power_mock, - update_secure_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: is_iscsi_boot_mock.return_value = False task.driver.boot.clean_up_instance(task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) pxe_cleanup_mock.assert_called_once_with(mock.ANY, task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) @mock.patch.object(ipxe.iPXEBoot, 'clean_up_instance', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume_bios( - self, pxe_cleanup_mock, node_power_mock, - update_secure_boot_mode_mock, is_iscsi_boot_mock): + self, pxe_cleanup_mock, node_power_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: is_iscsi_boot_mock.return_value = True task.driver.boot.clean_up_instance(task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) pxe_cleanup_mock.assert_called_once_with(mock.ANY, task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) @mock.patch.object(ilo_management.IloManagement, 'clear_iscsi_boot_target', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', + spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True, autospec=True) def test_clean_up_instance_boot_from_volume(self, node_power_mock, @@ -1311,7 +1287,7 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): clear_iscsi_boot_target_mock.assert_called_once_with(mock.ANY, task) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - update_secure_boot_mode_mock.assert_called_once_with(task, False) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1319,15 +1295,12 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, '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(ipxe.iPXEBoot, 'prepare_instance', spec_set=True, autospec=True) def test_prepare_instance(self, pxe_prepare_instance_mock, update_boot_mode_mock, - update_secure_boot_mode_mock, get_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -1336,7 +1309,6 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): get_boot_mode_mock.return_value = 'uefi' task.driver.boot.prepare_instance(task) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) pxe_prepare_instance_mock.assert_called_once_with(mock.ANY, task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1345,15 +1317,12 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, '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(ipxe.iPXEBoot, 'prepare_instance', spec_set=True, autospec=True) def test_prepare_instance_bios(self, pxe_prepare_instance_mock, update_boot_mode_mock, - update_secure_boot_mode_mock, get_boot_mode_mock, is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -1362,7 +1331,6 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): get_boot_mode_mock.return_value = 'bios' task.driver.boot.prepare_instance(task) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) pxe_prepare_instance_mock.assert_called_once_with(mock.ANY, task) self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1377,8 +1345,8 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True, autospec=True) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True, - autospec=True) + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', + spec_set=True, autospec=True) def test_prepare_instance_boot_from_volume( self, update_secure_boot_mode_mock, update_boot_mode_mock, set_boot_device_mock, @@ -1393,7 +1361,7 @@ class IloiPXEBootTestCase(test_common.BaseIloTest): set_boot_device_mock.assert_called_once_with( task, boot_devices.ISCSIBOOT, persistent=True) update_boot_mode_mock.assert_called_once_with(task) - update_secure_boot_mode_mock.assert_called_once_with(task, True) + update_secure_boot_mode_mock.assert_called_once_with(task) self.assertTrue(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) @@ -1994,7 +1962,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): task.driver.boot.clean_up_ramdisk(task) cleanup_iso_mock.assert_called_once_with(task) - @mock.patch.object(ilo_common, 'update_secure_boot_mode', + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', spec_set=True, autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', spec_set=True, autospec=True) @@ -2018,7 +1986,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): set_boot_device_mock.assert_called_once_with(task, boot_devices.DISK, persistent=True) - update_secureboot_mock.assert_called_once_with(task, True) + update_secureboot_mock.assert_called_once_with(task) cleanup_iso_mock.assert_called_once_with(task) prepare_iso_mock.assert_not_called() setup_uefi_https_mock.assert_not_called() @@ -2033,7 +2001,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_instance_local_or_whole_disk_image() - @mock.patch.object(ilo_common, 'update_secure_boot_mode', + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', spec_set=True, autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', spec_set=True, autospec=True) @@ -2071,13 +2039,13 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): parse_deploy_mock.assert_called_once_with(mock.ANY, task.node) prepare_iso_mock.assert_called_once_with( task, d_info, root_uuid='12312642-09d3-467f-8e09-12385826a123') - update_secureboot_mock.assert_called_once_with(task, True) + update_secureboot_mock.assert_called_once_with(task) setup_uefi_https_mock.assert_called_once_with( task, "recreated-iso", True) self.assertEqual(task.node.instance_info['ilo_boot_iso'], "recreated-iso") - @mock.patch.object(ilo_common, 'update_secure_boot_mode', + @mock.patch.object(boot_mode_utils, 'configure_secure_boot_if_needed', spec_set=True, autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', spec_set=True, autospec=True) @@ -2113,12 +2081,12 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): parse_deploy_mock.assert_called_once_with(mock.ANY, task.node) prepare_iso_mock.assert_called_once_with( task, d_info) - update_secureboot_mock.assert_called_once_with(task, True) + update_secureboot_mock.assert_called_once_with(task) setup_uefi_https_mock.assert_called_once_with( task, "recreated-iso", True) self.assertTrue('ilo_boot_iso' not in task.node.instance_info) - @mock.patch.object(ilo_boot, 'disable_secure_boot_if_supported', + @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', spec_set=True, autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', spec_set=True, autospec=True) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 30dad59846..cf51bf5acb 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -34,7 +34,6 @@ from ironic.common import images from ironic.common import swift from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils -from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils @@ -1094,32 +1093,6 @@ class IloCommonMethodsTestCase(BaseIloTest): func_set_boot_device.assert_called_once_with(task, boot_devices.CDROM) - @mock.patch.object(deploy_utils, 'is_secure_boot_requested', spec_set=True, - autospec=True) - @mock.patch.object(ilo_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - def test_update_secure_boot_mode_passed_true(self, - func_set_secure_boot_mode, - func_is_secure_boot_req): - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - func_is_secure_boot_req.return_value = True - ilo_common.update_secure_boot_mode(task, True) - func_set_secure_boot_mode.assert_called_once_with(task, True) - - @mock.patch.object(deploy_utils, 'is_secure_boot_requested', spec_set=True, - autospec=True) - @mock.patch.object(ilo_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - def test_update_secure_boot_mode_passed_false(self, - func_set_secure_boot_mode, - func_is_secure_boot_req): - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - func_is_secure_boot_req.return_value = False - ilo_common.update_secure_boot_mode(task, False) - self.assertFalse(func_set_secure_boot_mode.called) - @mock.patch.object(ironic_utils, 'unlink_without_raise', spec_set=True, autospec=True) def test_remove_single_or_list_of_files_with_file_list(self, unlink_mock): diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 0c97b9e899..3eab04672d 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -1384,121 +1384,27 @@ class IRMCPXEBootTestCase(test_common.BaseIRMCTest): mock_parent_prepare.assert_called_once_with( task.driver.boot, task, {}) - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', spec_set=True, autospec=True) - def test_prepare_instance_with_secure_boot(self, mock_prepare_instance, - mock_set_secure_boot_mode): - self.node.provision_state = states.DEPLOYING - self.node.target_provision_state = states.ACTIVE - self.node.instance_info = { - 'capabilities': { - "secure_boot": "true" - } - } - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.driver.boot.prepare_instance(task) - mock_set_secure_boot_mode.assert_called_once_with(task.node, - enable=True) - mock_prepare_instance.assert_called_once_with( - task.driver.boot, task) - - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', spec_set=True, - autospec=True) - def test_prepare_instance_with_secure_boot_false( - self, mock_prepare_instance, mock_set_secure_boot_mode): - self.node.provision_state = states.DEPLOYING - self.node.target_provision_state = states.ACTIVE - self.node.instance_info = { - 'capabilities': { - "secure_boot": "false" - } - } - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.driver.boot.prepare_instance(task) - self.assertFalse(mock_set_secure_boot_mode.called) - mock_prepare_instance.assert_called_once_with( - task.driver.boot, task) - - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', spec_set=True, - autospec=True) - def test_prepare_instance_without_secure_boot(self, mock_prepare_instance, - mock_set_secure_boot_mode): + def test_prepare_instance(self, mock_prepare_instance): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot.prepare_instance(task) - self.assertFalse(mock_set_secure_boot_mode.called) mock_prepare_instance.assert_called_once_with( task.driver.boot, task) - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', spec_set=True, autospec=True) - def test_clean_up_instance_with_secure_boot(self, mock_clean_up_instance, - mock_set_secure_boot_mode): - self.node.provision_state = states.CLEANING - self.node.target_provision_state = states.AVAILABLE - self.node.instance_info = { - 'capabilities': { - "secure_boot": "true" - } - } - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.driver.boot.clean_up_instance(task) - mock_set_secure_boot_mode.assert_called_once_with(task.node, - enable=False) - mock_clean_up_instance.assert_called_once_with( - task.driver.boot, task) - - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', spec_set=True, - autospec=True) - def test_clean_up_instance_secure_boot_false(self, mock_clean_up_instance, - mock_set_secure_boot_mode): - self.node.provision_state = states.CLEANING - self.node.target_provision_state = states.AVAILABLE - self.node.instance_info = { - 'capabilities': { - "secure_boot": "false" - } - } - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.driver.boot.clean_up_instance(task) - self.assertFalse(mock_set_secure_boot_mode.called) - mock_clean_up_instance.assert_called_once_with( - task.driver.boot, task) - - @mock.patch.object(irmc_common, 'set_secure_boot_mode', spec_set=True, - autospec=True) - @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', spec_set=True, - autospec=True) - def test_clean_up_instance_without_secure_boot( - self, mock_clean_up_instance, mock_set_secure_boot_mode): + def test_clean_up_instance(self, mock_clean_up_instance): self.node.provision_state = states.CLEANING self.node.target_provision_state = states.AVAILABLE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot.clean_up_instance(task) - self.assertFalse(mock_set_secure_boot_mode.called) mock_clean_up_instance.assert_called_once_with( task.driver.boot, task)