openstack/ironic
Code Issues Proposed changes

Bring boot_iso/deploy_iso handling in iLO closer to Redfish

Browse Source 
The iLO version is, as far as I remember, the oldest implementation of
virtual media in Ironic. Since then a few cool feature have been
developed in this area, but they are limited to the code using
image_utils (e.g. Redfish). The iLO boot interface only uses image_utils
when an ISO needs to be built, but not when one is provided. This patch
changes that.

There are a few visible side-effects to that:
1) file:/// images are now supported
2) ramdisk_image_download_source is now supported
3) the default caching behavior changes to caching http:// links
   (previously they were passed to the BMC directly).
4) glance images are supported with backends other than swift

Story: #2008987
Task: #42638
Change-Id: I23c21188776c511eddcdaf66a222ce64876678e2
This commit is contained in:
Dmitry Tantsur 2021-07-06 17:24:49 +02:00
parent e2b156ec8b
commit d3737b23bd
5 changed files with 106 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
ironic/drivers/modules/ilo/boot.py
View File

@ -132,14 +132,7 @@ def _get_boot_iso(task, root_uuid):
:param task: a TaskManager instance containing the node to act on.
:param root_uuid: the uuid of the root partition.
:returns: boot ISO URL. Should be either of below:
* A Swift object - It should be of format 'swift:<object-name>'. It is
assumed that the image object is present in
CONF.ilo.swift_ilo_container;
* A Glance image - It should be format 'glance://<glance-image-uuid>'
or just <glance-image-uuid>;
* An HTTP URL.
On error finding the boot iso, it returns None.
:returns: boot ISO HTTP or swift:// URL.
:raises: MissingParameterValue, if any of the required parameters are
missing in the node's driver_info or instance_info.
:raises: InvalidParameterValue, if any of the parameters have invalid
@ -156,35 +149,24 @@ def _get_boot_iso(task, root_uuid):
boot_iso = driver_utils.get_field(task.node, 'boot_iso',
deprecated_prefix='ilo',
collection='instance_info')
if boot_iso:
LOG.debug("Using boot_iso provided in node's instance_info")
if not service_utils.is_glance_image(boot_iso):
try:
image_service.HttpImageService().validate_href(boot_iso)
except exception.ImageRefValidationFailed:
with excutils.save_and_reraise_exception():
LOG.error("Virtual media deploy accepts only Glance "
"images or HTTP(S) URLs as "
"instance_info['boot_iso']. Either %s "
"is not a valid HTTP(S) URL or is "
"not reachable.", boot_iso)
return boot_iso
deploy_info = _parse_deploy_info(task.node)
# Option 2 - Check if user has provided a boot_iso in Glance. If boot_iso
# is a supported non-glance href execution will proceed to option 3.
deploy_info = _parse_deploy_info(task.node)
if not boot_iso:
# TODO(dtantsur): this approach should be generic across drivers.
image_href = deploy_info['image_source']
image_properties = (
images.get_image_properties(
task.context, image_href, ['boot_iso']))
image_href = deploy_info['image_source']
image_properties = (
images.get_image_properties(
task.context, image_href, ['boot_iso']))
boot_iso = image_properties.get('boot_iso')
boot_iso_uuid = image_properties.get('boot_iso')
if boot_iso_uuid:
LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid)
return boot_iso_uuid
if boot_iso:
i_info = task.node.instance_info
i_info['boot_iso'] = boot_iso
task.node.instance_info = i_info
task.node.save()
# NOTE(rameshg87): Functionality to share the boot ISOs created for
# similar instances (instances with same deployed image) is
@ -436,14 +418,12 @@ class IloVirtualMediaBoot(base.BootInterface):
mode = deploy_utils.rescue_or_deploy_mode(node)
d_info = parse_driver_info(node, mode)
if 'rescue_iso' in d_info:
ilo_common.setup_vmedia(task, d_info['rescue_iso'], ramdisk_params)
elif 'deploy_iso' in d_info:
ilo_common.setup_vmedia(task, d_info['deploy_iso'], ramdisk_params)
else:
iso = image_utils.prepare_deploy_iso(task, ramdisk_params,
mode, d_info)
ilo_common.setup_vmedia(task, iso)
iso = image_utils.prepare_deploy_iso(task, ramdisk_params,
mode, d_info)
if not d_info.get(f'{mode}_iso'):
# No need to attach a floopy when an ISO is built by us.
ramdisk_params = None
ilo_common.setup_vmedia(task, iso, ramdisk_params)
@METRICS.timer('IloVirtualMediaBoot.prepare_instance')
def prepare_instance(self, task):
@ -569,9 +549,6 @@ class IloVirtualMediaBoot(base.BootInterface):
node = task.node
boot_iso = _get_boot_iso(task, root_uuid)
if not boot_iso:
LOG.error("Cannot get boot ISO for node %s", node.uuid)
return
# Upon deploy complete, some distros cloud images reboot the system as
# part of its configuration. Hence boot device should be persistent and

