Tidy up nova.image.glance

* Add comment explaining relevance of _is_image_available
* Drop superfluous log statements
* Drop GlanceImageService.delete_all as it has never been implemented
* Drop the 'features' kwarg for GlanceImageService.update
* Code simplification

Change-Id: Ie538dd93dec729fb060bbcddfbddcba6cf303e52

7 files changed, 16 insertions, 51 deletions

diff --git a/nova/image/glance.py b/nova/image/glance.py
index 0b2d29c7d..0167a25fc 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -87,7 +87,7 @@ class GlanceClientWrapper(object):
 
     def __init__(self, context=None, host=None, port=None):
         if host is not None:
-            self._create_static_client(context, host, port)
+            self.client = self._create_static_client(context, host, port)
         else:
             self.client = None
         self.api_servers = None
@@ -96,7 +96,7 @@ class GlanceClientWrapper(object):
         """Create a client that we'll use for every call."""
         self.host = host
         self.port = port
-        self.client = _create_glance_client(context, self.host, self.port)
+        return _create_glance_client(context, self.host, self.port)
 
     def _create_onetime_client(self, context):
         """Create a client that will be used for one call."""
@@ -110,17 +110,13 @@ class GlanceClientWrapper(object):
         Call a glance client method.  If we get a connection error,
         retry the request according to FLAGS.glance_num_retries.
         """
-
         retry_excs = (glance_exception.ClientConnectionError,
                 glance_exception.ServiceUnavailable)
 
         num_attempts = 1 + FLAGS.glance_num_retries
 
         for attempt in xrange(1, num_attempts + 1):
-            if self.client:
-                client = self.client
-            else:
-                client = self._create_onetime_client(context)
+            client = self.client or self._create_onetime_client(context)
             try:
                 return getattr(client, method)(*args, **kwargs)
             except retry_excs as e:
@@ -136,16 +132,13 @@ class GlanceClientWrapper(object):
                             host=host, port=port, reason=str(e))
                 LOG.exception(error_msg, locals())
                 time.sleep(1)
-        # Not reached
 
 
 class GlanceImageService(object):
     """Provides storage and retrieval of disk image objects within Glance."""
 
     def __init__(self, client=None):
-        if client is None:
-            client = GlanceClientWrapper()
-        self._client = client
+        self._client = client or GlanceClientWrapper()
 
     def detail(self, context, **kwargs):
         """Calls out to Glance for a list of detailed image information."""
@@ -166,12 +159,10 @@ class GlanceImageService(object):
         for param in accepted_params:
             if param in params:
                 _params[param] = params.get(param)
-
         return _params
 
     def _get_images(self, context, **kwargs):
         """Get image entitites from images service"""
-
         # ensure filters is a dict
         kwargs['filters'] = kwargs.get('filters') or {}
         # NOTE(vish): don't filter out private images
@@ -242,23 +233,13 @@ class GlanceImageService(object):
         :raises: AlreadyExists if the image already exist.
 
         """
-        # Translate Base -> Service
-        LOG.debug(_('Creating image in Glance. Metadata passed in %s'),
-                  image_meta)
         sent_service_image_meta = self._translate_to_glance(image_meta)
-        LOG.debug(_('Metadata after formatting for Glance %s'),
-                  sent_service_image_meta)
-
         recv_service_image_meta = self._client.call(context,
                 'add_image', sent_service_image_meta, data)
-
-        # Translate Service -> Base
         base_image_meta = self._translate_from_glance(recv_service_image_meta)
-        LOG.debug(_('Metadata returned from Glance formatted for Base %s'),
-                  base_image_meta)
         return base_image_meta
 
-    def update(self, context, image_id, image_meta, data=None, features=None):
+    def update(self, context, image_id, image_meta, data=None):
         """Replace the contents of the given image with the new data.
 
         :raises: ImageNotFound if the image does not exist.
@@ -269,7 +250,7 @@ class GlanceImageService(object):
         image_meta = self._translate_to_glance(image_meta)
         try:
             image_meta = self._client.call(context, 'update_image',
-                    image_id, image_meta, data, features)
+                    image_id, image_meta, data)
         except Exception:
             _reraise_translated_image_exception(image_id)
 
@@ -291,18 +272,14 @@ class GlanceImageService(object):
             raise exception.ImageNotFound(image_id=image_id)
         return result
 
-    def delete_all(self):
-        """Clears out all images."""
-        pass
-
-    @classmethod
-    def _translate_to_glance(cls, image_meta):
+    @staticmethod
+    def _translate_to_glance(image_meta):
         image_meta = _convert_to_string(image_meta)
         image_meta = _remove_read_only(image_meta)
         return image_meta
 
-    @classmethod
-    def _translate_from_glance(cls, image_meta):
+    @staticmethod
+    def _translate_from_glance(image_meta):
         image_meta = _limit_attributes(image_meta)
         image_meta = _convert_timestamps_to_datetimes(image_meta)
         image_meta = _convert_from_string(image_meta)
@@ -312,10 +289,11 @@ class GlanceImageService(object):
     def _is_image_available(context, image_meta):
         """Check image availability.
 
