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.