From 0a8373872156865aeece0f4f68c940a6c399d481 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 6 May 2021 10:52:24 +0200 Subject: [PATCH] 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 --- doc/source/admin/drivers/redfish.rst | 9 +++- ironic/drivers/modules/redfish/boot.py | 46 ++++++++----------- ironic/drivers/utils.py | 42 +++++++++++++++++ .../unit/drivers/modules/redfish/test_boot.py | 16 ++++++- .../unit/drivers/modules/test_image_utils.py | 19 ++++++++ .../redfish-deploy-iso-9671ae83108f6385.yaml | 6 +++ 6 files changed, 108 insertions(+), 30 deletions(-) create mode 100644 releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index b0f5c7ea85..f92abbfe39 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 b854fc994d..0f48b4d1eb 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 fa826c2662..56b4409dfa 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 d4471cda51..e0f0dbf17e 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -75,6 +75,18 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertTrue(actual_driver_info['can_provide_config']) def test_parse_driver_info_iso(self): + 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( @@ -83,14 +95,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 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 c527c53dcb..d2420b32b6 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -627,6 +627,25 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True) def test_prepare_deploy_iso_existing_iso(self, mock__prepare_iso_image): + with task_manager.acquire(self.context, self.node.uuid, + 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: diff --git a/releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml b/releasenotes/notes/redfish-deploy-iso-9671ae83108f6385.yaml new file mode 100644 index 0000000000..10111a7957 --- /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.