-        Under Glance, images are always available if the context has
-        an auth_token.
-
+        This check is needed in case Nova and Glance are deployed
+        without authentication turned on.
         """
+        # The presence of an auth token implies this is an authenticated
+        # request and we need not handle the noauth use-case.
         if hasattr(context, 'auth_token') and context.auth_token:
             return True
 
diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py
index 19d752a9f..ef3995649 100644
--- a/nova/tests/api/openstack/compute/test_server_actions.py
+++ b/nova/tests/api/openstack/compute/test_server_actions.py
@@ -73,7 +73,6 @@ class ServerActionsControllerTest(test.TestCase):
         nova.tests.image.fake.stub_out_image_service(self.stubs)
         service_class = 'nova.image.glance.GlanceImageService'
         self.service = importutils.import_object(service_class)
-        self.service.delete_all()
         self.sent_to_glance = {}
         fakes.stub_out_glance_add_image(self.stubs, self.sent_to_glance)
         self.flags(allow_instance_snapshots=True,
diff --git a/nova/tests/glance/stubs.py b/nova/tests/glance/stubs.py
index b3cef0ff3..4811169fc 100644
--- a/nova/tests/glance/stubs.py
+++ b/nova/tests/glance/stubs.py
@@ -69,7 +69,7 @@ class StubGlanceClient(object):
 
         return self.images[-1]
 
-    def update_image(self, image_id, metadata, data, features):
+    def update_image(self, image_id, metadata, data):
         for i, image in enumerate(self.images):
             if image['id'] == str(image_id):
                 if 'id' in metadata:
diff --git a/nova/tests/image/fake.py b/nova/tests/image/fake.py
index 66e37e5d5..dea9c14e9 100644
--- a/nova/tests/image/fake.py
+++ b/nova/tests/image/fake.py
@@ -221,9 +221,6 @@ class _FakeImageService(object):
         if not removed:
             raise exception.ImageNotFound(image_id=image_id)
 
-    def delete_all(self):
-        """Clears out all images."""
-        self.images.clear()
 
 _fakeImageService = _FakeImageService()
 
diff --git a/nova/tests/image/test_fake.py b/nova/tests/image/test_fake.py
index ab7f043d2..70ce001b4 100644
--- a/nova/tests/image/test_fake.py
+++ b/nova/tests/image/test_fake.py
@@ -108,14 +108,6 @@ class FakeImageServiceTestCase(test.TestCase):
                           self.context,
                           '34')
 
-    def test_delete_all(self):
-        self.image_service.create(self.context, {'id': '32', 'foo': 'bar'})
-        self.image_service.create(self.context, {'id': '33', 'foo': 'bar'})
-        self.image_service.create(self.context, {'id': '34', 'foo': 'bar'})
-        self.image_service.delete_all()
-        index = self.image_service.detail(self.context)
-        self.assertEquals(len(index), 0)
-
     def test_create_then_get(self):
         blob = 'some data'
         s1 = StringIO.StringIO(blob)
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 4d866bb7b..fadea5a8c 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -103,7 +103,6 @@ class TestGlanceImageService(test.TestCase):
         client = glance_stubs.StubGlanceClient()
         self.service = self._create_image_service(client)
         self.context = context.RequestContext('fake', 'fake', auth_token=True)
-        self.service.delete_all()
 
     def _create_image_service(self, client):
         def _fake_create_glance_client(context, host, port):
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index e7f701add..4319726d8 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -96,7 +96,7 @@ IMAGE_FIXTURES = {
 
 def set_image_fixtures():
     image_service = fake_image.FakeImageService()
-    image_service.delete_all()
+    image_service.images.clear()
     for image_id, image_meta in IMAGE_FIXTURES.items():
         image_meta = image_meta['image_meta']
         image_meta['id'] = image_id