From d8ccfabda8649d82a28daa96fbd5a2891c0dea86 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 23 Jun 2021 16:37:18 +0200 Subject: [PATCH] Ramdisk: do not require image_source Because of the way validation works, the ramdisk deploy interface currently requires an image_source in addition to kernel/ramdisk. After 1d6441cc347cfe984721f34ebb0cd64fd9e4d876 it is no longer necessary, and this change removes this requirement. Change-Id: I59996fac059dade0ef186598be1e8971e073eb04 --- doc/source/admin/ramdisk-boot.rst | 8 +-- ironic/drivers/modules/deploy_utils.py | 51 ++++++++++++------- .../unit/drivers/modules/test_deploy_utils.py | 47 +++++++++++++++-- .../tests/unit/drivers/modules/test_ipxe.py | 12 +++-- ironic/tests/unit/drivers/modules/test_pxe.py | 15 ++++-- .../ramdisk-validate-acbc4acdb71d10c2.yaml | 5 ++ 6 files changed, 104 insertions(+), 34 deletions(-) create mode 100644 releasenotes/notes/ramdisk-validate-acbc4acdb71d10c2.yaml diff --git a/doc/source/admin/ramdisk-boot.rst b/doc/source/admin/ramdisk-boot.rst index d961b21abb..df8bb7ea19 100644 --- a/doc/source/admin/ramdisk-boot.rst +++ b/doc/source/admin/ramdisk-boot.rst @@ -72,13 +72,13 @@ source, for example, baremetal node set \ --instance-info kernel=http://path/to/ramdisk.kernel \ - --instance-info ramdisk=http://path/to/ramdisk.initramfs \ - --instance-info image_source=http://path/to/ramdisk.initramfs + --instance-info ramdisk=http://path/to/ramdisk.initramfs baremetal node deploy .. note:: - The requirement to pass ``image_source`` is artificial and will be fixed - in a future version of the Bare Metal service. + Before the Xena release, the ``image_source`` field was also required:: + + --instance-info image_source=http://path/to/ramdisk.initramfs Booting an ISO -------------- diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 22480a8983..0124516512 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -565,17 +565,23 @@ def validate_image_properties(task, deploy_info): image_href = boot_iso properties = [] - if (not boot_iso - and not task.node.driver_internal_info.get('is_whole_disk_image')): - if service_utils.is_glance_image(image_href): - properties = ['kernel_id', 'ramdisk_id'] - boot_option = get_boot_option(task.node) - if boot_option == 'kickstart': - properties.append('squashfs_id') - else: - properties = ['kernel', 'ramdisk'] + if boot_iso or task.node.driver_internal_info.get('is_whole_disk_image'): + # No image properties are required in this case + return - image_props = get_image_properties(task.context, image_href) + if service_utils.is_glance_image(image_href): + properties = ['kernel_id', 'ramdisk_id'] + boot_option = get_boot_option(task.node) + if boot_option == 'kickstart': + properties.append('squashfs_id') + else: + properties = ['kernel', 'ramdisk'] + + if image_href: + image_props = get_image_properties(task.context, image_href) + else: + # Ramdisk deploy, no image_source is present + image_props = [] missing_props = [] for prop in properties: @@ -783,17 +789,28 @@ def get_image_instance_info(node): # more explicit unit testing to exist. info = {} - is_whole_disk_image = node.driver_internal_info.get('is_whole_disk_image') boot_iso = node.instance_info.get('boot_iso') - if not boot_iso: - info['image_source'] = node.instance_info.get('image_source') - else: + image_source = node.instance_info.get('image_source') + if boot_iso: + if image_source: + raise exception.InvalidParameterValue(_( + "An 'image_source' and 'boot_iso' parameter may not be " + "specified at the same time.")) info['boot_iso'] = boot_iso - - if not is_whole_disk_image and not boot_iso: - if not service_utils.is_glance_image(info['image_source']): + else: + if get_boot_option(node) == 'ramdisk': + # Ramdisk deploy does not require an image info['kernel'] = node.instance_info.get('kernel') info['ramdisk'] = node.instance_info.get('ramdisk') + else: + info['image_source'] = image_source + + is_whole_disk_image = node.driver_internal_info.get( + 'is_whole_disk_image') + if (not is_whole_disk_image + and not service_utils.is_glance_image(image_source)): + info['kernel'] = node.instance_info.get('kernel') + info['ramdisk'] = node.instance_info.get('ramdisk') error_msg = (_("Cannot validate image information for node %s because one " "or more parameters are missing from its instance_info and " diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 247c33b0c5..4021558bf2 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1433,6 +1433,20 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase): instance_info) self.assertEqual(expected_error, str(error)) + @mock.patch.object(utils, 'get_boot_option', autospec=True, + return_value='ramdisk') + @mock.patch.object(image_service.HttpImageService, 'show', autospec=True) + def test_validate_image_properties_ramdisk_deploy( + self, image_service_show_mock, boot_options_mock): + instance_info = { + 'kernel': 'file://kernel', + 'ramdisk': 'file://initrd', + } + self.node.instance_info = instance_info + inst_info = utils.get_image_instance_info(self.node) + utils.validate_image_properties(self.task, inst_info) + image_service_show_mock.assert_not_called() + class ValidateParametersTestCase(db_base.DbTestCase): @@ -1449,12 +1463,11 @@ class ValidateParametersTestCase(db_base.DbTestCase): driver_internal_info=DRV_INTERNAL_INFO_DICT, ) - info = utils.get_image_instance_info(node) - self.assertIsNotNone(info['image_source']) - return info + return utils.get_image_instance_info(node) def test__get_img_instance_info_good(self): - self._test__get_img_instance_info() + info = self._test__get_img_instance_info() + self.assertIsNotNone(info['image_source']) def test__get_img_instance_info_good_non_glance_image(self): instance_info = INST_INFO_DICT.copy() @@ -1464,6 +1477,7 @@ class ValidateParametersTestCase(db_base.DbTestCase): info = self._test__get_img_instance_info(instance_info=instance_info) + self.assertIsNotNone(info['image_source']) self.assertIsNotNone(info['ramdisk']) self.assertIsNotNone(info['kernel']) @@ -1500,8 +1514,31 @@ class ValidateParametersTestCase(db_base.DbTestCase): driver_internal_info = DRV_INTERNAL_INFO_DICT.copy() driver_internal_info['is_whole_disk_image'] = True - self._test__get_img_instance_info( + info = self._test__get_img_instance_info( driver_internal_info=driver_internal_info) + self.assertIsNotNone(info['image_source']) + + def test__get_img_instance_info_boot_iso_only(self): + instance_info = { + 'boot_iso': 'http://iso' + } + + info = self._test__get_img_instance_info(instance_info=instance_info) + self.assertIsNotNone(info['boot_iso']) + self.assertNotIn('image_source', info) + + @mock.patch.object(utils, 'get_boot_option', autospec=True, + return_value='ramdisk') + def test__get_img_instance_info_ramdisk_deploy(self, mock_boot_opt): + instance_info = { + 'kernel': 'http://kernel', + 'ramdisk': 'http://ramdisk', + } + + info = self._test__get_img_instance_info(instance_info=instance_info) + self.assertIsNotNone(info['kernel']) + self.assertIsNotNone(info['ramdisk']) + self.assertNotIn('image_source', info) class InstanceInfoTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index b9f94a92b3..824bee4537 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -133,15 +133,17 @@ class iPXEBootTestCase(db_base.DbTestCase): @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option', - return_value='ramdisk', autospec=True) + autospec=True) def test_validate_with_boot_iso(self, mock_boot_option, mock_glance): - i_info = self.node.driver_info - i_info['boot_iso'] = "http://localhost:1234/boot.iso" + self.node.instance_info = { + 'boot_iso': "http://localhost:1234/boot.iso" + } + self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.driver.boot.validate(task) - self.assertTrue(mock_boot_option.called) - self.assertTrue(mock_glance.called) + mock_boot_option.assert_called_with(task.node) + mock_glance.assert_not_called() @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option', return_value='ramdisk', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index b6dbb146a7..ea78e6ddf8 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -853,7 +853,8 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase): 'default_%s_interface' % iface: impl} self.config(**config_kwarg) self.config(enabled_hardware_types=['fake-hardware']) - instance_info = INST_INFO_DICT + instance_info = {'kernel': 'kernelUUID', + 'ramdisk': 'ramdiskUUID'} self.node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -963,8 +964,16 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase): autospec=True) def test_validate(self, mock_validate_img): with task_manager.acquire(self.context, self.node.uuid) as task: - task.node.instance_info['capabilities'] = { - 'boot_option': 'netboot'} + task.driver.deploy.validate(task) + self.assertTrue(mock_validate_img.called) + + @mock.patch.object(deploy_utils, 'validate_image_properties', + autospec=True) + def test_validate_with_boot_iso(self, mock_validate_img): + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node.instance_info = { + 'boot_iso': 'isoUUID' + } task.driver.deploy.validate(task) self.assertTrue(mock_validate_img.called) diff --git a/releasenotes/notes/ramdisk-validate-acbc4acdb71d10c2.yaml b/releasenotes/notes/ramdisk-validate-acbc4acdb71d10c2.yaml new file mode 100644 index 0000000000..4ed577fd80 --- /dev/null +++ b/releasenotes/notes/ramdisk-validate-acbc4acdb71d10c2.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + The ``ramdisk`` deploy interface no longer requires a fake ``image_source`` + value to be provided when ``boot_iso`` is not used.