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>
			
			
This commit is contained in:
		| @@ -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' | ||||
|   | ||||
| @@ -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): | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Fan Zhang
					Fan Zhang