From c917f21994b9b80abf42823fc24d1c131ea12de4 Mon Sep 17 00:00:00 2001 From: PranaliDeore Date: Wed, 29 Apr 2015 05:31:04 -0700 Subject: [PATCH] Cinder os-force_detach api returns 500 If invalid 'attachment_id' is passed to the os-force_detach api, then it returns 500 error because even though VolumeAttachmentNotFound exception is raised from manager, it gets converted to RemoteError due to rpcapi call. Caught RemoteError exception in the api and returned 400 error to the user if exception type is VolumeAttachmentNotFound or InvalidVolume. In other cases 500 error is raised. Closes-Bug: 1450431 Change-Id: I5e308d9f4b3772284caaedb3d2d0b3f7d25e1e5d --- cinder/api/contrib/admin_actions.py | 17 +++- .../unit/api/contrib/test_admin_actions.py | 99 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/cinder/api/contrib/admin_actions.py b/cinder/api/contrib/admin_actions.py index cc5852474..5c51646d7 100644 --- a/cinder/api/contrib/admin_actions.py +++ b/cinder/api/contrib/admin_actions.py @@ -13,6 +13,7 @@ # under the License. from oslo_log import log as logging +import oslo_messaging as messaging from oslo_utils import strutils import webob from webob import exc @@ -188,7 +189,21 @@ class VolumeAdminController(AdminController): attachment_id = body['os-force_detach'].get('attachment_id', None) - self.volume_api.detach(context, volume, attachment_id) + try: + self.volume_api.detach(context, volume, attachment_id) + except messaging.RemoteError as error: + if error.exc_type in ['VolumeAttachmentNotFound', + 'InvalidVolume']: + msg = "Error force detaching 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 force-detach 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 return webob.Response(status_int=202) @wsgi.action('os-migrate_volume') diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py index a1dff4da4..71aece524 100644 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py @@ -17,6 +17,7 @@ import mock from oslo_concurrency import lockutils from oslo_config import cfg from oslo_config import fixture as config_fixture +import oslo_messaging as messaging from oslo_serialization import jsonutils from oslo_utils import timeutils import webob @@ -492,6 +493,104 @@ class AdminActionsTest(test.TestCase): # cleanup svc.stop() + def test_volume_force_detach_raises_remote_error(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', + 'provider_location': '', 'size': 1}) + connector = {'initiator': 'iqn.2012-07.org.fake:01'} + # start service to handle rpc messages for attach requests + svc = self.start_service('volume', host='test') + self.volume_api.reserve_volume(ctx, volume) + mountpoint = '/dev/vbd' + attachment = self.volume_api.attach(ctx, volume, stubs.FAKE_UUID, + None, mountpoint, 'rw') + # volume is attached + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['status'], 'in-use') + self.assertEqual(attachment['instance_uuid'], stubs.FAKE_UUID) + self.assertEqual(attachment['mountpoint'], mountpoint) + self.assertEqual(attachment['attach_status'], 'attached') + admin_metadata = volume['volume_admin_metadata'] + self.assertEqual(len(admin_metadata), 2) + self.assertEqual(admin_metadata[0]['key'], 'readonly') + self.assertEqual(admin_metadata[0]['value'], 'False') + self.assertEqual(admin_metadata[1]['key'], 'attached_mode') + self.assertEqual(admin_metadata[1]['value'], 'rw') + conn_info = self.volume_api.initialize_connection(ctx, + volume, + connector) + self.assertEqual(conn_info['data']['access_mode'], 'rw') + # build request to force detach + volume_remote_error = \ + messaging.RemoteError(exc_type='VolumeAttachmentNotFound') + with mock.patch.object(volume_api.API, 'detach', + side_effect=volume_remote_error): + req = webob.Request.blank('/v2/fake/volumes/%s/action' % + volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_detach': + {'attachment_id': 'fake'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + # make request + resp = req.get_response(app()) + self.assertEqual(resp.status_int, 400) + # cleanup + svc.stop() + + def test_volume_force_detach_raises_db_error(self): + # In case of DB error 500 error code is returned to user + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', + 'provider_location': '', 'size': 1}) + connector = {'initiator': 'iqn.2012-07.org.fake:01'} + # start service to handle rpc messages for attach requests + svc = self.start_service('volume', host='test') + self.volume_api.reserve_volume(ctx, volume) + mountpoint = '/dev/vbd' + attachment = self.volume_api.attach(ctx, volume, stubs.FAKE_UUID, + None, mountpoint, 'rw') + # volume is attached + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['status'], 'in-use') + self.assertEqual(attachment['instance_uuid'], stubs.FAKE_UUID) + self.assertEqual(attachment['mountpoint'], mountpoint) + self.assertEqual(attachment['attach_status'], 'attached') + admin_metadata = volume['volume_admin_metadata'] + self.assertEqual(len(admin_metadata), 2) + self.assertEqual(admin_metadata[0]['key'], 'readonly') + self.assertEqual(admin_metadata[0]['value'], 'False') + self.assertEqual(admin_metadata[1]['key'], 'attached_mode') + self.assertEqual(admin_metadata[1]['value'], 'rw') + conn_info = self.volume_api.initialize_connection(ctx, + volume, + connector) + self.assertEqual(conn_info['data']['access_mode'], 'rw') + # build request to force detach + volume_remote_error = \ + messaging.RemoteError(exc_type='DBError') + with mock.patch.object(volume_api.API, 'detach', + side_effect=volume_remote_error): + req = webob.Request.blank('/v2/fake/volumes/%s/action' % + volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_detach': + {'attachment_id': 'fake'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + # make request + self.assertRaises(messaging.RemoteError, + req.get_response, + app()) + # cleanup + svc.stop() + def test_attach_in_used_volume_by_instance(self): """Test that attaching to an in-use volume fails.""" # admin context -- 2.45.2