Fix exception handling in Glance image service

At the moment there is literally no exception handling in
glance_service.base_image_service, as it tries to catch Ironic
exceptions instead of Glance client exceptions. This change fixes
this behaviour.

Change-Id: Iaaeb234c68088538534f2a455f3678079cc60145
Closes-bug: #1441727

3 files changed, 31 insertions, 29 deletions

diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py
index 1b646d587..9a97cbbf0 100644
--- a/ironic/common/glance_service/base_image_service.py
+++ b/ironic/common/glance_service/base_image_service.py
@@ -22,6 +22,7 @@ import sys
 import time
 
 from glanceclient import client
+from glanceclient import exc as glance_exc
 from oslo_config import cfg
 import sendfile
 import six.moves.urllib.parse as urlparse
@@ -36,23 +37,23 @@ CONF = cfg.CONF
 
 
 def _translate_image_exception(image_id, exc_value):
-    if isinstance(exc_value, (exception.Forbidden,
-                              exception.Unauthorized)):
+    if isinstance(exc_value, (glance_exc.Forbidden,
+                              glance_exc.Unauthorized)):
         return exception.ImageNotAuthorized(image_id=image_id)
-    if isinstance(exc_value, exception.NotFound):
+    if isinstance(exc_value, glance_exc.NotFound):
         return exception.ImageNotFound(image_id=image_id)
-    if isinstance(exc_value, exception.BadRequest):
+    if isinstance(exc_value, glance_exc.BadRequest):
         return exception.Invalid(exc_value)
     return exc_value
 
 
 def _translate_plain_exception(exc_value):
-    if isinstance(exc_value, (exception.Forbidden,
-                              exception.Unauthorized)):
+    if isinstance(exc_value, (glance_exc.Forbidden,
+                              glance_exc.Unauthorized)):
         return exception.NotAuthorized(exc_value)
-    if isinstance(exc_value, exception.NotFound):
+    if isinstance(exc_value, glance_exc.NotFound):
         return exception.NotFound(exc_value)
-    if isinstance(exc_value, exception.BadRequest):
+    if isinstance(exc_value, glance_exc.BadRequest):
         return exception.Invalid(exc_value)
     return exc_value
 
