diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 40ad982176..ccd5f39635 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -674,20 +674,33 @@ def get_instance_image_info(task, ipxe_enabled=False): os.path.join(root_dir, node.uuid, 'boot_iso')) return image_info - image_properties = None d_info = deploy_utils.get_image_instance_info(node) + isap = node.driver_internal_info.get('is_source_a_path') def _get_image_properties(): - nonlocal image_properties - if not image_properties: + nonlocal image_properties, isap + if not image_properties and not isap: i_service = service.get_image_service( d_info['image_source'], context=ctx) image_properties = i_service.show( d_info['image_source'])['properties'] + # TODO(TheJulia): At some point, we should teach this code + # to understand that with a path, it *can* retrieve the + # manifest from the HTTP(S) endpoint, which can populate + # image_properties, and drive path to variable population + # like is done with basically Glance. labels = ('kernel', 'ramdisk') + if not isap: + anaconda_labels = ('stage2', 'ks_template', 'ks_cfg') + else: + # When a path is used, a stage2 ramdisk can be determiend + # automatically by anaconda, so it is not an explicit + # requirement. + anaconda_labels = ('ks_template', 'ks_cfg') + if not (i_info.get('kernel') and i_info.get('ramdisk')): # NOTE(rloo): If both are not specified in instance_info # we won't use any of them. We'll use the values specified @@ -700,20 +713,13 @@ def get_instance_image_info(task, ipxe_enabled=False): i_info[label] = str(image_properties[label + '_id']) node.instance_info = i_info node.save() + # TODO(TheJulia): Add functionality to look/grab the hints file + # for anaconda and just run with the entire path. - anaconda_labels = () - if deploy_utils.get_boot_option(node) == 'kickstart': - isap = node.driver_internal_info.get('is_source_a_path') # stage2: installer stage2 squashfs image # ks_template: anaconda kickstart template # ks_cfg - rendered ks_template - if not isap: - anaconda_labels = ('stage2', 'ks_template', 'ks_cfg') - else: - # When a path is used, a stage2 ramdisk can be determiend - # automatically by anaconda, so it is not an explicit - # requirement. - anaconda_labels = ('ks_template', 'ks_cfg') + # NOTE(rloo): We save stage2 & ks_template values in case they # are changed by the user after we start using them and to # prevent re-computing them again. @@ -733,26 +739,31 @@ def get_instance_image_info(task, ipxe_enabled=False): else: node.set_driver_internal_info( 'stage2', str(image_properties['stage2_id'])) - # NOTE(TheJulia): A kickstart template is entirely independent - # of the stage2 ramdisk. In the end, it was the configuration which - # told anaconda how to execute. - if i_info.get('ks_template'): - # If the value is set, we always overwrite it, in the event - # a rebuild is occuring or something along those lines. - node.set_driver_internal_info('ks_template', - i_info['ks_template']) + # NOTE(TheJulia): A kickstart template is entirely independent + # of the stage2 ramdisk. In the end, it was the configuration which + # told anaconda how to execute. + if i_info.get('ks_template'): + # If the value is set, we always overwrite it, in the event + # a rebuild is occuring or something along those lines. + node.set_driver_internal_info('ks_template', + i_info['ks_template']) + else: + _get_image_properties() + # ks_template is an optional property on the image + if image_properties and 'ks_template' in image_properties: + node.set_driver_internal_info( + 'ks_template', str(image_properties['ks_template'])) else: - _get_image_properties() - # ks_template is an optional property on the image - if 'ks_template' not in image_properties: - # If not defined, default to the overall system default - # kickstart template, as opposed to a user supplied - # template. - node.set_driver_internal_info( - 'ks_template', CONF.anaconda.default_ks_template) - else: - node.set_driver_internal_info( - 'ks_template', str(image_properties['ks_template'])) + # If not defined, default to the overall system default + # kickstart template, as opposed to a user supplied + # template. + node.set_driver_internal_info( + 'ks_template', + 'file://' + os.path.abspath( + CONF.anaconda.default_ks_template + ) + ) + node.save() for label in labels + anaconda_labels: @@ -1249,6 +1260,8 @@ def cache_ramdisk_kernel(task, pxe_info, ipxe_enabled=False): CONF.deploy.http_root, 'stage2') ensure_tree(os.path.dirname(file_path)) + + if 'ks_cfg' in pxe_info: # ks_cfg is rendered later by the driver using ks_template. It cannot # be fetched and cached. t_pxe_info.pop('ks_cfg') diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 037ad7449f..e10af1c2df 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -1357,7 +1357,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase): 'LiveOS', 'squashfs.img')), 'ks_template': - (CONF.anaconda.default_ks_template, + ('file://' + CONF.anaconda.default_ks_template, os.path.join(CONF.deploy.http_root, self.node.uuid, 'ks.cfg.template')), @@ -1375,63 +1375,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase): self.assertEqual(expected_info, image_info) # In the absense of kickstart template in both instance_info and # image default kickstart template is used - self.assertEqual(CONF.anaconda.default_ks_template, - image_info['ks_template'][0]) - calls = [mock.call(task.node), mock.call(task.node)] - boot_opt_mock.assert_has_calls(calls) - # Instance info gets presedence over kickstart template on the - # image - properties['properties'] = {'ks_template': 'glance://template_id'} - task.node.instance_info['ks_template'] = 'https://server/fake.tmpl' - image_show_mock.return_value = properties - image_info = pxe_utils.get_instance_image_info( - task, ipxe_enabled=False) - self.assertEqual('https://server/fake.tmpl', - image_info['ks_template'][0]) - - @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option', - return_value='kickstart', autospec=True) - @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) - def test_get_instance_image_info_with_kickstart_url( - self, image_show_mock, boot_opt_mock): - properties = {'properties': {u'kernel_id': u'instance_kernel_uuid', - u'ramdisk_id': u'instance_ramdisk_uuid', - u'image_source': u'http://path/to/os/'}} - - expected_info = {'ramdisk': - ('instance_ramdisk_uuid', - os.path.join(CONF.pxe.tftp_root, - self.node.uuid, - 'ramdisk')), - 'kernel': - ('instance_kernel_uuid', - os.path.join(CONF.pxe.tftp_root, - self.node.uuid, - 'kernel')), - 'ks_template': - (CONF.anaconda.default_ks_template, - os.path.join(CONF.deploy.http_root, - self.node.uuid, - 'ks.cfg.template')), - 'ks_cfg': - ('', - os.path.join(CONF.deploy.http_root, - self.node.uuid, - 'ks.cfg'))} - image_show_mock.return_value = properties - self.context.auth_token = 'fake' - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - dii = task.node.driver_internal_info - dii['is_source_a_path'] = True - task.node.driver_internal_info = dii - task.node.save() - image_info = pxe_utils.get_instance_image_info( - task, ipxe_enabled=False) - self.assertEqual(expected_info, image_info) - # In the absense of kickstart template in both instance_info and - # image default kickstart template is used - self.assertEqual(CONF.anaconda.default_ks_template, + self.assertEqual('file://' + CONF.anaconda.default_ks_template, image_info['ks_template'][0]) calls = [mock.call(task.node), mock.call(task.node)] boot_opt_mock.assert_has_calls(calls) @@ -1463,7 +1407,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase): self.node.uuid, 'kernel')), 'ks_template': - (CONF.anaconda.default_ks_template, + ('file://' + CONF.anaconda.default_ks_template, os.path.join(CONF.deploy.http_root, self.node.uuid, 'ks.cfg.template')), @@ -1490,7 +1434,7 @@ class PXEInterfacesTestCase(db_base.DbTestCase): self.assertEqual(expected_info, image_info) # In the absense of kickstart template in both instance_info and # image default kickstart template is used - self.assertEqual(CONF.anaconda.default_ks_template, + self.assertEqual('file://' + CONF.anaconda.default_ks_template, image_info['ks_template'][0]) calls = [mock.call(task.node), mock.call(task.node)] boot_opt_mock.assert_has_calls(calls) @@ -1577,6 +1521,46 @@ class PXEInterfacesTestCase(db_base.DbTestCase): list(fake_pxe_info.values()), True) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch.object(pxe_utils, 'TFTPImageCache', lambda: None) + @mock.patch.object(pxe_utils, 'ensure_tree', autospec=True) + @mock.patch.object(deploy_utils, 'fetch_images', autospec=True) + def test_cache_ramdisk_kernel_ipxe_anaconda(self, mock_fetch_image, + mock_ensure_tree, mock_chmod): + expected_path = os.path.join(CONF.deploy.http_root, + self.node.uuid) + fake_pxe_info = {'ramdisk': + ('instance_ramdisk_uuid', + os.path.join(CONF.pxe.tftp_root, + self.node.uuid, + 'ramdisk')), + 'kernel': + ('instance_kernel_uuid', + os.path.join(CONF.pxe.tftp_root, + self.node.uuid, + 'kernel')), + 'ks_template': + ('file://' + CONF.anaconda.default_ks_template, + os.path.join(CONF.deploy.http_root, + self.node.uuid, + 'ks.cfg.template')), + 'ks_cfg': + ('', + os.path.join(CONF.deploy.http_root, + self.node.uuid, + 'ks.cfg'))} + expected = fake_pxe_info.copy() + expected.pop('ks_cfg') + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + pxe_utils.cache_ramdisk_kernel(task, fake_pxe_info, + ipxe_enabled=True) + mock_ensure_tree.assert_called_with(expected_path) + mock_fetch_image.assert_called_once_with(self.context, mock.ANY, + list(expected.values()), + True) + @mock.patch.object(pxe.PXEBoot, '__init__', lambda self: None) class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml b/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml new file mode 100644 index 0000000000..10d270a45f --- /dev/null +++ b/releasenotes/notes/correct-source-path-handling-lookups-4ce2023a56372f10.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + Fixes an issue where image information retrieval would fail when a + path was supplied when using the ``anaconda`` deploy interface, + as `HTTP` ``HEAD`` requests on a URL path have no ``Content-Length``. + We now consider if a path is used prior to attempting to collect + additional configuration data from what is normally expected to + be Glance. + - | + Fixes an issue where the fallback to a default kickstart template + value would result in error indicating + "Scheme-less image href is not a UUID". + This was becaues the handling code falling back to the default + did not explicitly indicate it was a file URL before saving the + value.