Rename redfish_deploy_iso -> deploy_iso

We should only use prefixed driver_info parameters when they are either
unique to the driver or have different meanings for different drivers.
This is not the case for deploy_iso, but we use vendor prefixes for
historical reasons.

This change migrates the redfish-virtual-media boot interface and
creates helpers to fascilitate migration of other boot interfaces.

Change-Id: I698d1c90592e8de2cb24d6e2cf819e7f6ac3911f
Story: #2008880
Task: #42425

6 files changed, 108 insertions, 30 deletions

diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index b0f5c7ea8..f92abbfe3 100644
--- a/doc/source/admin/drivers/redfish.rst
+++ b/doc/source/admin/drivers/redfish.rst
@@ -208,13 +208,18 @@ with the Wallaby release it's possible to provide a pre-built ISO image:
 .. code-block:: bash
 
   baremetal node set node-0 \
-    --driver_info redfish_deploy_iso=http://url/of/deploy.iso \
-    --driver_info redfish_rescue_iso=http://url/of/rescue.iso
+    --driver_info deploy_iso=http://url/of/deploy.iso \
+    --driver_info rescue_iso=http://url/of/rescue.iso
 
 .. note::
    OpenStack Image service (glance) image IDs and ``file://`` links are also
    accepted.
 
+.. note::
+   Before the Xena release the parameters were called ``redfish_deploy_iso``
+   and ``redfish_rescue_iso`` accordingly. The old names are still supported
+   for backward compatibility.
+
 No customization is currently done to the image, so e.g.
 :doc:`/admin/dhcp-less` won't work. `Configuring an ESP image`_ is also
 unnecessary.
diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index b854fc994..0f48b4d1e 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/ironic/drivers/modules/redfish/boot.py
@@ -35,9 +35,11 @@ LOG = log.getLogger(__name__)
 
 REQUIRED_PROPERTIES = {
     'deploy_kernel': _("URL or Glance UUID of the deployment kernel. "
-                       "Required."),
-    'deploy_ramdisk': _("URL or Glance UUID of the ramdisk that is "
-                        "mounted at boot time. Required.")
+                       "Required if deploy_iso is not set."),
+    'deploy_ramdisk': _("URL or Glance UUID of the ramdisk that is mounted at "
+                        "boot time. Required if deploy_iso is not set."),
+    'deploy_iso': _("URL or Glance UUID of the deployment ISO to use. "
+                    "Required if deploy_kernel/deploy_ramdisk are not set.")
 }
 
 OPTIONAL_PROPERTIES = {
@@ -56,16 +58,19 @@ OPTIONAL_PROPERTIES = {
                     "image containing EFI boot loader. This image will be "
                     "used by ironic when building UEFI-bootable ISO "
                     "out of kernel and ramdisk. Required for UEFI "
-                    "boot from partition images."),
+                    "when deploy_iso is not provided."),
 
 }
 
 RESCUE_PROPERTIES = {
-    'rescue_kernel': _('URL or Glance UUID of the rescue kernel. This value '
-                       'is required for rescue mode.'),
+    'rescue_kernel': _('URL or Glance UUID of the rescue kernel. Required for '
+                       'rescue mode if rescue_iso is not set.'),
     'rescue_ramdisk': _('URL or Glance UUID of the rescue ramdisk with agent '
-                        'that is used at node rescue time. This value is '
-                        'required for rescue mode.'),
+                        'that is used at node rescue time. Required for '
+                        'rescue mode if rescue_iso is not set.'),
+    'rescue_iso': _("URL or Glance UUID of the rescue ISO to use. Required "
+                    "for rescue mode if rescue_kernel/rescue_ramdisk are "
+                    "not set.")
 }
 
 COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
@@ -73,11 +78,6 @@ COMMON_PROPERTIES.update(driver_utils.OPTIONAL_PROPERTIES)
 COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
 COMMON_PROPERTIES.update(RESCUE_PROPERTIES)
 
-KERNEL_RAMDISK_LABELS = {
-    'deploy': REQUIRED_PROPERTIES,
-    'rescue': RESCUE_PROPERTIES
-}
-
 IMAGE_SUBDIR = 'redfish'
 
 sushy = importutils.try_import('sushy')
@@ -100,23 +100,15 @@ def _parse_driver_info(node):
     d_info = node.driver_info
 
     mode = deploy_utils.rescue_or_deploy_mode(node)
-    iso_param = 'redfish_%s_iso' % mode
-    iso_ref = d_info.get(iso_param)
+    iso_param = f'{mode}_iso'
+    iso_ref = driver_utils.get_agent_iso(node, deprecated_prefix='redfish',
+                                         mode=mode)
     if iso_ref is not None:
         deploy_info = {iso_param: iso_ref}
         can_config = False
     else:
