From d18a8cadbcdb4c0a0513b060e00a1753a34d5087 Mon Sep 17 00:00:00 2001 From: Matthew Gilliard Date: Fri, 9 Jan 2015 09:24:00 +0000 Subject: [PATCH] Adds get_glance_image_properties get_glance_image_property would fetch an image's metadata then return a single property. Fetching multiple properties would need multiple calls to glance, which is wasteful. This patch adds a get_glance_image_properties method, and calls it where appropriate. This means there are no longer any consumers of the original method, so it has been removed. Change-Id: Id710dfda693b1f710c6fb4f69555c80a717bec07 --- ironic/common/images.py | 17 ++++++--- ironic/drivers/modules/ilo/deploy.py | 15 ++++---- ironic/tests/drivers/ilo/test_deploy.py | 44 ++++++++++++----------- ironic/tests/test_images.py | 48 +++++++++++++++++++++---- 4 files changed, 87 insertions(+), 37 deletions(-) diff --git a/ironic/common/images.py b/ironic/common/images.py index bdf43911a5..cff9897b88 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -303,17 +303,24 @@ def converted_size(path): return data.virtual_size -def get_glance_image_property(context, image_uuid, property): - """Returns the value of a glance image property. +def get_glance_image_properties(context, image_uuid, properties="all"): + """Returns the values of several properties of a glance image :param context: context :param image_uuid: the UUID of the image in glance - :param property: the property whose value is required. - :returns: the value of the property if it exists, otherwise None. + :param properties: the properties whose values are required. + This argument is optional, default value is "all", so if not specified + all properties will be returned. + :returns: a dict of the values of the properties. A property not on the + glance metadata will have a value of None. """ glance_service = service.Service(version=1, context=context) iproperties = glance_service.show(image_uuid)['properties'] - return iproperties.get(property) + + if properties == "all": + return iproperties + + return {p: iproperties.get(p) for p in properties} def get_temp_url_for_glance_image(context, image_uuid): diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 6f799f9fac..a75f0f445d 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -89,9 +89,16 @@ def _get_boot_iso(task, root_uuid): # Option 1 - Check if user has provided a boot_iso in Glance. LOG.debug("Trying to get a boot ISO to boot the baremetal node") deploy_info = _parse_deploy_info(task.node) + image_href = deploy_info['image_source'] - boot_iso_uuid = images.get_glance_image_property(task.context, - image_href, 'boot_iso') + glance_properties = ( + images.get_glance_image_properties(task.context, + image_href, ['boot_iso', 'kernel_id', 'ramdisk_id'])) + + boot_iso_uuid = glance_properties.get('boot_iso') + kernel_uuid = glance_properties.get('kernel_id') + ramdisk_uuid = glance_properties.get('ramdisk_id') + if boot_iso_uuid: LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid) return 'glance:%s' % boot_iso_uuid @@ -104,10 +111,6 @@ def _get_boot_iso(task, root_uuid): {'node': task.node.uuid}) return - kernel_uuid = images.get_glance_image_property(task.context, - image_href, 'kernel_id') - ramdisk_uuid = images.get_glance_image_property(task.context, - image_href, 'ramdisk_id') if not kernel_uuid or not ramdisk_uuid: LOG.error(_LE("Unable to find 'kernel_id' and 'ramdisk_id' in Glance " "image %(image)s for generating boot ISO for %(node)s"), diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index 93f1aa2cbb..60d56dd056 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -61,37 +61,43 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): boot_iso_expected = "boot-%s" % self.node.uuid self.assertEqual(boot_iso_expected, boot_iso_actual) - @mock.patch.object(images, 'get_glance_image_property') + @mock.patch.object(images, 'get_glance_image_properties') @mock.patch.object(ilo_deploy, '_parse_deploy_info') def test__get_boot_iso_glance_image(self, deploy_info_mock, - image_prop_mock): + image_props_mock): deploy_info_mock.return_value = {'image_source': 'image-uuid'} - image_prop_mock.return_value = 'boot-iso-uuid' + image_props_mock.return_value = {'boot_iso': 'boot-iso-uuid', + 'kernel_id': None, + 'ramdisk_id': None} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: boot_iso_actual = ilo_deploy._get_boot_iso(task, 'root-uuid') deploy_info_mock.assert_called_once_with(task.node) - image_prop_mock.assert_called_once_with(task.context, 'image-uuid', - 'boot_iso') + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) boot_iso_expected = 'glance:boot-iso-uuid' self.assertEqual(boot_iso_expected, boot_iso_actual) @mock.patch.object(driver_utils, 'get_node_capability') - @mock.patch.object(images, 'get_glance_image_property') + @mock.patch.object(images, 'get_glance_image_properties') @mock.patch.object(ilo_deploy, '_parse_deploy_info') def test__get_boot_iso_uefi_no_glance_image(self, deploy_info_mock, - image_prop_mock, get_node_cap_mock): + image_props_mock, get_node_cap_mock): deploy_info_mock.return_value = {'image_source': 'image-uuid'} - image_prop_mock.return_value = None + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': None, + 'ramdisk_id': None} get_node_cap_mock.return_value = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: boot_iso_result = ilo_deploy._get_boot_iso(task, 'root-uuid') deploy_info_mock.assert_called_once_with(task.node) - image_prop_mock.assert_called_once_with(task.context, 'image-uuid', - 'boot_iso') + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) get_node_cap_mock.assert_called_once_with(task.node, 'boot_mode') self.assertIsNone(boot_iso_result) @@ -99,9 +105,9 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): @mock.patch.object(images, 'create_boot_iso') @mock.patch.object(swift, 'SwiftAPI') @mock.patch.object(ilo_deploy, '_get_boot_iso_object_name') - @mock.patch.object(images, 'get_glance_image_property') + @mock.patch.object(images, 'get_glance_image_properties') @mock.patch.object(ilo_deploy, '_parse_deploy_info') - def test__get_boot_iso_create(self, deploy_info_mock, image_prop_mock, + def test__get_boot_iso_create(self, deploy_info_mock, image_props_mock, boot_object_name_mock, swift_api_mock, create_boot_iso_mock, tempfile_mock): CONF.keystone_authtoken.auth_uri = 'http://authurl' @@ -116,7 +122,9 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): tempfile_mock.return_value = mock_file_handle deploy_info_mock.return_value = {'image_source': 'image-uuid'} - image_prop_mock.side_effect = [None, 'kernel-uuid', 'ramdisk-uuid'] + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': 'kernel_uuid', + 'ramdisk_id': 'ramdisk_uuid'} boot_object_name_mock.return_value = 'abcdef' create_boot_iso_mock.return_value = '/path/to/boot-iso' @@ -124,15 +132,11 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): shared=False) as task: boot_iso_actual = ilo_deploy._get_boot_iso(task, 'root-uuid') deploy_info_mock.assert_called_once_with(task.node) - image_prop_mock.assert_any_call(task.context, 'image-uuid', - 'boot_iso') - image_prop_mock.assert_any_call(task.context, 'image-uuid', - 'kernel_id') - image_prop_mock.assert_any_call(task.context, 'image-uuid', - 'ramdisk_id') + image_props_mock.assert_any_call(task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) boot_object_name_mock.assert_called_once_with(task.node) create_boot_iso_mock.assert_called_once_with(task.context, - 'tmpfile', 'kernel-uuid', 'ramdisk-uuid', + 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', 'root-uuid', 'kernel-params') swift_obj_mock.create_object.assert_called_once_with('ilo-cont', 'abcdef', diff --git a/ironic/tests/test_images.py b/ironic/tests/test_images.py index c769796550..7ebfbe475c 100644 --- a/ironic/tests/test_images.py +++ b/ironic/tests/test_images.py @@ -440,20 +440,56 @@ class FsImageTestCase(base.TestCase): 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', params) @mock.patch.object(image_service, 'Service') - def test_get_glance_image_property(self, image_service_mock): + def test_get_glance_image_properties_no_such_prop( + self, image_service_mock): - prop_dict = {'properties': {'prop1': 'val1'}} + prop_dict = {'properties': {'p1': 'v1', + 'p2': 'v2'}} image_service_obj_mock = image_service_mock.return_value image_service_obj_mock.show.return_value = prop_dict - ret_val = images.get_glance_image_property('con', 'uuid', 'prop1') + ret_val = images.get_glance_image_properties('con', 'uuid', + ['p1', 'p2', 'p3']) image_service_mock.assert_called_once_with(version=1, context='con') image_service_obj_mock.show.assert_called_once_with('uuid') - self.assertEqual('val1', ret_val) + self.assertEqual({'p1': 'v1', + 'p2': 'v2', + 'p3': None}, ret_val) - ret_val = images.get_glance_image_property('con', 'uuid', 'prop2') - self.assertIsNone(ret_val) + @mock.patch.object(image_service, 'Service') + def test_get_glance_image_properties_default_all( + self, image_service_mock): + + prop_dict = {'properties': {'p1': 'v1', + 'p2': 'v2'}} + + image_service_obj_mock = image_service_mock.return_value + image_service_obj_mock.show.return_value = prop_dict + + ret_val = images.get_glance_image_properties('con', 'uuid') + image_service_mock.assert_called_once_with(version=1, context='con') + image_service_obj_mock.show.assert_called_once_with('uuid') + self.assertEqual({'p1': 'v1', + 'p2': 'v2'}, ret_val) + + @mock.patch.object(image_service, 'Service') + def test_get_glance_image_properties_with_prop_subset( + self, image_service_mock): + + prop_dict = {'properties': {'p1': 'v1', + 'p2': 'v2', + 'p3': 'v3'}} + + image_service_obj_mock = image_service_mock.return_value + image_service_obj_mock.show.return_value = prop_dict + + ret_val = images.get_glance_image_properties('con', 'uuid', + ['p1', 'p3']) + image_service_mock.assert_called_once_with(version=1, context='con') + image_service_obj_mock.show.assert_called_once_with('uuid') + self.assertEqual({'p1': 'v1', + 'p3': 'v3'}, ret_val) @mock.patch.object(image_service, 'Service') def test_get_temp_url_for_glance_image(self, image_service_mock):