From 349bf2d5bcb38b409fe01aa78cc281f016199efc Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 14 Jun 2021 15:03:42 +0200 Subject: [PATCH] Clean up vendor prefixes for iLO boot Generic fields, such as deploy_{kernel,ramdisk,iso) and bootloader should not have vendor prefixes. This patch removes them from the iLO boot interfaces with deprecation. Also clean up the code a bit for less repetition. Change-Id: Ib23203fa01837ee96f8a38ab50fa8252334c8cb8 Story: #2008880 Task: #42430 --- doc/source/admin/drivers/ilo.rst | 52 ++-- ironic/drivers/modules/ilo/boot.py | 146 +++------- ironic/drivers/utils.py | 12 +- ironic/tests/unit/conductor/test_manager.py | 2 +- .../unit/drivers/modules/ilo/test_boot.py | 273 ++++++++---------- .../unit/drivers/modules/test_image_utils.py | 38 +-- .../notes/ilo-prefix-d5a65ac637f82f65.yaml | 13 + 7 files changed, 229 insertions(+), 307 deletions(-) create mode 100644 releasenotes/notes/ilo-prefix-d5a65ac637f82f65.yaml diff --git a/doc/source/admin/drivers/ilo.rst b/doc/source/admin/drivers/ilo.rst index 838a3cb976..147a4aebb5 100644 --- a/doc/source/admin/drivers/ilo.rst +++ b/doc/source/admin/drivers/ilo.rst @@ -246,8 +246,12 @@ The following command can be used to enroll a ProLiant node with --driver-info ilo_address= \ --driver-info ilo_username= \ --driver-info ilo_password= \ - --driver-info ilo_deploy_iso= \ - --driver-info ilo_rescue_iso= + --driver-info deploy_iso= \ + --driver-info rescue_iso= + +.. note:: + The fields ``deploy_iso`` and ``rescue_iso`` used to be called + ``ilo_deploy_iso`` and ``ilo_rescue_iso`` before the Xena release. The following command can be used to enroll a ProLiant node with ``ilo5`` hardware type: @@ -262,8 +266,8 @@ The following command can be used to enroll a ProLiant node with --driver-info ilo_address= \ --driver-info ilo_username= \ --driver-info ilo_password= \ - --driver-info ilo_deploy_iso= \ - --driver-info ilo_rescue_iso= + --driver-info deploy_iso= \ + --driver-info rescue_iso= Please refer to :doc:`/install/enabling-drivers` for detailed explanation of hardware type. @@ -289,7 +293,7 @@ Node configuration * The following properties are also required in node object's ``driver_info`` if ``ilo-virtual-media`` boot interface is used: - - ``ilo_deploy_iso``: The glance UUID of the deploy ramdisk ISO image. + - ``deploy_iso``: The glance UUID of the deploy ramdisk ISO image. - ``instance info/ilo_boot_iso`` property to be either boot iso Glance UUID or a HTTP(S) URL. This is optional property and is used when ``boot_option`` is set to ``netboot`` or ``ramdisk``. @@ -299,7 +303,7 @@ Node configuration configured to use ``ramdisk`` deploy interface. See :ref:`ramdisk-deploy` for details. - - ``ilo_rescue_iso``: The glance UUID of the rescue ISO image. This is optional + - ``rescue_iso``: The glance UUID of the rescue ISO image. This is optional property and is used when ``rescue`` interface is set to ``agent``. * The following properties are also required in node object's @@ -318,22 +322,26 @@ Node configuration ``driver_info`` if ``ilo-uefi-https`` boot interface is used for ``ilo5`` hardware type: - - ``ilo_deploy_kernel``: The glance UUID or a HTTPS URL of the deployment kernel. - - ``ilo_deploy_ramdisk``: The glance UUID or a HTTPS URL of the deployment ramdisk. - - ``ilo_bootloader``: The glance UUID or a HTTPS URL of the bootloader. - - ``ilo_rescue_kernel``: The glance UUID or a HTTPS URL of the rescue kernel. + - ``deploy_kernel``: The glance UUID or a HTTPS URL of the deployment kernel. + - ``deploy_ramdisk``: The glance UUID or a HTTPS URL of the deployment ramdisk. + - ``bootloader``: The glance UUID or a HTTPS URL of the bootloader. + - ``rescue_kernel``: The glance UUID or a HTTPS URL of the rescue kernel. This is optional property and is used when ``rescue`` interface is set to ``agent``. - - ``ilo_rescue_ramdisk``: The glance UUID or a HTTP(S) URL of the rescue ramdisk. + - ``rescue_ramdisk``: The glance UUID or a HTTP(S) URL of the rescue ramdisk. This is optional property and is used when ``rescue`` interface is set to ``agent``. - .. note:: - ``ilo-uefi-https`` boot interface is supported by only ``ilo5`` hardware - type. If the images are not hosted in glance, the references - must be HTTPS URLs hosted by secure webserver. This boot interface can - be used only when the current boot mode is ``UEFI``. + .. note:: + ``ilo-uefi-https`` boot interface is supported by only ``ilo5`` hardware + type. If the images are not hosted in glance, the references + must be HTTPS URLs hosted by secure webserver. This boot interface can + be used only when the current boot mode is ``UEFI``. + .. note:: + The fields ``deploy_kernel``, ``deploy_ramdisk``, ``rescue_kernel`` + ``rescue_ramdisk`` and ``bootloader`` used to have an ``ilo_`` prefix + before the Xena release. * The following parameters are mandatory in ``driver_info`` if ``ilo-inspect`` inspect inteface is used and SNMPv3 inspection @@ -1033,7 +1041,7 @@ for whole disk image support. HTTP(S) Based Deploy Support ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -The user input for the images given in ``driver_info`` like ``ilo_deploy_iso``, +The user input for the images given in ``driver_info`` like ``deploy_iso``, ``deploy_kernel`` and ``deploy_ramdisk`` and in ``instance_info`` like ``image_source``, ``kernel``, ``ramdisk`` and ``ilo_boot_iso`` may also be given as HTTP(S) URLs. @@ -2153,8 +2161,8 @@ into the baremetal service using the iLO IPv6 addresses. --driver-info ilo_address=2001:0db8:85a3:0000:0000:8a2e:0370:7334 \ --driver-info ilo_username=test-user \ --driver-info ilo_password=test-password \ - --driver-info ilo_deploy_iso=test-iso \ - --driver-info ilo_rescue_iso=test-iso + --driver-info deploy_iso=test-iso \ + --driver-info rescue_iso=test-iso .. note:: @@ -2259,9 +2267,9 @@ and ``ilo-uefi-https`` boot interface: --driver-info ilo_address= \ --driver-info ilo_username= \ --driver-info ilo_password= \ - --driver-info ilo_deploy_kernel= \ - --driver-info ilo_deploy_ramdisk= \ - --driver-info ilo_bootloader= + --driver-info deploy_kernel= \ + --driver-info deploy_ramdisk= \ + --driver-info bootloader= Layer 3 or DHCP-less ramdisk booting ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 86f7d7006e..556886b25c 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -15,8 +15,6 @@ Boot Interface for iLO drivers and its supporting methods. """ -from urllib import parse as urlparse - from ironic_lib import metrics_utils from oslo_config import cfg from oslo_log import log as logging @@ -47,38 +45,35 @@ METRICS = metrics_utils.get_metrics_logger(__name__) CONF = cfg.CONF REQUIRED_PROPERTIES = { - 'ilo_deploy_iso': _("UUID (from Glance) of the deployment ISO. " - "Required.") + 'deploy_iso': _("UUID (from Glance) of the deployment ISO. Required.") } RESCUE_PROPERTIES = { - 'ilo_rescue_iso': _("UUID (from Glance) of the rescue ISO. Only " - "required if rescue mode is being used and ironic is " - "managing booting the rescue ramdisk.") + 'rescue_iso': _("UUID (from Glance) of the rescue ISO. Only " + "required if rescue mode is being used and ironic is " + "managing booting the rescue ramdisk.") } REQUIRED_PROPERTIES_UEFI_HTTPS_BOOT = { - 'ilo_deploy_kernel': _("URL or Glance UUID of the deployment kernel. " - "Required."), - 'ilo_deploy_ramdisk': _("URL or Glance UUID of the ramdisk that is " - "mounted at boot time. Required."), + '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."), } RESCUE_PROPERTIES_UEFI_HTTPS_BOOT = { - 'ilo_rescue_kernel': _('URL or Glance UUID of the rescue kernel. This ' - 'value is required for rescue mode.'), - 'ilo_rescue_ramdisk': _('URL or Glance UUID of the rescue ramdisk with ' - 'agent that is used at node rescue time. ' - 'The value is required for rescue mode.'), + 'rescue_kernel': _('URL or Glance UUID of the rescue kernel. This ' + 'value is required for rescue mode.'), + 'rescue_ramdisk': _('URL or Glance UUID of the rescue ramdisk with ' + 'agent that is used at node rescue time. ' + 'The value is required for rescue mode.'), } OPTIONAL_PROPERTIES = { - 'ilo_bootloader': _("URL or Glance UUID of the EFI system partition " - "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."), + 'bootloader': _("URL or Glance UUID of the EFI system partition " + "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."), 'ilo_add_certificates': _("Boolean value that indicates whether the " "certificates require to be added to the " - "iLO.") -} -KERNEL_PARAM_PROPERTIES = { + "iLO."), 'kernel_append_params': _("Additional kernel parameters to pass down " "to instance kernel. These parameters can " "be consumed by the kernel or by the " @@ -87,13 +82,6 @@ KERNEL_PARAM_PROPERTIES = { "[ilo]/kernel_append_params ironic option.") } COMMON_PROPERTIES = REQUIRED_PROPERTIES -VMEDIA_OPTIONAL_PROPERTIES = OPTIONAL_PROPERTIES.copy() -VMEDIA_OPTIONAL_PROPERTIES.update(KERNEL_PARAM_PROPERTIES) - -KERNEL_RAMDISK_LABELS = { - 'deploy': REQUIRED_PROPERTIES_UEFI_HTTPS_BOOT, - 'rescue': RESCUE_PROPERTIES_UEFI_HTTPS_BOOT -} def parse_driver_info(node, mode='deploy'): @@ -112,33 +100,22 @@ def parse_driver_info(node, mode='deploy'): :raises: MissingParameterValue, if any of the required parameters are missing. """ - info = node.driver_info d_info = {} - if mode == 'rescue' and info.get('ilo_rescue_iso'): - d_info['ilo_rescue_iso'] = info.get('ilo_rescue_iso') - elif mode == 'deploy' and info.get('ilo_deploy_iso'): - d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') + iso_ref = driver_utils.get_agent_iso(node, mode, deprecated_prefix='ilo') + if iso_ref: + d_info[f'{mode}_iso'] = iso_ref else: - params_to_check = KERNEL_RAMDISK_LABELS[mode] + d_info = driver_utils.get_agent_kernel_ramdisk( + node, mode, deprecated_prefix='ilo') + d_info['bootloader'] = driver_utils.get_field(node, 'bootloader', + deprecated_prefix='ilo', + use_conf=True) - d_info = {option: info.get(option) - for option in params_to_check} - - if not any(d_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. - d_info = {k: getattr(CONF.conductor, k.replace('ilo_', '')) - for k in params_to_check} - - error_msg = (_("Error validating iLO virtual media for %s. Some " - "parameters were missing in node's driver_info.") % mode) + error_msg = (_("Error validating iLO boot for %s. Some " + "parameters were missing in node's driver_info") % mode) deploy_utils.check_for_missing_params(d_info, error_msg) - - d_info.update( - {k: info.get(k, getattr(CONF.conductor, k.replace('ilo_', ''), None)) - for k in VMEDIA_OPTIONAL_PROPERTIES}) - d_info.pop('ilo_add_certificates', None) + d_info['kernel_append_params'] = node.driver_info.get( + 'kernel_append_params') return d_info @@ -458,16 +435,14 @@ class IloVirtualMediaBoot(base.BootInterface): deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) if deploy_nic_mac is not None: ramdisk_params['BOOTIF'] = deploy_nic_mac - if (node.driver_info.get('ilo_rescue_iso') - and node.provision_state == states.RESCUING): - iso = node.driver_info['ilo_rescue_iso'] - ilo_common.setup_vmedia(task, iso, ramdisk_params) - elif node.driver_info.get('ilo_deploy_iso'): - iso = node.driver_info['ilo_deploy_iso'] - ilo_common.setup_vmedia(task, iso, ramdisk_params) + + mode = deploy_utils.rescue_or_deploy_mode(node) + d_info = parse_driver_info(node, mode) + if 'rescue_iso' in d_info: + ilo_common.setup_vmedia(task, d_info['rescue_iso'], ramdisk_params) + elif 'deploy_iso' in d_info: + ilo_common.setup_vmedia(task, d_info['deploy_iso'], ramdisk_params) else: - mode = deploy_utils.rescue_or_deploy_mode(node) - d_info = parse_driver_info(node, mode) iso = image_utils.prepare_deploy_iso(task, ramdisk_params, mode, d_info) ilo_common.setup_vmedia(task, iso) @@ -583,12 +558,7 @@ class IloVirtualMediaBoot(base.BootInterface): :raises: IloOperationError, if some operation on iLO failed. """ ilo_common.cleanup_vmedia_boot(task) - - info = task.node.driver_info - mode = deploy_utils.rescue_or_deploy_mode(task.node) - if ((mode == 'rescue' and not info.get('ilo_rescue_iso')) - or (mode == 'deploy' and not info.get('ilo_deploy_iso'))): - image_utils.cleanup_iso_image(task) + image_utils.cleanup_iso_image(task) def _configure_vmedia_boot(self, task, root_uuid): """Configure vmedia boot for the node. @@ -848,10 +818,8 @@ class IloUefiHttpsBoot(base.BootInterface): for prop in image_dict: image_ref = image_dict.get(prop) - if not service_utils.is_glance_image(image_ref): - prefix = urlparse.urlparse(image_ref).scheme.lower() - if prefix == 'http': - insecure_props.append(prop) + if image_ref is not None and image_ref.startswith('http://'): + insecure_props.append(image_ref) if len(insecure_props) > 0: error = (_('Secure URLs exposed over HTTPS are expected. ' @@ -896,29 +864,9 @@ class IloUefiHttpsBoot(base.BootInterface): :raises: InvalidParameterValue, if any of the required parameters are invalid. """ - info = node.driver_info + deploy_info = parse_driver_info(node, mode) - if mode == 'rescue': - params_to_check = RESCUE_PROPERTIES_UEFI_HTTPS_BOOT.keys() - else: - params_to_check = REQUIRED_PROPERTIES_UEFI_HTTPS_BOOT.keys() - - deploy_info = {option: 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.replace('ilo_', '')) - for k in params_to_check} - - deploy_info.update( - {k: info.get(k, getattr(CONF.conductor, - k.replace('ilo_', ''), None)) - for k in OPTIONAL_PROPERTIES}) - - should_add_certs = deploy_info.pop('ilo_add_certificates', True) + should_add_certs = node.driver_info.get('ilo_add_certificates', True) if should_add_certs is not None: try: @@ -934,14 +882,6 @@ class IloUefiHttpsBoot(base.BootInterface): self._validate_hrefs(deploy_info) - error_msg = (_("Error validating %s for iLO UEFI HTTPS boot. Some " - "parameters were missing in node's driver_info") % mode) - deploy_utils.check_for_missing_params(deploy_info, error_msg) - - deploy_info.update( - {k: info.get(k, getattr(CONF.ilo, k.replace('ilo_', ''), None)) - for k in KERNEL_PARAM_PROPERTIES}) - deploy_info.update(ilo_common.parse_driver_info(node)) return deploy_info diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py index 56b4409dfa..e46c6275e6 100644 --- a/ironic/drivers/utils.py +++ b/ironic/drivers/utils.py @@ -406,7 +406,8 @@ def get_kernel_append_params(node, default): return default -def _get_field(node, name, deprecated_prefix=None): +def get_field(node, name, deprecated_prefix=None, use_conf=False): + """Get a driver_info field with deprecated prefix.""" value = node.driver_info.get(name) if value or not deprecated_prefix: return value @@ -419,14 +420,17 @@ def _get_field(node, name, deprecated_prefix=None): deprecated_name, node.uuid, name) return value + if use_conf: + return getattr(CONF.conductor, name) + 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), + 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, @@ -445,4 +449,4 @@ def get_agent_kernel_ramdisk(node, mode='deploy', deprecated_prefix=None): def get_agent_iso(node, mode='deploy', deprecated_prefix=None): """Get the agent ISO image.""" - return _get_field(node, f'{mode}_iso', deprecated_prefix) + return get_field(node, f'{mode}_iso', deprecated_prefix) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 7e6e4216e8..3bd12a3fb4 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -5737,7 +5737,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): enabled_console_interfaces=['ilo']) expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', - 'ilo_deploy_iso', 'console_port', 'ilo_change_password', + 'deploy_iso', 'console_port', 'ilo_change_password', 'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password', 'snmp_auth_priv_password', 'snmp_auth_protocol', 'snmp_auth_priv_protocol', 'ilo_verify_ca'] diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 6b7cfe7c8b..4aa7810c6d 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -16,7 +16,6 @@ """Test class for boot methods used by iLO modules.""" from unittest import mock -from urllib import parse as urlparse from oslo_config import cfg @@ -53,44 +52,64 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): boot_interface = 'ilo-virtual-media' def test_parse_driver_info_deploy_iso(self): + self.node.driver_info['deploy_iso'] = 'deploy-iso' + self.node.driver_info['kernel_append_params'] = 'kernel-param' + expected_driver_info = {'kernel_append_params': 'kernel-param', + 'deploy_iso': 'deploy-iso'} + + actual_driver_info = ilo_boot.parse_driver_info(self.node) + self.assertEqual(expected_driver_info, actual_driver_info) + + def test_parse_driver_info_deploy_iso_deprecated(self): self.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' self.node.driver_info['kernel_append_params'] = 'kernel-param' - expected_driver_info = {'ilo_bootloader': None, - 'kernel_append_params': 'kernel-param', - 'ilo_deploy_iso': 'deploy-iso'} + expected_driver_info = {'kernel_append_params': 'kernel-param', + 'deploy_iso': 'deploy-iso'} actual_driver_info = ilo_boot.parse_driver_info(self.node) self.assertEqual(expected_driver_info, actual_driver_info) def test_parse_driver_info_rescue_iso(self): - self.node.driver_info['ilo_rescue_iso'] = 'rescue-iso' - expected_driver_info = {'ilo_bootloader': None, - 'kernel_append_params': None, - 'ilo_rescue_iso': 'rescue-iso'} + self.node.driver_info['rescue_iso'] = 'rescue-iso' + expected_driver_info = {'kernel_append_params': None, + 'rescue_iso': 'rescue-iso'} actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') self.assertEqual(expected_driver_info, actual_driver_info) def test_parse_driver_info_deploy(self): + self.node.driver_info['deploy_kernel'] = 'kernel' + self.node.driver_info['deploy_ramdisk'] = 'ramdisk' + self.node.driver_info['bootloader'] = 'bootloader' + self.node.driver_info['kernel_append_params'] = 'kernel-param' + expected_driver_info = {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', + 'kernel_append_params': 'kernel-param'} + + actual_driver_info = ilo_boot.parse_driver_info(self.node) + self.assertEqual(expected_driver_info, actual_driver_info) + + def test_parse_driver_info_deploy_deprecated(self): self.node.driver_info['ilo_deploy_kernel'] = 'kernel' self.node.driver_info['ilo_deploy_ramdisk'] = 'ramdisk' self.node.driver_info['ilo_bootloader'] = 'bootloader' self.node.driver_info['kernel_append_params'] = 'kernel-param' - expected_driver_info = {'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader', + expected_driver_info = {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', 'kernel_append_params': 'kernel-param'} actual_driver_info = ilo_boot.parse_driver_info(self.node) self.assertEqual(expected_driver_info, actual_driver_info) def test_parse_driver_info_rescue(self): - self.node.driver_info['ilo_rescue_kernel'] = 'kernel' - self.node.driver_info['ilo_rescue_ramdisk'] = 'ramdisk' - self.node.driver_info['ilo_bootloader'] = 'bootloader' - expected_driver_info = {'ilo_rescue_kernel': 'kernel', - 'ilo_rescue_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader', + self.node.driver_info['rescue_kernel'] = 'kernel' + self.node.driver_info['rescue_ramdisk'] = 'ramdisk' + self.node.driver_info['bootloader'] = 'bootloader' + expected_driver_info = {'rescue_kernel': 'kernel', + 'rescue_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', 'kernel_append_params': None} actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') @@ -100,9 +119,9 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): CONF.conductor.deploy_kernel = 'kernel' CONF.conductor.deploy_ramdisk = 'ramdisk' CONF.conductor.bootloader = 'bootloader' - expected_driver_info = {'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader', + expected_driver_info = {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', 'kernel_append_params': None} actual_driver_info = ilo_boot.parse_driver_info(self.node) @@ -113,9 +132,9 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): CONF.conductor.rescue_ramdisk = 'ramdisk' CONF.conductor.bootloader = 'bootloader' - expected_driver_info = {'ilo_rescue_kernel': 'kernel', - 'ilo_rescue_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader', + expected_driver_info = {'rescue_kernel': 'kernel', + 'rescue_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', 'kernel_append_params': None} actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') @@ -125,13 +144,8 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): CONF.conductor.deploy_kernel = 'kernel' CONF.conductor.deploy_ramdisk = 'ramdisk' - expected_driver_info = {'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': None, - 'kernel_append_params': None} - - actual_driver_info = ilo_boot.parse_driver_info(self.node) - self.assertEqual(expected_driver_info, actual_driver_info) + self.assertRaisesRegex(exception.MissingParameterValue, 'bootloader', + ilo_boot.parse_driver_info, self.node) def test_parse_driver_info_exc(self): self.assertRaises(exception.MissingParameterValue, @@ -438,7 +452,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' + task.node.driver_info['deploy_iso'] = 'deploy-iso' storage_mock.return_value = True task.driver.boot.validate(task) mock_val_instance_image_info.assert_called_once_with(task) @@ -536,7 +550,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): mock_val_driver_info, storage_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' + task.node.driver_info['deploy_iso'] = 'deploy-iso' storage_mock.return_value = False task.driver.boot.validate(task) mock_val_driver_info.assert_called_once_with(task) @@ -546,7 +560,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): def test_validate_inspection(self, mock_val_driver_info): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' + task.node.driver_info['deploy_iso'] = 'deploy-iso' task.driver.boot.validate_inspection(task) mock_val_driver_info.assert_called_once_with(task) @@ -695,9 +709,9 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): mode = 'deploy' ramdisk_params = {'a': 'b'} d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } driver_info_mock.return_value = d_info prepare_deploy_iso_mock.return_value = 'recreated-iso' @@ -851,18 +865,13 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'cleanup_vmedia_boot', spec_set=True, autospec=True) - @mock.patch.object(deploy_utils, 'rescue_or_deploy_mode', - spec_set=True, autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', spec_set=True, autospec=True) - def test_clean_up_ramdisk(self, cleanup_iso_mock, mode_mock, - cleanup_vmedia_mock): - mode_mock.return_value = 'deploy' + def test_clean_up_ramdisk(self, cleanup_iso_mock, cleanup_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot.clean_up_ramdisk(task) cleanup_vmedia_mock.assert_called_once_with(task) - mode_mock.assert_called_once_with(task.node) cleanup_iso_mock.assert_called_once_with(task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', @@ -1033,6 +1042,14 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): update_secure_boot_mode_mock.assert_called_once_with(task) def test_validate_rescue(self): + driver_info = self.node.driver_info + driver_info['rescue_iso'] = 'rescue.iso' + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.boot.validate_rescue(task) + + def test_validate_rescue_deprecated(self): driver_info = self.node.driver_info driver_info['ilo_rescue_iso'] = 'rescue.iso' self.node.driver_info = driver_info @@ -1394,86 +1411,29 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): enabled_raid_interfaces=['ilo5']) self.node = obj_utils.create_test_node(self.context, **n) - @mock.patch.object(urlparse, 'urlparse', spec_set=True, - autospec=True) - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - def test__validate_hrefs_https_image(self, is_glance_mock, urlparse_mock): - is_glance_mock.return_value = False - urlparse_mock.return_value.scheme = 'https' + def test__validate_hrefs_https_image(self): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: data = { - 'ilo_deploy_kernel': 'https://a.b.c.d/kernel', - 'ilo_deploy_ramdisk': 'https://a.b.c.d/ramdisk', - 'ilo_bootloader': 'https://a.b.c.d/bootloader' + 'deploy_kernel': 'https://a.b.c.d/kernel', + 'deploy_ramdisk': 'https://a.b.c.d/ramdisk', + 'bootloader': 'https://a.b.c.d/bootloader' } task.driver.boot._validate_hrefs(data) - glance_calls = [ - mock.call('https://a.b.c.d/kernel'), - mock.call('https://a.b.c.d/ramdisk'), - mock.call('https://a.b.c.d/bootloader') - ] - - is_glance_mock.assert_has_calls(glance_calls) - urlparse_mock.assert_has_calls(glance_calls) - - @mock.patch.object(urlparse, 'urlparse', spec_set=True, - autospec=True) - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - def test__validate_hrefs_http_image(self, is_glance_mock, urlparse_mock): - is_glance_mock.return_value = False - scheme_mock = mock.PropertyMock( - side_effect=['http', 'https', 'http']) - type(urlparse_mock.return_value).scheme = scheme_mock - + def test__validate_hrefs_http_image(self): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: data = { - 'ilo_deploy_kernel': 'http://a.b.c.d/kernel', - 'ilo_deploy_ramdisk': 'https://a.b.c.d/ramdisk', - 'ilo_bootloader': 'http://a.b.c.d/bootloader' + 'deploy_kernel': 'http://a.b.c.d/kernel', + 'deploy_ramdisk': 'https://a.b.c.d/ramdisk', + 'bootloader': 'http://a.b.c.d/bootloader' } - glance_calls = [ - mock.call('http://a.b.c.d/kernel'), - mock.call('https://a.b.c.d/ramdisk'), - mock.call('http://a.b.c.d/bootloader') - ] self.assertRaisesRegex(exception.InvalidParameterValue, "Secure URLs exposed over HTTPS are .*" - "['ilo_deploy_kernel', 'ilo_bootloader']", + "['deploy_kernel', 'bootloader']", task.driver.boot._validate_hrefs, data) - is_glance_mock.assert_has_calls(glance_calls) - urlparse_mock.assert_has_calls(glance_calls) - - @mock.patch.object(urlparse, 'urlparse', spec_set=True, - autospec=True) - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - def test__validate_hrefs_glance_image(self, is_glance_mock, urlparse_mock): - is_glance_mock.return_value = True - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - data = { - 'ilo_deploy_kernel': 'https://a.b.c.d/kernel', - 'ilo_deploy_ramdisk': 'https://a.b.c.d/ramdisk', - 'ilo_bootloader': 'https://a.b.c.d/bootloader' - } - - task.driver.boot._validate_hrefs(data) - - glance_calls = [ - mock.call('https://a.b.c.d/kernel'), - mock.call('https://a.b.c.d/ramdisk'), - mock.call('https://a.b.c.d/bootloader') - ] - - is_glance_mock.assert_has_calls(glance_calls) - urlparse_mock.assert_not_called() @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_parse_driver_info', autospec=True) @@ -1482,9 +1442,9 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): def test__parse_deploy_info(self, get_img_inst_mock, parse_driver_mock): parse_driver_mock.return_value = { - 'ilo_deploy_kernel': 'deploy-kernel', - 'ilo_deploy_ramdisk': 'deploy-ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'deploy-kernel', + 'deploy_ramdisk': 'deploy-ramdisk', + 'bootloader': 'bootloader' } get_img_inst_mock.return_value = { 'ilo_boot_iso': 'boot-iso', @@ -1497,18 +1457,18 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): instance_info['image_source'] = '6b2f0c0c-79e8-4db6-842e-43c9764204af' self.node.instance_info = instance_info - driver_info['ilo_deploy_kernel'] = 'deploy-kernel' - driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk' - driver_info['ilo_bootloader'] = 'bootloader' + driver_info['deploy_kernel'] = 'deploy-kernel' + driver_info['deploy_ramdisk'] = 'deploy-ramdisk' + driver_info['bootloader'] = 'bootloader' self.node.driver_info = driver_info self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: expected_info = { - 'ilo_deploy_kernel': 'deploy-kernel', - 'ilo_deploy_ramdisk': 'deploy-ramdisk', - 'ilo_bootloader': 'bootloader', + 'deploy_kernel': 'deploy-kernel', + 'deploy_ramdisk': 'deploy-ramdisk', + 'bootloader': 'bootloader', 'ilo_boot_iso': 'boot-iso', 'image_source': '6b2f0c0c-79e8-4db6-842e-43c9764204af' } @@ -1530,11 +1490,11 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): 'ilo_password': 'admin' } driver_info = self.node.driver_info - driver_info['ilo_deploy_kernel'] = 'deploy-kernel' - driver_info['ilo_rescue_kernel'] = 'rescue-kernel' - driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk' - driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk' - driver_info['ilo_bootloader'] = 'bootloader' + driver_info['deploy_kernel'] = 'deploy-kernel' + driver_info['rescue_kernel'] = 'rescue-kernel' + driver_info['deploy_ramdisk'] = 'deploy-ramdisk' + driver_info['rescue_ramdisk'] = 'rescue-ramdisk' + driver_info['bootloader'] = 'bootloader' driver_info['ilo_add_certificates'] = True driver_info['dummy_key'] = 'dummy-value' self.node.driver_info = driver_info @@ -1543,10 +1503,10 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: deploy_info = { - 'ilo_deploy_kernel': 'deploy-kernel', - 'ilo_deploy_ramdisk': 'deploy-ramdisk', - 'ilo_bootloader': 'bootloader', - 'kernel_append_params': 'nofb nomodeset vga=normal' + 'deploy_kernel': 'deploy-kernel', + 'deploy_ramdisk': 'deploy-ramdisk', + 'bootloader': 'bootloader', + 'kernel_append_params': None, } deploy_info.update({'ilo_username': 'admin', @@ -1571,11 +1531,11 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): } mode = 'rescue' driver_info = self.node.driver_info - driver_info['ilo_deploy_kernel'] = 'deploy-kernel' - driver_info['ilo_rescue_kernel'] = 'rescue-kernel' - driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk' - driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk' - driver_info['ilo_bootloader'] = 'bootloader' + driver_info['deploy_kernel'] = 'deploy-kernel' + driver_info['rescue_kernel'] = 'rescue-kernel' + driver_info['deploy_ramdisk'] = 'deploy-ramdisk' + driver_info['rescue_ramdisk'] = 'rescue-ramdisk' + driver_info['bootloader'] = 'bootloader' driver_info['ilo_add_certificates'] = 'false' driver_info['kernel_append_params'] = 'kernel-param' driver_info['dummy_key'] = 'dummy-value' @@ -1585,10 +1545,10 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: deploy_info = { - 'ilo_rescue_kernel': 'rescue-kernel', - 'ilo_rescue_ramdisk': 'rescue-ramdisk', - 'ilo_bootloader': 'bootloader', - 'kernel_append_params': 'kernel-param' + 'rescue_kernel': 'rescue-kernel', + 'rescue_ramdisk': 'rescue-ramdisk', + 'bootloader': 'bootloader', + 'kernel_append_params': 'kernel-param', } deploy_info.update({'ilo_username': 'admin', @@ -1603,21 +1563,19 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_validate_hrefs', autospec=True) - @mock.patch.object(deploy_utils, 'check_for_missing_params', - autospec=True) @mock.patch.object(ilo_common, 'parse_driver_info', autospec=True) def test__parse_driver_info_invalid_params( - self, parse_driver_mock, check_missing_mock, validate_href_mock): + self, parse_driver_mock, validate_href_mock): parse_driver_mock.return_value = { 'ilo_username': 'admin', 'ilo_password': 'admin' } driver_info = self.node.driver_info - driver_info['ilo_deploy_kernel'] = 'deploy-kernel' - driver_info['ilo_rescue_kernel'] = 'rescue-kernel' - driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk' - driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk' - driver_info['ilo_bootloader'] = 'bootloader' + driver_info['deploy_kernel'] = 'deploy-kernel' + driver_info['rescue_kernel'] = 'rescue-kernel' + driver_info['deploy_ramdisk'] = 'deploy-ramdisk' + driver_info['rescue_ramdisk'] = 'rescue-ramdisk' + driver_info['bootloader'] = 'bootloader' driver_info['dummy_key'] = 'dummy-value' driver_info['ilo_add_certificates'] = 'xyz' self.node.driver_info = driver_info @@ -1626,9 +1584,9 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: deploy_info = { - 'ilo_deploy_kernel': 'deploy-kernel', - 'ilo_deploy_ramdisk': 'deploy-ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'deploy-kernel', + 'deploy_ramdisk': 'deploy-ramdisk', + 'bootloader': 'bootloader' } deploy_info.update({'ilo_username': 'admin', @@ -1638,7 +1596,6 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): task.driver.boot._parse_driver_info, task.node) validate_href_mock.assert_not_called() - check_missing_mock.assert_not_called() parse_driver_mock.assert_not_called() @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_validate_hrefs', @@ -1764,9 +1721,9 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node.driver_info['ilo_deploy_kernel'] = 'deploy-kernel' - task.node.driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk' - task.node.driver_info['ilo_bootloader'] = 'bootloader' + task.node.driver_info['deploy_kernel'] = 'deploy-kernel' + task.node.driver_info['deploy_ramdisk'] = 'deploy-ramdisk' + task.node.driver_info['bootloader'] = 'bootloader' storage_mock.return_value = True task.driver.boot.validate(task) mock_val_instance_image_info.assert_called_once_with( @@ -2158,9 +2115,9 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): def test_validate_rescue(self): driver_info = self.node.driver_info - driver_info['ilo_rescue_kernel'] = 'rescue-kernel' - driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk' - driver_info['ilo_bootloader'] = 'bootloader' + driver_info['rescue_kernel'] = 'rescue-kernel' + driver_info['rescue_ramdisk'] = 'rescue-ramdisk' + driver_info['bootloader'] = 'bootloader' self.node.driver_info = driver_info self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: @@ -2168,13 +2125,13 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase): def test_validate_rescue_no_rescue_ramdisk(self): driver_info = self.node.driver_info - driver_info['ilo_rescue_kernel'] = 'rescue-kernel' - driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk' - driver_info.pop('ilo_bootloader', None) + driver_info['rescue_kernel'] = 'rescue-kernel' + driver_info['rescue_ramdisk'] = 'rescue-ramdisk' + driver_info.pop('bootloader', None) self.node.driver_info = driver_info self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaisesRegex(exception.MissingParameterValue, - "Error validating rescue for iLO UEFI " - "HTTPS boot.* ['ilo_bootloader']", + "Error validating iLO boot for rescue.*" + "['bootloader']", task.driver.boot.validate_rescue, task) diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index 91c0e514cf..b520c72b96 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -608,9 +608,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): def test__find_param(self): param_dict = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } param_str = "deploy_kernel" expected = "kernel" @@ -620,8 +620,8 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): def test__find_param_not_found(self): param_dict = { - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } param_str = "deploy_kernel" expected = None @@ -634,9 +634,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): shared=True) as task: d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } task.node.driver_info.update(d_info) @@ -691,8 +691,8 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): shared=True) as task: d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk' } task.node.driver_info.update(d_info) @@ -729,9 +729,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): shared=True) as task: d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } task.node.driver_info.update(d_info) @@ -757,9 +757,9 @@ cafile = /etc/ironic-python-agent/ironic.crt shared=True) as task: d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } task.node.driver_info.update(d_info) @@ -780,9 +780,9 @@ cafile = /etc/ironic-python-agent/ironic.crt with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: d_info = { - 'ilo_deploy_kernel': 'kernel', - 'ilo_deploy_ramdisk': 'ramdisk', - 'ilo_bootloader': 'bootloader' + 'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader' } task.node.driver_info.update(d_info) diff --git a/releasenotes/notes/ilo-prefix-d5a65ac637f82f65.yaml b/releasenotes/notes/ilo-prefix-d5a65ac637f82f65.yaml new file mode 100644 index 0000000000..5620cef9c7 --- /dev/null +++ b/releasenotes/notes/ilo-prefix-d5a65ac637f82f65.yaml @@ -0,0 +1,13 @@ +--- +deprecations: + - | + The following ``driver_info`` parameters have been renamed with + deprecation: + + * ``ilo_deploy_kernel`` -> ``deploy_kernel`` + * ``ilo_deploy_ramdisk`` -> ``deploy_ramdisk`` + * ``ilo_deploy_iso`` -> ``deploy_iso`` + * ``ilo_rescue_kernel`` -> ``rescue_kernel`` + * ``ilo_rescue_ramdisk`` -> ``rescue_ramdisk`` + * ``ilo_rescue_iso`` -> ``rescue_iso`` + * ``ilo_bootloader`` -> ``bootloader``