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 abea4d96f..3b05291e2 100644 --- 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 153dde940..e13c5435f 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 @@ 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 7f95a88d5..5936462ff 100644 --- 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): -- 2.45.2