From 0f9311fae19e4e204cdc601acbe7dafc49c73661 Mon Sep 17 00:00:00 2001
From: Fan Zhang <zh.f@outlook.com>
Date: Wed, 28 Aug 2019 11:16:25 +0800
Subject: [PATCH] Fix exception translation when creating volume

Method `nova.volume.cinder.API#create` accepts `size` as the 3rd args,
but in wrapper of `nova.volume.cinder.translate_volume_exception`, the 3rd
parameter is volume_id. If we hit cinder exception when creating volumes
like the response body down below:
```
{"itemNotFound": {"message": "Volume type with name xxx could not be found.",
"code": 404}}
```
we may get exception in nova compute log like this:
```
BuildAbortException: Build of instance xxx aborted: Volume 40 could not be
found.
```
actually, `40` is volume size, not voluem id.

This could be a little misleading.

Closes-Bug: #1846532

Change-Id: If5b0c2f28773e0b2fcb008d5251fb69d950ca569
Signed-off-by: Fan Zhang <zh.f@outlook.com>
---
 nova/tests/unit/volume/test_cinder.py | 23 +++++++++++++++++++++++
 nova/volume/cinder.py                 | 16 +++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py
index 605e83c227f3..bcf1637b9b96 100644
--- a/nova/tests/unit/volume/test_cinder.py
+++ b/nova/tests/unit/volume/test_cinder.py
@@ -210,6 +210,15 @@ class CinderApiTestCase(test.NoDBTestCase):
         self.assertRaises(exception.InvalidInput,
                           self.api.create, self.ctx, 1, '', '')
 
+    @mock.patch('nova.volume.cinder.cinderclient')
+    def test_create_failed_not_found(self, mock_cinderclient):
+        mock_cinderclient.return_value.volumes.create.side_effect = (
+            cinder_exception.NotFound(404, 'Volume type can not be found.'))
+
+        ex = self.assertRaises(exception.NotFound,
+                               self.api.create, self.ctx, 1, '', '')
+        self.assertEqual('Volume type can not be found.', six.text_type(ex))
+
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_create_over_quota_failed(self, mock_cinderclient):
         mock_cinderclient.return_value.volumes.create.side_effect = (
@@ -991,6 +1000,16 @@ class CinderApiTestCase(test.NoDBTestCase):
             keystone_exception.NotFound,
             exception.VolumeNotFound)
 
+    def test_translate_create_exception_keystone_not_found(self):
+        self._do_translate_create_exception_test(
+            keystone_exception.NotFound,
+            exception.NotFound)
+
+    def test_translate_create_exception_volume_not_found(self):
+        self._do_translate_create_exception_test(
+            cinder_exception.NotFound('Volume type could not be found'),
+            exception.NotFound)
+
     def _do_translate_cinder_exception_test(self, raised_exc, expected_exc):
         self._do_translate_exception_test(raised_exc, expected_exc,
                                           cinder.translate_cinder_exception)
@@ -999,6 +1018,10 @@ class CinderApiTestCase(test.NoDBTestCase):
         self._do_translate_exception_test(raised_exc, expected_exc,
                                           cinder.translate_mixed_exceptions)
 
+    def _do_translate_create_exception_test(self, raised_exc, expected_exc):
+        self._do_translate_exception_test(raised_exc, expected_exc,
+                                          cinder.translate_create_exception)
+
     def _do_translate_exception_test(self, raised_exc, expected_exc, wrapper):
         my_func = mock.Mock()
         my_func.__name__ = 'my_func'
diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py
index 803672189dd7..d139b9f480cc 100644
--- a/nova/volume/cinder.py
+++ b/nova/volume/cinder.py
@@ -411,6 +411,20 @@ def translate_cinder_exception(method):
     return wrapper
 
 
+def translate_create_exception(method):
+    """Transforms the exception for create but keeps its traceback intact.
+    """
+    def wrapper(self, ctx, size, *args, **kwargs):
+        try:
+            res = method(self, ctx, size, *args, **kwargs)
+        except (keystone_exception.NotFound, cinder_exception.NotFound) as e:
+            _reraise(exception.NotFound(message=e.message))
+        except cinder_exception.OverLimit as e:
+            _reraise(exception.OverQuota(message=e.message))
+        return res
+    return translate_cinder_exception(wrapper)
+
+
 def translate_volume_exception(method):
     """Transforms the exception for the volume but keeps its traceback intact.
     """
@@ -625,7 +639,7 @@ class API(object):
         return cinderclient(context).volumes.migrate_volume_completion(
             old_volume_id, new_volume_id, error)
 
-    @translate_volume_exception
+    @translate_create_exception
     def create(self, context, size, name, description, snapshot=None,
                image_id=None, volume_type=None, metadata=None,
                availability_zone=None):