-        params_to_check = KERNEL_RAMDISK_LABELS[mode]
-
-        deploy_info = {option: d_info.get(option)
-                       for option in params_to_check}
-
-        if not any(deploy_info.values()):
-            # NOTE(dtantsur): avoid situation when e.g. deploy_kernel comes
-            # from driver_info but deploy_ramdisk comes from configuration,
-            # since it's a sign of a potential operator's mistake.
-            deploy_info = {k: getattr(CONF.conductor, k)
-                           for k in params_to_check}
+        # There was never a deprecated prefix for kernel/ramdisk
+        deploy_info = driver_utils.get_agent_kernel_ramdisk(node, mode)
 
         error_msg = _("Error validating Redfish virtual media. Some "
                       "parameters were missing in node's driver_info")
@@ -378,6 +370,8 @@ class RedfishVirtualMediaBoot(base.BootInterface):
         node = task.node
 
         _parse_driver_info(node)
+        # Issue the deprecation warning if needed
+        driver_utils.get_agent_iso(node, deprecated_prefix='redfish')
 
     def _validate_instance_info(self, task):
         """Validate instance image information for the task's node.
diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py
index fa826c266..56b4409df 100644
--- a/ironic/drivers/utils.py
+++ b/ironic/drivers/utils.py
@@ -404,3 +404,45 @@ def get_kernel_append_params(node, default):
             return result
 
     return default
+
+
+def _get_field(node, name, deprecated_prefix=None):
+    value = node.driver_info.get(name)
+    if value or not deprecated_prefix:
+        return value
+
+    deprecated_name = f'{deprecated_prefix}_{name}'
+    value = node.driver_info.get(deprecated_name)
+    if value:
+        LOG.warning("The driver_info field %s of node %s is deprecated, "
+                    "please use %s instead",
+                    deprecated_name, node.uuid, name)
+        return value
+
+
+def get_agent_kernel_ramdisk(node, mode='deploy', deprecated_prefix=None):
+    """Get the agent kernel/ramdisk as a dictionary."""
+    kernel_name = f'{mode}_kernel'
+    ramdisk_name = f'{mode}_ramdisk'
+    kernel, ramdisk = (
+        _get_field(node, kernel_name, deprecated_prefix),
+        _get_field(node, ramdisk_name, deprecated_prefix),
+    )
+    # NOTE(dtantsur): avoid situation when e.g. deploy_kernel comes
+    # from driver_info but deploy_ramdisk comes from configuration,
+    # since it's a sign of a potential operator's mistake.
+    if not kernel or not ramdisk:
+        return {
+            kernel_name: getattr(CONF.conductor, kernel_name),
+            ramdisk_name: getattr(CONF.conductor, ramdisk_name),
+        }
+    else:
+        return {
+            kernel_name: kernel,
+            ramdisk_name: ramdisk
+        }
+
+
+def get_agent_iso(node, mode='deploy', deprecated_prefix=None):
+    """Get the agent ISO image."""
+    return _get_field(node, f'{mode}_iso', deprecated_prefix)
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index d4471cda5..e0f0dbf17 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -78,19 +78,31 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.node.driver_info.update(
+                {'deploy_iso': 'http://boot.iso'})
+
+            actual_driver_info = redfish_boot._parse_driver_info(task.node)
+
+            self.assertEqual('http://boot.iso',
+                             actual_driver_info['deploy_iso'])
+            self.assertFalse(actual_driver_info['can_provide_config'])
+
+    def test_parse_driver_info_iso_deprecated(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            task.node.driver_info.update(
                 {'redfish_deploy_iso': 'http://boot.iso'})
 
             actual_driver_info = redfish_boot._parse_driver_info(task.node)
 
             self.assertEqual('http://boot.iso',
-                             actual_driver_info['redfish_deploy_iso'])
+                             actual_driver_info['deploy_iso'])
             self.assertFalse(actual_driver_info['can_provide_config'])
 
     def test_parse_driver_info_removable(self):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.node.driver_info.update(
-                {'redfish_deploy_iso': 'http://boot.iso',
+                {'deploy_iso': 'http://boot.iso',
                  'config_via_removable': True}
             )
 
diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py
index c527c53dc..d2420b32b 100644
--- a/ironic/tests/unit/drivers/modules/test_image_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_image_utils.py
@@ -631,6 +631,25 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
                                   shared=True) as task:
 
             d_info = {
+                'deploy_iso': 'iso',
+            }
+            task.node.driver_info.update(d_info)
+
+            task.node.instance_info.update(deploy_boot_mode='uefi')
+
+            image_utils.prepare_deploy_iso(task, {}, 'deploy', d_info)
+
+            mock__prepare_iso_image.assert_called_once_with(
+                task, None, None, None, params={},
+                inject_files={}, base_iso='iso')
+
+    @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True)
+    def test_prepare_deploy_iso_existing_iso_vendor_prefix(
+            self, mock__prepare_iso_image):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+
+            d_info = {
                 'redfish_deploy_iso': 'iso',
             }
             task.node.driver_info.update(d_info)
diff --git a/releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml b/releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml
new file mode 100644
index 000000000..10111a795
--- /dev/null
+++ b/releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml
@@ -0,0 +1,6 @@
+---
+deprecations:
+  - |
+    The node's ``driver_info`` parameters ``redfish_deploy_iso`` and
+    ``redfish_rescue_iso`` have been renamed to ``deploy_iso`` and
+    ``rescue_iso`` accordingly. The old names are deprecated.