12
ironic/drivers/modules/image_utils.py
View File

@ -437,13 +437,23 @@ def _prepare_iso_image(task, kernel_href, ramdisk_href,
# NOTE(rpittau): if base_iso is defined as http address, we just access
# it directly.
if base_iso:
scheme = urlparse.urlparse(base_iso).scheme.lower()
if scheme == 'swift':
# FIXME(dtantsur): iLO supports swift: scheme. In the long run we
# should support it for all boot interfaces by using temporary
# URLs. Until it's done, return base_iso as it is.
return base_iso
if (download_source == 'swift'
and service_utils.is_glance_image(base_iso)):
base_iso = (
images.get_temp_url_for_glance_image(task.context, base_iso))
# get_temp_url_for_glance_image return an HTTP (or HTTPS - doesn't
# matter here) image.
scheme = 'http'
if download_source != 'local':
if base_iso.startswith(('http://', 'https://')):
if scheme in ('http', 'https'):
return base_iso
LOG.debug("ramdisk_image_download_source set to http but "
"boot_iso is not an HTTP URL: %(boot_iso)s",

92
ironic/tests/unit/drivers/modules/ilo/test_boot.py
View File

@ -156,9 +156,12 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest):
boot_interface = 'ilo-virtual-media'
@mock.patch.object(image_service.HttpImageService, 'validate_href',
spec_set=True, autospec=True)
def test__get_boot_iso_http_url(self, service_mock):
@mock.patch.object(image_utils, 'prepare_boot_iso', spec_set=True,
autospec=True)
@mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True,
autospec=True)
def test__get_boot_iso_http_url(self, deploy_info_mock,
prepare_iso_mock):
url = 'http://abc.org/image/qcow2'
i_info = self.node.instance_info
i_info['boot_iso'] = url
@ -168,31 +171,20 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid')
service_mock.assert_called_once_with(mock.ANY, url)
self.assertEqual(url, boot_iso_actual)
@mock.patch.object(image_service.HttpImageService, 'validate_href',
spec_set=True, autospec=True)
def test__get_boot_iso_unsupported_url(self, validate_href_mock):
validate_href_mock.side_effect = exception.ImageRefValidationFailed(
image_href='file://img.qcow2', reason='fail')
url = 'file://img.qcow2'
i_info = self.node.instance_info
i_info['boot_iso'] = url
self.node.instance_info = i_info
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
self.assertRaises(exception.ImageRefValidationFailed,
ilo_boot._get_boot_iso, task, 'root-uuid')
deploy_info_mock.assert_called_once_with(task.node)
prepare_iso_mock.assert_called_once_with(
task, deploy_info_mock.return_value, 'root-uuid')
self.assertEqual(prepare_iso_mock.return_value, boot_iso_actual)
@mock.patch.object(image_utils, 'prepare_boot_iso', spec_set=True,
autospec=True)
@mock.patch.object(images, 'get_image_properties', spec_set=True,
autospec=True)
@mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True,
autospec=True)
def test__get_boot_iso_glance_image(self, deploy_info_mock,
image_props_mock):
image_props_mock,
prepare_iso_mock):
deploy_info_mock.return_value = {'image_source': 'image-uuid',
'ilo_deploy_iso': 'deploy_iso_uuid'}
image_props_mock.return_value = {'boot_iso': u'glance://uui\u0111'}
@ -204,7 +196,26 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest):
image_props_mock.assert_called_once_with(
task.context, 'image-uuid', ['boot_iso'])
boot_iso_expected = u'glance://uui\u0111'
self.assertEqual(boot_iso_expected, boot_iso_actual)
prepare_iso_mock.assert_called_once_with(
task, deploy_info_mock.return_value, 'root-uuid')
self.assertEqual(prepare_iso_mock.return_value, boot_iso_actual)
self.assertEqual(boot_iso_expected,
task.node.instance_info['boot_iso'])
@mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True,
autospec=True)
def test__get_boot_iso_swift_image(self, deploy_info_mock):
url = 'swift://abc.org/image/qcow2'
i_info = self.node.instance_info
i_info['boot_iso'] = url
self.node.instance_info = i_info
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid')
self.assertEqual(url, boot_iso_actual)
deploy_info_mock.assert_called_once_with(task.node)
@mock.patch.object(image_utils, 'prepare_boot_iso', spec_set=True,
autospec=True)
@ -549,6 +560,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest):
self.assertRaises(exception.UnsupportedDriverExtension,
task.driver.boot.validate_inspection, task)
@mock.patch.object(image_utils, 'prepare_deploy_iso', autospec=True)
@mock.patch.object(ilo_boot, 'prepare_node_for_deploy',
spec_set=True, autospec=True)
@mock.patch.object(ilo_common, 'eject_vmedia_devices',
@ -560,6 +572,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest):
def _test_prepare_ramdisk(self, get_nic_mock, setup_vmedia_mock,
eject_mock,
prepare_node_for_deploy_mock,
prepare_boot_iso_mock,
ilo_boot_iso, image_source,
ramdisk_params={'a': 'b'},
mode='deploy'):
@ -586,8 +599,12 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest):
'ipa-agent-token': mock.ANY,
'boot_method': 'vmedia'}
get_nic_mock.assert_called_once_with(task)
setup_vmedia_mock.assert_called_once_with(task, iso,
expected_ramdisk_opts)
prepare_boot_iso_mock.assert_called_once_with(
task, expected_ramdisk_opts, mode,
{f'{mode}_iso': iso, 'kernel_append_params': None})
setup_vmedia_mock.assert_called_once_with(
task, prepare_boot_iso_mock.return_value,
expected_ramdisk_opts)
@mock.patch.object(service_utils, 'is_glance_image', spec_set=True,
autospec=True)
@ -703,7 +720,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest):
driver_info_mock.assert_called_once_with(task.node, mode)
prepare_deploy_iso_mock.assert_called_once_with(
task, ramdisk_params, mode, d_info)
setup_vmedia_mock.assert_called_once_with(task, 'recreated-iso')
setup_vmedia_mock.assert_called_once_with(
task, 'recreated-iso', None)
@mock.patch.object(manager_utils, 'node_set_boot_device', spec_set=True,
autospec=True)
@ -731,28 +749,6 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest):
self.assertEqual('boot.iso',
task.node.instance_info['boot_iso'])
@mock.patch.object(manager_utils, 'node_set_boot_device', spec_set=True,
autospec=True)
@mock.patch.object(ilo_common, 'setup_vmedia_for_boot', spec_set=True,
autospec=True)
@mock.patch.object(ilo_boot, '_get_boot_iso', spec_set=True,
autospec=True)
def test__configure_vmedia_boot_without_boot_iso(
self, get_boot_iso_mock, setup_vmedia_mock, set_boot_device_mock):
root_uuid = {'root uuid': 'root_uuid'}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
get_boot_iso_mock.return_value = None
task.driver.boot._configure_vmedia_boot(
task, root_uuid)
get_boot_iso_mock.assert_called_once_with(
task, root_uuid)
self.assertFalse(setup_vmedia_mock.called)
self.assertFalse(set_boot_device_mock.called)
@mock.patch.object(deploy_utils, 'is_iscsi_boot',
spec_set=True, autospec=True)
@mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed',

