From a813c769e8d27803e87d46280db84607fdfce153 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Wed, 19 Jan 2022 13:27:44 +0100
Subject: [PATCH] Explicit parameter to distinguish partition/whole-disk images
Using kernel/ramdisk makes no sense with local boot, we need a better
way. We already have an internal image_type instance parameter, let's
make it public.
Glance support will be added in the next patch.
Change-Id: I4ce5f7a2317d952f976194d2022328f4afbb0258
---
doc/source/user/creating-images.rst | 4 +-
doc/source/user/deploy.rst | 22 ++-
ironic/common/images.py | 11 ++
ironic/conductor/deployments.py | 8 +-
ironic/conductor/manager.py | 10 +-
ironic/conductor/utils.py | 22 +++
ironic/drivers/modules/agent.py | 7 +
.../playbooks/roles/configure/tasks/main.yaml | 4 +-
.../playbooks/roles/prepare/tasks/main.yaml | 2 +-
ironic/drivers/modules/deploy_utils.py | 28 ++--
ironic/objects/node.py | 12 ++
ironic/tests/unit/common/test_images.py | 12 ++
.../tests/unit/conductor/test_deployments.py | 5 +-
ironic/tests/unit/conductor/test_manager.py | 21 ++-
.../tests/unit/drivers/modules/test_agent.py | 9 ++
.../unit/drivers/modules/test_deploy_utils.py | 130 ++++++++++++++++--
.../tests/unit/drivers/modules/test_ipxe.py | 2 +-
.../notes/image-type-ac259a90393bdd2c.yaml | 9 ++
18 files changed, 267 insertions(+), 51 deletions(-)
create mode 100644 releasenotes/notes/image-type-ac259a90393bdd2c.yaml
diff --git a/doc/source/user/creating-images.rst b/doc/source/user/creating-images.rst
index 6aeaaf529c..efcd632a5f 100644
--- a/doc/source/user/creating-images.rst
+++ b/doc/source/user/creating-images.rst
@@ -9,8 +9,8 @@ the end user. There are two types of user images:
*partition images*
contain only the contents of the root partition. Additionally, two more
- images are used together with them: an image with a kernel and with
- an initramfs.
+ images are used together with them when booting from network: an image with
+ a kernel and with an initramfs.
.. warning::
To use partition images with local boot, Grub2 must be installed on
diff --git a/doc/source/user/deploy.rst b/doc/source/user/deploy.rst
index f462fa0e52..9b46092894 100644
--- a/doc/source/user/deploy.rst
+++ b/doc/source/user/deploy.rst
@@ -100,19 +100,18 @@ You need to specify image information in the node's ``instance_info``
$ cd /path/to/http/root
$ md5sum *.img > checksums
-* ``kernel``, ``ramdisk`` - HTTP(s) or file URLs of the kernel and
- initramfs of the target OS. Must be added **only** for partition images.
- Supports the same schemes as ``image_source``.
+* ``kernel``, ``ramdisk`` - HTTP(s) or file URLs of the kernel and initramfs of
+ the target OS. Must be added **only** for partition images and only if
+ network boot is required. Supports the same schemes as ``image_source``.
-An example for a partition image:
+An example for a partition image with local boot:
.. code-block:: shell
baremetal node set $NODE_UUID \
--instance-info image_source=http://image.server/my-image.qcow2 \
--instance-info image_checksum=1f9c0e1bad977a954ba40928c1e11f33 \
- --instance-info kernel=http://image.server/my-image.kernel \
- --instance-info ramdisk=http://image.server/my-image.initramfs \
+ --instance-info image_type=partition \
--instance-info root_gb=10
With a SHA256 hash:
@@ -123,6 +122,17 @@ With a SHA256 hash:
--instance-info image_source=http://image.server/my-image.qcow2 \
--instance-info image_os_hash_algo=sha256 \
--instance-info image_os_hash_value=a64dd95e0c48e61ed741ff026d8c89ca38a51f3799955097c5123b1705ef13d4 \
+ --instance-info image_type=partition \
+ --instance-info root_gb=10
+
+If you use network boot (or Ironic before Yoga), two more fields must be set:
+
+.. code-block:: shell
+
+ baremetal node set $NODE_UUID \
+ --instance-info image_source=http://image.server/my-image.qcow2 \
+ --instance-info image_checksum=1f9c0e1bad977a954ba40928c1e11f33 \
+ --instance-info image_type=partition \
--instance-info kernel=http://image.server/my-image.kernel \
--instance-info ramdisk=http://image.server/my-image.initramfs \
--instance-info root_gb=10
diff --git a/ironic/common/images.py b/ironic/common/images.py
index cf51d723c8..939fcb6767 100644
--- a/ironic/common/images.py
+++ b/ironic/common/images.py
@@ -570,6 +570,11 @@ def create_boot_iso(context, output_filename, kernel_href,
kernel_params=params, inject_files=inject_files)
+IMAGE_TYPE_PARTITION = 'partition'
+IMAGE_TYPE_WHOLE_DISK = 'whole-disk'
+VALID_IMAGE_TYPES = frozenset((IMAGE_TYPE_PARTITION, IMAGE_TYPE_WHOLE_DISK))
+
+
def is_whole_disk_image(ctx, instance_info):
"""Find out if the image is a partition image or a whole disk image.
@@ -583,12 +588,18 @@ def is_whole_disk_image(ctx, instance_info):
if not image_source:
return
+ image_type = instance_info.get('image_type')
+ if image_type:
+ # This logic reflects the fact that whole disk images are the default
+ return image_type != IMAGE_TYPE_PARTITION
+
is_whole_disk_image = False
if glance_utils.is_glance_image(image_source):
try:
iproperties = get_image_properties(ctx, image_source)
except Exception:
return
+
is_whole_disk_image = (not iproperties.get('kernel_id')
and not iproperties.get('ramdisk_id'))
else:
diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py
index 30f24e4041..fee436c32e 100644
--- a/ironic/conductor/deployments.py
+++ b/ironic/conductor/deployments.py
@@ -22,7 +22,6 @@ from oslo_utils import excutils
from ironic.common import exception
from ironic.common.glance_service import service_utils as glance_utils
from ironic.common.i18n import _
-from ironic.common import images
from ironic.common import states
from ironic.common import swift
from ironic.conductor import notification_utils as notify_utils
@@ -88,11 +87,8 @@ def start_deploy(task, manager, configdrive=None, event='deploy',
# Infer the image type to make sure the deploy driver
# validates only the necessary variables for different
# image types.
- # NOTE(sirushtim): The iwdi variable can be None. It's up to
- # the deploy driver to validate this.
- iwdi = images.is_whole_disk_image(task.context, node.instance_info)
- node.set_driver_internal_info('is_whole_disk_image', iwdi)
- node.save()
+ if utils.update_image_type(task.context, task.node):
+ node.save()
try:
task.driver.power.validate(task)
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 983b504c76..9b1c20d9be 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -56,7 +56,6 @@ from ironic.common import driver_factory
from ironic.common import exception
from ironic.common import faults
from ironic.common.i18n import _
-from ironic.common import images
from ironic.common import network
from ironic.common import nova
from ironic.common import states
@@ -1711,10 +1710,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# being triggered, as such we need to populate the
# internal info based on the configuration the user has
# supplied.
- iwdi = images.is_whole_disk_image(task.context,
- task.node.instance_info)
- if iwdi is not None:
- node.set_driver_internal_info('is_whole_disk_image', iwdi)
+ utils.update_image_type(task.context, task.node)
if deploy_utils.get_boot_option(node) != 'local':
# Calling boot validate to ensure that sufficient information
# is supplied to allow the node to be able to boot if takeover
@@ -1894,9 +1890,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# the meantime, we don't know if the is_whole_disk_image value will
# change or not. It isn't saved to the DB, but only used with this
# node instance for the current validations.
- iwdi = images.is_whole_disk_image(context,
- task.node.instance_info)
- task.node.set_driver_internal_info('is_whole_disk_image', iwdi)
+ utils.update_image_type(context, task.node)
for iface_name in task.driver.non_vendor_interfaces:
iface = getattr(task.driver, iface_name)
result = reason = None
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index ab091ab1c8..ff56560522 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -33,6 +33,7 @@ from ironic.common import boot_devices
from ironic.common import exception
from ironic.common import faults
from ironic.common.i18n import _
+from ironic.common import images
from ironic.common import network
from ironic.common import nova
from ironic.common import states
@@ -1649,3 +1650,24 @@ def node_history_record(node, conductor=None, event=None,
severity=error and "ERROR" or "INFO",
event=event,
event_type=event_type or "UNKNOWN").create()
+
+
+def update_image_type(context, node):
+ """Updates is_whole_disk_image and image_type based on the node data.
+
+ :param context: Request context.
+ :param node: Node object.
+ :return: True if any changes have been done, else False.
+ """
+ iwdi = images.is_whole_disk_image(context, node.instance_info)
+ if iwdi is None:
+ return False
+
+ node.set_driver_internal_info('is_whole_disk_image', iwdi)
+ # We need to gradually phase out is_whole_disk_image in favour of
+ # image_type, so make sure to set it as well. The primary use case is to
+ # cache information detected from Glance or the presence of kernel/ramdisk.
+ node.set_instance_info(
+ 'image_type',
+ images.IMAGE_TYPE_WHOLE_DISK if iwdi else images.IMAGE_TYPE_PARTITION)
+ return True
diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index f309a10112..2dcd8a819c 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -471,6 +471,13 @@ class AgentDeploy(CustomAgentDeploy):
deploy_utils.check_for_missing_params(params, error_msg)
+ image_type = node.instance_info.get('image_type')
+ if image_type and image_type not in images.VALID_IMAGE_TYPES:
+ raise exception.InvalidParameterValue(
+ _('Invalid image_type "%(value)s", valid are %(valid)s')
+ % {'value': image_type,
+ 'valid': ', '.join(images.VALID_IMAGE_TYPES)})
+
# NOTE(dtantsur): glance images contain a checksum; for file images we
# will recalculate the checksum anyway.
if (not service_utils.is_glance_image(image_source)
diff --git a/ironic/drivers/modules/ansible/playbooks/roles/configure/tasks/main.yaml b/ironic/drivers/modules/ansible/playbooks/roles/configure/tasks/main.yaml
index 9baa882a60..ed8d168c3c 100644
--- a/ironic/drivers/modules/ansible/playbooks/roles/configure/tasks/main.yaml
+++ b/ironic/drivers/modules/ansible/playbooks/roles/configure/tasks/main.yaml
@@ -1,4 +1,4 @@
- import_tasks: mounts.yaml
- when: ironic.image.type | default('whole-disk-image') == 'partition'
+ when: ironic.image.type | default('whole-disk') == 'partition'
- import_tasks: grub.yaml
- when: ironic.image.type | default('whole-disk-image') == 'partition'
+ when: ironic.image.type | default('whole-disk') == 'partition'
diff --git a/ironic/drivers/modules/ansible/playbooks/roles/prepare/tasks/main.yaml b/ironic/drivers/modules/ansible/playbooks/roles/prepare/tasks/main.yaml
index e92aba69d5..1c13bc5236 100644
--- a/ironic/drivers/modules/ansible/playbooks/roles/prepare/tasks/main.yaml
+++ b/ironic/drivers/modules/ansible/playbooks/roles/prepare/tasks/main.yaml
@@ -1,2 +1,2 @@
- import_tasks: parted.yaml
- when: ironic.image.type | default('whole-disk-image') == 'partition'
+ when: ironic.image.type | default('whole-disk') == 'partition'
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 1aeda7acf5..812d007757 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -570,14 +570,21 @@ def validate_image_properties(task, deploy_info):
if not image_href:
image_href = boot_iso
- properties = []
- if boot_iso or task.node.driver_internal_info.get('is_whole_disk_image'):
- # No image properties are required in this case
+ boot_option = get_boot_option(task.node)
+
+ if (boot_iso
+ or task.node.driver_internal_info.get('is_whole_disk_image')
+ or boot_option == 'local'):
+ # No image properties are required in this case, but validate that the
+ # image at least looks reasonable.
+ try:
+ image_service.get_image_service(image_href, context=task.context)
+ except exception.ImageRefValidationFailed as e:
+ raise exception.InvalidParameterValue(err=e)
return
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('stage2_id')
image_props = get_image_properties(task.context, image_href)
@@ -799,7 +806,8 @@ def get_image_instance_info(node):
"specified at the same time."))
info['boot_iso'] = boot_iso
else:
- if get_boot_option(node) == 'ramdisk':
+ boot_option = get_boot_option(node)
+ if boot_option == 'ramdisk':
# Ramdisk deploy does not require an image
info['kernel'] = node.instance_info.get('kernel')
info['ramdisk'] = node.instance_info.get('ramdisk')
@@ -809,6 +817,7 @@ def get_image_instance_info(node):
is_whole_disk_image = node.driver_internal_info.get(
'is_whole_disk_image')
if (not is_whole_disk_image
+ and boot_option != 'local'
and not service_utils.is_glance_image(image_source)):
info['kernel'] = node.instance_info.get('kernel')
info['ramdisk'] = node.instance_info.get('ramdisk')
@@ -846,8 +855,10 @@ def parse_instance_info(node):
i_info = {}
i_info['image_source'] = info.get('image_source')
iwdi = node.driver_internal_info.get('is_whole_disk_image')
+ boot_option = get_boot_option(node)
if not iwdi:
if (i_info['image_source']
+ and boot_option != 'local'
and not service_utils.is_glance_image(
i_info['image_source'])):
i_info['kernel'] = info.get('kernel')
@@ -1176,6 +1187,7 @@ def build_instance_info_for_deploy(task):
iwdi = node.driver_internal_info.get('is_whole_disk_image')
image_source = instance_info['image_source']
image_download_source = get_image_download_source(node)
+ boot_option = get_boot_option(task.node)
if service_utils.is_glance_image(image_source):
glance = image_service.GlanceImageService(context=task.context)
@@ -1198,7 +1210,7 @@ def build_instance_info_for_deploy(task):
instance_info['image_tags'] = image_info.get('tags', [])
instance_info['image_properties'] = image_info['properties']
- if not iwdi:
+ if not iwdi and boot_option != 'local':
instance_info['kernel'] = image_info['properties']['kernel_id']
instance_info['ramdisk'] = image_info['properties']['ramdisk_id']
elif (image_source.startswith('file://')
@@ -1209,11 +1221,11 @@ def build_instance_info_for_deploy(task):
instance_info['image_url'] = image_source
if not iwdi:
- instance_info['image_type'] = 'partition'
+ instance_info['image_type'] = images.IMAGE_TYPE_PARTITION
i_info = parse_instance_info(node)
instance_info.update(i_info)
else:
- instance_info['image_type'] = 'whole-disk-image'
+ instance_info['image_type'] = images.IMAGE_TYPE_WHOLE_DISK
return instance_info
diff --git a/ironic/objects/node.py b/ironic/objects/node.py
index cdb2183017..7c6c8bc1ae 100644
--- a/ironic/objects/node.py
+++ b/ironic/objects/node.py
@@ -729,6 +729,18 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
"""
self.set_driver_internal_info(key, timeutils.utcnow().isoformat())
+ def set_instance_info(self, key, value):
+ """Set an `instance_info` value.
+
+ Setting a `instance_info` dict value via this method ensures that this
+ field will be flagged for saving.
+
+ :param key: Key of item to set
+ :param value: Value of item to set
+ """
+ self.instance_info[key] = value
+ self._changed_fields.add('instance_info')
+
@base.IronicObjectRegistry.register
class NodePayload(notification.NotificationPayloadBase):
diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py
index 9892d671c8..f699fb7ce6 100644
--- a/ironic/tests/unit/common/test_images.py
+++ b/ironic/tests/unit/common/test_images.py
@@ -216,6 +216,18 @@ class IronicImagesTestCase(base.TestCase):
self.assertFalse(mock_igi.called)
self.assertFalse(mock_gip.called)
+ @mock.patch.object(images, 'get_image_properties', autospec=True)
+ @mock.patch.object(glance_utils, 'is_glance_image', autospec=True)
+ def test_is_whole_disk_image_explicit(self, mock_igi, mock_gip):
+ for value, result in [(images.IMAGE_TYPE_PARTITION, False),
+ (images.IMAGE_TYPE_WHOLE_DISK, True)]:
+ instance_info = {'image_source': 'glance://partition_image',
+ 'image_type': value}
+ iwdi = images.is_whole_disk_image('context', instance_info)
+ self.assertIs(iwdi, result)
+ self.assertFalse(mock_igi.called)
+ self.assertFalse(mock_gip.called)
+
@mock.patch.object(images, 'get_image_properties', autospec=True)
@mock.patch.object(glance_utils, 'is_glance_image', autospec=True)
def test_is_whole_disk_image_partition_image(self, mock_igi, mock_gip):
diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py
index 65bb1eca13..d86292aa9d 100644
--- a/ironic/tests/unit/conductor/test_deployments.py
+++ b/ironic/tests/unit/conductor/test_deployments.py
@@ -353,7 +353,10 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
deployments.start_deploy(task, self.service, configdrive=None,
event='deploy', deploy_steps=deploy_steps)
node.refresh()
- self.assertTrue(mock_iwdi.called)
+ mock_iwdi.assert_called_once_with(task.context,
+ task.node.instance_info)
+ self.assertFalse(node.driver_internal_info['is_whole_disk_image'])
+ self.assertEqual('partition', node.instance_info['image_type'])
mock_power_validate.assert_called_once_with(task.driver.power, task)
mock_deploy_validate.assert_called_once_with(task.driver.deploy, task)
mock_validate_traits.assert_called_once_with(task.node)
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index dee5b6974c..7016576b7f 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -1859,12 +1859,15 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin,
# exc_info[1]
self.assertIn(r'node 1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
str(exc.exc_info[1]))
+ node.refresh()
# This is a sync operation last_error should be None.
self.assertIsNone(node.last_error)
# Verify reservation has been cleared.
self.assertIsNone(node.reservation)
mock_iwdi.assert_called_once_with(self.context, node.instance_info)
- self.assertNotIn('is_whole_disk_image', node.driver_internal_info)
+ # The image type must be set for validation to actually work
+ self.assertFalse(node.driver_internal_info['is_whole_disk_image'])
+ self.assertEqual('partition', node.instance_info['image_type'])
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.validate',
autospec=True)
@@ -3410,6 +3413,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
self.context, driver='fake-hardware',
target_raid_config=target_raid_config,
network_interface='noop')
+ expected_info = dict(node.instance_info, image_type='partition')
ret = self.service.validate_driver_interfaces(self.context,
node.uuid)
expected = {'console': {'result': True},
@@ -3424,7 +3428,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
'rescue': {'result': True},
'bios': {'result': True}}
self.assertEqual(expected, ret)
- mock_iwdi.assert_called_once_with(self.context, node.instance_info)
+ mock_iwdi.assert_called_once_with(self.context, expected_info)
@mock.patch.object(fake.FakeDeploy, 'validate', autospec=True)
@mock.patch.object(images, 'is_whole_disk_image', autospec=True)
@@ -3435,11 +3439,12 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
network_interface='noop')
reason = 'fake reason'
mock_val.side_effect = exception.InvalidParameterValue(reason)
+ expected_info = dict(node.instance_info, image_type='partition')
ret = self.service.validate_driver_interfaces(self.context,
node.uuid)
self.assertFalse(ret['deploy']['result'])
self.assertEqual(reason, ret['deploy']['reason'])
- mock_iwdi.assert_called_once_with(self.context, node.instance_info)
+ mock_iwdi.assert_called_once_with(self.context, expected_info)
@mock.patch.object(fake.FakeDeploy, 'validate', autospec=True)
@mock.patch.object(images, 'is_whole_disk_image', autospec=True)
@@ -3447,6 +3452,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
self, mock_iwdi, mock_val):
node = obj_utils.create_test_node(self.context, driver='fake-hardware')
mock_val.side_effect = Exception('boom')
+ expected_info = dict(node.instance_info, image_type='whole-disk')
ret = self.service.validate_driver_interfaces(self.context,
node.uuid)
reason = ('Unexpected exception, traceback saved '
@@ -3454,8 +3460,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
'that is running on test-host: boom')
self.assertFalse(ret['deploy']['result'])
self.assertEqual(reason, ret['deploy']['reason'])
-
- mock_iwdi.assert_called_once_with(self.context, node.instance_info)
+ mock_iwdi.assert_called_once_with(self.context, expected_info)
@mock.patch.object(images, 'is_whole_disk_image', autospec=True)
def test_validate_driver_interfaces_validation_fail_instance_traits(
@@ -3463,6 +3468,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
mock_iwdi.return_value = False
node = obj_utils.create_test_node(self.context, driver='fake-hardware',
network_interface='noop')
+ expected_info = dict(node.instance_info, image_type='partition')
with mock.patch(
'ironic.conductor.utils.validate_instance_info_traits',
autospec=True) as ii_traits:
@@ -3472,7 +3478,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
node.uuid)
self.assertFalse(ret['deploy']['result'])
self.assertEqual(reason, ret['deploy']['reason'])
- mock_iwdi.assert_called_once_with(self.context, node.instance_info)
+ mock_iwdi.assert_called_once_with(self.context, expected_info)
@mock.patch.object(images, 'is_whole_disk_image', autospec=True)
def test_validate_driver_interfaces_validation_fail_deploy_templates(
@@ -3480,6 +3486,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
mock_iwdi.return_value = False
node = obj_utils.create_test_node(self.context, driver='fake-hardware',
network_interface='noop')
+ expected_info = dict(node.instance_info, image_type='partition')
with mock.patch(
'ironic.conductor.steps'
'.validate_user_deploy_steps_and_templates',
@@ -3490,7 +3497,7 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
node.uuid)
self.assertFalse(ret['deploy']['result'])
self.assertEqual(reason, ret['deploy']['reason'])
- mock_iwdi.assert_called_once_with(self.context, node.instance_info)
+ mock_iwdi.assert_called_once_with(self.context, expected_info)
@mock.patch.object(manager.ConductorManager, '_fail_if_in_state',
autospec=True)
diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py
index 586bdc07fb..4b6d271d4a 100644
--- a/ironic/tests/unit/drivers/modules/test_agent.py
+++ b/ironic/tests/unit/drivers/modules/test_agent.py
@@ -692,6 +692,15 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
task.driver.boot, task)
validate_http_mock.assert_called_once_with(task.node)
+ def test_validate_invalid_image_type(self):
+ with task_manager.acquire(
+ self.context, self.node['uuid'], shared=False) as task:
+ task.node.instance_info['image_source'] = 'http://image-ref'
+ task.node.instance_info['image_type'] = 'passport photo'
+ self.assertRaisesRegex(exception.InvalidParameterValue,
+ 'passport photo',
+ self.driver.validate, task)
+
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
@mock.patch.object(deploy_utils, 'check_for_missing_params',
autospec=True)
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index c3ef5e2043..e85ab631d6 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -1362,8 +1362,15 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
self.task = mock.Mock(context=self.context, node=self.node,
spec=['context', 'node'])
+ def test_validate_image_properties_local_boot(self):
+ inst_info = utils.get_image_instance_info(self.node)
+ utils.validate_image_properties(self.task, inst_info)
+
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
@mock.patch.object(image_service, 'get_image_service', autospec=True)
- def test_validate_image_properties_glance_image(self, image_service_mock):
+ def test_validate_image_properties_glance_image(self, image_service_mock,
+ boot_options_mock):
inst_info = utils.get_image_instance_info(self.node)
image_service_mock.return_value.show.return_value = {
'properties': {'kernel_id': '1111', 'ramdisk_id': '2222'},
@@ -1374,9 +1381,11 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
self.node.instance_info['image_source'], context=self.context
)
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
@mock.patch.object(image_service, 'get_image_service', autospec=True)
def test_validate_image_properties_glance_image_missing_prop(
- self, image_service_mock):
+ self, image_service_mock, boot_options_mock):
inst_info = utils.get_image_instance_info(self.node)
image_service_mock.return_value.show.return_value = {
'properties': {'kernel_id': '1111'},
@@ -1406,9 +1415,11 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
self.node.instance_info['image_source'], context=self.context
)
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
@mock.patch.object(image_service, 'get_image_service', autospec=True)
def test_validate_image_properties_glance_image_not_authorized(
- self, image_service_mock):
+ self, image_service_mock, boot_options_mock):
inst_info = {'image_source': 'uuid'}
show_mock = image_service_mock.return_value.show
show_mock.side_effect = exception.ImageNotAuthorized(image_id='uuid')
@@ -1416,9 +1427,11 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
utils.validate_image_properties, self.task,
inst_info)
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
@mock.patch.object(image_service, 'get_image_service', autospec=True)
def test_validate_image_properties_glance_image_not_found(
- self, image_service_mock):
+ self, image_service_mock, boot_options_mock):
inst_info = {'image_source': 'uuid'}
show_mock = image_service_mock.return_value.show
show_mock.side_effect = exception.ImageNotFound(image_id='uuid')
@@ -1432,7 +1445,10 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
utils.validate_image_properties, self.task,
inst_info)
- def test_validate_image_properties_nonglance_image(self):
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
+ def test_validate_image_properties_nonglance_image(
+ self, boot_options_mock):
instance_info = {
'image_source': 'http://ubuntu',
'kernel': 'kernel_uuid',
@@ -1496,6 +1512,19 @@ class ValidateParametersTestCase(db_base.DbTestCase):
def test__get_img_instance_info_good_non_glance_image(self):
instance_info = INST_INFO_DICT.copy()
instance_info['image_source'] = 'http://image'
+
+ info = self._test__get_img_instance_info(instance_info=instance_info)
+
+ self.assertIsNotNone(info['image_source'])
+ self.assertNotIn('kernel', info)
+ self.assertNotIn('ramdisk', info)
+
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
+ def test__get_img_instance_info_good_non_glance_image_netboot(
+ self, mock_boot_opt):
+ instance_info = INST_INFO_DICT.copy()
+ instance_info['image_source'] = 'http://image'
instance_info['kernel'] = 'http://kernel'
instance_info['ramdisk'] = 'http://ramdisk'
@@ -1505,7 +1534,10 @@ class ValidateParametersTestCase(db_base.DbTestCase):
self.assertIsNotNone(info['ramdisk'])
self.assertIsNotNone(info['kernel'])
- def test__get_img_instance_info_non_glance_image_missing_kernel(self):
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
+ def test__get_img_instance_info_non_glance_image_missing_kernel(
+ self, mock_boot_opt):
instance_info = INST_INFO_DICT.copy()
instance_info['image_source'] = 'http://image'
instance_info['ramdisk'] = 'http://ramdisk'
@@ -1515,7 +1547,10 @@ class ValidateParametersTestCase(db_base.DbTestCase):
self._test__get_img_instance_info,
instance_info=instance_info)
- def test__get_img_instance_info_non_glance_image_missing_ramdisk(self):
+ @mock.patch.object(utils, 'get_boot_option', autospec=True,
+ return_value='netboot')
+ def test__get_img_instance_info_non_glance_image_missing_ramdisk(
+ self, mock_boot_opt):
instance_info = INST_INFO_DICT.copy()
instance_info['image_source'] = 'http://image'
instance_info['kernel'] = 'http://kernel'
@@ -1772,10 +1807,20 @@ class InstanceInfoTestCase(db_base.DbTestCase):
self.assertEqual('http://1.2.3.4/cd', instance_info['configdrive'])
def test_parse_instance_info_nonglance_image(self):
+ info = INST_INFO_DICT.copy()
+ info['image_source'] = 'file:///image.qcow2'
+ node = obj_utils.create_test_node(
+ self.context, instance_info=info,
+ driver_internal_info=DRV_INTERNAL_INFO_DICT,
+ )
+ utils.parse_instance_info(node)
+
+ def test_parse_instance_info_nonglance_image_netboot(self):
info = INST_INFO_DICT.copy()
info['image_source'] = 'file:///image.qcow2'
info['kernel'] = 'file:///image.vmlinuz'
info['ramdisk'] = 'file:///image.initrd'
+ info['capabilities'] = {'boot_option': 'netboot'}
node = obj_utils.create_test_node(
self.context, instance_info=info,
driver_internal_info=DRV_INTERNAL_INFO_DICT,
@@ -1786,6 +1831,7 @@ class InstanceInfoTestCase(db_base.DbTestCase):
info = INST_INFO_DICT.copy()
info['image_source'] = 'file:///image.qcow2'
info['ramdisk'] = 'file:///image.initrd'
+ info['capabilities'] = {'boot_option': 'netboot'}
node = obj_utils.create_test_node(
self.context, instance_info=info,
driver_internal_info=DRV_INTERNAL_INFO_DICT,
@@ -1873,8 +1919,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
self, glance_mock, parse_instance_info_mock, validate_mock):
i_info = {}
i_info['image_source'] = '733d1c44-a2ea-414b-aca7-69decf20d810'
- i_info['kernel'] = '13ce5a56-1de3-4916-b8b2-be778645d003'
- i_info['ramdisk'] = 'a5a370a8-1b39-433f-be63-2c7d708e4b4e'
+ i_info['image_type'] = 'partition'
i_info['root_gb'] = 5
i_info['swap_mb'] = 4
i_info['ephemeral_gb'] = 0
@@ -1898,6 +1943,73 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):
parse_instance_info_mock.return_value = {'swap_mb': 4}
image_source = '733d1c44-a2ea-414b-aca7-69decf20d810'
expected_i_info = {'root_gb': 5,
+ 'swap_mb': 4,
+ 'ephemeral_gb': 0,
+ 'ephemeral_format': None,
+ 'configdrive': 'configdrive',
+ 'image_source': image_source,
+ 'image_url': 'http://temp-url',
+ 'image_type': 'partition',
+ 'image_tags': [],
+ 'image_properties': {'kernel_id': 'kernel',
+ 'ramdisk_id': 'ramdisk'},
+ 'image_checksum': 'aa',
+ 'image_os_hash_algo': 'sha512',
+ 'image_os_hash_value': 'fake-sha512',
+ 'image_container_format': 'bare',
+ 'image_disk_format': 'qcow2'}
+ with task_manager.acquire(
+ self.context, self.node.uuid, shared=False) as task:
+
+ info = utils.build_instance_info_for_deploy(task)
+
+ glance_mock.assert_called_once_with(context=task.context)
+ glance_mock.return_value.show.assert_called_once_with(
+ self.node.instance_info['image_source'])
+ glance_mock.return_value.swift_temp_url.assert_called_once_with(
+ image_info)
+ validate_mock.assert_called_once_with(
+ mock.ANY, 'http://temp-url', secret=True)
+ image_type = task.node.instance_info['image_type']
+ self.assertEqual('partition', image_type)
+ self.assertEqual(expected_i_info, info)
+ parse_instance_info_mock.assert_called_once_with(task.node)
+
+ @mock.patch.object(image_service.HttpImageService, 'validate_href',
+ autospec=True)
+ @mock.patch.object(utils, 'parse_instance_info', autospec=True)
+ @mock.patch.object(image_service, 'GlanceImageService', autospec=True)
+ def test_build_instance_info_for_deploy_glance_partition_image_netboot(
+ self, glance_mock, parse_instance_info_mock, validate_mock):
+ i_info = {}
+ i_info['image_source'] = '733d1c44-a2ea-414b-aca7-69decf20d810'
+ i_info['kernel'] = '13ce5a56-1de3-4916-b8b2-be778645d003'
+ i_info['ramdisk'] = 'a5a370a8-1b39-433f-be63-2c7d708e4b4e'
+ i_info['root_gb'] = 5
+ i_info['swap_mb'] = 4
+ i_info['ephemeral_gb'] = 0
+ i_info['ephemeral_format'] = None
+ i_info['configdrive'] = 'configdrive'
+ i_info['capabilities'] = {'boot_option': 'netboot'}
+ driver_internal_info = self.node.driver_internal_info
+ driver_internal_info['is_whole_disk_image'] = False
+ self.node.driver_internal_info = driver_internal_info
+ self.node.instance_info = i_info
+ self.node.save()
+
+ image_info = {'checksum': 'aa', 'disk_format': 'qcow2',
+ 'os_hash_algo': 'sha512', 'os_hash_value': 'fake-sha512',
+ 'container_format': 'bare',
+ 'properties': {'kernel_id': 'kernel',
+ 'ramdisk_id': 'ramdisk'}}
+ glance_mock.return_value.show = mock.MagicMock(spec_set=[],
+ return_value=image_info)
+ glance_obj_mock = glance_mock.return_value
+ glance_obj_mock.swift_temp_url.return_value = 'http://temp-url'
+ parse_instance_info_mock.return_value = {'swap_mb': 4}
+ image_source = '733d1c44-a2ea-414b-aca7-69decf20d810'
+ expected_i_info = {'capabilities': {'boot_option': 'netboot'},
+ 'root_gb': 5,
'swap_mb': 4,
'ephemeral_gb': 0,
'ephemeral_format': None,
diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py
index 085c96c411..87af05bcae 100644
--- a/ironic/tests/unit/drivers/modules/test_ipxe.py
+++ b/ironic/tests/unit/drivers/modules/test_ipxe.py
@@ -136,7 +136,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
autospec=True)
def test_validate_with_boot_iso(self, mock_boot_option, mock_glance):
self.node.instance_info = {
- 'boot_iso': "http://localhost:1234/boot.iso"
+ 'boot_iso': "glance://image"
}
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
diff --git a/releasenotes/notes/image-type-ac259a90393bdd2c.yaml b/releasenotes/notes/image-type-ac259a90393bdd2c.yaml
new file mode 100644
index 0000000000..3d2dfd064f
--- /dev/null
+++ b/releasenotes/notes/image-type-ac259a90393bdd2c.yaml
@@ -0,0 +1,9 @@
+---
+features:
+ - |
+ Introduces a new explicit ``instance_info`` parameter ``image_type``,
+ which can be used to distinguish between partition and whole disk images
+ instead of a ``kernel``/``ramdisk`` pair.
+
+ Adding ``kernel`` and ``ramdisk`` is no longer necessary for partition
+ images if ``image_type`` is set to ``partition`` and local boot is used.