Handle terminate_connection() exception in volume manager

Due to the fact that we sometimes need to manually terminate a volume's
connection through volume api, it's possile that these backend drivers
throw exceptions while doing that. Currently exceptions are bubbled up to
volume API not being handled. This patch logs exception in volume manager
and then raises VolumeBackendAPIException to caller.

Change-Id: If809f97998f52516af09ec21b3052b67d3a62f36
Closes-bug: #1263820

diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py
index abea4d96faa4305cab7f549a251a029f95e8056c..3b05291e2fafd88c5af54be8f0f2c04f977952da 100644 (file)
--- a/cinder/api/contrib/volume_actions.py
+++ b/cinder/api/contrib/volume_actions.py
@@ -205,7 +205,11 @@ class VolumeActionsController(wsgi.Controller):
             connector = body['os-terminate_connection']['connector']
         except KeyError:
             raise webob.exc.HTTPBadRequest("Must specify 'connector'")
-        self.volume_api.terminate_connection(context, volume, connector)
+        try:
+            self.volume_api.terminate_connection(context, volume, connector)
+        except exception.VolumeBackendAPIException as error:
+            msg = _("Unable to terminate volume connection from backend.")
+            raise webob.exc.HTTPInternalServerError(explanation=msg)
         return webob.Response(status_int=202)
 
     @wsgi.response(202)

diff --git a/cinder/tests/api/contrib/test_volume_actions.py b/cinder/tests/api/contrib/test_volume_actions.py
index 153dde9404b12325e0634f5f6a5351d665f1b8ea..e13c5435f5f86c3b00306480ab5ef9b39d1ebb2a 100644 (file)
--- a/cinder/tests/api/contrib/test_volume_actions.py
+++ b/cinder/tests/api/contrib/test_volume_actions.py
@@ -17,6 +17,8 @@
 import datetime
 import json
 import uuid
+
+import mock
 import webob
 
 from cinder.api.contrib import volume_actions
@@ -95,34 +97,44 @@ class VolumeActionsTest(test.TestCase):
         self.assertEqual(res.status_int, 400)
 
     def test_terminate_connection(self):
-        def fake_terminate_connection(*args, **kwargs):
-            return {}
-        self.stubs.Set(volume.API, 'terminate_connection',
-                       fake_terminate_connection)
-
-        body = {'os-terminate_connection': {'connector': 'fake'}}
-        req = webob.Request.blank('/v2/fake/volumes/1/action')
-        req.method = "POST"
-        req.body = jsonutils.dumps(body)
-        req.headers["content-type"] = "application/json"
+        with mock.patch.object(volume_api.API,
+                               'terminate_connection') as terminate_conn:
+            terminate_conn.return_value = {}
+            body = {'os-terminate_connection': {'connector': 'fake'}}
+            req = webob.Request.blank('/v2/fake/volumes/1/action')
+            req.method = "POST"
+            req.body = jsonutils.dumps(body)
+            req.headers["content-type"] = "application/json"
 
-        res = req.get_response(fakes.wsgi_app())
-        self.assertEqual(res.status_int, 202)
+            res = req.get_response(fakes.wsgi_app())
+            self.assertEqual(res.status_int, 202)
 
     def test_terminate_connection_without_connector(self):
-        def fake_terminate_connection(*args, **kwargs):
-            return {}
-        self.stubs.Set(volume.API, 'terminate_connection',
-                       fake_terminate_connection)
+        with mock.patch.object(volume_api.API,
+                               'terminate_connection') as terminate_conn:
+            terminate_conn.return_value = {}
+            body = {'os-terminate_connection': {}}
+            req = webob.Request.blank('/v2/fake/volumes/1/action')
+            req.method = "POST"
+            req.body = jsonutils.dumps(body)
+            req.headers["content-type"] = "application/json"
 
-        body = {'os-terminate_connection': {}}
-        req = webob.Request.blank('/v2/fake/volumes/1/action')
-        req.method = "POST"
-        req.body = jsonutils.dumps(body)
-        req.headers["content-type"] = "application/json"
+            res = req.get_response(fakes.wsgi_app())
+            self.assertEqual(res.status_int, 400)
+
+    def test_terminate_connection_with_exception(self):
+        with mock.patch.object(volume_api.API,
+                               'terminate_connection') as terminate_conn:
+            terminate_conn.side_effect = \
+                exception.VolumeBackendAPIException(data=None)
+            body = {'os-terminate_connection': {'connector': 'fake'}}
+            req = webob.Request.blank('/v2/fake/volumes/1/action')
+            req.method = "POST"
+            req.body = jsonutils.dumps(body)
+            req.headers["content-type"] = "application/json"
 
-        res = req.get_response(fakes.wsgi_app())
-        self.assertEqual(res.status_int, 400)
+            res = req.get_response(fakes.wsgi_app())
+            self.assertEqual(res.status_int, 500)
 
     def test_attach_to_instance(self):
         body = {'os-attach': {'instance_uuid': 'fake',

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 7f95a88d51a7074d211508e4f5930d9a6e0eba53..5936462fffd7bfa49689c655de9e805926079550 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -744,7 +744,14 @@ class VolumeManager(manager.SchedulerDependentManager):
         The format of connector is the same as for initialize_connection.
         """
         volume_ref = self.db.volume_get(context, volume_id)
-        self.driver.terminate_connection(volume_ref, connector, force=force)
+        try:
+            self.driver.terminate_connection(volume_ref,
+                                             connector, force=force)
+        except Exception as err:
+            err_msg = (_('Unable to terminate volume connection: %(err)s')
+                       % {'err': str(err)})
+            LOG.error(err_msg)
+            raise exception.VolumeBackendAPIException(data=err_msg)
 
     @utils.require_driver_initialized
     def accept_transfer(self, context, volume_id, new_user, new_project):