Update images API for get/list/search interface

Images API now conforms to the new search interface. This required
us to no longer return a dict of images keyed by image ID, but, rather,
just a list if images.

The 'exclude' argument to get_image() was not being used by either
nodepool or the Ansible modules, so this was simply removed to make
that method conform to the standard interface. However, it is still
used by the Ansible modules in get_image_id(), so that functionality
is kept intact.

Change-Id: Icf825fcb0471de4acb08b59b93a4dfcd399b4c69

shade/__init__.py

@@ -167,7 +167,7 @@ def _no_pending_volumes(volumes):
 
 def _no_pending_images(images):
     '''If there are any images not in a steady state, don't cache'''
-    for image_id, image in iter(images.items()):
+    for image in images:
         if image.status not in ('active', 'deleted', 'killed'):
             return False
     return True
@@ -870,6 +870,10 @@ class OpenStackCloud(object):
         servers = self.list_servers()
         return self._filter_list(servers, name_or_id, filters)
 
+    def search_images(self, name_or_id=None, filters=None):
+        images = self.list_images()
+        return self._filter_list(images, name_or_id, filters)
+
     def list_networks(self):
         return self.manager.submitTask(_tasks.NetworkList())['networks']
 
@@ -916,6 +920,46 @@ class OpenStackCloud(object):
             raise OpenStackCloudException(
                 "Error fetching server list: %s" % e)
 
+    @_cache_on_arguments(should_cache_fn=_no_pending_images)
+    def list_images(self, filter_deleted=True):
+        """Get available glance images.
+
+        :param filter_deleted: Control whether deleted images are returned.
+        :returns: A list of glance images.
+        """
+        # First, try to actually get images from glance, it's more efficient
+        images = []
+        try:
+            # If the cloud does not expose the glance API publically
+            image_gen = self.manager.submitTask(_tasks.GlanceImageList())
+
+            # Deal with the generator to make a list
+            image_list = [image for image in image_gen]
+
+            if image_list:
+                if getattr(image_list[0], 'validate', None):
+                    # glanceclient returns a "warlock" object if you use v2
+                    image_list = meta.warlock_list_to_dict(image_list)
+                else:
+                    # glanceclient returns a normal object if you use v1
+                    image_list = meta.obj_list_to_dict(image_list)
+        except (OpenStackCloudException,
+                glanceclient.exc.HTTPInternalServerError):
+            # We didn't have glance, let's try nova
+            # If this doesn't work - we just let the exception propagate
+            image_list = meta.obj_list_to_dict(
+                self.manager.submitTask(_tasks.NovaImageList())
+            )
+
+        for image in image_list:
+            # The cloud might return DELETED for invalid images.
+            # While that's cute and all, that's an implementation detail.
+            if not filter_deleted:
+                images.append(image)
+            elif image.status != 'DELETED':
+                images.append(image)
+        return images
+
     def get_network(self, name_or_id, filters=None):
         return self._get_entity(self.search_networks, name_or_id, filters)
 
@@ -938,6 +982,9 @@ class OpenStackCloud(object):
     def get_server(self, name_or_id, filters=None):
         return self._get_entity(self.search_servers, name_or_id, filters)
 
+    def get_image(self, name_or_id, filters=None):
+        return self._get_entity(self.search_images, name_or_id, filters)
+
     # TODO(Shrews): This will eventually need to support tenant ID and
     # provider networks, which are admin-level params.
     def create_network(self, name, shared=False, admin_state_up=True):
@@ -1082,71 +1129,30 @@ class OpenStackCloud(object):
             raise OpenStackCloudException(
                 "Error deleting router %s: %s" % (name_or_id, e))
 
-    def _get_images_from_cloud(self, filter_deleted):
-        # First, try to actually get images from glance, it's more efficient
-        images = dict()
-        try:
-            # If the cloud does not expose the glance API publically
-            image_list = self.manager.submitTask(_tasks.GlanceImageList())
-        except (OpenStackCloudException,
-                glanceclient.exc.HTTPInternalServerError):
-            # We didn't have glance, let's try nova
-            # If this doesn't work - we just let the exception propagate
-            image_list = self.manager.submitTask(_tasks.NovaImageList())
-        for image in image_list:
-            # The cloud might return DELETED for invalid images.
-            # While that's cute and all, that's an implementation detail.
-            if not filter_deleted:
-                images[image.id] = image
-            elif image.status != 'DELETED':
-                images[image.id] = image
-        return images
-
     def _reset_image_cache(self):
         self._image_cache = None
 
