force_detach terminate_connection needs connector

The function force_detach in cinder/api/contrib/admin_actions does not
take a connector in the body of the request. When it calls terminate
connection it passes in an empty dict as the connector:
self.volume_api.terminate_connection(context, volume, {}, force=True)
This patch pulls the connector out of the body of the request, and
throws an exception if it is missing.

Change-Id: I14990eb4f42283231e4249a81d0b7fa18a8dfeae
Closes-Bug:#1485766

diff --git a/cinder/api/contrib/admin_actions.py b/cinder/api/contrib/admin_actions.py
index 46bb3fd8ac207e8d5a7f2b9d8e192a0236a212ca..38838ef3253f196291ae1ba4ef4a355a30eac23b 100644 (file)
--- a/cinder/api/contrib/admin_actions.py
+++ b/cinder/api/contrib/admin_actions.py
@@ -185,8 +185,16 @@ class VolumeAdminController(AdminController):
             volume = self._get(context, id)
         except exception.VolumeNotFound as e:
             raise exc.HTTPNotFound(explanation=e.msg)
-        self.volume_api.terminate_connection(context, volume,
-                                             {}, force=True)
+        try:
+            connector = body['os-force_detach'].get('connector', None)
+        except KeyError:
+            raise webob.exc.HTTPBadRequest(
+                explanation=_("Must specify '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)
 
         attachment_id = body['os-force_detach'].get('attachment_id', None)
 

diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py
index 688f1ea523145d8db1fecae772058f83b8ad167f..87f99f97ec6e4090506ad5f1711e1b584bf433e9 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_admin_actions.py
+++ b/cinder/tests/unit/api/contrib/test_admin_actions.py
@@ -453,7 +453,8 @@ class AdminActionsTest(test.TestCase):
         req.headers['content-type'] = 'application/json'
         # request status of 'error'
         req.body = jsonutils.dumps({'os-force_detach':
-                                    {'attachment_id': attachment['id']}})
+                                   {'attachment_id': attachment['id'],
+                                    'connector': connector}})
         # attach admin context to request
         req.environ['cinder.context'] = ctx
         # make request
@@ -510,7 +511,8 @@ class AdminActionsTest(test.TestCase):
         req.headers['content-type'] = 'application/json'
         # request status of 'error'
         req.body = jsonutils.dumps({'os-force_detach':
-                                    {'attachment_id': attachment['id']}})
+                                    {'attachment_id': attachment['id'],
+                                     'connector': connector}})
         # attach admin context to request
         req.environ['cinder.context'] = ctx
         # make request
@@ -575,6 +577,44 @@ class AdminActionsTest(test.TestCase):
             # make request
             resp = req.get_response(app())
             self.assertEqual(400, resp.status_int)
+
+        # test for KeyError when missing connector
+        volume_remote_error = (
+            messaging.RemoteError(exc_type='KeyError'))
+        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())
+
+        # test for VolumeBackendAPIException
+        volume_remote_error = (
+            messaging.RemoteError(exc_type='VolumeBackendAPIException'))
+        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',
+                                        'connector': connector}})
+
+            # attach admin context to request
+            req.environ['cinder.context'] = ctx
+            # make request
+            self.assertRaises(messaging.RemoteError,
+                              req.get_response,
+                              app())
         # cleanup
         svc.stop()
 
@@ -618,7 +658,8 @@ class AdminActionsTest(test.TestCase):
             req.method = 'POST'
             req.headers['content-type'] = 'application/json'
             req.body = jsonutils.dumps({'os-force_detach':
-                                        {'attachment_id': 'fake'}})
+                                       {'attachment_id': 'fake',
+                                        'connector': connector}})
             # attach admin context to request
             req.environ['cinder.context'] = ctx
             # make request