Merge "Make boot_mode more consistent with other capabilities"

5 files changed, 77 insertions, 53 deletions

diff --git a/doc/source/install/standalone.rst b/doc/source/install/standalone.rst
index d747eb059..7aa5ecd13 100644
--- a/doc/source/install/standalone.rst
+++ b/doc/source/install/standalone.rst
@@ -266,7 +266,7 @@ Populating instance_info
 #. :ref:`Boot mode <boot_mode_support>` can be specified per instance::
 
     baremetal node set $NODE_UUID \
-        --instance-info deploy_boot_mode=uefi
+        --instance-info capabilities='{"boot_mode": "uefi"}'
 
    Otherwise, the ``boot_mode`` capability from the node's ``properties`` will
    be used.
@@ -275,9 +275,9 @@ Populating instance_info
         The two settings must not contradict each other.
 
    .. note::
-        The ``boot_mode`` capability is only used in the node's ``properties``,
-        not in ``instance_info`` like most other capabilities. Use the separate
-        ``instance_info/deploy_boot_mode`` field instead.
+        This capability was introduced in the Wallaby release series,
+        previously ironic used a separate ``instance_info/deploy_boot_mode``
+        field instead.
 
 #. To override the :ref:`boot option <local-boot-partition-images>` used for
    this instance, set the ``boot_option`` capability::
diff --git a/ironic/drivers/modules/boot_mode_utils.py b/ironic/drivers/modules/boot_mode_utils.py
index 6eebe50aa..9ddf64849 100644
--- a/ironic/drivers/modules/boot_mode_utils.py
+++ b/ironic/drivers/modules/boot_mode_utils.py
@@ -224,27 +224,35 @@ def get_boot_mode_for_deploy(node):
     # NOTE(etingof):
     # The search for a boot mode should be in the priority order:
     #
-    # 1) instance_info
-    # 2) properties.capabilities
-    # 3) driver_internal_info
+    # 1) instance_info.capabilities
+    # 2) instance_info.deploy_boot_mode (deprecated in Wallaby)
+    # 3) properties.capabilities
+    # 4) driver_internal_info.deploy_boot_mode (internal)
     #
     # Because:
     #
-    # (1) can be deleted before teardown
-    # (3) will never be touched if node properties/capabilities
+    # (1) and (2) are deleted during teardown
+    # (4) will never be touched if node properties/capabilities
     #     are still present.
-    # (2) becomes operational default as the last resort
-
-    instance_info = node.instance_info
+    # (3) becomes operational default as the last resort
 
+    inst_boot_mode = (
+        common_utils.parse_instance_info_capabilities(node).get('boot_mode')
+    )
     cap_boot_mode = driver_utils.get_node_capability(node, 'boot_mode')
 
-    boot_mode = instance_info.get('deploy_boot_mode')
-    if boot_mode is None:
-        boot_mode = cap_boot_mode
-        if cap_boot_mode is None:
-            driver_internal_info = node.driver_internal_info
-            boot_mode = driver_internal_info.get('deploy_boot_mode')
+    old_boot_mode = node.instance_info.get('deploy_boot_mode')
+    if old_boot_mode:
+        LOG.warning('Using instance_info/deploy_boot_mode is deprecated, '
+                    'please use instance_info/capabilities with boot mode '
+                    'for node %s', node.uuid)
+
+    boot_mode = (
+        inst_boot_mode
+        or old_boot_mode
+        or cap_boot_mode
+        or node.driver_internal_info.get('deploy_boot_mode')
+    )
 
     if not boot_mode:
         return
diff --git a/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py
index 6b52f6c76..611bc0ec4 100644
--- a/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_boot_mode_utils.py
@@ -69,6 +69,49 @@ class GetBootModeTestCase(tests_base.TestCase):
         self.assertEqual(boot_modes.UEFI, boot_mode)
         self.assertEqual(0, mock_log.warning.call_count)
 
