diff options
author | Dmitry Tantsur <dtantsur@protonmail.com> | 2022-01-19 13:27:44 +0100 |
---|---|---|
committer | Dmitry Tantsur <dtantsur@protonmail.com> | 2022-01-28 19:13:13 +0100 |
commit | a813c769e8d27803e87d46280db84607fdfce153 (patch) | |
tree | 2aced89fce0b491961f4fe65a768a7bf88a8f36c /ironic | |
parent | 374d5e5a3037a6b985933e734e902b2e05d805bb (diff) | |
download | ironic-a813c769e8d27803e87d46280db84607fdfce153.tar.gz |
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
Diffstat (limited to 'ironic')
-rw-r--r-- | ironic/common/images.py | 11 | ||||
-rw-r--r-- | ironic/conductor/deployments.py | 8 | ||||
-rw-r--r-- | ironic/conductor/manager.py | 10 | ||||
-rw-r--r-- | ironic/conductor/utils.py | 22 | ||||
-rw-r--r-- | ironic/drivers/modules/agent.py | 7 | ||||
-rw-r--r-- | ironic/drivers/modules/ansible/playbooks/roles/configure/tasks/main.yaml | 4 | ||||
-rw-r--r-- | ironic/drivers/modules/ansible/playbooks/roles/prepare/tasks/main.yaml | 2 | ||||
-rw-r--r-- | ironic/drivers/modules/deploy_utils.py | 28 | ||||
-rw-r--r-- | ironic/objects/node.py | 12 | ||||
-rw-r--r-- | ironic/tests/unit/common/test_images.py | 12 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_deployments.py | 5 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_manager.py | 21 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_agent.py | 9 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_deploy_utils.py | 128 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_ipxe.py | 2 |
15 files changed, 239 insertions, 42 deletions
diff --git a/ironic/common/images.py b/ironic/common/images.py index cf51d723c..939fcb676 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 30f24e404..fee436c32 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 983b504c7..9b1c20d9b 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 ab091ab1c..ff5656052 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 f309a1011..2dcd8a819 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 9baa882a6..ed8d168c3 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 e92aba69d..1c13bc523 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 1aeda7acf..812d00775 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 cdb218301..7c6c8bc1a 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 9892d671c..f699fb7ce 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -218,6 +218,18 @@ class IronicImagesTestCase(base.TestCase): @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): mock_igi.return_value = True mock_gip.return_value = {'kernel_id': 'kernel', diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 65bb1eca1..d86292aa9 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 dee5b6974..7016576b7 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 586bdc07f..4b6d271d4 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 c3ef5e204..e85ab631d 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' @@ -1774,8 +1809,18 @@ class InstanceInfoTestCase(db_base.DbTestCase): 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,6 +1919,70 @@ 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['image_type'] = 'partition' + i_info['root_gb'] = 5 + i_info['swap_mb'] = 4 + i_info['ephemeral_gb'] = 0 + i_info['ephemeral_format'] = None + i_info['configdrive'] = 'configdrive' + 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 = {'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 @@ -1880,6 +1990,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase): 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 @@ -1897,7 +2008,8 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase): 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 = {'root_gb': 5, + 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 085c96c41..87af05bca 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, |