Allow cinderclient to handle exception response

For all cinder commands, the error message returned by the cinder
service is gobbled by keystoneclient as raise_exc is enabled by
default and it doesn't have the knowledge of how to interpret
exception returned by the cinder service correctly.

Set raise_exc to False explicitly in request method of cinderclient
and pass it to the keystoneclient session request method so that
keystoneclient raises exception only when the exception is raised
by the keystone server else cinderclient should interpret the
exception returned by the cinder service.

Closes-Bug: 1431693
Closes-Bug: 1428764
Change-Id: I1160a2c998919b669c39a4b739e83f5bd9d8d235

cinderclient/client.py

@@ -73,7 +73,16 @@ class SessionClient(adapter.LegacyJsonAdapter):
 
     def request(self, *args, **kwargs):
         kwargs.setdefault('authenticated', False)
-        return super(SessionClient, self).request(*args, **kwargs)
+        # Note(tpatil): The standard call raises errors from
+        # keystoneclient, here we need to raise the cinderclient errors.
+        raise_exc = kwargs.pop('raise_exc', True)
+        resp, body = super(SessionClient, self).request(*args,
+                                                        raise_exc=False,
+                                                        **kwargs)
+        if raise_exc and resp.status_code >= 400:
+            raise exceptions.from_response(resp, body)
+
+        return resp, body
 
     def _cs_request(self, url, method, **kwargs):
         # this function is mostly redundant but makes compatibility easier

cinderclient/tests/unit/test_client.py

@@ -12,13 +12,18 @@
 # limitations under the License.
 
 import logging
+import json
 
 import fixtures
+import mock
 
 import cinderclient.client
 import cinderclient.v1.client
 import cinderclient.v2.client
+from cinderclient import exceptions
 from cinderclient.tests.unit import utils
+from keystoneclient import adapter
+from keystoneclient import exceptions as keystone_exception
 
 
 class ClientTest(utils.TestCase):
@@ -74,3 +79,111 @@ class ClientTest(utils.TestCase):
         self.assertRaises(cinderclient.exceptions.UnsupportedVersion,
                           cinderclient.client.get_volume_api_from_url,
                           unknown_url)
+
+    @mock.patch.object(adapter.Adapter, 'request')
+    @mock.patch.object(exceptions, 'from_response')
+    def test_sessionclient_request_method(
+            self, mock_from_resp, mock_request):
+        kwargs = {
+            "body": {
+                "volume": {
+                    "status": "creating",
+                    "imageRef": "username",
+                    "attach_status": "detached"
+                },
+                "authenticated": "True"
+            }
+        }
+
+        resp = {
+            "text": {
+                "volume": {
+                    "status": "creating",
+                    "id": "431253c0-e203-4da2-88df-60c756942aaf",
+                    "size": 1
+                }
+            },
+            "code": 202
+        }
+
+        mock_response = utils.TestResponse({
+            "status_code": 202,
+            "text": json.dumps(resp),
+        })
+
+        # 'request' method of Adaptor will return 202 response
+        mock_request.return_value = mock_response
+        session_client = cinderclient.client.SessionClient(session=mock.Mock())
+        response, body = session_client.request(mock.sentinel.url,
+                                                'POST', **kwargs)
+
+        # In this case, from_response method will not get called
+        # because response status_code is < 400
+        self.assertEqual(202, response.status_code)
+        self.assertFalse(mock_from_resp.called)
+
+    @mock.patch.object(adapter.Adapter, 'request')
+    def test_sessionclient_request_method_raises_badrequest(
+            self, mock_request):
+        kwargs = {
+            "body": {
+                "volume": {
+                    "status": "creating",
+                    "imageRef": "username",
+                    "attach_status": "detached"
+                },
+                "authenticated": "True"
+            }
+        }
+
+        resp = {
+            "badRequest": {
+                "message": "Invalid image identifier or unable to access "
+                           "requested image.",
+                "code": 400
+            }
+        }
+
+        mock_response = utils.TestResponse({
+            "status_code": 400,
+            "text": json.dumps(resp),
+        })
+
+        # 'request' method of Adaptor will return 400 response
+        mock_request.return_value = mock_response
+        session_client = cinderclient.client.SessionClient(
+            session=mock.Mock())
+
+        # 'from_response' method will raise BadRequest because
+        # resp.status_code is 400
+        self.assertRaises(exceptions.BadRequest, session_client.request,
+                          mock.sentinel.url, 'POST', **kwargs)
+
+    @mock.patch.object(exceptions, 'from_response')
+    def test_keystone_request_raises_auth_failure_exception(
+            self, mock_from_resp):
+
+        kwargs = {
+            "body": {
+                "volume": {
+                    "status": "creating",
+                    "imageRef": "username",
+                    "attach_status": "detached"
+                },
+                "authenticated": "True"
+            }
+        }
+
+        with mock.patch.object(adapter.Adapter, 'request',
+                               side_effect=
+                               keystone_exception.AuthorizationFailure()):
+            session_client = cinderclient.client.SessionClient(
+                session=mock.Mock())
+            self.assertRaises(keystone_exception.AuthorizationFailure,
+                              session_client.request,
+                              mock.sentinel.url, 'POST', **kwargs)
+
+        # As keystonesession.request method will raise
+        # AuthorizationFailure exception, check exceptions.from_response
+        # is not getting called.
+        self.assertFalse(mock_from_resp.called)