+    def test_get_boot_mode_for_deploy_using_capabilities(self):
+        properties = {'capabilities': 'boot_mode:uefi,cap2:value2'}
+        self.node.properties = properties
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('uefi', result)
+
+    def test_get_boot_mode_for_deploy_using_instance_info_secure_boot(self):
+        instance_info = {'capabilities': {'secure_boot': 'True'}}
+        self.node.instance_info = instance_info
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('uefi', result)
+
+        instance_info = {'capabilities': {'trusted_boot': 'True'}}
+        self.node.instance_info = instance_info
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('bios', result)
+
+        instance_info = {'capabilities': {'trusted_boot': 'True',
+                                          'secure_boot': 'True'}}
+        self.node.instance_info = instance_info
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('uefi', result)
+
+    def test_get_boot_mode_for_deploy_using_instance_info_cap(self):
+        instance_info = {'capabilities': {'boot_mode': 'uefi'}}
+        self.node.instance_info = instance_info
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('uefi', result)
+
+    @mock.patch.object(boot_mode_utils.LOG, 'warning', autospec=True)
+    def test_get_boot_mode_for_deploy_using_instance_info(self, mock_log):
+        instance_info = {'deploy_boot_mode': 'bios'}
+        self.node.instance_info = instance_info
+
+        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
+        self.assertEqual('bios', result)
+        self.assertTrue(mock_log.called)
+
 
 @mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', autospec=True)
 class SecureBootTestCase(db_base.DbTestCase):
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index 41cb502ee..c2bfa0fa9 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -31,7 +31,6 @@ from ironic.common import states
 from ironic.common import utils as common_utils
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
-from ironic.drivers.modules import boot_mode_utils
 from ironic.drivers.modules import deploy_utils as utils
 from ironic.drivers.modules import fake
 from ironic.drivers.modules import image_cache
@@ -955,40 +954,6 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase):
         self.node.instance_info = {'capabilities': {"trusted_boot": "invalid"}}
         self.assertFalse(utils.is_trusted_boot_requested(self.node))
 
-    def test_get_boot_mode_for_deploy_using_capabilities(self):
-        properties = {'capabilities': 'boot_mode:uefi,cap2:value2'}
-        self.node.properties = properties
-
-        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
-        self.assertEqual('uefi', result)
-
-    def test_get_boot_mode_for_deploy_using_instance_info_cap(self):
-        instance_info = {'capabilities': {'secure_boot': 'True'}}
-        self.node.instance_info = instance_info
-
-        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
-        self.assertEqual('uefi', result)
-
-        instance_info = {'capabilities': {'trusted_boot': 'True'}}
-        self.node.instance_info = instance_info
-
-        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
-        self.assertEqual('bios', result)
-
-        instance_info = {'capabilities': {'trusted_boot': 'True',
-                                          'secure_boot': 'True'}}
-        self.node.instance_info = instance_info
-
-        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
-        self.assertEqual('uefi', result)
-
-    def test_get_boot_mode_for_deploy_using_instance_info(self):
-        instance_info = {'deploy_boot_mode': 'bios'}
-        self.node.instance_info = instance_info
-
-        result = boot_mode_utils.get_boot_mode_for_deploy(self.node)
-        self.assertEqual('bios', result)
-
     def test_validate_boot_mode_capability(self):
         prop = {'capabilities': 'boot_mode:uefi,cap2:value2'}
         self.node.properties = prop
diff --git a/releasenotes/notes/instance-info-boot-mode-25732c767593f849.yaml b/releasenotes/notes/instance-info-boot-mode-25732c767593f849.yaml
new file mode 100644
index 000000000..65e77010e
--- /dev/null
+++ b/releasenotes/notes/instance-info-boot-mode-25732c767593f849.yaml
@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Supports setting boot mode via an ``instance_info`` capability.
+deprecations:
+  - |
+    Using ``instance_info/deploy_boot_mode`` is deprecated, use the
+    ``boot_mode`` capability in ``instance_info/capabilities`` instead.