From 5e8f2e39d4f3508b2189dc09d6eecc9492c42f0c Mon Sep 17 00:00:00 2001 From: Ilya Etingof Date: Thu, 7 Dec 2017 17:13:59 +0100 Subject: [PATCH] Adds boot mode support to ManagementInterface This change introduces optional boot mode get/set methods to driver management interface [1] [2] alongside existing get/set boot device calls. The management interface is called at deploy time to synchronize BM machine boot mode setting with Ironic node configuration whenever needed. Also, this change introduces common exception class to be eventually used by the drivers to communicate their runtime errors to Ironic code in a uniformed way. 1. http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2018-01-09.log.html#t2018-01-09T15:54:16 2. http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2018-05-11.log.html#t2018-05-11T12:47:12 Story: 1734131 Task: 10640 Change-Id: If3db6ab8d3fae35d17808a231b7eecf11cf58327 --- ironic/common/boot_modes.py | 29 ++++ ironic/common/exception.py | 24 +-- ironic/conductor/manager.py | 1 + ironic/conductor/utils.py | 87 ++++++++-- ironic/conf/deploy.py | 12 ++ ironic/drivers/base.py | 65 ++++++++ ironic/drivers/modules/boot_mode_utils.py | 148 ++++++++++++++++++ ironic/drivers/modules/deploy_utils.py | 56 ++++++- ironic/drivers/modules/pxe.py | 5 + ironic/tests/unit/conductor/test_utils.py | 72 +++++++++ .../unit/drivers/modules/irmc/test_boot.py | 3 + ironic/tests/unit/drivers/modules/test_pxe.py | 132 +++++++++++++++- ironic/tests/unit/drivers/test_base.py | 21 +++ .../tests/unit/drivers/test_fake_hardware.py | 8 + ...de-boot-mode-control-9761d4bcbd8c3a0d.yaml | 16 ++ 15 files changed, 648 insertions(+), 31 deletions(-) create mode 100644 ironic/common/boot_modes.py create mode 100644 ironic/drivers/modules/boot_mode_utils.py create mode 100644 releasenotes/notes/add-node-boot-mode-control-9761d4bcbd8c3a0d.yaml diff --git a/ironic/common/boot_modes.py b/ironic/common/boot_modes.py new file mode 100644 index 0000000000..cd148dcbbc --- /dev/null +++ b/ironic/common/boot_modes.py @@ -0,0 +1,29 @@ +# Copyright 2018 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +""" +Mapping of boot modes used when requesting the system to boot +using alternative firmware interfaces. + +The options presented were based on the Redfish protocol capabilities, +specifically on the BootSourceOverrideMode property. +""" + +LEGACY_BIOS = 'bios' +"Boot over legacy PC BIOS firmware interface" + +UEFI = 'uefi' +"Boot over Unified Extensible Firmware Interface (UEFI) firmware interface" diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 8ab107d9fe..6487ca66d2 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -586,6 +586,10 @@ class DriverLoadError(IronicException): "loaded. Reason: %(reason)s.") +class DriverOperationError(IronicException): + _msg_fmt = _("Runtime driver %(driver)s failure. Reason: %(reason)s.") + + class ConsoleError(IronicException): pass @@ -602,15 +606,15 @@ class PasswordFileFailedToCreate(IronicException): _msg_fmt = _("Failed to create the password file. %(error)s") -class IloOperationError(IronicException): +class IloOperationError(DriverOperationError): _msg_fmt = _("%(operation)s failed, error: %(error)s") -class IloOperationNotSupported(IronicException): +class IloOperationNotSupported(DriverOperationError): _msg_fmt = _("%(operation)s not supported. error: %(error)s") -class DracOperationError(IronicException): +class DracOperationError(DriverOperationError): _msg_fmt = _('DRAC operation failed. Reason: %(error)s') @@ -643,7 +647,7 @@ class SwiftObjectNotFoundError(SwiftOperationError): "not found. Operation '%(operation)s' failed.") -class SNMPFailure(IronicException): +class SNMPFailure(DriverOperationError): _msg_fmt = _("SNMP operation '%(operation)s' failed: %(error)s") @@ -652,11 +656,11 @@ class FileSystemNotSupported(IronicException): "File system %(fs)s is not supported.") -class IRMCOperationError(IronicException): +class IRMCOperationError(DriverOperationError): _msg_fmt = _('iRMC %(operation)s failed. Reason: %(error)s') -class IRMCSharedFileSystemNotMounted(IronicException): +class IRMCSharedFileSystemNotMounted(DriverOperationError): _msg_fmt = _("iRMC shared file system '%(share)s' is not mounted.") @@ -676,7 +680,7 @@ class DirectoryNotWritable(IronicException): _msg_fmt = _("Directory %(dir)s is not writable.") -class UcsOperationError(IronicException): +class UcsOperationError(DriverOperationError): _msg_fmt = _("Cisco UCS client: operation %(operation)s failed for node" " %(node)s. Reason: %(error)s") @@ -691,11 +695,11 @@ class ImageUploadFailed(IronicException): "%(web_server)s, reason: %(reason)s") -class CIMCException(IronicException): +class CIMCException(DriverOperationError): _msg_fmt = _("Cisco IMC exception occurred for node %(node)s: %(error)s") -class OneViewError(IronicException): +class OneViewError(DriverOperationError): _msg_fmt = _("OneView exception occurred. Error: %(error)s") @@ -737,7 +741,7 @@ class StorageError(IronicException): _msg_fmt = _("Storage operation failure.") -class RedfishError(IronicException): +class RedfishError(DriverOperationError): _msg_fmt = _("Redfish exception occurred. Error: %(error)s") diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 76d493c0fd..d9c6f39953 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -944,6 +944,7 @@ class ConductorManager(base_manager.BaseConductorManager): driver_internal_info.pop('clean_steps', None) driver_internal_info.pop('root_uuid_or_disk_id', None) driver_internal_info.pop('is_whole_disk_image', None) + driver_internal_info.pop('deploy_boot_mode', None) node.driver_internal_info = driver_internal_info network.remove_vifs_from_node(task) node.save() diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 68f9cbc2a7..46e0f446d8 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -47,9 +47,6 @@ CLEANING_INTERFACE_PRIORITY = { def node_set_boot_device(task, device, persistent=False): """Set the boot device for a node. - Sets the boot device for a node if the node's driver interface - contains a 'management' interface. - If the node that the boot device change is being requested for is in ADOPTING state, the boot device will not be set as that change could potentially result in the future running state of @@ -63,12 +60,84 @@ def node_set_boot_device(task, device, persistent=False): ManagementInterface fails. """ - if getattr(task.driver, 'management', None): - task.driver.management.validate(task) - if task.node.provision_state != states.ADOPTING: - task.driver.management.set_boot_device(task, - device=device, - persistent=persistent) + # TODO(etingof): remove `if` once classic drivers are gone + if not getattr(task.driver, 'management', None): + return + + task.driver.management.validate(task) + if task.node.provision_state != states.ADOPTING: + task.driver.management.set_boot_device(task, + device=device, + persistent=persistent) + + +def node_get_boot_mode(task): + """Read currently set boot mode from a node. + + Reads the boot mode for a node. If boot mode can't be discovered, + `None` is returned. + + :param task: a TaskManager instance. + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if current driver does not have + management interface or `get_boot_mode()` method is + not supported. + :returns: Boot mode. One of :mod:`ironic.common.boot_mode` or `None` + if boot mode can't be discovered + """ + # TODO(etingof): remove `if` once classic drivers are gone + if not getattr(task.driver, 'management', None): + return + + task.driver.management.validate(task) + return task.driver.management.get_boot_mode(task) + + +# TODO(ietingof): remove `Sets the boot mode...` from the docstring +# once classic drivers are gone +@task_manager.require_exclusive_lock +def node_set_boot_mode(task, mode): + """Set the boot mode for a node. + + Sets the boot mode for a node if the node's driver interface + contains a 'management' interface. + + If the node that the boot mode change is being requested for + is in ADOPTING state, the boot mode will not be set as that + change could potentially result in the future running state of + an adopted node being modified erroneously. + + :param task: a TaskManager instance. + :param mode: Boot mode. Values are one of + :mod:`ironic.common.boot_modes` + :raises: InvalidParameterValue if the validation of the + ManagementInterface fails. + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: UnsupportedDriverExtension if current driver does not have + vendor interface or method is unsupported. + """ + if task.node.provision_state == states.ADOPTING: + return + + # TODO(etingof): remove `if` once classic drivers are gone + if not getattr(task.driver, 'management', None): + return + + task.driver.management.validate(task) + + boot_modes = task.driver.management.get_supported_boot_modes(task) + + if mode not in boot_modes: + msg = _("Unsupported boot mode %(mode)s specified for " + "node %(node_id)s. Supported boot modes are: " + "%(modes)s") % {'mode': mode, + 'modes': ', '.join(boot_modes), + 'node_id': task.node.uuid} + raise exception.InvalidParameterValue(msg) + + task.driver.management.set_boot_mode(task, mode=mode) def node_wait_for_power_state(task, new_state, timeout=None): diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 6a94895fcb..05bd7f3c7e 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -16,6 +16,7 @@ from oslo_config import cfg +from ironic.common import boot_modes from ironic.common.i18n import _ @@ -73,6 +74,17 @@ opts = [ 'default is "netboot", but it will be changed to ' '"local" in the future. It is recommended to set ' 'an explicit value for this option.')), + cfg.StrOpt('default_boot_mode', + choices=[(boot_modes.UEFI, _('UEFI boot mode')), + (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], + default=boot_modes.LEGACY_BIOS, + help=_('Default boot mode to use when no boot mode is ' + 'requested in node\'s driver_info, capabilities or ' + 'in the `instance_info` configuration. Currently the ' + 'default boot mode is "%(bios)s". This option only ' + 'has effect when management interface supports boot ' + 'mode management') % { + 'bios': boot_modes.LEGACY_BIOS}), cfg.BoolOpt('configdrive_use_object_store', default=False, deprecated_group='conductor', diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 4862cabc53..45131ba33d 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -849,6 +849,71 @@ class ManagementInterface(BaseInterface): """ + def get_supported_boot_modes(self, task): + """Get a list of the supported boot modes. + + NOTE: Not all drivers support this method. Older hardware + may not implement that. + + :param task: A task from TaskManager. + :raises: UnsupportedDriverExtension if requested operation is + not supported by the driver + :raises: DriverOperationError or its derivative in case + of driver runtime error. + :raises: MissingParameterValue if a required parameter is missing + :returns: A list with the supported boot modes defined + in :mod:`ironic.common.boot_modes`. If boot + mode support can't be determined, empty list + is returned. + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_supported_boot_modes') + + def set_boot_mode(self, task, mode): + """Set the boot mode for a node. + + Set the boot mode to use on next reboot of the node. + + Drivers implementing this method are required to implement + the `get_supported_boot_modes` method as well. + + NOTE: Not all drivers support this method. Hardware supporting only + one boot mode may not implement that. + + :param task: A task from TaskManager. + :param mode: The boot mode, one of + :mod:`ironic.common.boot_modes`. + :raises: InvalidParameterValue if an invalid boot mode is + specified. + :raises: MissingParameterValue if a required parameter is missing + :raises: UnsupportedDriverExtension if requested operation is + not supported by the driver + :raises: DriverOperationError or its derivative in case + of driver runtime error. + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='set_boot_mode') + + def get_boot_mode(self, task): + """Get the current boot mode for a node. + + Provides the current boot mode of 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 requested operation is + not supported by the driver + :returns: The boot mode, one of :mod:`ironic.common.boot_mode` or + None if it is unknown. + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_boot_mode') + @abc.abstractmethod def get_sensors_data(self, task): """Get sensors data method. diff --git a/ironic/drivers/modules/boot_mode_utils.py b/ironic/drivers/modules/boot_mode_utils.py new file mode 100644 index 0000000000..acddf898ec --- /dev/null +++ b/ironic/drivers/modules/boot_mode_utils.py @@ -0,0 +1,148 @@ +# Copyright 2018 Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_log import log as logging + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.conductor import utils as manager_utils +from ironic.conf import CONF +from ironic.drivers.modules import deploy_utils + +LOG = logging.getLogger(__name__) + + +def _set_boot_mode_on_bm(task, ironic_boot_mode, fail_if_unsupported=False): + try: + manager_utils.node_set_boot_mode(task, ironic_boot_mode) + + except exception.UnsupportedDriverExtension as ex: + if fail_if_unsupported: + msg = (_("Baremetal node %(uuid)s boot mode is not set " + "to boot mode %(boot_mode)s: %(error)s") % + {'uuid': task.node.uuid, + 'boot_mode': ironic_boot_mode, + 'error': ex}) + LOG.error(msg) + raise exception.UnsupportedDriverExtension(msg) + + msg_tmpl = _("Baremetal node %(uuid)s boot mode is not set " + "to boot mode %(boot_mode)s. Assuming " + "baremetal node is already in %(boot_mode)s or " + "driver set boot mode via some other " + "mechanism: %(error)s") + + LOG.debug(msg_tmpl, {'uuid': task.node.uuid, + 'boot_mode': ironic_boot_mode, + 'error': ex}) + + except exception.InvalidParameterValue as ex: + msg = (_("Node %(uuid)s boot mode is not set. " + "Attempt to set %(ironic_boot_mode)s boot mode " + "on the baremetal node failed with error %(error)s") % + {'uuid': task.node.uuid, + 'ironic_boot_mode': ironic_boot_mode, + 'error': ex}) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + else: + LOG.info("Baremetal node boot mode is set to boot " + "mode %(boot_mode)s", + {'uuid': task.node.uuid, 'boot_mode': ironic_boot_mode}) + + +def sync_boot_mode(task): + """Set node's boot mode from bare metal configuration + + Attempt to read currently set boot mode off the bare metal machine. + Also read node's boot mode configuration: + + * If BM driver does not implement getting boot mode, assume + BM boot mode is not set and apply the logic that follows + * If Ironic node boot mode is not set and BM node boot mode is + not set - set Ironic boot mode to `[deploy]/default_boot_mode` + * If Ironic node boot mode is not set and BM node boot mode + is set - set BM node boot mode on the Ironic node + * If Ironic node boot mode is set and BM node boot mode is + not set - set Ironic boot mode to BM boot mode + * If both Ironic and BM node boot modes are set but they + differ - try to set Ironic boot mode to BM boot mode and fail hard + if underlying hardware type does not support setting boot mode + + In the end, the new boot mode may be set in + 'driver_internal_info/deploy_boot_mode'. + + :param task: a task object + """ + node = task.node + + try: + bm_boot_mode = manager_utils.node_get_boot_mode(task) + + except exception.UnsupportedDriverExtension as ex: + bm_boot_mode = None + + LOG.debug("Cannot determine node %(uuid)s boot mode: %(error)s", + {'uuid': node.uuid, 'error': ex}) + + ironic_boot_mode = deploy_utils.get_boot_mode_for_deploy(node) + + # NOTE(etingof): the outcome of the branching that follows is that + # the new boot mode may be set in 'driver_internal_info/deploy_boot_mode' + + if not ironic_boot_mode and not bm_boot_mode: + driver_internal_info = node.driver_internal_info + default_boot_mode = CONF.deploy.default_boot_mode + driver_internal_info['deploy_boot_mode'] = default_boot_mode + node.driver_internal_info = driver_internal_info + node.save() + + LOG.debug("Ironic node %(uuid)s boot mode will be set to default " + "boot mode %(boot_mode)s", + {'uuid': node.uuid, 'boot_mode': default_boot_mode}) + + _set_boot_mode_on_bm(task, default_boot_mode) + + elif not ironic_boot_mode and bm_boot_mode: + driver_internal_info = node.driver_internal_info + driver_internal_info['deploy_boot_mode'] = bm_boot_mode + node.driver_internal_info = driver_internal_info + node.save() + + LOG.debug("Ironic node %(uuid)s boot mode is set to boot mode " + "%(boot_mode)s reported by the driver", + {'uuid': node.uuid, 'boot_mode': bm_boot_mode}) + + elif ironic_boot_mode and not bm_boot_mode: + # NOTE(etingof): if only ironic boot mode is known, try to synchronize + # (e.g. ironic -> bm) and do not fail if setting boot mode is not + # supported by the underlying hardware type + _set_boot_mode_on_bm(task, ironic_boot_mode) + + elif ironic_boot_mode != bm_boot_mode: + msg = (_("Boot mode %(node_boot_mode)s currently configured " + "on node %(uuid)s does not match the boot mode " + "%(ironic_boot_mode)s requested for provisioning." + "Attempting to set node boot mode to %(ironic_boot_mode)s.") % + {'uuid': node.uuid, 'node_boot_mode': bm_boot_mode, + 'ironic_boot_mode': ironic_boot_mode}) + LOG.info(msg) + + # NOTE(etingof): if boot modes are known and different, try + # to synchronize them (e.g. ironic -> bm) and fail hard if + # underlying hardware type does not support setting boot mode as + # it seems to be a hopeless misconfiguration + _set_boot_mode_on_bm(task, ironic_boot_mode, fail_if_unsupported=True) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index e18209adff..11b74e2c9d 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -771,12 +771,17 @@ def get_boot_mode_for_deploy(node): 'trusted_boot' is set to 'true' in 'instance_info/capabilities' of node. Otherwise it returns value of 'boot_mode' in 'properties/capabilities' of node if set. If that is not set, it returns boot mode in + 'driver_internal_info/deploy_boot_mode' for the node. + If that is not set, it returns boot mode in 'instance_info/deploy_boot_mode' for the node. It would return None if boot mode is present neither in 'capabilities' of - node 'properties' nor in node's 'instance_info' (which could also be None). + node 'properties' nor in node's 'driver_internal_info' nor in node's + 'instance_info' (which could also be None). :param node: an ironic node object. :returns: 'bios', 'uefi' or None + :raises: InvalidParameterValue, if the node boot mode disagrees with + the boot mode set to node properties/capabilities """ if is_secure_boot_requested(node): @@ -789,15 +794,56 @@ def get_boot_mode_for_deploy(node): LOG.debug('Deploy boot mode is bios for %s.', node.uuid) return 'bios' - boot_mode = driver_utils.get_node_capability(node, 'boot_mode') + # NOTE(etingof): + # The search for a boot mode should be in the priority order: + # + # 1) instance_info + # 2) properties.capabilities + # 3) driver_internal_info + # + # Because: + # + # (1) can be deleted before teardown + # (3) will never be touched if node properties/capabilities + # are still present. + # (2) becomes operational default as the last resort + + instance_info = node.instance_info + + cap_boot_mode = driver_utils.get_node_capability(node, 'boot_mode') + + boot_mode = instance_info.get('deploy_boot_mode') if boot_mode is None: - instance_info = node.instance_info - boot_mode = instance_info.get('deploy_boot_mode') + boot_mode = cap_boot_mode + if cap_boot_mode is None: + driver_internal_info = node.driver_internal_info + boot_mode = driver_internal_info.get('deploy_boot_mode') + + if not boot_mode: + return + + boot_mode = boot_mode.lower() + + # NOTE(etingof): + # Make sure that the ultimate boot_mode agrees with the one set to + # node properties/capabilities. This locks down node to use only + # boot mode specified in properties/capabilities. + # TODO(etingof): this logic will have to go away when we switch to traits + if cap_boot_mode: + cap_boot_mode = cap_boot_mode.lower() + if cap_boot_mode != boot_mode: + msg = (_("Node %(uuid)s boot mode %(boot_mode)s violates " + "node properties/capabilities %(caps)s") % + {'uuid': node.uuid, + 'boot_mode': boot_mode, + 'caps': cap_boot_mode}) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) LOG.debug('Deploy boot mode is %(boot_mode)s for %(node)s.', {'boot_mode': boot_mode, 'node': node.uuid}) - return boot_mode.lower() if boot_mode else boot_mode + return boot_mode def get_pxe_boot_file(node): diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index c2c2ee3da9..f2310c64e2 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -35,6 +35,7 @@ from ironic.common import states from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers import base +from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_cache from ironic.drivers import utils as driver_utils @@ -528,6 +529,7 @@ class PXEBoot(base.BootInterface): # if we are in DEPLOYING state. if node.provision_state == states.DEPLOYING: pxe_info.update(_get_instance_image_info(node, task.context)) + boot_mode_utils.sync_boot_mode(task) pxe_options = _build_pxe_config_options(task, pxe_info) pxe_options.update(ramdisk_params) @@ -590,6 +592,8 @@ class PXEBoot(base.BootInterface): :param task: a task from TaskManager. :returns: None """ + boot_mode_utils.sync_boot_mode(task) + node = task.node boot_option = deploy_utils.get_boot_option(node) boot_device = None @@ -679,6 +683,7 @@ class PXEBoot(base.BootInterface): :returns: None """ node = task.node + try: images_info = _get_instance_image_info(node, task.context) except exception.MissingParameterValue as e: diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index c09db144db..428012e8a9 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -14,6 +14,7 @@ import mock from oslo_config import cfg from oslo_utils import uuidutils +from ironic.common import boot_modes from ironic.common import exception from ironic.common import network from ironic.common import states @@ -77,6 +78,77 @@ class NodeSetBootDeviceTestCase(db_base.DbTestCase): self.assertFalse(mock_sbd.called) +class NodeGetBootModeTestCase(db_base.DbTestCase): + + def setUp(self): + super(NodeGetBootModeTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + self.task = task_manager.TaskManager(self.context, self.node.uuid) + + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_node_get_boot_mode_valid(self, mock_gbm): + mock_gbm.return_value = 'bios' + boot_mode = conductor_utils.node_get_boot_mode(self.task) + self.assertEqual(boot_mode, 'bios') + mock_gbm.assert_called_once_with(mock.ANY, self.task) + + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_node_get_boot_mode_unsupported(self, mock_gbm): + mock_gbm.side_effect = exception.UnsupportedDriverExtension( + driver=self.task.node.driver, extension='get_boot_mode') + self.assertRaises(exception.UnsupportedDriverExtension, + conductor_utils.node_get_boot_mode, self.task) + + +class NodeSetBootModeTestCase(db_base.DbTestCase): + + def setUp(self): + super(NodeSetBootModeTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + self.task = task_manager.TaskManager(self.context, self.node.uuid) + + @mock.patch.object(fake.FakeManagement, 'get_supported_boot_modes', + autospec=True) + def test_node_set_boot_mode_non_existent_mode(self, mock_gsbm): + + mock_gsbm.return_value = [boot_modes.LEGACY_BIOS] + + self.assertRaises(exception.InvalidParameterValue, + conductor_utils.node_set_boot_mode, + self.task, + mode='non-existing') + + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_supported_boot_modes', + autospec=True) + def test_node_set_boot_mode_valid(self, mock_gsbm, mock_sbm): + mock_gsbm.return_value = [boot_modes.LEGACY_BIOS] + + conductor_utils.node_set_boot_mode(self.task, + mode=boot_modes.LEGACY_BIOS) + mock_sbm.assert_called_once_with(mock.ANY, self.task, + mode=boot_modes.LEGACY_BIOS) + + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_supported_boot_modes', + autospec=True) + def test_node_set_boot_mode_adopting(self, mock_gsbm, mock_sbm): + mock_gsbm.return_value = [boot_modes.LEGACY_BIOS] + + old_provision_state = self.task.node.provision_state + self.task.node.provision_state = states.ADOPTING + try: + conductor_utils.node_set_boot_mode(self.task, + mode=boot_modes.LEGACY_BIOS) + + finally: + self.task.node.provision_state = old_provision_state + + self.assertFalse(mock_sbm.called) + + class NodePowerActionTestCase(db_base.DbTestCase): @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) def test_node_power_action_power_on(self, get_power_mock): diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 284655b778..1382624828 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -1905,6 +1905,9 @@ class IRMCPXEBootBasicTestCase(test_pxe.PXEBootTestCase): driver = 'pxe_irmc' boot_interface = None + # NOTE(etingof): add driver-specific configuration + driver_info = dict(test_pxe.PXEBootTestCase.driver_info) + driver_info.update(PARSED_IFNO) def test_get_properties(self): with task_manager.acquire(self.context, self.node.uuid, diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index ba14c04d2b..b82de3d0f0 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -26,6 +26,7 @@ from oslo_utils import fileutils from oslo_utils import uuidutils from ironic.common import boot_devices +from ironic.common import boot_modes from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import base_image_service @@ -752,6 +753,8 @@ class PXEBootTestCase(db_base.DbTestCase): driver = 'fake-hardware' boot_interface = 'pxe' + driver_info = DRV_INFO_DICT + driver_internal_info = DRV_INTERNAL_INFO_DICT def setUp(self): super(PXEBootTestCase, self).setUp() @@ -776,8 +779,8 @@ class PXEBootTestCase(db_base.DbTestCase): vendor_interface=('no-vendor' if self.driver == 'fake-hardware' else None), instance_info=instance_info, - driver_info=DRV_INFO_DICT, - driver_internal_info=DRV_INTERNAL_INFO_DICT) + driver_info=self.driver_info, + driver_internal_info=self.driver_internal_info) self.port = obj_utils.create_test_port(self.context, node_id=self.node.id) self.config(group='conductor', api_url='http://127.0.0.1:1234/') @@ -908,6 +911,7 @@ class PXEBootTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.boot.validate, task) + @mock.patch.object(manager_utils, 'node_get_boot_mode', autospec=True) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(dhcp_factory, 'DHCPFactory') @mock.patch.object(pxe, '_get_instance_image_info', autospec=True) @@ -921,11 +925,13 @@ class PXEBootTestCase(db_base.DbTestCase): mock_instance_img_info, dhcp_factory_mock, set_boot_device_mock, + get_boot_mode_mock, uefi=False, cleaning=False, ipxe_use_swift=False, whole_disk_image=False, - mode='deploy'): + mode='deploy', + node_boot_mode=None): mock_build_pxe.return_value = {} kernel_label = '%s_kernel' % mode ramdisk_label = '%s_ramdisk' % mode @@ -939,6 +945,7 @@ class PXEBootTestCase(db_base.DbTestCase): mock_cache_r_k.return_value = None provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock + get_boot_mode_mock.return_value = node_boot_mode driver_internal_info = self.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = whole_disk_image self.node.driver_internal_info = driver_internal_info @@ -952,6 +959,8 @@ class PXEBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {'foo': 'bar'}) mock_deploy_img_info.assert_called_once_with(task.node, mode=mode) provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts) + if self.node.provision_state == states.DEPLOYING: + get_boot_mode_mock.assert_called_once_with(task) set_boot_device_mock.assert_called_once_with(task, boot_devices.PXE, persistent=False) @@ -1097,6 +1106,104 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk(cleaning=True) + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_set_boot_mode_on_bm( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + properties = self.node.properties + properties['capabilities'] = 'boot_mode:uefi' + self.node.properties = properties + self.node.save() + self._test_prepare_ramdisk(uefi=True) + set_boot_mode_mock.assert_called_once_with(mock.ANY, boot_modes.UEFI) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_set_boot_mode_on_ironic( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + self.node.save() + self._test_prepare_ramdisk(node_boot_mode=boot_modes.LEGACY_BIOS) + + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_internal_info = task.node.driver_internal_info + self.assertIn('deploy_boot_mode', driver_internal_info) + self.assertEqual(boot_modes.LEGACY_BIOS, + driver_internal_info['deploy_boot_mode']) + self.assertEqual(set_boot_mode_mock.call_count, 0) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_set_default_boot_mode_on_ironic_bios( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + self.node.save() + + self.config(default_boot_mode=boot_modes.LEGACY_BIOS, group='deploy') + + self._test_prepare_ramdisk() + + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_internal_info = task.node.driver_internal_info + self.assertIn('deploy_boot_mode', driver_internal_info) + self.assertEqual(boot_modes.LEGACY_BIOS, + driver_internal_info['deploy_boot_mode']) + self.assertEqual(set_boot_mode_mock.call_count, 1) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_set_default_boot_mode_on_ironic_uefi( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + self.node.save() + + self.config(default_boot_mode=boot_modes.UEFI, group='deploy') + + self._test_prepare_ramdisk(uefi=True) + + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_internal_info = task.node.driver_internal_info + self.assertIn('deploy_boot_mode', driver_internal_info) + self.assertEqual(boot_modes.UEFI, + driver_internal_info['deploy_boot_mode']) + self.assertEqual(set_boot_mode_mock.call_count, 1) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_conflicting_boot_modes( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + properties = self.node.properties + properties['capabilities'] = 'boot_mode:uefi' + self.node.properties = properties + self.node.save() + self._test_prepare_ramdisk(uefi=True, + node_boot_mode=boot_modes.LEGACY_BIOS) + set_boot_mode_mock.assert_called_once_with(mock.ANY, boot_modes.UEFI) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_conflicting_boot_modes_set_unsupported( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + properties = self.node.properties + properties['capabilities'] = 'boot_mode:uefi' + self.node.properties = properties + self.node.save() + set_boot_mode_mock.side_effect = exception.UnsupportedDriverExtension( + extension='management', driver='test-driver' + ) + self.assertRaises(exception.UnsupportedDriverExtension, + self._test_prepare_ramdisk, + uefi=True, node_boot_mode=boot_modes.LEGACY_BIOS) + + @mock.patch.object(manager_utils, 'node_set_boot_mode', autospec=True) + def test_prepare_ramdisk_set_boot_mode_not_called( + self, set_boot_mode_mock): + self.node.provision_state = states.DEPLOYING + self.node.save() + properties = self.node.properties + properties['capabilities'] = 'boot_mode:uefi' + self.node.properties = properties + self.node.save() + self._test_prepare_ramdisk(uefi=True, node_boot_mode=boot_modes.UEFI) + self.assertEqual(set_boot_mode_mock.call_count, 0) + @mock.patch.object(pxe, '_clean_up_pxe_env', autospec=True) @mock.patch.object(pxe, '_get_image_info', autospec=True) def _test_clean_up_ramdisk(self, get_image_info_mock, @@ -1290,6 +1397,7 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_opts = pxe_utils.dhcp_options_for_instance(task) pxe_config_path = pxe_utils.get_pxe_config_file_path( task.node.uuid) + task.node.properties['capabilities'] = 'boot_mode:bios' task.driver.boot.prepare_instance(task) self.assertFalse(get_image_info_mock.called) self.assertFalse(cache_mock.called) @@ -1297,7 +1405,8 @@ class PXEBootTestCase(db_base.DbTestCase): create_pxe_config_mock.assert_called_once_with( task, mock.ANY, CONF.pxe.pxe_config_template) switch_pxe_config_mock.assert_called_once_with( - pxe_config_path, None, None, False, iscsi_boot=True) + pxe_config_path, None, boot_modes.LEGACY_BIOS, False, + iscsi_boot=True) set_boot_device_mock.assert_called_once_with(task, boot_devices.PXE, persistent=True) @@ -1307,7 +1416,10 @@ class PXEBootTestCase(db_base.DbTestCase): def test_prepare_instance_localboot(self, clean_up_pxe_config_mock, set_boot_device_mock): with task_manager.acquire(self.context, self.node.uuid) as task: - task.node.instance_info['capabilities'] = {'boot_option': 'local'} + instance_info = task.node.instance_info + instance_info['capabilities'] = {'boot_option': 'local'} + task.node.instance_info = instance_info + task.node.save() task.driver.boot.prepare_instance(task) clean_up_pxe_config_mock.assert_called_once_with(task) set_boot_device_mock.assert_called_once_with(task, @@ -1319,7 +1431,10 @@ class PXEBootTestCase(db_base.DbTestCase): def test_is_force_persistent_boot_device_enabled( self, clean_up_pxe_config_mock, set_boot_device_mock): with task_manager.acquire(self.context, self.node.uuid) as task: - task.node.instance_info['capabilities'] = {'boot_option': 'local'} + instance_info = task.node.instance_info + instance_info['capabilities'] = {'boot_option': 'local'} + task.node.instance_info = instance_info + task.node.save() task.driver.boot.prepare_instance(task) clean_up_pxe_config_mock.assert_called_once_with(task) driver_info = task.node.driver_info @@ -1336,7 +1451,10 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: - task.node.instance_info['capabilities'] = {'boot_option': 'local'} + instance_info = task.node.instance_info + instance_info['capabilities'] = {'boot_option': 'local'} + task.node.instance_info = instance_info + task.node.save() task.driver.boot.prepare_instance(task) clean_up_pxe_config_mock.assert_called_once_with(task) self.assertFalse(set_boot_device_mock.called) diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index 49a40f2c48..015ba8e218 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -478,6 +478,27 @@ class TestManagementInterface(base.TestCase): self.assertRaises(exception.UnsupportedDriverExtension, management.inject_nmi, task_mock) + def test_get_supported_boot_modes_default_impl(self): + management = fake.FakeManagement() + task_mock = mock.MagicMock(spec_set=['node']) + + self.assertRaises(exception.UnsupportedDriverExtension, + management.get_supported_boot_modes, task_mock) + + def test_set_boot_mode_default_impl(self): + management = fake.FakeManagement() + task_mock = mock.MagicMock(spec_set=['node']) + + self.assertRaises(exception.UnsupportedDriverExtension, + management.set_boot_mode, task_mock, 'whatever') + + def test_get_boot_mode_default_impl(self): + management = fake.FakeManagement() + task_mock = mock.MagicMock(spec_set=['node']) + + self.assertRaises(exception.UnsupportedDriverExtension, + management.get_boot_mode, task_mock) + class TestBaseDriver(base.TestCase): diff --git a/ironic/tests/unit/drivers/test_fake_hardware.py b/ironic/tests/unit/drivers/test_fake_hardware.py index 7091db4a50..792940de0a 100644 --- a/ironic/tests/unit/drivers/test_fake_hardware.py +++ b/ironic/tests/unit/drivers/test_fake_hardware.py @@ -19,6 +19,7 @@ from ironic.common import boot_devices +from ironic.common import boot_modes from ironic.common import exception from ironic.common import states from ironic.conductor import task_manager @@ -111,6 +112,13 @@ class FakeHardwareTestCase(db_base.DbTestCase): self.assertEqual(expected, self.driver.management.get_boot_device(self.task)) + def test_management_interface_set_boot_mode_good(self): + self.assertRaises( + exception.UnsupportedDriverExtension, + self.driver.management.set_boot_mode, + self.task, boot_modes.LEGACY_BIOS + ) + def test_inspect_interface(self): self.assertEqual({}, self.driver.inspect.get_properties()) self.driver.inspect.validate(self.task) diff --git a/releasenotes/notes/add-node-boot-mode-control-9761d4bcbd8c3a0d.yaml b/releasenotes/notes/add-node-boot-mode-control-9761d4bcbd8c3a0d.yaml new file mode 100644 index 0000000000..1c13abd2a3 --- /dev/null +++ b/releasenotes/notes/add-node-boot-mode-control-9761d4bcbd8c3a0d.yaml @@ -0,0 +1,16 @@ +--- +other: + - Adds ``get_boot_mode``, ``set_boot_mode`` and + ``get_supported_boot_modes`` methods to driver management interface. + Drivers can override these methods implementing boot mode management + calls to the BMC of the baremetal nodes being managed. +features: + - The new ironic configuration setting ``[deploy]/default_boot_mode`` + allows to set the default boot mode when ironic can't pick boot mode + automatically based on node configuration, hardware capabilities, + bare-metal machine configuration. +fixes: + - If the bare metal machine's boot mode differs from the requested one, + ironic will attempt to set requested boot mode on the bare metal + machine and fail explicitly if the driver does not support setting + boot mode on the node. \ No newline at end of file