diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 03828fc34c..bc28bb39e5 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -572,7 +572,7 @@ if [[ "$IRONIC_BOOT_MODE" == "uefi" ]]; then fi # TODO(dtantsur): change this when we change the default value. -IRONIC_DEFAULT_BOOT_OPTION=${IRONIC_DEFAULT_BOOT_OPTION:-netboot} +IRONIC_DEFAULT_BOOT_OPTION=${IRONIC_DEFAULT_BOOT_OPTION:-local} if [ $IRONIC_DEFAULT_BOOT_OPTION != "netboot" ] && [ $IRONIC_DEFAULT_BOOT_OPTION != "local" ]; then die $LINENO "Supported values for IRONIC_DEFAULT_BOOT_OPTION are 'netboot' and 'local' only." fi diff --git a/ironic/cmd/conductor.py b/ironic/cmd/conductor.py index d8fbcfec9c..c9c56a2fdf 100644 --- a/ironic/cmd/conductor.py +++ b/ironic/cmd/conductor.py @@ -49,14 +49,6 @@ def warn_about_unsafe_shred_parameters(conf): 'Secure Erase. This is a possible SECURITY ISSUE!') -def warn_about_missing_default_boot_option(conf): - if not conf.deploy.default_boot_option: - LOG.warning('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') - - def warn_about_agent_token_deprecation(conf): if not conf.require_agent_token: LOG.warning('The ``[DEFAULT]require_agent_token`` option is not ' @@ -70,7 +62,6 @@ def warn_about_agent_token_deprecation(conf): def issue_startup_warnings(conf): warn_about_unsafe_shred_parameters(conf) - warn_about_missing_default_boot_option(conf) warn_about_agent_token_deprecation(conf) diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 092238ec8c..500324f1c5 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -80,11 +80,11 @@ opts = [ cfg.StrOpt('default_boot_option', choices=[('netboot', _('boot from a network')), ('local', _('local boot'))], + default='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.')), + 'requested in node\'s driver_info. Defaults to ' + '"local". Prior to the Ussuri release, the default ' + 'was "netboot".')), cfg.StrOpt('default_boot_mode', choices=[(boot_modes.UEFI, _('UEFI boot mode')), (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 787ea21fe0..9a9bb94b3b 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -563,6 +563,12 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): # 'instance_info' to 'local for backward compatibility. # TODO(stendulker): Fail here once the default boot # option is local. + # NOTE(TheJulia): Fixing the default boot mode only + # masks the failure as the lack of a user definition + # can be perceived as both an invalid configuration and + # reliance upon the default configuration. The reality + # being that in most scenarios, users do not want network + # booting, so the changed default should be valid. with excutils.save_and_reraise_exception(reraise=False) as ctx: instance_info = node.instance_info capabilities = utils.parse_instance_info_capabilities(node) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index a0e69e5294..cb0af75c8e 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -497,7 +497,8 @@ def validate_image_properties(ctx, deploy_info, properties): def get_default_boot_option(): """Gets the default boot option.""" - return CONF.deploy.default_boot_option or 'netboot' + # TODO(TheJulia): Deprecated: Remove after Ussuri. + return CONF.deploy.default_boot_option def get_boot_option(node): @@ -514,7 +515,8 @@ def get_boot_option(node): if is_software_raid(node): return 'local' capabilities = utils.parse_instance_info_capabilities(node) - return capabilities.get('boot_option', get_default_boot_option()).lower() + return capabilities.get('boot_option', + CONF.deploy.default_boot_option).lower() def is_software_raid(node): diff --git a/ironic/tests/unit/cmd/test_conductor.py b/ironic/tests/unit/cmd/test_conductor.py index 2da3db2a23..8de4ebb405 100644 --- a/ironic/tests/unit/cmd/test_conductor.py +++ b/ironic/tests/unit/cmd/test_conductor.py @@ -50,8 +50,3 @@ class ConductorStartTestCase(db_base.DbTestCase): 'deploy') conductor.warn_about_unsafe_shred_parameters(cfg.CONF) self.assertTrue(log_mock.warning.called) - - @mock.patch.object(conductor, 'LOG', autospec=True) - def test_warn_on_missing_default_boot_option(self, log_mock): - conductor.warn_about_missing_default_boot_option(cfg.CONF) - self.assertTrue(log_mock.warning.called) diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 449fafe9df..6950bfe8a5 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -998,9 +998,17 @@ class PXEInterfacesTestCase(db_base.DbTestCase): def test_get_instance_image_info(self): # Tests when 'is_whole_disk_image' exists in driver_internal_info + # NOTE(TheJulia): The method being tested is primarily geared for + # only netboot operation as the information should only need to be + # looked up again during network booting. + self.config(group="deploy", default_boot_option="netboot") self._test_get_instance_image_info() def test_get_instance_image_info_without_is_whole_disk_image(self): + # NOTE(TheJulia): The method being tested is primarily geared for + # only netboot operation as the information should only need to be + # looked up again during network booting. + self.config(group="deploy", default_boot_option="netboot") # Tests when 'is_whole_disk_image' doesn't exists in # driver_internal_info del self.node.driver_internal_info['is_whole_disk_image'] diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 1e133a5402..056467ad2c 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -1105,6 +1105,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): is_iscsi_boot_mock): self.node.driver_internal_info = {'root_uuid_or_disk_id': ( "12312642-09d3-467f-8e09-12385826a123")} + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.save() is_iscsi_boot_mock.return_value = False with task_manager.acquire(self.context, self.node.uuid, diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 6831759a77..fd83b8cc85 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -1074,6 +1074,8 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): autospec=True) def test_prepare_instance_partition_image( self, _cleanup_vmedia_boot_mock, _configure_vmedia_mock): + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.driver_internal_info = {'root_uuid_or_disk_id': "some_uuid"} self.node.save() with task_manager.acquire(self.context, self.node.uuid, @@ -1149,7 +1151,7 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.target_provision_state = states.ACTIVE self.node.instance_info = { 'capabilities': { - "secure_boot": "true" + "secure_boot": "true", 'boot_option': 'netboot' } } self.node.save() @@ -1177,7 +1179,7 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.target_provision_state = states.ACTIVE self.node.instance_info = { 'capabilities': { - "secure_boot": "false" + "secure_boot": "false", 'boot_option': 'netboot' } } self.node.save() @@ -1202,6 +1204,11 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.driver_internal_info = {'root_uuid_or_disk_id': "12312642"} self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE + self.node.instance_info = { + 'capabilities': { + 'boot_option': 'netboot' + } + } self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 354e041598..590a22e08e 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -1362,6 +1362,8 @@ class TestAgentDeploy(db_base.DbTestCase): power_on_node_if_needed_mock, resume_mock): check_deploy_mock.return_value = None + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} uuid_mock.return_value = 'root_uuid' self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 20e578bfb2..a39b33a351 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -701,7 +701,7 @@ class OtherFunctionTestCase(db_base.DbTestCase): def test_get_boot_option_default_value(self): self.node.instance_info = {} result = utils.get_boot_option(self.node) - self.assertEqual("netboot", result) + self.assertEqual("local", result) def test_get_boot_option_overridden_default_value(self): cfg.CONF.set_override('default_boot_option', 'local', 'deploy') diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index 2c78a75504..1b4b0a48c6 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -179,9 +179,11 @@ class iPXEBootTestCase(db_base.DbTestCase): @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) def test_validate_fail_no_image_kernel_ramdisk_props(self, mock_glance): + instance_info = {"boot_option": "netboot"} mock_glance.return_value = {'properties': {}} with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + task.node.instance_info['capabilities'] = instance_info self.assertRaises(exception.MissingParameterValue, task.driver.boot.validate, task) @@ -626,6 +628,7 @@ class iPXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( @@ -633,6 +636,7 @@ class iPXEBootTestCase(db_base.DbTestCase): pxe_config_path = pxe_utils.get_pxe_config_file_path( task.node.uuid, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False @@ -666,6 +670,7 @@ class iPXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info self.node.provision_state = states.ACTIVE self.node.save() @@ -675,6 +680,7 @@ class iPXEBootTestCase(db_base.DbTestCase): pxe_config_path = pxe_utils.get_pxe_config_file_path( task.node.uuid, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False @@ -708,10 +714,12 @@ class iPXEBootTestCase(db_base.DbTestCase): image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} get_image_info_mock.return_value = image_info + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = False task.driver.boot.prepare_instance(task) @@ -739,10 +747,12 @@ class iPXEBootTestCase(db_base.DbTestCase): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock get_image_info_mock.return_value = {} + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index ab887e96b1..e723b3518e 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -423,7 +423,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): def test_get_deploy_info_boot_option_default(self): ret_val = self._test_get_deploy_info() - self.assertEqual('netboot', ret_val['boot_option']) + self.assertEqual('local', ret_val['boot_option']) def test_get_deploy_info_netboot_specified(self): capabilities = {'capabilities': {'boot_option': 'netboot'}} @@ -1001,11 +1001,14 @@ class ISCSIDeployTestCase(db_base.DbTestCase): def test_continue_deploy_netboot(self, do_agent_iscsi_deploy_mock, reboot_and_finish_deploy_mock): + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE self.node.save() uuid_dict_returned = {'root uuid': 'some-root-uuid'} do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned + self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: with mock.patch.object( task.driver.boot, 'prepare_instance') as m_prep_instance: @@ -1395,7 +1398,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): 'configdrive': deploy_args['configdrive'], # boot_option defaults to 'netboot' if # not set - 'boot_option': deploy_args['boot_option'] or 'netboot', + 'boot_option': deploy_args['boot_option'] or 'local', 'boot_mode': deploy_args['boot_mode'], 'disk_label': deploy_args['disk_label'], 'cpu_arch': deploy_args['cpu_arch'] or '' @@ -1762,7 +1765,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): ephemeral_format, image_path, node_uuid, configdrive=None, preserve_ephemeral=False, - boot_option="netboot", + boot_option="local", boot_mode="bios", disk_label=None, cpu_arch="")] diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 33b487db2b..7f10297a2d 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -180,9 +180,11 @@ class PXEBootTestCase(db_base.DbTestCase): @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) def test_validate_fail_no_image_kernel_ramdisk_props(self, mock_glance): + instance_info = {"boot_option": "netboot"} mock_glance.return_value = {'properties': {}} with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + task.node.instance_info['capabilities'] = instance_info self.assertRaises(exception.MissingParameterValue, task.driver.boot.validate, task) @@ -559,7 +561,8 @@ class PXEBootTestCase(db_base.DbTestCase): task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False - + task.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( @@ -589,6 +592,7 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info self.node.provision_state = states.ACTIVE self.node.save() @@ -601,7 +605,7 @@ class PXEBootTestCase(db_base.DbTestCase): task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False - + task.node.instance_info['capabilities'] = instance_info task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( @@ -630,11 +634,13 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=False) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = False task.driver.boot.prepare_instance(task) @@ -660,10 +666,12 @@ class PXEBootTestCase(db_base.DbTestCase): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock get_image_info_mock.return_value = {} + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=False) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with(task, diff --git a/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml b/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml new file mode 100644 index 0000000000..7e7cd49be8 --- /dev/null +++ b/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The default value of ``[deploy]/default_boot_option`` is changed from + ``netboot`` to ``local``. + - Due to the default boot option change, partition images without ``grub2`` + will be unable to be deployed without the ``boot_option`` for the node + to be explicitly set to ``netboot``. diff --git a/zuul.d/ironic-jobs.yaml b/zuul.d/ironic-jobs.yaml index 6e41c7d984..b381a12823 100644 --- a/zuul.d/ironic-jobs.yaml +++ b/zuul.d/ironic-jobs.yaml @@ -151,6 +151,7 @@ IRONIC_ENABLED_POWER_INTERFACES: redfish IRONIC_ENABLED_MANAGEMENT_INTERFACES: redfish IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-partition-uefi-redfish-vmedia @@ -198,6 +199,7 @@ IRONIC_ENABLED_BOOT_INTERFACES: "fake,pxe" IRONIC_IPXE_ENABLED: False IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot devstack_services: mysql: False postgresql: True @@ -248,6 +250,7 @@ IRONIC_BOOT_MODE: uefi IRONIC_VM_SPECS_RAM: 3096 IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-ipa-partition-pxe_ipmitool @@ -262,6 +265,7 @@ # cause this job to fail easily due to the extra steps # and boot cycle of the cleaning operation. IRONIC_TEMPEST_BUILD_TIMEOUT: 850 + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-bfv @@ -305,6 +309,7 @@ IRONIC_AUTOMATED_CLEAN_ENABLED: False SWIFT_ENABLE_TEMPURLS: True SWIFT_TEMPURL_KEY: secretkey + IRONIC_DEFAULT_BOOT_OPTION: netboot devstack_plugins: ironic-inspector: https://opendev.org/openstack/ironic-inspector devstack_services: @@ -337,6 +342,7 @@ IRONIC_AUTOMATED_CLEAN_ENABLED: False IRONIC_DEFAULT_RESCUE_INTERFACE: no-rescue IRONIC_ENABLED_RESCUE_INTERFACES: "fake,no-rescue" + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-functional-python3 @@ -569,6 +575,7 @@ IRONIC_IPXE_ENABLED: False IRONIC_BOOT_MODE: uefi IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: # Security testing for known issues