9
ironic/tests/unit/drivers/modules/test_image_utils.py
View File

@ -666,6 +666,15 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
base_iso=base_image_url)
self.assertEqual(url, base_image_url)
def test__prepare_iso_image_bootable_iso_swift_schema(self):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
base_image_url = 'swift://bearmetal.net/boot.iso'
url = image_utils._prepare_iso_image(
task, None, None, bootloader_href=None, root_uuid=None,
base_iso=base_image_url)
self.assertEqual(url, base_image_url)
def test__find_param(self):
param_dict = {
'deploy_kernel': 'kernel',

22
releasenotes/notes/ilo-deploy-iso-0c88edb5daff8a4e.yaml Normal file
View File

@ -0,0 +1,22 @@
---
features:
- |
The ``ilo-virtual-media`` deploy interface now supports ``file:///`` URLs
for ``boot_iso`` and ``deploy_iso``.
- |
The ``ilo-virtual-media`` deploy interface now supports the
``[deploy]ramdisk_image_download_source`` configuration option.
fixes:
- |
The ``ilo-virtual-media`` deploy interface no longer requires the Image
service backend to be Swift for Glance images in ``boot_iso`` and
``deploy_iso``.
upgrade:
- |
Since ``ilo-virtual-media`` deploy interface now respects the
``[deploy]ramdisk_image_download_source`` configuration options, its
default caching behavior has changed. Now HTTP ``boot_iso``/``deploy_iso``
are cached locally and served from the conductor's HTTP server instead of
passing them directly to the BMC. Glance images are also cached locally.
To revert to the previous behavior, set the
``[deploy]ramdisk_image_download_source`` option to ``swift``.