Correct Image properties lookup for paths
The image lookup process, when handed a path attempts to issue a HEAD request against the path and gets a response which is devoid of details like a content length or any properties. This is expected behavior, however if we have a path, we also know we don't need to explicitly attempt to make an HTTP HEAD request in an attempt to match the glance ``kernel_id`` -> ``kernel`` and similar value population behavior. Also removes an invalid test which was written before the overall method was fully understood. And fixes the default fallback for kickstart template configuration, so that it uses a URL instead of a direct file path. And fix logic in the handling of image property result set, where the code previously assumed a ``stage2`` ramdisk was always required, and based other cleanup upon that. Change-Id: I589e9586d1279604a743746952aeabbc483825df
This commit is contained in:
Julia Kreger
parent
3d3a67daf7
commit
4d653ac225
@ -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')
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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.
|
||||
Loading…
Reference in New Issue
Block a user