From dfcd0dd7c4dad64b282843f681213b335ab735f4 Mon Sep 17 00:00:00 2001 From: xingzhou Date: Tue, 21 May 2013 02:11:18 -0400 Subject: [PATCH] ImagePollster record duplicate counter during one poll When using ImagePollster to poll image status from Glance, there might be duplicate image status event recorded down if there are private images together with public images stored in Glance. In Glance, the get image list API looks like: def _get_images(self, context, filters, **params): ... # NOTE(markwash): for backwards compatibility, is_public=True for # admins actually means "treat me as if I'm not an admin and show # all my images" if context.is_admin and params.get('is_public') is True: context.is_admin = False del params['is_public'] ... The above code shows that if a user is of 'admin_role'(this option is configured in the glance-api.conf) and querying for public images, Glance will return all the public images together with the private ones While in ceilometer/image/glance.py#_Base.iter_images method, ceilometer get the image list by querying public and private images respectively and then chain the result lists together. This causes the duplication of image in the image list. This fix changes the code in ImagePollster.get_counters method to filter out the duplicate images based on their image id. Change-Id: Ib5e213161043033c20195efc0c6e7edd78982bd6 Fixes: Bug 1180630 --- ceilometer/image/glance.py | 23 ++++++++++++++++++-- tests/image/test_glance.py | 43 +++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/ceilometer/image/glance.py b/ceilometer/image/glance.py index 83b0e1894..52e699857 100644 --- a/ceilometer/image/glance.py +++ b/ceilometer/image/glance.py @@ -45,11 +45,30 @@ class _Base(plugin.PollsterBase): client = self.get_glance_client(ksclient) #TODO(eglynn): use pagination to protect against unbounded # memory usage - return itertools.chain( + rawImageList = list(itertools.chain( client.images.list(filters={"is_public": True}), #TODO(eglynn): extend glance API with all_tenants logic to # avoid second call to retrieve private images - client.images.list(filters={"is_public": False})) + client.images.list(filters={"is_public": False}))) + + # When retrieving images from glance, glance will check + # whether the user is of 'admin_role' which is + # configured in glance-api.conf. If the user is of + # admin_role, and is querying public images(which means + # that the 'is_public' param is set to be True), + # glance will ignore 'is_public' parameter and returns + # all the public images together with private images. + # As a result, if the user/tenant has an admin role + # for ceilometer to collect image list, + # the _Base.iter_images method will return a image list + # which contains duplicate images. Add the following + # code to avoid recording down duplicate image events. + imageIdSet = set(image.id for image in rawImageList) + + for image in rawImageList: + if image.id in imageIdSet: + imageIdSet -= set([image.id]) + yield image @staticmethod def extract_image_metadata(image): diff --git a/tests/image/test_glance.py b/tests/image/test_glance.py index 8500f138a..593646d4a 100644 --- a/tests/image/test_glance.py +++ b/tests/image/test_glance.py @@ -82,9 +82,33 @@ IMAGE_LIST = [ u'deleted_at': None, u'min_ram': 0, u'size': 1024}), + # Make one duplicate private image to test the iter_images method. + type('Image', (object,), + {u'status': u'queued', + u'name': "some name", + u'deleted': False, + u'container_format': None, + u'created_at': u'2012-09-18T16:29:46', + u'disk_format': None, + u'updated_at': u'2012-09-18T16:29:46', + u'properties': {}, + u'min_disk': 0, + u'protected': False, + u'id': u'1d21a8d0-25f4-4e0a-b4ec-85f40237676b', + u'location': None, + u'checksum': None, + u'owner': u'4c8364fc20184ed7971b76602aa96184', + u'is_public': True, + u'deleted_at': None, + u'min_ram': 0, + u'size': 2048}), ] +class _BaseObject(object): + pass + + class TestManager(manager.AgentManager): def __init__(self): @@ -94,17 +118,26 @@ class TestManager(manager.AgentManager): class TestImagePollster(base.TestCase): - @staticmethod - def fake_glance_iter_images(self, ksclient): - return iter(IMAGE_LIST) + def fake_get_glance_client(self, ksclient): + glanceclient = _BaseObject() + setattr(glanceclient, "images", _BaseObject()) + setattr(glanceclient.images, + "list", lambda *args, **kwargs: iter(IMAGE_LIST)) + return glanceclient @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock()) def setUp(self): super(TestImagePollster, self).setUp() self.context = context.get_admin_context() self.manager = TestManager() - self.stubs.Set(glance._Base, 'iter_images', - self.fake_glance_iter_images) + self.stubs.Set(glance._Base, 'get_glance_client', + self.fake_get_glance_client) + + # Tests whether the iter_images method returns an unique image list. + def test_iter_images(self): + images = list(glance.ImagePollster(). + iter_images(self.manager.keystone)) + self.assertEqual(len(images), len(set(image.id for image in images))) def test_glance_image_counter(self): counters = list(glance.ImagePollster().get_counters(self.manager))