From 93f947c852409af2c56a499428b09ff69ab345a1 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 23 Sep 2016 14:52:16 +0200 Subject: [PATCH] Introduce default_boot_option configuration option This is the first step to changing the default boot option from netboot to local. The new configuration allows setting the default boot option globally. A warning is issued if no explicit value is provided for this configuration. Change-Id: I3bd4a165fa2ec1105f34bf5a2150da99136ba4a6 Partial-Bug: #1619339 --- devstack/lib/ironic | 3 +++ etc/ironic/ironic.conf.sample | 8 ++++++++ ironic/conf/deploy.py | 7 +++++++ ironic/drivers/modules/deploy_utils.py | 20 +++++++++++++++++-- .../unit/drivers/modules/test_deploy_utils.py | 13 ++++++++++++ .../default_boot_option-f22c01f976bc2de7.yaml | 8 ++++++++ 6 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/default_boot_option-f22c01f976bc2de7.yaml diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 18237fbf7b..0bea7033e6 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -770,6 +770,9 @@ function configure_ironic_conductor { if [[ -n "$IRONIC_ENABLED_NETWORK_INTERFACES" ]]; then iniset $IRONIC_CONF_FILE DEFAULT enabled_network_interfaces $IRONIC_ENABLED_NETWORK_INTERFACES fi + + # TODO(dtantsur): change this when we change the default value. + iniset $IRONIC_CONF_FILE deploy default_boot_option netboot } # create_ironic_cache_dir() - Part of the init_ironic() process diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index a1eeb3e5ce..4e386c05a6 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -996,6 +996,14 @@ # to True. (boolean value) #power_off_after_deploy_failure = true +# Default boot option to use when no boot option is requested +# in node's driver_info. Currently the default is "netboot", +# but it will be changed to "local" in the future. It is +# recommended to set an explicit value for this option. +# (string value) +# Allowed values: netboot, local +#default_boot_option = + [dhcp] diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 6e109a2d9f..1cb988f63f 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -68,6 +68,13 @@ opts = [ default=True, help=_('Whether to power off a node after deploy failure. ' 'Defaults to True.')), + cfg.StrOpt('default_boot_option', + choices=['netboot', 'local'], + help=_('Default boot option to use when no boot option is ' + 'requested in node\'s driver_info. Currently the ' + 'default is "netboot", but it will be changed to ' + '"local" in the future. It is recommended to set ' + 'an explicit value for this option.')), ] diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 32bb16deb1..0e64843fca 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -79,7 +79,17 @@ def warn_about_unsafe_shred_parameters(): 'Secure Erase. This is a possible SECURITY ISSUE!')) +def warn_about_missing_default_boot_option(): + if not CONF.deploy.default_boot_option: + LOG.warning(_LW('The default value of default_boot_option ' + 'configuration will change eventually from ' + '"netboot" to "local". It is recommended to set ' + 'an explicit value for it during the transition ' + 'period')) + + warn_about_unsafe_shred_parameters() +warn_about_missing_default_boot_option() # All functions are called from deploy() directly or indirectly. # They are split for stub-out. @@ -311,7 +321,7 @@ def deploy_partition_image( address, port, iqn, lun, image_path, root_mb, swap_mb, ephemeral_mb, ephemeral_format, node_uuid, preserve_ephemeral=False, configdrive=None, - boot_option="netboot", boot_mode="bios", disk_label=None): + boot_option=None, boot_mode="bios", disk_label=None): """All-in-one function to deploy a partition image to a node. :param address: The iSCSI IP address. @@ -345,6 +355,7 @@ def deploy_partition_image( NOTE: If key exists but value is None, it means partition doesn't exist. """ + boot_option = boot_option or get_default_boot_option() image_mb = disk_utils.get_image_mb(image_path) if image_mb > root_mb: msg = (_('Root partition is too small for requested image. Image ' @@ -907,6 +918,11 @@ def validate_image_properties(ctx, deploy_info, properties): "%(properties)s") % {'image': image_href, 'properties': props}) +def get_default_boot_option(): + """Gets the default boot option.""" + return CONF.deploy.default_boot_option or 'netboot' + + def get_boot_option(node): """Gets the boot option. @@ -917,7 +933,7 @@ def get_boot_option(node): 'netboot'. """ capabilities = parse_instance_info_capabilities(node) - return capabilities.get('boot_option', 'netboot').lower() + return capabilities.get('boot_option', get_default_boot_option()).lower() # TODO(vdrok): This method is left here for backwards compatibility with out of diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 683d60b465..23dc42ebce 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1396,6 +1396,19 @@ class OtherFunctionTestCase(db_base.DbTestCase): result = utils.get_boot_option(self.node) self.assertEqual("netboot", result) + def test_get_boot_option_overriden_default_value(self): + cfg.CONF.set_override('default_boot_option', 'local', 'deploy') + self.node.instance_info = {} + result = utils.get_boot_option(self.node) + self.assertEqual("local", result) + + def test_get_boot_option_instance_info_priority(self): + cfg.CONF.set_override('default_boot_option', 'local', 'deploy') + self.node.instance_info = {'capabilities': + '{"boot_option": "netboot"}'} + result = utils.get_boot_option(self.node) + self.assertEqual("netboot", result) + @mock.patch.object(image_cache, 'clean_up_caches', autospec=True) def test_fetch_images(self, mock_clean_up_caches): diff --git a/releasenotes/notes/default_boot_option-f22c01f976bc2de7.yaml b/releasenotes/notes/default_boot_option-f22c01f976bc2de7.yaml new file mode 100644 index 0000000000..c0e430fd9d --- /dev/null +++ b/releasenotes/notes/default_boot_option-f22c01f976bc2de7.yaml @@ -0,0 +1,8 @@ +--- +features: + - Adds new option "[deploy]/default_boot_option" for setting the default + boot option when no explicit boot option is requested via capabilities. +upgrade: + - We are going to change the default value of "[deploy]/default_boot_option" + from "netboot" to "local" eventually. To avoid disruptions, it is + recommended to an explicit value to this option.