Translate OverLimit exceptions in Cinder calls

The cinder wrapper on all cinder API calls can check for the cinder
client returning OverLimit, so it can get correctly translated to
OverQuota. The OverQuota is different in volumes vs. snapshots, so they
need to be separated out into the different wrappers. But also, because
in snapshot creations, we need to catch a NotFound as a VolumeNotFound
and an OverLimit as an OverQuota for snapshots, we need to make a new
wrapper that mixes those two together for when we create snapshots.

Change-Id: Ia03f15232df71ca9a31ffbcca60f33949312a686
Partial-Bug: #1554631

2 files changed, 58 insertions, 33 deletions

diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py
index 218fa337e8..facf3ffc1f 100644
--- a/nova/tests/unit/volume/test_cinder.py
+++ b/nova/tests/unit/volume/test_cinder.py
@@ -482,11 +482,32 @@ class CinderApiTestCase(test.NoDBTestCase):
             keystone_exception.Forbidden,
             exception.Forbidden)
 
+    def test_translate_mixed_exception_over_limit(self):
+        self._do_translate_mixed_exception_test(
+            cinder_exception.OverLimit(''),
+            exception.OverQuota)
+
+    def test_translate_mixed_exception_volume_not_found(self):
+        self._do_translate_mixed_exception_test(
+            cinder_exception.NotFound(''),
+            exception.VolumeNotFound)
+
+    def test_translate_mixed_exception_keystone_not_found(self):
+        self._do_translate_mixed_exception_test(
+            keystone_exception.NotFound,
+            exception.VolumeNotFound)
+
     def _do_translate_cinder_exception_test(self, raised_exc, expected_exc):
+        self._do_translate_exception_test(raised_exc, expected_exc,
+                                          cinder.translate_cinder_exception)
+
+    def _do_translate_mixed_exception_test(self, raised_exc, expected_exc):
+        self._do_translate_exception_test(raised_exc, expected_exc,
+                                          cinder.translate_mixed_exceptions)
+
+    def _do_translate_exception_test(self, raised_exc, expected_exc, wrapper):
         my_func = mock.Mock()
         my_func.__name__ = 'my_func'
         my_func.side_effect = raised_exc
 
-        self.assertRaises(expected_exc,
-                          cinder.translate_cinder_exception(my_func),
-                          'foo', 'bar', 'baz')
+        self.assertRaises(expected_exc, wrapper(my_func), 'foo', 'bar', 'baz')
diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py
index 9d283b170e..87fb6d753a 100644
--- a/nova/volume/cinder.py
+++ b/nova/volume/cinder.py
@@ -233,20 +233,16 @@ def translate_cinder_exception(method):
         except (cinder_exception.ConnectionError,
                 keystone_exception.ConnectionError):
             exc_type, exc_value, exc_trace = sys.exc_info()
-            exc_value = exception.CinderConnectionFailed(
-                reason=six.text_type(exc_value))
-            six.reraise(exc_value, None, exc_trace)
+            _reraise(exception.CinderConnectionFailed(
+                reason=six.text_type(exc_value)))
         except (keystone_exception.BadRequest,
                 cinder_exception.BadRequest):
             exc_type, exc_value, exc_trace = sys.exc_info()
-            exc_value = exception.InvalidInput(
-                reason=six.text_type(exc_value))
-            six.reraise(exc_value, None, exc_trace)
+            _reraise(exception.InvalidInput(reason=six.text_type(exc_value)))
         except (keystone_exception.Forbidden,
                 cinder_exception.Forbidden):
             exc_type, exc_value, exc_trace = sys.exc_info()
-            exc_value = exception.Forbidden(message=six.text_type(exc_value))
-            six.reraise(exc_value, None, exc_trace)
+            _reraise(exception.Forbidden(six.text_type(exc_value)))
         return res
     return wrapper
 
@@ -257,13 +253,10 @@ def translate_volume_exception(method):
     def wrapper(self, ctx, volume_id, *args, **kwargs):
         try:
             res = method(self, ctx, volume_id, *args, **kwargs)
