Optionally preserve original system boot order upon instance deployment

By default, Ironic makes persistent changes to the system boot order
at the end of an instance deployment. This may not be desired in
all cases, e.g. when DC policies require to leave the persistent
system boot order unchanged.
While keeping the persistent approach as the default, this patch
proposes to extent the existing 'force_persistent_boot_device'
field in the node's driver_info for (i)PXE in a way that allows
to have all boot device changes to be non-persistent.

Change-Id: If3a19f74fb0dfbcff2cde4cd5a8cc1edf5de3058
Story: #2004846
Task: #29058

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