Cinder os-attach api returns 500

If volume is in 'in-use' state or invalid instance UUID
is passed to cinder os-attach api, then it returns 500 error
because InvalidVolume and InvalidUUID exceptions are raised
from manager, but not caught at api level.

Caught RemoteError exception in the api and returned 400 error
code if user passes invalid information of the volume or invalid
instance UUID.

Closes-Bug: 1449454
Change-Id: I2121b03b9f2310c40e0158627b7d4b9ba73a8f5d

diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py
index 423cf17a288de4ba43c0926011ddb5b9de3addf7..6f13d2640088d179a527a1f3c5994837e8e88b66 100644 (file)
--- a/cinder/api/contrib/volume_actions.py
+++ b/cinder/api/contrib/volume_actions.py
@@ -114,9 +114,21 @@ class VolumeActionsController(wsgi.Controller):
             msg = _("Invalid request to attach volume with an invalid mode. "
                     "Attaching mode should be 'rw' or 'ro'")
             raise webob.exc.HTTPBadRequest(explanation=msg)
+        try:
+            self.volume_api.attach(context, volume,
+                                   instance_uuid, host_name, mountpoint, mode)
+        except messaging.RemoteError as error:
+            if error.exc_type in ['InvalidVolume', 'InvalidUUID',
+                                  'InvalidVolumeAttachMode']:
+                msg = "Error attaching volume - %(err_type)s: %(err_msg)s" % {
+                      'err_type': error.exc_type, 'err_msg': error.value}
+                raise webob.exc.HTTPBadRequest(explanation=msg)
+            else:
+                # There are also few cases where attach call could fail due to
+                # db or volume driver errors. These errors shouldn't be exposed
+                # to the user and in such cases it should raise 500 error.
+                raise
 
-        self.volume_api.attach(context, volume,
-                               instance_uuid, host_name, mountpoint, mode)
         return webob.Response(status_int=202)
 
     @wsgi.action('os-detach')

diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py
index b7a5225188c89548c205e5759a75772f78ff9bf1..00792dc5c3dd53572f6e143e24f18a0274c68499 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_volume_actions.py
+++ b/cinder/tests/unit/api/contrib/test_volume_actions.py
@@ -44,6 +44,7 @@ class VolumeActionsTest(test.TestCase):
     def setUp(self):
         super(VolumeActionsTest, self).setUp()
         self.UUID = uuid.uuid4()
+        self.controller = volume_actions.VolumeActionsController()
         self.api_patchers = {}
         for _meth in self._methods:
             self.api_patchers[_meth] = mock.patch('cinder.volume.API.' + _meth)
@@ -179,6 +180,41 @@ class VolumeActionsTest(test.TestCase):
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 202)
 
+    def test_volume_attach_to_instance_raises_remote_error(self):
+        volume_remote_error = \
+            messaging.RemoteError(exc_type='InvalidUUID')
+        with mock.patch.object(volume_api.API, 'attach',
+                               side_effect=volume_remote_error):
+            id = 1
+            vol = {"instance_uuid": self.UUID,
+                   "mountpoint": "/dev/vdc",
+                   "mode": "rw"}
+            body = {"os-attach": vol}
+            req = fakes.HTTPRequest.blank('/v2/tenant1/volumes/%s/action' % id)
+            self.assertRaises(webob.exc.HTTPBadRequest,
+                              self.controller._attach,
+                              req,
+                              id,
+                              body)
+
+    def test_volume_attach_to_instance_raises_db_error(self):
+        # In case of DB error 500 error code is returned to user
+        volume_remote_error = \
+            messaging.RemoteError(exc_type='DBError')
+        with mock.patch.object(volume_api.API, 'attach',
+                               side_effect=volume_remote_error):
+            id = 1
+            vol = {"instance_uuid": self.UUID,
+                   "mountpoint": "/dev/vdc",
+                   "mode": "rw"}
+            body = {"os-attach": vol}
+            req = fakes.HTTPRequest.blank('/v2/tenant1/volumes/%s/action' % id)
+            self.assertRaises(messaging.RemoteError,
+                              self.controller._attach,
+                              req,
+                              id,
+                              body)
+
     def test_detach(self):
         body = {'os-detach': {'attachment_id': 'fakeuuid'}}
         req = webob.Request.blank('/v2/fake/volumes/1/action')