-        except (cinder_exception.ClientException,
-                keystone_exception.ClientException):
-            exc_type, exc_value, exc_trace = sys.exc_info()
-            if isinstance(exc_value, (keystone_exception.NotFound,
-                                      cinder_exception.NotFound)):
-                exc_value = exception.VolumeNotFound(volume_id=volume_id)
-            six.reraise(exc_value, None, exc_trace)
+        except (keystone_exception.NotFound, cinder_exception.NotFound):
+            _reraise(exception.VolumeNotFound(volume_id=volume_id))
+        except cinder_exception.OverLimit:
+            _reraise(exception.OverQuota(overs='volumes'))
         return res
     return translate_cinder_exception(wrapper)
 
@@ -275,17 +268,31 @@ def translate_snapshot_exception(method):
     def wrapper(self, ctx, snapshot_id, *args, **kwargs):
         try:
             res = method(self, ctx, snapshot_id, *args, **kwargs)
-        except (cinder_exception.ClientException,
-                keystone_exception.ClientException):
-            exc_type, exc_value, exc_trace = sys.exc_info()
-            if isinstance(exc_value, (keystone_exception.NotFound,
-                                      cinder_exception.NotFound)):
-                exc_value = exception.SnapshotNotFound(snapshot_id=snapshot_id)
-            six.reraise(exc_value, None, exc_trace)
+        except (keystone_exception.NotFound, cinder_exception.NotFound):
+            _reraise(exception.SnapshotNotFound(snapshot_id=snapshot_id))
+        return res
+    return translate_cinder_exception(wrapper)
+
+
+def translate_mixed_exceptions(method):
+    """Transforms exceptions that can come from both volumes and snapshots."""
+    def wrapper(self, ctx, res_id, *args, **kwargs):
+        try:
+            res = method(self, ctx, res_id, *args, **kwargs)
+        except (keystone_exception.NotFound, cinder_exception.NotFound):
+            _reraise(exception.VolumeNotFound(volume_id=res_id))
+        except cinder_exception.OverLimit:
+            _reraise(exception.OverQuota(overs='snapshots'))
         return res
     return translate_cinder_exception(wrapper)
 
 
+def _reraise(desired_exc):
+    exc_type, exc_value, exc_trace = sys.exc_info()
+    exc_value = desired_exc
+    six.reraise(exc_value, None, exc_trace)
+
+
 class API(object):
     """API for interacting with the volume manager."""
 
@@ -447,7 +454,7 @@ class API(object):
         return cinderclient(context).volumes.migrate_volume_completion(
             old_volume_id, new_volume_id, error)
 
-    @translate_cinder_exception
+    @translate_volume_exception
     def create(self, context, size, name, description, snapshot=None,
                image_id=None, volume_type=None, metadata=None,
                availability_zone=None):
@@ -473,11 +480,8 @@ class API(object):
             kwargs['name'] = name
             kwargs['description'] = description
 
-        try:
-            item = client.volumes.create(size, **kwargs)
-            return _untranslate_volume_summary_view(context, item)
-        except cinder_exception.OverLimit:
-            raise exception.OverQuota(overs='volumes')
+        item = client.volumes.create(size, **kwargs)
+        return _untranslate_volume_summary_view(context, item)
 
     @translate_volume_exception
     def delete(self, context, volume_id):
@@ -502,7 +506,7 @@ class API(object):
 
         return rvals
 
-    @translate_volume_exception
+    @translate_mixed_exceptions
     def create_snapshot(self, context, volume_id, name, description):
         item = cinderclient(context).volume_snapshots.create(volume_id,
                                                              False,
@@ -510,7 +514,7 @@ class API(object):
                                                              description)
         return _untranslate_snapshot_summary_view(context, item)
 
-    @translate_volume_exception
+    @translate_mixed_exceptions
     def create_snapshot_force(self, context, volume_id, name, description):
         item = cinderclient(context).volume_snapshots.create(volume_id,
                                                              True,