-    @_cache_on_arguments(should_cache_fn=_no_pending_images)
+    def get_image_exclude(self, name_or_id, exclude):
-    def list_images(self, filter_deleted=True):
+        for image in self.search_images(name_or_id):
-        """Get available glance images.
+            if exclude:
-
+                if exclude not in image.name:
-        :param filter_deleted: Control whether deleted images are returned.
+                    return image
-        :returns: A dictionary of glance images indexed by image UUID.
+            else:
-        """
+                return image
-        return self._get_images_from_cloud(filter_deleted)
+        return None
 
     def get_image_name(self, image_id, exclude=None):
-        image = self.get_image(image_id, exclude)
+        image = self.get_image_exclude(image_id, exclude)
         if image:
             return image.name
         return None
 
     def get_image_id(self, image_name, exclude=None):
-        image = self.get_image(image_name, exclude)
+        image = self.get_image_exclude(image_name, exclude)
         if image:
             return image.id
         return None
 
-    def get_image(self, name_or_id, exclude=None):
-        for (image_id, image) in self.list_images().items():
-            if image_id == name_or_id:
-                return image
-            if (image is not None and
-                    name_or_id == image.name and (
-                    not exclude or exclude not in image.name)):
-                return image
-        return None
-
-    def get_image_dict(self, name_or_id, exclude=None):
-        image = self.get_image(name_or_id, exclude)
-        if not image:
-            return image
-        if getattr(image, 'validate', None):
-            # glanceclient returns a "warlock" object if you use v2
-            return meta.warlock_to_dict(image)
-        else:
-            # glanceclient returns a normal object if you use v1
-            return meta.obj_to_dict(image)
-
     def create_image_snapshot(self, name, **metadata):
         image = self.manager.submitTask(_tasks.ImageSnapshotCreate(
             name=name, **metadata))
@@ -1186,7 +1192,7 @@ class OpenStackCloud(object):
             wait=False, timeout=3600, **kwargs):
         if not md5 or not sha256:
             (md5, sha256) = self._get_file_hashes(filename)
