From: PranaliDeore <pranali11.deore@nttdata.com>
Date: Mon, 27 Apr 2015 11:52:46 +0000 (-0700)
Subject: Cinder os-attach api returns 500
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=42d4d1cc7fe8b2f5eb797905baf3a82220c1d711;p=openstack-build%2Fcinder-build.git

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 423cf17a2..6f13d2640 100644
--- 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 b7a522518..00792dc5c 100644
--- 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')