From d48479b52da7ede21f26299db4bd0370bb71ec21 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 12 Oct 2020 16:50:29 +0200 Subject: [PATCH] Simplify injecting network data into an ISO image Currently we're building a VFAT image with the network data just to unpack it back on the next step. Just pass the file directly. This fixes a permission denied problem on Bifrost on Fedora (at least). As a nice side effect, the change reduces the amount of IO done for virtual media quite substantially. Change-Id: I5499fa42c1d82a1a29099fbbba6f45d440448b72 --- ironic/common/images.py | 158 ++++++------------ ironic/drivers/modules/image_utils.py | 96 +++-------- ironic/tests/unit/common/test_images.py | 82 ++++----- .../unit/drivers/modules/test_image_utils.py | 30 ++-- .../vmedia-inject-files-b6e226e2db4cff06.yaml | 10 ++ 5 files changed, 132 insertions(+), 244 deletions(-) create mode 100644 releasenotes/notes/vmedia-inject-files-b6e226e2db4cff06.yaml diff --git a/ironic/common/images.py b/ironic/common/images.py index cdddddf8e3..3f9fb88379 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -19,7 +19,6 @@ Handling of VM disk images. """ -import contextlib import os import shutil @@ -48,9 +47,11 @@ def _create_root_fs(root_directory, files_info): :param root_directory: the filesystem root directory. :param files_info: A dict containing absolute path of file to be copied - -> relative path within the vfat image. For example:: + or its content as bytes -> relative path within + the vfat image. For example:: { - '/absolute/path/to/file' -> 'relative/path/within/root' + '/absolute/path/to/file': 'relative/path/within/root', + b'{"some": "json"}': 'another/relative/path' ... } :raises: OSError, if creation of any directory failed. @@ -59,10 +60,14 @@ def _create_root_fs(root_directory, files_info): for src_file, path in files_info.items(): target_file = os.path.join(root_directory, path) dirname = os.path.dirname(target_file) - if not os.path.exists(dirname): - os.makedirs(dirname) + if dirname: + os.makedirs(dirname, exist_ok=True) - shutil.copyfile(src_file, target_file) + if isinstance(src_file, bytes): + with open(target_file, 'wb') as fp: + fp.write(src_file) + else: + shutil.copyfile(src_file, target_file) def _umount_without_raise(mount_dir): @@ -155,67 +160,19 @@ def _generate_cfg(kernel_params, template, options): return utils.render_template(template, options) -def _read_dir(root_dir, prefix_dir=None): - """Gather files under given directory. +def _label(files_info): + """Get a suitable label for the files. - :param root_dir: a directory to traverse. - :returns: a dict mapping absolute paths to relative to the `root_dir`. + Returns "config-2" if the openstack metadata is present. """ - files_info = {} - - if not prefix_dir: - prefix_dir = root_dir - - for entry in os.listdir(root_dir): - path = os.path.join(root_dir, entry) - if os.path.isdir(path): - files_info.update(_read_dir(path, prefix_dir)) - - else: - files_info[path] = path[len(prefix_dir) + 1:] - - return files_info - - -@contextlib.contextmanager -def _collect_files(image_path): - """Mount image and return a dictionary of paths found there. - - Mounts given image under a temporary directory, walk its contents - and produce a dictionary of absolute->relative paths found on the - image. - - :param image_path: ISO9660 or FAT-formatted image to mount. - :raises: ImageCreationFailed, if image inspection failed. - :returns: a dict mapping absolute paths to relative to the mount point. - """ - if not image_path: - yield {} - return - - with utils.tempdir() as mount_dir: - try: - utils.mount(image_path, mount_dir, '-o', 'loop') - - except processutils.ProcessExecutionError as e: - LOG.exception("Mounting filesystem image %(image)s " - "failed", {'image': image_path}) - raise exception.ImageCreationFailed(image_type='iso', error=e) - - try: - yield _read_dir(mount_dir) - - except EnvironmentError as e: - LOG.exception( - "Examining image %(images)s failed: ", {'image': image_path}) - _umount_without_raise(mount_dir) - raise exception.ImageCreationFailed(image_type='iso', error=e) - - _umount_without_raise(mount_dir) + if any(x.startswith('openstack/') for x in files_info.values()): + return 'config-2' + else: + return 'VMEDIA_BOOT_ISO' def create_isolinux_image_for_bios( - output_file, kernel, ramdisk, kernel_params=None, configdrive=None): + output_file, kernel, ramdisk, kernel_params=None, inject_files=None): """Creates an isolinux image on the specified file. Copies the provided kernel, ramdisk to a directory, generates the isolinux @@ -229,8 +186,8 @@ def create_isolinux_image_for_bios( :param kernel_params: a list of strings(each element being a string like 'K=V' or 'K' or combination of them like 'K1=V1,K2,...') to be added as the kernel cmdline. - :param configdrive: ISO9660 or FAT-formatted OpenStack config drive - image. This image will be written onto the built ISO image. Optional. + :param inject_files: Mapping of local source file paths to their location + on the final ISO image. :raises: ImageCreationFailed, if image creation failed while copying files or while running command to generate iso. """ @@ -248,6 +205,8 @@ def create_isolinux_image_for_bios( ramdisk: 'initrd', CONF.isolinux_bin: ISOLINUX_BIN, } + if inject_files: + files_info.update(inject_files) # ldlinux.c32 is required for syslinux 5.0 or later. if CONF.ldlinux_c32: @@ -262,15 +221,12 @@ def create_isolinux_image_for_bios( if ldlinux_src: files_info[ldlinux_src] = LDLINUX_BIN - with _collect_files(configdrive) as cfgdrv_files: - files_info.update(cfgdrv_files) + try: + _create_root_fs(tmpdir, files_info) - try: - _create_root_fs(tmpdir, files_info) - - except EnvironmentError as e: - LOG.exception("Creating the filesystem root failed.") - raise exception.ImageCreationFailed(image_type='iso', error=e) + except EnvironmentError as e: + LOG.exception("Creating the filesystem root failed.") + raise exception.ImageCreationFailed(image_type='iso', error=e) cfg = _generate_cfg(kernel_params, CONF.isolinux_config_template, options) @@ -279,8 +235,7 @@ def create_isolinux_image_for_bios( utils.write_to_file(isolinux_cfg, cfg) try: - utils.execute('mkisofs', '-r', '-V', - 'config-2' if configdrive else 'VMEDIA_BOOT_ISO', + utils.execute('mkisofs', '-r', '-V', _label(files_info), '-cache-inodes', '-J', '-l', '-no-emul-boot', '-boot-load-size', '4', '-boot-info-table', '-b', ISOLINUX_BIN, '-o', output_file, tmpdir) @@ -291,7 +246,7 @@ def create_isolinux_image_for_bios( def create_esp_image_for_uefi( output_file, kernel, ramdisk, deploy_iso=None, esp_image=None, - kernel_params=None, configdrive=None): + kernel_params=None, inject_files=None): """Creates an ESP image on the specified file. Copies the provided kernel, ramdisk and EFI system partition image (ESP) to @@ -311,8 +266,8 @@ def create_esp_image_for_uefi( :param kernel_params: a list of strings(each element being a string like 'K=V' or 'K' or combination of them like 'K1=V1,K2,...') to be added as the kernel cmdline. - :param configdrive: ISO9660 or FAT-formatted OpenStack config drive - image. This image will be written onto the built ISO image. Optional. + :param inject_files: Mapping of local source file paths to their location + on the final ISO image. :raises: ImageCreationFailed, if image creation failed while copying files or while running command to generate iso. """ @@ -325,6 +280,8 @@ def create_esp_image_for_uefi( kernel: 'vmlinuz', ramdisk: 'initrd', } + if inject_files: + files_info.update(inject_files) with utils.tempdir() as mountdir: # Open the deploy iso used to initiate deploy and copy the @@ -359,20 +316,17 @@ def create_esp_image_for_uefi( files_info.update(uefi_path_info) - with _collect_files(configdrive) as cfgdrv_files: - files_info.update(cfgdrv_files) + try: + _create_root_fs(tmpdir, files_info) - try: - _create_root_fs(tmpdir, files_info) + except EnvironmentError as e: + LOG.exception("Creating the filesystem root failed.") + raise exception.ImageCreationFailed( + image_type='iso', error=e) - except EnvironmentError as e: - LOG.exception("Creating the filesystem root failed.") - raise exception.ImageCreationFailed( - image_type='iso', error=e) - - finally: - if deploy_iso: - _umount_without_raise(mountdir) + finally: + if deploy_iso: + _umount_without_raise(mountdir) # Generate and copy grub config file. grub_conf = _generate_cfg(kernel_params, @@ -381,8 +335,7 @@ def create_esp_image_for_uefi( # Create the boot_iso. try: - utils.execute('mkisofs', '-r', '-V', - 'config-2' if configdrive else 'VMEDIA_BOOT_ISO', + utils.execute('mkisofs', '-r', '-V', _label(files_info), '-l', '-e', e_img_rel_path, '-no-emul-boot', '-o', output_file, tmpdir) @@ -391,10 +344,6 @@ def create_esp_image_for_uefi( raise exception.ImageCreationFailed(image_type='iso', error=e) -# NOTE(etingof): backward compatibility -create_isolinux_image_for_uefi = create_esp_image_for_uefi - - def fetch(context, image_href, path, force_raw=False): # TODO(vish): Improve context handling and add owner and auth data # when it is added to glance. Right now there is no @@ -526,7 +475,7 @@ def get_temp_url_for_glance_image(context, image_uuid): def create_boot_iso(context, output_filename, kernel_href, ramdisk_href, deploy_iso_href=None, esp_image_href=None, root_uuid=None, kernel_params=None, boot_mode=None, - configdrive_href=None, base_iso=None): + base_iso=None, inject_files=None): """Creates a bootable ISO image for a node. Given the hrefs for kernel, ramdisk, root partition's UUID and @@ -550,12 +499,11 @@ def create_boot_iso(context, output_filename, kernel_href, :param kernel_params: a string containing whitespace separated values kernel cmdline arguments of the form K=V or K (optional). :boot_mode: the boot mode in which the deploy is to happen. - :param configdrive_href: URL to ISO9660 or FAT-formatted OpenStack config - drive image. This image will be embedded into the built ISO image. - Optional. :param base_iso: URL or glance UUID of a to be used as an override of what should be retrieved for to use, instead of building an ISO bootable ramdisk. + :param inject_files: Mapping of local source file paths to their location + on the final ISO image. :raises: ImageCreationFailed, if creating boot ISO failed. """ with utils.tempdir() as tmpdir: @@ -574,14 +522,6 @@ def create_boot_iso(context, output_filename, kernel_href, fetch(context, kernel_href, kernel_path) fetch(context, ramdisk_href, ramdisk_path) - if configdrive_href: - configdrive_path = os.path.join( - tmpdir, configdrive_href.split('/')[-1]) - fetch(context, configdrive_href, configdrive_path) - - else: - configdrive_path = None - params = [] if root_uuid: params.append('root=UUID=%s' % root_uuid) @@ -612,12 +552,12 @@ def create_boot_iso(context, output_filename, kernel_href, create_esp_image_for_uefi( output_filename, kernel_path, ramdisk_path, deploy_iso=deploy_iso_path, esp_image=esp_image_path, - kernel_params=params, configdrive=configdrive_path) + kernel_params=params, inject_files=inject_files) else: create_isolinux_image_for_bios( output_filename, kernel_path, ramdisk_path, - kernel_params=params, configdrive=configdrive_path) + kernel_params=params, inject_files=inject_files) def is_whole_disk_image(ctx, instance_info): diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index dcdeff9d1f..b50ffe867f 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -22,7 +22,6 @@ from urllib import parse as urlparse from ironic_lib import utils as ironic_utils from oslo_log import log -from oslo_serialization import base64 from ironic.common import exception from ironic.common.i18n import _ @@ -288,28 +287,24 @@ def cleanup_floppy_image(task): def _prepare_iso_image(task, kernel_href, ramdisk_href, - bootloader_href=None, configdrive=None, - root_uuid=None, params=None, base_iso=None): + bootloader_href=None, root_uuid=None, params=None, + base_iso=None, inject_files=None): """Prepare an ISO to boot the node. Build bootable ISO out of `kernel_href` and `ramdisk_href` (and `bootloader` if it's UEFI boot), then push built image up to Swift and return a temporary URL. - If `configdrive` is specified it will be eventually written onto - the boot ISO image. - :param task: a TaskManager instance containing the node to act on. :param kernel_href: URL or Glance UUID of the kernel to use :param ramdisk_href: URL or Glance UUID of the ramdisk to use :param bootloader_href: URL or Glance UUID of the EFI bootloader image to use when creating UEFI bootbable ISO - :param configdrive: URL to or a compressed blob of a ISO9660 or - FAT-formatted OpenStack config drive image. This image will be - written onto the built ISO image. Optional. :param root_uuid: optional uuid of the root partition. :param params: a dictionary containing 'parameter name'->'value' mapping to be passed to kernel command line. + :param inject_files: Mapping of local source file paths to their location + on the final ISO image. :returns: bootable ISO HTTP URL. :raises: MissingParameterValue, if any of the required parameters are missing. @@ -360,44 +355,21 @@ def _prepare_iso_image(task, kernel_href, ramdisk_href, with tempfile.NamedTemporaryFile( dir=CONF.tempdir, suffix='.iso') as boot_fileobj: - with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.img') as cfgdrv_fileobj: + boot_iso_tmp_file = boot_fileobj.name + images.create_boot_iso( + task.context, boot_iso_tmp_file, + kernel_href, ramdisk_href, + esp_image_href=bootloader_href, + root_uuid=root_uuid, + kernel_params=kernel_params, + boot_mode=boot_mode, + base_iso=base_iso, + inject_files=inject_files) - configdrive_href = configdrive + iso_object_name = _get_iso_image_name(task.node) - # FIXME(TheJulia): This is treated as conditional with - # a base_iso as the intent, eventually, is to support - # injection into the supplied image. - - if configdrive and not base_iso: - parsed_url = urlparse.urlparse(configdrive) - if not parsed_url.scheme: - cfgdrv_blob = base64.decode_as_bytes(configdrive) - - with open(cfgdrv_fileobj.name, 'wb') as f: - f.write(cfgdrv_blob) - - configdrive_href = urlparse.urlunparse( - ('file', '', cfgdrv_fileobj.name, '', '', '')) - - LOG.debug("Built configdrive out of configdrive blob " - "for node %(node)s", {'node': task.node.uuid}) - - boot_iso_tmp_file = boot_fileobj.name - images.create_boot_iso( - task.context, boot_iso_tmp_file, - kernel_href, ramdisk_href, - esp_image_href=bootloader_href, - configdrive_href=configdrive_href, - root_uuid=root_uuid, - kernel_params=kernel_params, - boot_mode=boot_mode, - base_iso=base_iso) - - iso_object_name = _get_iso_image_name(task.node) - - image_url = img_handler.publish_image( - boot_iso_tmp_file, iso_object_name) + image_url = img_handler.publish_image( + boot_iso_tmp_file, iso_object_name) LOG.debug("Created ISO %(name)s in object store for node %(node)s, " "exposed as temporary URL " @@ -426,8 +398,8 @@ def prepare_deploy_iso(task, params, mode, d_info): and return temporary Swift URL to the image. If network interface supplies network configuration (`network_data`), - a new `configdrive` will be created with `network_data.json` inside, - and eventually written down onto the boot ISO. + a `network_data.json` will be written into an appropriate location on + the final ISO. :param task: a TaskManager instance containing the node to act on. :param params: a dictionary containing 'parameter name'->'value' @@ -457,35 +429,17 @@ def prepare_deploy_iso(task, params, mode, d_info): _prepare_iso_image, task, kernel_href, ramdisk_href, bootloader_href=bootloader_href, params=params) + inject_files = {} network_data = task.driver.network.get_node_network_data(task) if network_data: LOG.debug('Injecting custom network data for node %s', task.node.uuid) - with tempfile.NamedTemporaryFile(dir=CONF.tempdir, - suffix='.iso') as metadata_fileobj: + network_data = json.dumps(network_data, indent=2).encode('utf-8') + inject_files[network_data] = ( + 'openstack/latest/network_data.json' + ) - with open(metadata_fileobj.name, 'w') as f: - json.dump(network_data, f, indent=2) - - files_info = { - metadata_fileobj.name: 'openstack/latest/network_data.json' - } - - with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.img') as cfgdrv_fileobj: - - images.create_vfat_image(cfgdrv_fileobj.name, files_info) - - configdrive_href = urlparse.urlunparse( - ('file', '', cfgdrv_fileobj.name, '', '', '')) - - LOG.debug("Built configdrive %(name)s out of network data " - "for node %(node)s", {'name': configdrive_href, - 'node': task.node.uuid}) - - return prepare_iso_image(configdrive=configdrive_href) - - return prepare_iso_image() + return prepare_iso_image(inject_files=inject_files) def prepare_boot_iso(task, d_info, root_uuid=None): diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 8a90ab55bb..f5cb8ac835 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -250,34 +250,28 @@ class IronicImagesTestCase(base.TestCase): class FsImageTestCase(base.TestCase): + @mock.patch.object(builtins, 'open', autospec=True) @mock.patch.object(shutil, 'copyfile', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) - @mock.patch.object(os.path, 'dirname', autospec=True) - @mock.patch.object(os.path, 'exists', autospec=True) - def test__create_root_fs(self, path_exists_mock, - dirname_mock, mkdir_mock, cp_mock): - - def path_exists_mock_func(path): - return path == 'root_dir' - + def test__create_root_fs(self, mkdir_mock, cp_mock, open_mock): files_info = { 'a1': 'b1', 'a2': 'b2', - 'a3': 'sub_dir/b3'} + 'a3': 'sub_dir/b3', + b'a4': 'b4'} - path_exists_mock.side_effect = path_exists_mock_func - dirname_mock.side_effect = ['root_dir', 'root_dir', 'root_dir/sub_dir', - 'root_dir/sub_dir'] images._create_root_fs('root_dir', files_info) + cp_mock.assert_any_call('a1', 'root_dir/b1') cp_mock.assert_any_call('a2', 'root_dir/b2') cp_mock.assert_any_call('a3', 'root_dir/sub_dir/b3') - path_exists_mock.assert_any_call('root_dir/sub_dir') - dirname_mock.assert_any_call('root_dir/b1') - dirname_mock.assert_any_call('root_dir/b2') - dirname_mock.assert_any_call('root_dir/sub_dir/b3') - mkdir_mock.assert_called_once_with('root_dir/sub_dir') + open_mock.assert_called_once_with('root_dir/b4', 'wb') + fp = open_mock.return_value.__enter__.return_value + fp.write.assert_called_once_with(b'a4') + + mkdir_mock.assert_any_call('root_dir', exist_ok=True) + mkdir_mock.assert_any_call('root_dir/sub_dir', exist_ok=True) @mock.patch.object(images, '_create_root_fs', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) @@ -413,21 +407,6 @@ class FsImageTestCase(base.TestCase): options) self.assertEqual(expected_cfg, cfg) - @mock.patch.object(images, 'os', autospec=True) - def test__read_dir(self, mock_os): - mock_os.path.join = os.path.join - mock_os.path.isdir.side_effect = (False, True, False) - mock_os.listdir.side_effect = [['a', 'b'], ['c']] - - file_info = images._read_dir('/mnt') - - expected = { - '/mnt/a': 'a', - '/mnt/b/c': 'b/c' - } - - self.assertEqual(expected, file_info) - @mock.patch.object(os.path, 'relpath', autospec=True) @mock.patch.object(os, 'walk', autospec=True) @mock.patch.object(utils, 'mount', autospec=True) @@ -599,7 +578,8 @@ class FsImageTestCase(base.TestCase): @mock.patch.object(images, '_generate_cfg', autospec=True) def _test_create_isolinux_image_for_bios( self, gen_cfg_mock, execute_mock, tempdir_mock, - write_to_file_mock, create_root_fs_mock, ldlinux_path=None): + write_to_file_mock, create_root_fs_mock, ldlinux_path=None, + inject_files=None): mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'tmpdir' @@ -616,13 +596,16 @@ class FsImageTestCase(base.TestCase): images.create_isolinux_image_for_bios('tgt_file', 'path/to/kernel', 'path/to/ramdisk', - kernel_params=params) + kernel_params=params, + inject_files=inject_files) files_info = { 'path/to/kernel': 'vmlinuz', 'path/to/ramdisk': 'initrd', CONF.isolinux_bin: 'isolinux/isolinux.bin' } + if inject_files: + files_info.update(inject_files) if ldlinux_path: files_info[ldlinux_path] = 'isolinux/ldlinux.c32' create_root_fs_mock.assert_called_once_with('tmpdir', files_info) @@ -653,6 +636,12 @@ class FsImageTestCase(base.TestCase): self._test_create_isolinux_image_for_bios( ldlinux_path='/usr/share/syslinux/ldlinux.c32') + @mock.patch.object(os.path, 'isfile', autospec=True) + def test_create_isolinux_image_for_bios_inject_files(self, mock_isfile): + mock_isfile.return_value = False + self._test_create_isolinux_image_for_bios( + inject_files={'/source': 'target'}) + @mock.patch.object(images, '_umount_without_raise', autospec=True) @mock.patch.object(images, '_create_root_fs', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) @@ -765,7 +754,7 @@ class FsImageTestCase(base.TestCase): create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', deploy_iso='tmpdir/deploy_iso-uuid', - esp_image=None, kernel_params=params, configdrive=None) + esp_image=None, kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -793,7 +782,7 @@ class FsImageTestCase(base.TestCase): create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', deploy_iso=None, esp_image='tmpdir/efiboot-uuid', - kernel_params=params, configdrive=None) + kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -821,7 +810,7 @@ class FsImageTestCase(base.TestCase): create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-href', 'tmpdir/ramdisk-href', deploy_iso='tmpdir/deploy_iso-href', - esp_image=None, kernel_params=params, configdrive=None) + esp_image=None, kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -849,7 +838,7 @@ class FsImageTestCase(base.TestCase): create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-href', 'tmpdir/ramdisk-href', deploy_iso=None, esp_image='tmpdir/efiboot-href', - kernel_params=params, configdrive=None) + kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -863,26 +852,24 @@ class FsImageTestCase(base.TestCase): images.create_boot_iso('ctx', 'output_file', 'kernel-uuid', 'ramdisk-uuid', 'deploy_iso-uuid', 'efiboot-uuid', 'root-uuid', - 'kernel-params', 'bios', 'configdrive') + 'kernel-params', 'bios') fetch_images_mock.assert_any_call( 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') fetch_images_mock.assert_any_call( 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') - fetch_images_mock.assert_any_call( - 'ctx', 'configdrive', 'tmpdir/configdrive') # Note (NobodyCam): the original assert asserted that fetch_images # was not called with parameters, this did not # work, So I instead assert that there were only # Two calls to the mock validating the above # asserts. - self.assertEqual(3, fetch_images_mock.call_count) + self.assertEqual(2, fetch_images_mock.call_count) params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', - kernel_params=params, configdrive='tmpdir/configdrive') + kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -897,19 +884,17 @@ class FsImageTestCase(base.TestCase): images.create_boot_iso('ctx', 'output_file', 'kernel-uuid', 'ramdisk-uuid', 'deploy_iso-uuid', 'efiboot-uuid', 'root-uuid', - 'kernel-params', None, 'http://configdrive') + 'kernel-params', None) fetch_images_mock.assert_any_call( 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') fetch_images_mock.assert_any_call( 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') - fetch_images_mock.assert_any_call( - 'ctx', 'http://configdrive', 'tmpdir/configdrive') params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', - configdrive='tmpdir/configdrive', kernel_params=params) + kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @@ -924,8 +909,7 @@ class FsImageTestCase(base.TestCase): images.create_boot_iso('ctx', 'output_file', 'kernel-uuid', 'ramdisk-uuid', 'deploy_iso-uuid', 'efiboot-uuid', None, - None, None, 'http://configdrive', - base_iso=base_iso) + None, None, base_iso=base_iso) fetch_images_mock.assert_any_call( 'ctx', base_iso, 'output_file') diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index e5dd94849a..7c177a5522 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -252,10 +252,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', boot_mode='uefi', esp_image_href='http://bootloader/img', - configdrive_href=mock.ANY, kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None) + base_iso=None, inject_files=None) self.assertEqual(expected_url, url) @@ -275,10 +274,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', boot_mode='uefi', esp_image_href=None, - configdrive_href=mock.ANY, kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso') + base_iso='/path/to/baseiso', inject_files=None) @mock.patch.object(image_utils.ImageHandler, 'publish_image', autospec=True) @@ -304,10 +302,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', boot_mode='bios', esp_image_href=None, - configdrive_href=mock.ANY, kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None) + base_iso=None, inject_files=None) self.assertEqual(expected_url, url) @@ -330,10 +327,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', boot_mode='bios', esp_image_href=None, - configdrive_href=mock.ANY, kernel_params=kernel_params, root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso') + base_iso='/path/to/baseiso', inject_files=None) def test__find_param(self): param_dict = { @@ -385,16 +381,15 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): image_utils.prepare_deploy_iso(task, {}, 'deploy', d_info) mock__prepare_iso_image.assert_called_once_with( - task, 'kernel', 'ramdisk', 'bootloader', params={}) + task, 'kernel', 'ramdisk', 'bootloader', params={}, + inject_files={}) 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) - @mock.patch.object(images, 'create_vfat_image', autospec=True) def test_prepare_deploy_iso_network_data( - self, mock_create_vfat_image, mock__prepare_iso_image, - find_mock): + self, mock__prepare_iso_image, find_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -421,12 +416,17 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): image_utils.prepare_deploy_iso(task, {}, 'deploy', d_info) - mock_create_vfat_image.assert_called_once_with( - mock.ANY, mock.ANY) + expected_files = { + b"""{ + "a": [ + "b" + ] +}""": 'openstack/latest/network_data.json' + } mock__prepare_iso_image.assert_called_once_with( task, 'kernel', 'ramdisk', bootloader_href=None, - configdrive=mock.ANY, params={}) + params={}, inject_files=expected_files) find_mock.assert_has_calls(find_call_list) diff --git a/releasenotes/notes/vmedia-inject-files-b6e226e2db4cff06.yaml b/releasenotes/notes/vmedia-inject-files-b6e226e2db4cff06.yaml new file mode 100644 index 0000000000..260533b5f5 --- /dev/null +++ b/releasenotes/notes/vmedia-inject-files-b6e226e2db4cff06.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes permission issues when injecting network data into a virtual media. +other: + - | + The ``configdrive`` argument to some utils in ``ironic.common.images`` and + ``ironic.drivers.modules.image_utils`` has been replaced with a new + ``inject_files`` argument. The previous approach did not really work in + all situations and we don't expect 3rd party drivers to use it.