diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 48163f9ea9..0f457f9d41 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -735,7 +735,12 @@ class AgentDeployMixin(HeartbeatMixin): log_and_raise_deployment_error(task, msg) try: - deploy_utils.try_set_boot_device(task, boot_devices.DISK) + persistent = True + if node.driver_info.get('force_persistent_boot_device', + 'Default') == 'Never': + persistent = False + deploy_utils.try_set_boot_device(task, boot_devices.DISK, + persistent=persistent) except Exception as e: msg = (_("Failed to change the boot device to %(boot_dev)s " "when deploying node %(node)s. Error: %(error)s") % diff --git a/ironic/drivers/modules/ipxe.py b/ironic/drivers/modules/ipxe.py index 9dca81d732..0a334779a2 100644 --- a/ironic/drivers/modules/ipxe.py +++ b/ironic/drivers/modules/ipxe.py @@ -166,9 +166,14 @@ class iPXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): pxe_utils.create_pxe_config(task, pxe_options, pxe_config_template, ipxe_enabled=True) - persistent = strutils.bool_from_string( - node.driver_info.get('force_persistent_boot_device', - False)) + persistent = False + value = node.driver_info.get('force_persistent_boot_device', + 'Default') + if value in {'Always', 'Default', 'Never'}: + if value == 'Always': + persistent = True + else: + persistent = strutils.bool_from_string(value, False) manager_utils.node_set_boot_device(task, boot_devices.PXE, persistent=persistent) @@ -266,8 +271,12 @@ class iPXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): # NOTE(pas-ha) do not re-set boot device on ACTIVE nodes # during takeover if boot_device and task.node.provision_state != states.ACTIVE: + persistent = True + if node.driver_info.get('force_persistent_boot_device', + 'Default') == 'Never': + persistent = False manager_utils.node_set_boot_device(task, boot_device, - persistent=True) + persistent=persistent) @METRICS.timer('iPXEBoot.clean_up_instance') def clean_up_instance(self, task): diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 4ee5c35b2d..d4b0bce8ee 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -172,9 +172,15 @@ class PXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): pxe_utils.create_pxe_config(task, pxe_options, pxe_config_template, ipxe_enabled=CONF.pxe.ipxe_enabled) - persistent = strutils.bool_from_string( - node.driver_info.get('force_persistent_boot_device', - False)) + + persistent = False + value = node.driver_info.get('force_persistent_boot_device', + 'Default') + if value in {'Always', 'Default', 'Never'}: + if value == 'Always': + persistent = True + else: + persistent = strutils.bool_from_string(value, False) manager_utils.node_set_boot_device(task, boot_devices.PXE, persistent=persistent) @@ -274,8 +280,12 @@ class PXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): # NOTE(pas-ha) do not re-set boot device on ACTIVE nodes # during takeover if boot_device and task.node.provision_state != states.ACTIVE: + persistent = True + if node.driver_info.get('force_persistent_boot_device', + 'Default') == 'Never': + persistent = False manager_utils.node_set_boot_device(task, boot_device, - persistent=True) + persistent=persistent) @METRICS.timer('PXEBoot.clean_up_instance') def clean_up_instance(self, task): diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py index c9077743d2..39851f5ab3 100644 --- a/ironic/drivers/modules/pxe_base.py +++ b/ironic/drivers/modules/pxe_base.py @@ -31,10 +31,16 @@ REQUIRED_PROPERTIES = { "mounted at boot time. Required."), } OPTIONAL_PROPERTIES = { - 'force_persistent_boot_device': _("True to enable persistent behavior " - "when the boot device is set during " - "deploy and cleaning operations. " - "Defaults to False. Optional."), + 'force_persistent_boot_device': _("Controls the persistency of boot order " + "changes. 'Always' will make all " + "changes persistent, 'Default' will " + "make all but the final one upon " + "instance deployment non-persistent, " + "and 'Never' will make no persistent " + "changes at all. The old values 'True' " + "and 'False' are still supported but " + "deprecated in favor of the new ones." + "Defaults to 'Default'. Optional."), } RESCUE_PROPERTIES = { 'rescue_kernel': _('UUID (from Glance) of the rescue kernel. This value ' diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index cc071bb4cd..fb81cfcf40 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -761,7 +761,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): task.node.driver_internal_info['is_whole_disk_image'] = False self.deploy.configure_local_boot(task, root_uuid='some-root-uuid') try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid=None, prep_boot_part_uuid=None) @@ -779,7 +779,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot(task, root_uuid='some-root-uuid', prep_boot_part_uuid='fake-prep') try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid=None, prep_boot_part_uuid='fake-prep') @@ -798,7 +798,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): task, root_uuid='some-root-uuid', efi_system_part_uuid='efi-system-part-uuid') try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid='efi-system-part-uuid', @@ -814,7 +814,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot(task) self.assertFalse(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', @@ -827,7 +827,52 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot(task) self.assertFalse(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) + + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) + @mock.patch.object(agent_client.AgentClient, 'install_bootloader', + autospec=True) + def test_configure_local_boot_enforce_persistent_boot_device_default( + self, install_bootloader_mock, try_set_boot_device_mock): + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + driver_info = task.node.driver_info + driver_info['force_persistent_boot_device'] = 'Default' + task.node.driver_info = driver_info + self.deploy.configure_local_boot(task) + self.assertFalse(install_bootloader_mock.called) + try_set_boot_device_mock.assert_called_once_with( + task, boot_devices.DISK, persistent=True) + + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) + @mock.patch.object(agent_client.AgentClient, 'install_bootloader', + autospec=True) + def test_configure_local_boot_enforce_persistent_boot_device_always( + self, install_bootloader_mock, try_set_boot_device_mock): + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + driver_info = task.node.driver_info + driver_info['force_persistent_boot_device'] = 'Always' + task.node.driver_info = driver_info + self.deploy.configure_local_boot(task) + self.assertFalse(install_bootloader_mock.called) + try_set_boot_device_mock.assert_called_once_with( + task, boot_devices.DISK, persistent=True) + + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) + @mock.patch.object(agent_client.AgentClient, 'install_bootloader', + autospec=True) + def test_configure_local_boot_enforce_persistent_boot_device_never( + self, install_bootloader_mock, try_set_boot_device_mock): + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + driver_info = task.node.driver_info + driver_info['force_persistent_boot_device'] = 'Never' + task.node.driver_info = driver_info + self.deploy.configure_local_boot(task) + self.assertFalse(install_bootloader_mock.called) + try_set_boot_device_mock.assert_called_once_with( + task, boot_devices.DISK, persistent=False) @mock.patch.object(agent_client.AgentClient, 'collect_system_logs', autospec=True) @@ -878,7 +923,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid=None, prep_boot_part_uuid=None) try_set_boot_device_mock.assert_called_once_with( - task, boot_devices.DISK) + task, boot_devices.DISK, persistent=True) collect_logs_mock.assert_called_once_with(mock.ANY, task.node) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index 2a556c6d6e..e05cdcd47d 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -308,7 +308,15 @@ class iPXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk() - def test_prepare_ramdisk_force_persistent_boot_device_enabled(self): + def test_prepare_ramdisk_force_persistent_boot_device_true(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'True' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_bool_true(self): self.node.provision_state = states.DEPLOYING driver_info = self.node.driver_info driver_info['force_persistent_boot_device'] = True @@ -316,13 +324,63 @@ class iPXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk(persistent=True) - def test_prepare_ramdisk_force_persistent_boot_device_disabled(self): + def test_prepare_ramdisk_force_persistent_boot_device_sloppy_true(self): + for value in ['true', 't', '1', 'on', 'y', 'YES']: + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = value + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_false(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'False' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk() + + def test_prepare_ramdisk_force_persistent_boot_device_bool_false(self): self.node.provision_state = states.DEPLOYING driver_info = self.node.driver_info driver_info['force_persistent_boot_device'] = False self.node.driver_info = driver_info self.node.save() - self._test_prepare_ramdisk() + self._test_prepare_ramdisk(persistent=False) + + def test_prepare_ramdisk_force_persistent_boot_device_sloppy_false(self): + for value in ['false', 'f', '0', 'off', 'n', 'NO', 'yxz']: + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = value + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk() + + def test_prepare_ramdisk_force_persistent_boot_device_default(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Default' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=False) + + def test_prepare_ramdisk_force_persistent_boot_device_always(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Always' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_never(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Never' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=False) def test_prepare_ramdisk_rescue(self): self.node.provision_state = states.RESCUING diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 7121fc18ff..98fe26f7f1 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -306,7 +306,15 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk() - def test_prepare_ramdisk_force_persistent_boot_device_enabled(self): + def test_prepare_ramdisk_force_persistent_boot_device_true(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'True' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_bool_true(self): self.node.provision_state = states.DEPLOYING driver_info = self.node.driver_info driver_info['force_persistent_boot_device'] = True @@ -314,13 +322,63 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk(persistent=True) - def test_prepare_ramdisk_force_persistent_boot_device_disabled(self): + def test_prepare_ramdisk_force_persistent_boot_device_sloppy_true(self): + for value in ['true', 't', '1', 'on', 'y', 'YES']: + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = value + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_false(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'False' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk() + + def test_prepare_ramdisk_force_persistent_boot_device_bool_false(self): self.node.provision_state = states.DEPLOYING driver_info = self.node.driver_info driver_info['force_persistent_boot_device'] = False self.node.driver_info = driver_info self.node.save() - self._test_prepare_ramdisk() + self._test_prepare_ramdisk(persistent=False) + + def test_prepare_ramdisk_force_persistent_boot_device_sloppy_false(self): + for value in ['false', 'f', '0', 'off', 'n', 'NO', 'yxz']: + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = value + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk() + + def test_prepare_ramdisk_force_persistent_boot_device_default(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Default' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=False) + + def test_prepare_ramdisk_force_persistent_boot_device_always(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Always' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=True) + + def test_prepare_ramdisk_force_persistent_boot_device_never(self): + self.node.provision_state = states.DEPLOYING + driver_info = self.node.driver_info + driver_info['force_persistent_boot_device'] = 'Never' + self.node.driver_info = driver_info + self.node.save() + self._test_prepare_ramdisk(persistent=False) def test_prepare_ramdisk_rescue(self): self.node.provision_state = states.RESCUING diff --git a/releasenotes/notes/add-option-persistent-boot-device-139cf280fb66f4f7.yaml b/releasenotes/notes/add-option-persistent-boot-device-139cf280fb66f4f7.yaml new file mode 100644 index 0000000000..55e6bcad40 --- /dev/null +++ b/releasenotes/notes/add-option-persistent-boot-device-139cf280fb66f4f7.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + Adds capability to control the persistency of boot order changes during + instance deployment via (i)PXE on a per-node level. The option + 'force_persistent_boot_device' in the node's driver info for the (i)PXE + drivers is extended to allow the values 'Default' (make all changes + but the last one upon deployment non-persistent), 'Always' (make all + changes persistent), and 'Never' (make all boot order changes + non-persistent). +deprecations: + - | + The values 'True'/'False' for the option 'force_persistent_boot_device' + in the node's driver info for the (i)PXE drivers are deprecated and + support for them may be removed in a future release. The former default + value 'False' is replaced by the new value 'Default', the value 'True' + is replaced by 'Always'.