@@ -109,13 +110,13 @@ class BaseImageService(object):
 
         :raises: GlanceConnectionFailed
         """
-        retry_excs = (exception.ServiceUnavailable,
-                      exception.InvalidEndpoint,
-                      exception.CommunicationError)
-        image_excs = (exception.Forbidden,
-                      exception.Unauthorized,
-                      exception.NotFound,
-                      exception.BadRequest)
+        retry_excs = (glance_exc.ServiceUnavailable,
+                      glance_exc.InvalidEndpoint,
+                      glance_exc.CommunicationError)
+        image_excs = (glance_exc.Forbidden,
+                      glance_exc.Unauthorized,
+                      glance_exc.NotFound,
+                      glance_exc.BadRequest)
         num_attempts = 1 + CONF.glance.glance_num_retries
 
         for attempt in range(1, num_attempts + 1):
diff --git a/ironic/tests/stubs.py b/ironic/tests/stubs.py
index 7d43d2676..d20c1fd8a 100644
--- a/ironic/tests/stubs.py
+++ b/ironic/tests/stubs.py
@@ -12,7 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from ironic.common import exception
+from glanceclient import exc as glance_exc
 
 
 NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"
@@ -40,7 +40,7 @@ class StubGlanceClient(object):
                     index += 1
                     break
             else:
-                raise exception.BadRequest('Marker not found')
+                raise glance_exc.BadRequest('Marker not found')
 
         return self._images[index:index + limit]
 
@@ -48,7 +48,7 @@ class StubGlanceClient(object):
         for image in self._images:
             if image.id == str(image_id):
                 return image
-        raise exception.ImageNotFound(image_id)
+        raise glance_exc.NotFound(image_id)
 
     def data(self, image_id):
         self.get(image_id)
@@ -76,7 +76,7 @@ class StubGlanceClient(object):
                 for k, v in metadata.items():
                     setattr(self._images[i], k, v)
                 return self._images[i]
-        raise exception.NotFound(image_id)
+        raise glance_exc.NotFound(image_id)
 
     def delete(self, image_id):
         for i, image in enumerate(self._images):
@@ -86,10 +86,10 @@ class StubGlanceClient(object):
                 # HTTPForbidden.
                 image_data = self._images[i]
                 if image_data.deleted:
-                    raise exception.Forbidden()
+                    raise glance_exc.Forbidden()
                 image_data.deleted = True
                 return
-        raise exception.NotFound(image_id)
+        raise glance_exc.NotFound(image_id)
 
 
 class FakeImage(object):
diff --git a/ironic/tests/test_glance_service.py b/ironic/tests/test_glance_service.py
index 8482ed751..6c3276da6 100644
--- a/ironic/tests/test_glance_service.py
+++ b/ironic/tests/test_glance_service.py
@@ -20,8 +20,11 @@ import os
 import tempfile
 import time
 
+from glanceclient import exc as glance_exc
 import mock
+from oslo_config import cfg
 from oslo_context import context
+from oslo_serialization import jsonutils
 import testtools
 
 
@@ -33,8 +36,6 @@ from ironic.tests import base
 from ironic.tests import matchers
 from ironic.tests import stubs
 
-from oslo_config import cfg
-from oslo_serialization import jsonutils
 
 CONF = cfg.CONF
 
@@ -468,7 +469,7 @@ class TestGlanceImageService(base.TestCase):
             def get(self, image_id):
                 if tries[0] == 0:
                     tries[0] = 1
-                    raise exception.ServiceUnavailable('')
+                    raise glance_exc.ServiceUnavailable('')
                 else:
                     return {}
 
@@ -536,7 +537,7 @@ class TestGlanceImageService(base.TestCase):
         class MyGlanceStubClient(stubs.StubGlanceClient):
             """A client that raises a Forbidden exception."""
             def get(self, image_id):
-                raise exception.Forbidden(image_id)
+                raise glance_exc.Forbidden(image_id)
 
         stub_client = MyGlanceStubClient()
         stub_context = context.RequestContext(auth_token=True)
@@ -552,7 +553,7 @@ class TestGlanceImageService(base.TestCase):
         class MyGlanceStubClient(stubs.StubGlanceClient):
             """A client that raises a HTTPForbidden exception."""
             def get(self, image_id):
-                raise exception.HTTPForbidden(image_id)
+                raise glance_exc.HTTPForbidden(image_id)
 
         stub_client = MyGlanceStubClient()
         stub_context = context.RequestContext(auth_token=True)
@@ -568,7 +569,7 @@ class TestGlanceImageService(base.TestCase):
         class MyGlanceStubClient(stubs.StubGlanceClient):
             """A client that raises a NotFound exception."""
             def get(self, image_id):
-                raise exception.NotFound(image_id)
+                raise glance_exc.NotFound(image_id)
 
         stub_client = MyGlanceStubClient()
         stub_context = context.RequestContext(auth_token=True)
@@ -584,7 +585,7 @@ class TestGlanceImageService(base.TestCase):
         class MyGlanceStubClient(stubs.StubGlanceClient):
             """A client that raises a HTTPNotFound exception."""
             def get(self, image_id):
-                raise exception.HTTPNotFound(image_id)
+                raise glance_exc.HTTPNotFound(image_id)
 
         stub_client = MyGlanceStubClient()
         stub_context = context.RequestContext(auth_token=True)
@@ -635,7 +636,7 @@ def _create_failing_glance_client(info):
         def get(self, image_id):
             info['num_calls'] += 1
             if info['num_calls'] == 1:
-                raise exception.ServiceUnavailable('')
+                raise glance_exc.ServiceUnavailable('')
             return {}
 
     return MyGlanceStubClient()