-        current_image = self.get_image_dict(name)
+        current_image = self.get_image(name)
         if (current_image and current_image.get(IMAGE_MD5_KEY, '') == md5
                 and current_image.get(IMAGE_SHA256_KEY, '') == sha256):
             self.log.debug(
@@ -1215,7 +1221,7 @@ class OpenStackCloud(object):
         self.manager.submitTask(_tasks.ImageUpdate(
             image=image, data=open(filename, 'rb')))
         self._cache.invalidate()
-        return self.get_image_dict(image.id)
+        return self.get_image(image.id)
 
     def _upload_image_task(
             self, name, filename, container, current_image=None,
@@ -1263,7 +1269,7 @@ class OpenStackCloud(object):
                         image=image,
                         **image_properties)
                     self.list_images.invalidate(self)
-                    return self.get_image_dict(status.result['image_id'])
+                    return self.get_image(status.result['image_id'])
                 if status.status == 'failure':
                     raise OpenStackCloudException(
                         "Image creation failed: {message}".format(

shade/meta.py

@@ -191,3 +191,10 @@ def warlock_to_dict(obj):
         if isinstance(value, NON_CALLABLES) and not key.startswith('_'):
             obj_dict[key] = value
     return obj_dict
+
+
+def warlock_list_to_dict(list):
+    new_list = []
+    for obj in list:
+        new_list.append(warlock_to_dict(obj))
+    return new_list

shade/tests/unit/test_caching.py

@@ -232,16 +232,17 @@ class TestMemoryCache(base.TestCase):
     @mock.patch.object(shade.OpenStackCloud, 'glance_client')
     def test_list_images(self, glance_mock):
         glance_mock.images.list.return_value = []
-        self.assertEqual({}, self.cloud.list_images())
+        self.assertEqual([], self.cloud.list_images())
 
         class Image(object):
             id = '22'
             name = '22 name'
             status = 'success'
         fake_image = Image()
+        fake_image_dict = meta.obj_to_dict(fake_image)
         glance_mock.images.list.return_value = [fake_image]
         self.cloud.list_images.invalidate(self.cloud)
-        self.assertEqual({'22': fake_image}, self.cloud.list_images())
+        self.assertEqual([fake_image_dict], self.cloud.list_images())
 
     @mock.patch.object(shade.OpenStackCloud, 'glance_client')
     def test_list_images_ignores_unsteady_status(self, glance_mock):
@@ -253,18 +254,19 @@ class TestMemoryCache(base.TestCase):
         steady_image.id = '68'
         steady_image.name = 'Jagr'
         steady_image.status = 'active'
+        steady_image_dict = meta.obj_to_dict(steady_image)
         for status in ('queued', 'saving', 'pending_delete'):
             active_image = Image()
             active_image.id = self.getUniqueString()
             active_image.name = self.getUniqueString()
             active_image.status = status
             glance_mock.images.list.return_value = [active_image]
-            self.assertEqual({active_image.id: active_image},
+            active_image_dict = meta.obj_to_dict(active_image)
+            self.assertEqual([active_image_dict],
                              self.cloud.list_images())
             glance_mock.images.list.return_value = [active_image, steady_image]
             # Should expect steady_image to appear if active wasn't cached
-            self.assertEqual({active_image.id: active_image,
+            self.assertEqual([active_image_dict, steady_image_dict],
-                              '68': steady_image},
                              self.cloud.list_images())
 
     @mock.patch.object(shade.OpenStackCloud, 'glance_client')
@@ -283,17 +285,16 @@ class TestMemoryCache(base.TestCase):
             active_image.id = self.getUniqueString()
             active_image.name = self.getUniqueString()
             active_image.status = status
+            active_image_dict = meta.obj_to_dict(active_image)
             if not first_image:
-                first_image = active_image
+                first_image = active_image_dict
             glance_mock.images.list.return_value = [active_image]
-            self.assertEqual({first_image.id: first_image},
+            self.assertEqual([first_image], self.cloud.list_images())
-                             self.cloud.list_images())
             glance_mock.images.list.return_value = [active_image, steady_image]
             # because we skipped the create_image code path, no invalidation
             # was done, so we _SHOULD_ expect steady state images to cache and
             # therefore we should _not_ expect to see the new one here
-            self.assertEqual({first_image.id: first_image},
+            self.assertEqual([first_image], self.cloud.list_images())
-                             self.cloud.list_images())
 
     def _call_create_image(self, name, container=None):
         imagefile = tempfile.NamedTemporaryFile(delete=False)
@@ -306,7 +307,7 @@ class TestMemoryCache(base.TestCase):
     def test_create_image_put(self, glance_mock):
         self.cloud.api_versions['image'] = '1'
         glance_mock.images.list.return_value = []
-        self.assertEqual({}, self.cloud.list_images())
+        self.assertEqual([], self.cloud.list_images())
 
         class Image(object):
             id = '42'
@@ -322,7 +323,8 @@ class TestMemoryCache(base.TestCase):
         glance_mock.images.create.assert_called_with(**args)
         glance_mock.images.update.assert_called_with(data=mock.ANY,
                                                      image=fake_image)
-        self.assertEqual({'42': fake_image}, self.cloud.list_images())
+        fake_image_dict = meta.obj_to_dict(fake_image)
+        self.assertEqual([fake_image_dict], self.cloud.list_images())
 
     @mock.patch.object(shade.OpenStackCloud, 'glance_client')
     @mock.patch.object(shade.OpenStackCloud, 'swift_client')
@@ -337,7 +339,7 @@ class TestMemoryCache(base.TestCase):
         swift_mock.put_container.return_value = fake_container
         swift_mock.head_object.return_value = {}
         glance_mock.images.list.return_value = []
-        self.assertEqual({}, self.cloud.list_images())
+        self.assertEqual([], self.cloud.list_images())
 
         # V2's warlock objects just work like dicts
         class FakeImage(dict):
@@ -382,4 +384,5 @@ class TestMemoryCache(base.TestCase):
                 'owner_specified.shade.sha256': mock.ANY,
                 'image_id': '99'}
         glance_mock.images.update.assert_called_with(**args)
-        self.assertEqual({'99': fake_image}, self.cloud.list_images())
+        fake_image_dict = meta.obj_to_dict(fake_image)
+        self.assertEqual([fake_image_dict], self.cloud.list_images())

shade/tests/unit/test_shade.py

@@ -72,6 +72,20 @@ class TestShade(base.TestCase):
                                       }})
         self.assertEquals([el2, el3], ret)
 
+    @mock.patch.object(shade.OpenStackCloud, 'search_images')
+    def test_get_images(self, mock_search):
+        image1 = dict(id='123', name='mickey')
+        mock_search.return_value = [image1]
+        r = self.cloud.get_image('mickey')
+        self.assertIsNotNone(r)
+        self.assertDictEqual(image1, r)
+
+    @mock.patch.object(shade.OpenStackCloud, 'search_images')
+    def test_get_image_not_found(self, mock_search):
+        mock_search.return_value = []
+        r = self.cloud.get_image('doesNotExist')
+        self.assertIsNone(r)
+
     @mock.patch.object(shade.OpenStackCloud, 'search_servers')
     def test_get_server(self, mock_search):
         server1 = dict(id='123', name='mickey')