Merge "Adds caches for image and flavor in compute agent"

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_flavor(instance, flavor_cache)
-            self._with_image(instance)
+            self._with_image(instance, image_cache)
 
         return instances
 
-    def _with_flavor(self, instance):
+    def _with_flavor(self, instance, cache):
         fid = instance.flavor['id']
-        try:
+        if fid in cache:
-            flavor = self.nova_client.flavors.get(fid)
+            flavor = cache.get(fid)
-        except novaclient.exceptions.NotFound:
+        else:
-            flavor = None
+            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:
+        if iid in cache:
-            image = self.nova_client.images.get(iid)
+            image = cache.get(iid)
-        except novaclient.exceptions.NotFound:
+        else:
-            instance.image['name'] = 'unknown-id-%s' % iid
+            try:
-            instance.kernel_id = None
+                image = self.nova_client.images.get(iid)
-            instance.ramdisk_id = None
+            except novaclient.exceptions.NotFound:
-            return
+                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']:
+        for attr, default in attr_defaults:
-            ameta = image_metadata.get(attr) if image_metadata else None
+            ameta = image_metadata.get(attr) if image_metadata else default
             setattr(instance, attr, ameta)
 
     @logged

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(self, *args, **kwargs):
-    def fake_flavors_get(*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(self, *args, **kwargs):
-    def fake_images_get(*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)