diff --git a/ironic/common/images.py b/ironic/common/images.py index ece093ae1b..d2eb0d76ee 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -498,7 +498,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, - base_iso=None, inject_files=None): + inject_files=None): """Creates a bootable ISO image for a node. Given the hrefs for kernel, ramdisk, root partition's UUID and @@ -522,28 +522,15 @@ 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 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: - if base_iso: - # NOTE(TheJulia): Eventually we want to use the creation method - # to perform the massaging of the image, because oddly enough - # we need to do all the same basic things, just a little - # differently. - fetch_into(context, base_iso, output_filename) - # Temporary, return to the caller until we support the combined - # operation. - return - else: - kernel_path = os.path.join(tmpdir, kernel_href.split('/')[-1]) - ramdisk_path = os.path.join(tmpdir, ramdisk_href.split('/')[-1]) - fetch(context, kernel_href, kernel_path) - fetch(context, ramdisk_href, ramdisk_path) + kernel_path = os.path.join(tmpdir, kernel_href.split('/')[-1]) + ramdisk_path = os.path.join(tmpdir, ramdisk_href.split('/')[-1]) + fetch(context, kernel_href, kernel_path) + fetch(context, ramdisk_href, ramdisk_path) params = [] if root_uuid: diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index dd93e71292..5d4f7fe5fd 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -465,58 +465,53 @@ def _prepare_iso_image(task, kernel_href, ramdisk_href, img_handler = ImageHandler(task.node.driver) - # NOTE(TheJulia): Until we support modifying a base iso, most of - # this logic actually does nothing in the end. But it should! - if is_ramdisk_boot: - if not base_iso: - kernel_params = "root=/dev/ram0 text " - kernel_params += i_info.get("ramdisk_kernel_arguments", "") - else: - kernel_params = None - - else: - kernel_params = driver_utils.get_kernel_append_params( - task.node, default=img_handler.kernel_params) - - if params and not base_iso: - kernel_params = ' '.join( - (kernel_params, ' '.join( - ('%s=%s' % kv) if kv[1] is not None else kv[0] - for kv in params.items()))) - boot_mode = boot_mode_utils.get_boot_mode(task.node) - 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: 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) + if base_iso: + # NOTE(dtantsur): this should be "params or inject_files", but + # params are always populated in the calling code. + log_func = LOG.warning if inject_files else LOG.debug + log_func('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}) + images.fetch_into(task.context, base_iso, boot_iso_tmp_file) + else: + if is_ramdisk_boot: + kernel_params = "root=/dev/ram0 text " + kernel_params += i_info.get("ramdisk_kernel_arguments", "") + else: + kernel_params = driver_utils.get_kernel_append_params( + task.node, default=img_handler.kernel_params) + + if params: + kernel_params = ' '.join( + (kernel_params, ' '.join( + ('%s=%s' % kv) if kv[1] is not None else kv[0] + for kv in params.items()))) + + 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}) + 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, + inject_files=inject_files) iso_object_name = _get_name(task.node, prefix='boot', suffix='.iso') diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 50604342f6..2f9875f469 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -919,26 +919,6 @@ class FsImageTestCase(base.TestCase): 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', kernel_params=params, inject_files=None) - @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) - @mock.patch.object(images, 'fetch_into', autospec=True) - @mock.patch.object(utils, 'tempdir', autospec=True) - def test_create_boot_iso_for_existing_iso(self, tempdir_mock, - fetch_images_mock, - create_isolinux_mock): - mock_file_handle = mock.MagicMock(spec=io.BytesIO) - mock_file_handle.__enter__.return_value = 'tmpdir' - tempdir_mock.return_value = mock_file_handle - base_iso = 'http://fake.local:1234/fake.iso' - images.create_boot_iso('ctx', 'output_file', 'kernel-uuid', - 'ramdisk-uuid', 'deploy_iso-uuid', - 'efiboot-uuid', None, - None, None, base_iso=base_iso) - - fetch_images_mock.assert_any_call( - 'ctx', base_iso, 'output_file') - - create_isolinux_mock.assert_not_called() - @mock.patch.object(image_service, 'get_image_service', autospec=True) def test_get_glance_image_properties_no_such_prop(self, image_service_mock): diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index edfb7b29b7..549a97d91c 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -481,7 +481,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): boot_mode='uefi', esp_image_href='http://bootloader/img', kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None, inject_files=None) + inject_files=None) self.assertEqual(expected_url, url) @@ -495,15 +495,14 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): shared=True) as task: image_utils._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid, - base_iso='/path/to/baseiso') + bootloader_href=None, root_uuid=task.node.uuid) 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, kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso', inject_files=None) + inject_files=None) @mock.patch.object(image_utils.ImageHandler, 'publish_image', autospec=True) @@ -531,7 +530,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): boot_mode='bios', esp_image_href=None, kernel_params='nofb nomodeset vga=normal', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None, inject_files=None) + inject_files=None) self.assertEqual(expected_url, url) @@ -548,15 +547,14 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): image_utils._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid, - base_iso='/path/to/baseiso') + bootloader_href=None, root_uuid=task.node.uuid) 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, kernel_params=kernel_params, root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso', inject_files=None) + inject_files=None) @mock.patch.object(image_utils.ImageHandler, 'publish_image', autospec=True) @@ -571,15 +569,14 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): image_utils._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid, - base_iso='/path/to/baseiso') + bootloader_href=None, root_uuid=task.node.uuid) 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, kernel_params=kernel_params, root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso', inject_files=None) + inject_files=None) @mock.patch.object(deploy_utils, 'get_boot_option', lambda node: 'ramdisk') @mock.patch.object(image_utils.ImageHandler, 'publish_image', @@ -602,32 +599,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): boot_mode='bios', esp_image_href=None, kernel_params="root=/dev/ram0 text " + kernel_params, root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None, inject_files=None) - - @mock.patch.object(deploy_utils, 'get_boot_option', lambda node: 'ramdisk') - @mock.patch.object(image_utils.ImageHandler, 'publish_image', - autospec=True) - @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_kernel_params_for_ramdisk_boot_iso( - self, mock_create_boot_iso, mock_publish_image): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - kernel_params = 'network-config=base64-cloudinit-blob' - - task.node.instance_info['ramdisk_kernel_arguments'] = kernel_params - - image_utils._prepare_iso_image( - task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid, - base_iso='/path/to/baseiso') - - 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, - # No custom parameters with a boot ISO present - kernel_params=None, - root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso', inject_files=None) + inject_files=None) @mock.patch.object(deploy_utils, 'get_boot_option', lambda node: 'ramdisk') @mock.patch.object(image_utils.ImageHandler, 'publish_image', @@ -644,15 +616,14 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): image_utils._prepare_iso_image( task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid, - base_iso='/path/to/baseiso') + bootloader_href=None, root_uuid=task.node.uuid) 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, kernel_params=kernel_params, root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso='/path/to/baseiso', inject_files=None) + inject_files=None) @mock.patch.object(image_utils.ImageHandler, 'publish_image', autospec=True) @@ -675,7 +646,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): boot_mode='bios', esp_image_href=None, kernel_params=kernel_params + ' foo=bar banana', root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - base_iso=None, inject_files=None) + inject_files=None) def test__prepare_iso_image_bootable_iso(self): with task_manager.acquire(self.context, self.node.uuid, @@ -700,8 +671,10 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): @mock.patch.object(image_utils.ImageHandler, 'publish_image', autospec=True) + @mock.patch.object(images, 'fetch_into', autospec=True) @mock.patch.object(images, 'create_boot_iso', autospec=True) def test__prepare_iso_image_bootable_iso_file(self, mock_create_boot_iso, + mock_fetch, mock_publish_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -711,12 +684,9 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase): task, 'http://kernel/img', 'http://ramdisk/img', bootloader_href=None, root_uuid=task.node.uuid, base_iso=base_image_url) - mock_create_boot_iso.assert_called_once_with( - mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - esp_image_href=None, - root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - kernel_params='nofb nomodeset vga=normal', boot_mode='bios', - base_iso='/path/to/baseiso', inject_files=None) + mock_fetch.assert_called_once_with(task.context, + base_image_url, mock.ANY) + mock_create_boot_iso.assert_not_called() @mock.patch.object(images, 'get_temp_url_for_glance_image', autospec=True)