Merge "Optionally preserve original system boot order upon instance deployment"

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") %

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):

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):

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 '

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)

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

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

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'.