From 1e73e64e7407294dc68af9c09323e2b396335c4a Mon Sep 17 00:00:00 2001 From: ling-yun Date: Tue, 24 Dec 2013 11:45:16 +0800 Subject: [PATCH] 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 --- cinder/api/contrib/volume_actions.py | 6 +- .../tests/api/contrib/test_volume_actions.py | 58 +++++++++++-------- cinder/volume/manager.py | 9 ++- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py index abea4d96fa..3b05291e2f 100644 --- a/cinder/api/contrib/volume_actions.py +++ b/cinder/api/contrib/volume_actions.py @@ -205,7 +205,11 @@ def _terminate_connection(self, req, id, body): 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 153dde9404..e13c5435f5 100644 --- 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 @@ def fake_initialize_connection(*args, **kwargs): 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 7f95a88d51..5936462fff 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -744,7 +744,14 @@ def terminate_connection(self, context, volume_id, connector, force=False): 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):