From 60767aee611bb46cf4b3fd1639224f817551bfb9 Mon Sep 17 00:00:00 2001 From: Hironori Shiina Date: Sun, 14 Oct 2018 22:13:47 +0900 Subject: [PATCH] Recommend to set boot mode explicitly A future release will change the default boot mode from legacy BIOS to UEFI. The default boot mode can be set to [deploy]/default_boot_mode option for hardware types which support setting boot mode. Otherwise, the default boot mode is hard-coded as legacy BIOS. This patch recommends to set boot mode explicitly in a help message of [deploy]/default_boot_mode option. A warning message is also logged when a default hard-coded boot mode is used. This message is logged once even if there are multiple nodes whose boot modes are configured explicitly for not emitting too many messages. Change-Id: Ib90ebf59ba72d49cb757e44f3741b5373a411ddf Story: 2003936 Task: 27475 --- ironic/common/pxe_utils.py | 11 ++-- ironic/conf/deploy.py | 11 ++-- ironic/drivers/modules/agent.py | 9 +-- ironic/drivers/modules/boot_mode_utils.py | 29 ++++++++++ ironic/drivers/modules/deploy_utils.py | 6 +- ironic/drivers/modules/ilo/boot.py | 6 +- ironic/drivers/modules/ipmitool.py | 2 +- ironic/drivers/modules/irmc/boot.py | 2 +- ironic/drivers/modules/iscsi_deploy.py | 14 +---- .../drivers/modules/test_boot_mode_utils.py | 55 +++++++++++++++++++ .../uefi-first-prepare-e7fa1e2a78b4af99.yaml | 10 ++++ 11 files changed, 118 insertions(+), 37 deletions(-) create mode 100644 ironic/tests/unit/drivers/modules/test_boot_mode_utils.py create mode 100644 releasenotes/notes/uefi-first-prepare-e7fa1e2a78b4af99.yaml diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index bd29b947d4..3128082578 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -279,7 +279,7 @@ def create_pxe_config(task, pxe_options, template=None, ipxe_enabled=False): pxe_config_file_path = get_pxe_config_file_path( task.node.uuid, ipxe_enabled=ipxe_enabled) - is_uefi_boot_mode = (boot_mode_utils.get_boot_mode_for_deploy(task.node) + is_uefi_boot_mode = (boot_mode_utils.get_boot_mode(task.node) == 'uefi') # grub bootloader panics with '{}' around any of its tags in its @@ -351,8 +351,7 @@ def clean_up_pxe_config(task, ipxe_enabled=False): """ LOG.debug("Cleaning up PXE config for node %s", task.node.uuid) - is_uefi_boot_mode = (boot_mode_utils.get_boot_mode_for_deploy(task.node) - == 'uefi') + is_uefi_boot_mode = (boot_mode_utils.get_boot_mode(task.node) == 'uefi') if is_uefi_boot_mode and not ipxe_enabled: api = dhcp_factory.DHCPFactory().provider @@ -777,7 +776,7 @@ def build_service_pxe_config(task, instance_image_info, iwdi = node.driver_internal_info.get('is_whole_disk_image') deploy_utils.switch_pxe_config( pxe_config_path, root_uuid_or_disk_id, - boot_mode_utils.get_boot_mode_for_deploy(node), + boot_mode_utils.get_boot_mode(node), iwdi, deploy_utils.is_trusted_boot_requested(node), deploy_utils.is_iscsi_boot(task), ramdisk_boot, ipxe_enabled=ipxe_enabled) @@ -852,7 +851,7 @@ def get_volume_pxe_options(task): def validate_boot_parameters_for_trusted_boot(node): """Check if boot parameters are valid for trusted boot.""" - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(node) + boot_mode = boot_mode_utils.get_boot_mode(node) boot_option = deploy_utils.get_boot_option(node) is_whole_disk_image = node.driver_internal_info.get('is_whole_disk_image') # 'is_whole_disk_image' is not supported by trusted boot, because there is @@ -906,7 +905,7 @@ def prepare_instance_pxe_config(task, image_info, ipxe_enabled=ipxe_enabled) deploy_utils.switch_pxe_config( pxe_config_path, None, - boot_mode_utils.get_boot_mode_for_deploy(node), False, + boot_mode_utils.get_boot_mode(node), False, iscsi_boot=iscsi_boot, ramdisk_boot=ramdisk_boot, ipxe_enabled=ipxe_enabled) diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 0c763fd751..6ae153b7dd 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -85,10 +85,13 @@ opts = [ 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}), + 'default boot mode is "%(bios)s", but it will be ' + 'changed to "%(uefi)s in the future. It is recommended ' + 'to set an explicit value for this option. This option ' + 'only has effect when management interface supports ' + 'boot mode management') % { + 'bios': boot_modes.LEGACY_BIOS, + 'uefi': boot_modes.UEFI}), cfg.BoolOpt('configdrive_use_object_store', default=False, deprecated_group='conductor', diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 80be59d0f7..b0fccadd76 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -237,11 +237,8 @@ class AgentDeployMixin(agent_base_vendor.AgentDeployMixin): for label in PARTITION_IMAGE_LABELS: image_info[label] = node.instance_info.get(label) boot_option = deploy_utils.get_boot_option(node) - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(node) - if boot_mode: - image_info['deploy_boot_mode'] = boot_mode - else: - image_info['deploy_boot_mode'] = 'bios' + image_info['deploy_boot_mode'] = ( + boot_mode_utils.get_boot_mode(node)) image_info['boot_option'] = boot_option disk_label = deploy_utils.get_disk_label(node) if disk_label is not None: @@ -336,7 +333,7 @@ class AgentDeployMixin(agent_base_vendor.AgentDeployMixin): efi_sys_uuid = None if not iwdi: - if boot_mode_utils.get_boot_mode_for_deploy(node) == 'uefi': + if boot_mode_utils.get_boot_mode(node) == 'uefi': efi_sys_uuid = (self._get_uuid_from_result(task, 'efi_system_partition_uuid')) diff --git a/ironic/drivers/modules/boot_mode_utils.py b/ironic/drivers/modules/boot_mode_utils.py index 7fb39d5d54..e7ab7017a6 100644 --- a/ironic/drivers/modules/boot_mode_utils.py +++ b/ironic/drivers/modules/boot_mode_utils.py @@ -15,6 +15,7 @@ from oslo_log import log as logging +from ironic.common import boot_modes from ironic.common import exception from ironic.common.i18n import _ from ironic.common import utils as common_utils @@ -24,6 +25,8 @@ from ironic.drivers import utils as driver_utils LOG = logging.getLogger(__name__) +warn_about_default_boot_mode = False + def _set_boot_mode_on_bm(task, ironic_boot_mode, fail_if_unsupported=False): try: @@ -266,3 +269,29 @@ def get_boot_mode_for_deploy(node): {'boot_mode': boot_mode, 'node': node.uuid}) return boot_mode + + +def get_boot_mode(node): + """Returns the boot mode. + + :param node: an ironic node object. + :returns: 'bios' or 'uefi' + :raises: InvalidParameterValue, if the node boot mode disagrees with + the boot mode set to node properties/capabilities + """ + boot_mode = get_boot_mode_for_deploy(node) + if boot_mode: + return boot_mode + # TODO(hshiina): The default boot mode will be changed to UEFI. + global warn_about_default_boot_mode + if not warn_about_default_boot_mode: + warn_about_default_boot_mode = True + LOG.warning('Boot mode is not configured for node %(node_uuid)s ' + 'explicitly. The default boot mode is "%(bios)s", but, ' + 'the default will be changed to "%(uefi)s" in the future. ' + 'It is recommended to set the boot option into ' + 'properties/capabilities/boot_mode for all nodes.', + {'node_uuid': node.uuid, + 'bios': boot_modes.LEGACY_BIOS, + 'uefi': boot_modes.UEFI}) + return boot_modes.LEGACY_BIOS diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index cb57d34194..51e04f5aa1 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -690,7 +690,7 @@ def try_set_boot_device(task, device, persistent=True): persistent=persistent) except exception.IPMIFailure: with excutils.save_and_reraise_exception() as ctxt: - if get_boot_mode_for_deploy(task.node) == 'uefi': + if boot_mode_utils.get_boot_mode(task.node) == 'uefi': ctxt.reraise = False LOG.warning("ipmitool is unable to set boot device while " "the node %s is in UEFI boot mode. Please set " @@ -722,7 +722,7 @@ def get_pxe_boot_file(node): cpu_arch = node.properties.get('cpu_arch') boot_file = CONF.pxe.pxe_bootfile_name_by_arch.get(cpu_arch) if boot_file is None: - if get_boot_mode_for_deploy(node) == 'uefi': + if boot_mode_utils.get_boot_mode(node) == 'uefi': boot_file = CONF.pxe.uefi_pxe_bootfile_name else: boot_file = CONF.pxe.pxe_bootfile_name @@ -743,7 +743,7 @@ def get_pxe_config_template(node): cpu_arch = node.properties.get('cpu_arch') config_template = CONF.pxe.pxe_config_template_by_arch.get(cpu_arch) if config_template is None: - if get_boot_mode_for_deploy(node) == 'uefi': + if boot_mode_utils.get_boot_mode(node) == 'uefi': config_template = CONF.pxe.uefi_pxe_config_template else: config_template = CONF.pxe.pxe_config_template diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 804e6772ce..670c65d952 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -185,7 +185,7 @@ def _get_boot_iso(task, root_uuid): # Option 3 - Create boot_iso from kernel/ramdisk, upload to Swift # or web server and provide its name. deploy_iso_uuid = deploy_info['ilo_deploy_iso'] - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + boot_mode = boot_mode_utils.get_boot_mode(task.node) boot_iso_object_name = _get_boot_iso_object_name(task.node) kernel_params = "" if deploy_utils.get_boot_option(task.node) == "ramdisk": @@ -525,7 +525,7 @@ class IloVirtualMediaBoot(base.BootInterface): """ ilo_common.cleanup_vmedia_boot(task) - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + boot_mode = boot_mode_utils.get_boot_mode(task.node) boot_option = deploy_utils.get_boot_option(task.node) if deploy_utils.is_iscsi_boot(task): @@ -700,7 +700,7 @@ class IloPXEBoot(pxe.PXEBoot): # Need to enable secure boot, if being requested ilo_common.update_secure_boot_mode(task, True) - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + boot_mode = boot_mode_utils.get_boot_mode(task.node) if deploy_utils.is_iscsi_boot(task) and boot_mode == 'uefi': # Need to set 'ilo_uefi_iscsi_boot' param for clean up diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 1dfc13cb12..85457dae56 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -938,7 +938,7 @@ class IPMIManagement(base.ManagementInterface): # uefi mode, this will work with newer and older versions of the # ipmitool utility. Also see: # https://bugs.launchpad.net/ironic/+bug/1611306 - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + boot_mode = boot_mode_utils.get_boot_mode(task.node) if persistent and boot_mode == 'uefi': raw_cmd = ('0x00 0x08 0x05 0xe0 %s 0x00 0x00 0x00' % BOOT_DEVICE_HEXA_MAP[device]) diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index e046f48ab7..4acfa06011 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -305,7 +305,7 @@ def _prepare_boot_iso(task, root_uuid): or image_properties['ramdisk_id']) deploy_iso_href = deploy_info['irmc_deploy_iso'] - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + boot_mode = boot_mode_utils.get_boot_mode(task.node) kernel_params = CONF.pxe.pxe_append_params boot_iso_filename = _get_iso_name(task.node, label='boot') diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 5bb23eb458..5cfaef2b59 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -117,7 +117,7 @@ def get_deploy_info(node, address, iqn, port=None, lun='1', conv_flags=None): 'ephemeral_mb': i_info['ephemeral_mb'], 'preserve_ephemeral': i_info['preserve_ephemeral'], 'boot_option': deploy_utils.get_boot_option(node), - 'boot_mode': _get_boot_mode(node), + 'boot_mode': boot_mode_utils.get_boot_mode(node), 'cpu_arch': node.properties.get('cpu_arch')}) # Append disk label if specified @@ -299,18 +299,6 @@ def do_agent_iscsi_deploy(task, agent_client): return uuid_dict_returned -def _get_boot_mode(node): - """Gets the boot mode. - - :param node: A single Node. - :returns: A string representing the boot mode type. Defaults to 'bios'. - """ - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(node) - if boot_mode: - return boot_mode - return "bios" - - @METRICS.timer('validate') def validate(task): """Validates the pre-requisites for iSCSI deploy. diff --git a/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py new file mode 100644 index 0000000000..11b5b5f7bb --- /dev/null +++ b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py @@ -0,0 +1,55 @@ +# Copyright 2018 FUJITSU LIMITED. +# 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. + +import mock + +from ironic.common import boot_modes +from ironic.drivers.modules import boot_mode_utils +from ironic.tests import base as tests_base +from ironic.tests.unit.objects import utils as obj_utils + + +class GetBootModeTestCase(tests_base.TestCase): + + def setUp(self): + super(GetBootModeTestCase, self).setUp() + self.node = obj_utils.get_test_node(self.context, + driver='fake-hardware') + + @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', + autospect=True) + def test_get_boot_mode_bios(self, mock_for_deploy): + mock_for_deploy.return_value = boot_modes.LEGACY_BIOS + boot_mode = boot_mode_utils.get_boot_mode(self.node) + self.assertEqual(boot_modes.LEGACY_BIOS, boot_mode) + + @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', + autospec=True) + def test_get_boot_mode_uefi(self, mock_for_deploy): + mock_for_deploy.return_value = boot_modes.UEFI + boot_mode = boot_mode_utils.get_boot_mode(self.node) + self.assertEqual(boot_modes.UEFI, boot_mode) + + @mock.patch.object(boot_mode_utils, 'LOG', autospect=True) + @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', + autospect=True) + def test_get_boot_mode_default(self, mock_for_deploy, mock_log): + boot_mode_utils.warn_about_default_boot_mode = False + mock_for_deploy.return_value = None + boot_mode = boot_mode_utils.get_boot_mode(self.node) + self.assertEqual(boot_modes.LEGACY_BIOS, boot_mode) + boot_mode = boot_mode_utils.get_boot_mode(self.node) + self.assertEqual(boot_modes.LEGACY_BIOS, boot_mode) + self.assertEqual(1, mock_log.warning.call_count) diff --git a/releasenotes/notes/uefi-first-prepare-e7fa1e2a78b4af99.yaml b/releasenotes/notes/uefi-first-prepare-e7fa1e2a78b4af99.yaml new file mode 100644 index 0000000000..2a8bf75228 --- /dev/null +++ b/releasenotes/notes/uefi-first-prepare-e7fa1e2a78b4af99.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + A future release will change the default value of + ``[deploy]/default_boot_mode`` from "bios" to "uefi". It is recommended to + set an explicit value for this option. For hardware types which don't + support setting boot mode, a future release will assume boot mode is set + to UEFI if no boot mode is set to node's capabilities. It is also + recommended to set ``boot_mode`` into ``properties/capabilities`` + of a node.