From d55929fc7d673290115a7cbab41d139176155507 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 22 Mar 2021 15:32:15 +0100 Subject: [PATCH] Support pre-built deploy/rescue ISO in Redfish This change adds support to pre-built ISO images via the new driver_info parameters redfish_deploy_iso and redfish_rescue_iso, similarly to the iLO hardware type. Also removes overly eager mocking in image unit tests. Change-Id: I1366791a6c6eb34f3a43337c4199592783765912 --- doc/source/admin/drivers/redfish.rst | 21 ++++++ ironic/drivers/modules/image_utils.py | 33 ++++++--- ironic/drivers/modules/redfish/boot.py | 29 ++++---- .../unit/drivers/modules/redfish/test_boot.py | 11 +++ .../unit/drivers/modules/test_image_utils.py | 68 +++++++------------ .../redfish-deploy-iso-60873289278bf28f.yaml | 5 ++ 6 files changed, 100 insertions(+), 67 deletions(-) create mode 100644 releasenotes/notes/redfish-deploy-iso-60873289278bf28f.yaml diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index f2b6e8f314..f2e273a7fa 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -198,6 +198,27 @@ property can be used to pass user-specified kernel command line parameters. For ramdisk kernel, ``[instance_info]/kernel_append_params`` property serves the same purpose. +Pre-built ISO images +~~~~~~~~~~~~~~~~~~~~ + +By default an ISO images is built per node using the deploy kernel and +initramfs provided in the configuration or the node's ``driver_info``. Starting +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 + +.. note:: + OpenStack Image service (glance) image IDs and ``file://`` links are also + accepted. + +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. + Configuring an ESP image ~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index 786e9bb4c5..e26ecddf1c 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -436,15 +436,23 @@ def _prepare_iso_image(task, kernel_href, ramdisk_href, boot_mode = boot_mode_utils.get_boot_mode(task.node) - LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " - "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " - "bootloader %(bootloader_href)s and kernel params %(params)s" - "", {'node': task.node.uuid, - 'boot_mode': boot_mode, - 'kernel_href': kernel_href, - 'ramdisk_href': ramdisk_href, - 'bootloader_href': bootloader_href, - 'params': kernel_params}) + if base_iso: + # TODO(dtantsur): fix this limitation eventually (see + # images.create_boot_iso). + LOG.debug('Using pre-built %(boot_mode)s ISO %(iso)s for node ' + '%(node)s, custom configuration will not be available', + {'boot_mode': boot_mode, 'node': task.node.uuid, + 'iso': base_iso}) + else: + LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " + "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " + "bootloader %(bootloader_href)s and kernel params %(params)s" + "", {'node': task.node.uuid, + 'boot_mode': boot_mode, + 'kernel_href': kernel_href, + 'ramdisk_href': ramdisk_href, + 'bootloader_href': bootloader_href, + 'params': kernel_params}) with tempfile.NamedTemporaryFile( dir=CONF.tempdir, suffix='.iso') as boot_fileobj: @@ -479,7 +487,8 @@ def _find_param(param_str, param_dict): for param_key in param_dict: if param_str in param_key: val = param_dict.get(param_key) - return val + if val is not None: + return val _TLS_REMOTE_FILE = 'etc/ironic-python-agent/ironic.crt' @@ -516,10 +525,12 @@ def prepare_deploy_iso(task, params, mode, d_info): kernel_str = '%s_kernel' % mode ramdisk_str = '%s_ramdisk' % mode + iso_str = '%s_iso' % mode bootloader_str = 'bootloader' kernel_href = _find_param(kernel_str, d_info) ramdisk_href = _find_param(ramdisk_str, d_info) + iso_href = _find_param(iso_str, d_info) bootloader_href = _find_param(bootloader_str, d_info) # TODO(TheJulia): At some point we should support something like @@ -527,7 +538,7 @@ def prepare_deploy_iso(task, params, mode, d_info): # injection. prepare_iso_image = functools.partial( _prepare_iso_image, task, kernel_href, ramdisk_href, - bootloader_href=bootloader_href, params=params) + bootloader_href=bootloader_href, params=params, base_iso=iso_href) inject_files = {} if CONF.agent.api_ca_file: diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 94d25eee3a..2a6815f531 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -100,22 +100,27 @@ def _parse_driver_info(node): d_info = node.driver_info mode = deploy_utils.rescue_or_deploy_mode(node) - params_to_check = KERNEL_RAMDISK_LABELS[mode] + iso_param = 'redfish_%s_iso' % mode + iso_ref = d_info.get(iso_param) + if iso_ref is not None: + deploy_info = {iso_param: iso_ref} + else: + params_to_check = KERNEL_RAMDISK_LABELS[mode] - deploy_info = {option: d_info.get(option) - for option in params_to_check} + 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} + 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} - error_msg = _("Error validating Redfish virtual media. Some " - "parameters were missing in node's driver_info") + error_msg = _("Error validating Redfish virtual media. Some " + "parameters were missing in node's driver_info") - deploy_utils.check_for_missing_params(deploy_info, error_msg) + deploy_utils.check_for_missing_params(deploy_info, error_msg) deploy_info.update( {option: d_info.get(option, getattr(CONF.conductor, option, None)) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 6a403a5229..e60eafea7c 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -72,6 +72,17 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertIn('ramdisk', actual_driver_info['deploy_ramdisk']) self.assertIn('bootloader', actual_driver_info['bootloader']) + 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( + {'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']) + def test_parse_driver_info_rescue(self): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index c0be7147b2..a8305d0233 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -529,10 +529,8 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): actual = image_utils._find_param(param_str, param_dict) self.assertEqual(actual, expected) - @mock.patch.object(image_utils, '_find_param', autospec=True) @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True) - def test_prepare_deploy_iso(self, mock__prepare_iso_image, - find_mock): + def test_prepare_deploy_iso(self, mock__prepare_iso_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -543,29 +541,34 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): } task.node.driver_info.update(d_info) - find_call_list = [ - mock.call('deploy_kernel', d_info), - mock.call('deploy_ramdisk', d_info), - mock.call('bootloader', d_info) - ] - find_mock.side_effect = [ - 'kernel', 'ramdisk', 'bootloader' - ] - 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, 'kernel', 'ramdisk', 'bootloader', params={}, - inject_files={}) + inject_files={}, base_iso=None) - find_mock.assert_has_calls(find_call_list) - - @mock.patch.object(image_utils, '_find_param', autospec=True) @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True) - def test_prepare_deploy_iso_network_data( - self, mock__prepare_iso_image, find_mock): + 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 = { + 'redfish_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_network_data(self, mock__prepare_iso_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -577,14 +580,6 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): task.node.instance_info.update() - find_call_list = [ - mock.call('deploy_kernel', d_info), - mock.call('deploy_ramdisk', d_info), - mock.call('bootloader', d_info) - ] - find_mock.side_effect = [ - 'kernel', 'ramdisk', None - ] network_data = {'a': ['b']} mock_get_node_nw_data = mock.MagicMock(return_value=network_data) @@ -602,14 +597,10 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): mock__prepare_iso_image.assert_called_once_with( task, 'kernel', 'ramdisk', bootloader_href=None, - params={}, inject_files=expected_files) + params={}, inject_files=expected_files, base_iso=None) - find_mock.assert_has_calls(find_call_list) - - @mock.patch.object(image_utils, '_find_param', autospec=True) @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True) - def test_prepare_deploy_iso_tls(self, mock__prepare_iso_image, - find_mock): + def test_prepare_deploy_iso_tls(self, mock__prepare_iso_image): with tempfile.NamedTemporaryFile(delete=False) as tf: temp_name = tf.name self.addCleanup(lambda: os.unlink(temp_name)) @@ -626,15 +617,6 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): } task.node.driver_info.update(d_info) - find_call_list = [ - mock.call('deploy_kernel', d_info), - mock.call('deploy_ramdisk', d_info), - mock.call('bootloader', d_info) - ] - find_mock.side_effect = [ - 'kernel', 'ramdisk', 'bootloader' - ] - task.node.instance_info.update(deploy_boot_mode='uefi') image_utils.prepare_deploy_iso(task, {}, 'deploy', d_info) @@ -648,9 +630,7 @@ cafile = /etc/ironic-python-agent/ironic.crt mock__prepare_iso_image.assert_called_once_with( task, 'kernel', 'ramdisk', 'bootloader', params={}, - inject_files=expected_files) - - find_mock.assert_has_calls(find_call_list) + inject_files=expected_files, base_iso=None) @mock.patch.object(image_utils, '_find_param', autospec=True) @mock.patch.object(image_utils, '_prepare_iso_image', autospec=True) diff --git a/releasenotes/notes/redfish-deploy-iso-60873289278bf28f.yaml b/releasenotes/notes/redfish-deploy-iso-60873289278bf28f.yaml new file mode 100644 index 0000000000..690765577c --- /dev/null +++ b/releasenotes/notes/redfish-deploy-iso-60873289278bf28f.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds support for pre-built ISO images to the ``redfish-virtual-media`` + boot interface and its derivatives.