diff --git a/ceilometer/nova_client.py b/ceilometer/nova_client.py index 092695e83..e8b67b8ef 100644 --- a/ceilometer/nova_client.py +++ b/ceilometer/nova_client.py @@ -58,18 +58,24 @@ class Client(object): no_cache=True) def _with_flavor_and_image(self, instances): + flavor_cache = {} + image_cache = {} for instance in instances: - self._with_flavor(instance) - self._with_image(instance) + self._with_flavor(instance, flavor_cache) + self._with_image(instance, image_cache) return instances - def _with_flavor(self, instance): + def _with_flavor(self, instance, cache): fid = instance.flavor['id'] - try: - flavor = self.nova_client.flavors.get(fid) - except novaclient.exceptions.NotFound: - flavor = None + if fid in cache: + flavor = cache.get(fid) + else: + try: + flavor = self.nova_client.flavors.get(fid) + except novaclient.exceptions.NotFound: + flavor = None + cache[fid] = flavor attr_defaults = [('name', 'unknown-id-%s' % fid), ('vcpus', 0), ('ram', 0), ('disk', 0), @@ -81,7 +87,7 @@ class Client(object): continue instance.flavor[attr] = getattr(flavor, attr, default) - def _with_image(self, instance): + def _with_image(self, instance, cache): try: iid = instance.image['id'] except TypeError: @@ -90,19 +96,24 @@ class Client(object): instance.ramdisk_id = None return - try: - image = self.nova_client.images.get(iid) - except novaclient.exceptions.NotFound: - instance.image['name'] = 'unknown-id-%s' % iid - instance.kernel_id = None - instance.ramdisk_id = None - return + if iid in cache: + image = cache.get(iid) + else: + try: + image = self.nova_client.images.get(iid) + except novaclient.exceptions.NotFound: + image = None + cache[iid] = image - instance.image['name'] = getattr(image, 'name') + attr_defaults = [('kernel_id', None), + ('ramdisk_id', None)] + + instance.image['name'] = \ + getattr(image, 'name') if image else 'unknown-id-%s' % iid image_metadata = getattr(image, 'metadata', None) - for attr in ['kernel_id', 'ramdisk_id']: - ameta = image_metadata.get(attr) if image_metadata else None + for attr, default in attr_defaults: + ameta = image_metadata.get(attr) if image_metadata else default setattr(instance, attr, ameta) @logged diff --git a/ceilometer/tests/test_novaclient.py b/ceilometer/tests/test_novaclient.py index 7733921bd..be01e6fe5 100644 --- a/ceilometer/tests/test_novaclient.py +++ b/ceilometer/tests/test_novaclient.py @@ -29,6 +29,8 @@ class TestNovaClient(test.BaseTestCase): def setUp(self): super(TestNovaClient, self).setUp() + self._flavors_count = 0 + self._images_count = 0 self.nv = nova_client.Client() self.useFixture(mockpatch.PatchObject( self.nv.nova_client.flavors, 'get', @@ -37,8 +39,8 @@ class TestNovaClient(test.BaseTestCase): self.nv.nova_client.images, 'get', side_effect=self.fake_images_get)) - @staticmethod - def fake_flavors_get(*args, **kwargs): + def fake_flavors_get(self, *args, **kwargs): + self._flavors_count += 1 a = mock.MagicMock() a.id = args[0] if a.id == 1: @@ -49,8 +51,8 @@ class TestNovaClient(test.BaseTestCase): raise novaclient.exceptions.NotFound('foobar') return a - @staticmethod - def fake_images_get(*args, **kwargs): + def fake_images_get(self, *args, **kwargs): + self._images_count += 1 a = mock.MagicMock() a.id = args[0] image_details = { @@ -70,30 +72,24 @@ class TestNovaClient(test.BaseTestCase): return a - @staticmethod - def fake_flavors_list(): - a = mock.MagicMock() - a.id = 1 - a.name = 'm1.tiny' - b = mock.MagicMock() - b.id = 2 - b.name = 'm1.large' - return [a, b] - @staticmethod def fake_servers_list(*args, **kwargs): a = mock.MagicMock() a.id = 42 a.flavor = {'id': 1} a.image = {'id': 1} - return [a] + b = mock.MagicMock() + b.id = 43 + b.flavor = {'id': 2} + b.image = {'id': 2} + return [a, b] def test_instance_get_all_by_host(self): with patch.object(self.nv.nova_client.servers, 'list', side_effect=self.fake_servers_list): instances = self.nv.instance_get_all_by_host('foobar') - self.assertEqual(1, len(instances)) + self.assertEqual(2, len(instances)) self.assertEqual('m1.tiny', instances[0].flavor['name']) self.assertEqual('ubuntu-12.04-x86', instances[0].image['name']) self.assertEqual(11, instances[0].kernel_id) @@ -150,6 +146,7 @@ class TestNovaClient(test.BaseTestCase): def test_with_flavor_and_image(self): results = self.nv._with_flavor_and_image(self.fake_servers_list()) instance = results[0] + self.assertEqual(2, len(results)) self.assertEqual('ubuntu-12.04-x86', instance.image['name']) self.assertEqual('m1.tiny', instance.flavor['name']) self.assertEqual(11, instance.kernel_id) @@ -204,6 +201,30 @@ class TestNovaClient(test.BaseTestCase): self.assertIsNone(instance.kernel_id) self.assertEqual(21, instance.ramdisk_id) + def test_with_flavor_and_image_no_cache(self): + results = self.nv._with_flavor_and_image(self.fake_servers_list()) + self.assertEqual(2, len(results)) + self.assertEqual(2, self._flavors_count) + self.assertEqual(2, self._images_count) + + def test_with_flavor_and_image_cache(self): + results = self.nv._with_flavor_and_image(self.fake_servers_list() * 2) + self.assertEqual(4, len(results)) + self.assertEqual(2, self._flavors_count) + self.assertEqual(2, self._images_count) + + def test_with_flavor_and_image_unknown_image_cache(self): + instances = self.fake_servers_list_unknown_image() + results = self.nv._with_flavor_and_image(instances * 2) + self.assertEqual(2, len(results)) + self.assertEqual(1, self._flavors_count) + self.assertEqual(1, self._images_count) + for instance in results: + self.assertEqual('unknown-id-666', instance.image['name']) + self.assertNotEqual(instance.flavor['name'], 'unknown-id-666') + self.assertIsNone(instance.kernel_id) + self.assertIsNone(instance.ramdisk_id) + def test_with_missing_image_instance(self): instances = self.fake_instance_image_missing() results = self.nv._with_flavor_and_image(instances)