From fe380bbbab9d99554ceff82404f6033215c36063 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 11 Jan 2021 20:00:47 +0100 Subject: [PATCH] Follow-up for ramdisk deploy configdrive support 1) Do not issue a warning if the boot interface supports configdrive 2) Implement missing support for Swift URLs in configdrives Change-Id: I4b06478a14ab514d785f8e3972e5afbd79f8d3b5 --- doc/source/admin/drivers/redfish.rst | 3 +-- ironic/common/images.py | 8 +++++-- ironic/drivers/modules/image_utils.py | 18 +++++++++----- ironic/drivers/modules/pxe.py | 15 +++++++----- ironic/drivers/modules/redfish/boot.py | 3 ++- .../unit/drivers/modules/test_image_utils.py | 24 +++++++++++++++++++ .../ramdisk-configdrive-142149339dd00b47.yaml | 3 +-- 7 files changed, 55 insertions(+), 19 deletions(-) diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index 5ebebfc19b..80b981984a 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -272,8 +272,7 @@ parameter injection, as such the ``[instance_info]/kernel_append_params`` setting is ignored. Configuration drives are supported starting with the Wallaby release -for nodes that have a free virtual USB slot. The configuration option -``[deploy]configdrive_use_object_store`` must be set to ``False`` for now. +for nodes that have a free virtual USB slot. Layer 3 or DHCP-less ramdisk booting ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic/common/images.py b/ironic/common/images.py index 778f9a12df..84fbd93b30 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -346,7 +346,7 @@ def create_esp_image_for_uefi( raise exception.ImageCreationFailed(image_type='iso', error=e) -def fetch(context, image_href, path, force_raw=False): +def fetch_into(context, image_href, image_file): # TODO(vish): Improve context handling and add owner and auth data # when it is added to glance. Right now there is no # auth checking in glance, so we assume that access was @@ -357,9 +357,13 @@ def fetch(context, image_href, path, force_raw=False): {'image_service': image_service.__class__, 'image_href': image_href}) + image_service.download(image_href, image_file) + + +def fetch(context, image_href, path, force_raw=False): with fileutils.remove_path_on_error(path): with open(path, "wb") as image_file: - image_service.download(image_href, image_file) + fetch_into(context, image_href, image_file) if force_raw: image_to_raw(image_href, path, "%s.part" % path) diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index b904e35a35..d12d23b0a2 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -299,20 +299,26 @@ def cleanup_floppy_image(task): def prepare_configdrive_image(task, content): """Prepare an image with configdrive. + Decodes base64 contents and writes it into a disk image that can be + attached e.g. to a virtual USB device. Images stored in Swift are + downloaded first. + :param task: a TaskManager instance containing the node to act on. :param content: Config drive as a base64-encoded string. :raises: ImageCreationFailed, if it failed while creating the image. :raises: SwiftOperationError, if any operation with Swift fails. :returns: image URL for the image. """ - # FIXME(dtantsur): download and convert? - if '://' in content: - raise exception.ImageCreationFailed( - _('URLs are not supported for configdrive images yet')) - with tempfile.TemporaryFile(dir=CONF.tempdir) as comp_tmpfile_obj: - comp_tmpfile_obj.write(base64.b64decode(content)) + if '://' in content: + with tempfile.TemporaryFile(dir=CONF.tempdir) as tmpfile2: + images.fetch_into(task.context, content, tmpfile2) + tmpfile2.seek(0) + base64.decode(tmpfile2, comp_tmpfile_obj) + else: + comp_tmpfile_obj.write(base64.b64decode(content)) comp_tmpfile_obj.seek(0) + gz = gzip.GzipFile(fileobj=comp_tmpfile_obj, mode='rb') with tempfile.NamedTemporaryFile( dir=CONF.tempdir, suffix='.img') as image_tmpfile_obj: diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 222e952dac..81e04eb758 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -58,12 +58,15 @@ class PXERamdiskDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin, @base.deploy_step(priority=100) @task_manager.require_exclusive_lock def deploy(self, task): - if 'configdrive' in task.node.instance_info: - LOG.warning('A configuration drive is present with ' - 'in the deployment request of node %(node)s. ' - 'The configuration drive will be ignored for ' - 'this deployment.', - {'node': task.node}) + if ('configdrive' in task.node.instance_info + and 'ramdisk_boot_configdrive' not in + task.driver.boot.capabilities): + # TODO(dtantsur): make it an actual error? + LOG.warning('A configuration drive is present in the ramdisk ' + 'deployment request of node %(node)s with boot ' + 'interface %(drv)s. The configuration drive will be ' + 'ignored for this deployment.', + {'node': task.node, 'drv': task.node.boot_interface}) manager_utils.node_power_action(task, states.POWER_OFF) # Tenant neworks must enable connectivity to the boot # location, as reboot() can otherwise be very problematic. diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 21aae30fe1..bd57969a91 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -279,7 +279,8 @@ class RedfishVirtualMediaBoot(base.BootInterface): `[instance_info]image_source` node property. """ - capabilities = ['iscsi_volume_boot', 'ramdisk_boot'] + capabilities = ['iscsi_volume_boot', 'ramdisk_boot', + 'ramdisk_boot_configdrive'] def __init__(self): """Initialize the Redfish virtual media boot interface. diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index fd51d1c738..c4d817af20 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -304,6 +304,30 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): result = image_utils.prepare_configdrive_image(task, encoded) self.assertEqual(expected_url, result) + @mock.patch.object(images, 'fetch_into', autospec=True) + @mock.patch.object(image_utils, 'prepare_disk_image', autospec=True) + def test_prepare_configdrive_image_url(self, mock_prepare, mock_fetch): + content = 'https://swift/path' + expected_url = 'https://a.b/c.f?e=f' + encoded = b'H4sIAPJ8418C/0vOzytJzSsBAKkwxf4HAAAA' + + def _fetch(context, image_href, image_file): + self.assertEqual(content, image_href) + image_file.write(encoded) + + def _prepare(task, content, prefix): + with open(content, 'rb') as fp: + self.assertEqual(b'content', fp.read()) + return expected_url + + mock_fetch.side_effect = _fetch + mock_prepare.side_effect = _prepare + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + result = image_utils.prepare_configdrive_image(task, content) + self.assertEqual(expected_url, result) + @mock.patch.object(image_utils.ImageHandler, 'unpublish_image', autospec=True) def test_cleanup_iso_image(self, mock_unpublish): diff --git a/releasenotes/notes/ramdisk-configdrive-142149339dd00b47.yaml b/releasenotes/notes/ramdisk-configdrive-142149339dd00b47.yaml index 57741dc95f..617a5efc48 100644 --- a/releasenotes/notes/ramdisk-configdrive-142149339dd00b47.yaml +++ b/releasenotes/notes/ramdisk-configdrive-142149339dd00b47.yaml @@ -3,5 +3,4 @@ features: - | Supports attaching configdrives when doing ``ramdisk`` deploy with the ``redfish-virtual-media`` boot. A configdrive is attached to a free USB - slot. Swift must not be used for configdrive storage (this limitation will - be fixed later). + slot.