]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Add admin only action for force detach
authorclayg <clay.gerrard@gmail.com>
Thu, 25 Oct 2012 09:55:13 +0000 (02:55 -0700)
committerClay Gerrard <clay.gerrard@gmail.com>
Wed, 7 Nov 2012 18:09:48 +0000 (12:09 -0600)
This action calls the same methods nova would after it successfully
detaches a volume.  By exposing it to the administrator it makes it's
easier to repair un-syncronized state between services.  Generally when
the host is no longer attached, but the volume state is wrong.

Future work: The Iscsi based drivers don't seem to use
initialize_connection and terminate_connection to create the export for
the volume.  This would be more useful with drivers that do that.

I added the force parameter to terminate_connection for drivers that may
want to differintiate between a normal terminate and the force detach.

Future Nova work:  Nova will want an admin action to update the bdm
tables - today it's a bit of nova-manage shell work.

Change-Id: Icc1cff0f50a5ace9ebdae62c85524ee8d6ec23e0

19 files changed:
cinder/api/openstack/volume/contrib/admin_actions.py
cinder/tests/api/openstack/volume/contrib/test_admin_actions.py
cinder/tests/fake_driver.py
cinder/tests/fake_flags.py
cinder/tests/policy.json
cinder/volume/api.py
cinder/volume/driver.py
cinder/volume/drivers/rbd.py
cinder/volume/drivers/sheepdog.py
cinder/volume/iscsi.py
cinder/volume/manager.py
cinder/volume/netapp.py
cinder/volume/nfs.py
cinder/volume/san/hp_lefthand.py
cinder/volume/storwize_svc.py
cinder/volume/windows.py
cinder/volume/xensm.py
cinder/volume/xiv.py
cinder/volume/zadara.py

index 8aa6863c0350d8315631b0acc11a6bb9913a1372..c427f3e45bff31761588f4b9086d36b17af80549 100644 (file)
@@ -123,6 +123,23 @@ class VolumeAdminController(AdminController):
             update['attach_status'] = body['attach_status']
         return update
 
+    @wsgi.action('os-force_detach')
+    def _force_detach(self, req, id, body):
+        """
+        Roll back a bad detach after the volume been disconnected from
+        the hypervisor.
+        """
+        context = req.environ['cinder.context']
+        self.authorize(context, 'force_detach')
+        try:
+            volume = self._get(context, id)
+        except exception.NotFound:
+            raise exc.HTTPNotFound()
+        self.volume_api.terminate_connection(context, volume,
+                                             {}, force=True)
+        self.volume_api.detach(context, volume)
+        return webob.Response(status_int=202)
+
 
 class SnapshotAdminController(AdminController):
     """AdminController for Snapshots."""
index 91b6b73be070751aeb4c981853d8a61b6363be58..e8fcc688de0be7618070ca4c4ddc24e0766edd93 100644 (file)
@@ -4,6 +4,7 @@ from cinder import context
 from cinder import db
 from cinder import exception
 from cinder import test
+from cinder.volume import api as volume_api
 from cinder.openstack.common import jsonutils
 from cinder.tests.api.openstack import fakes
 
@@ -21,6 +22,7 @@ class AdminActionsTest(test.TestCase):
     def setUp(self):
         super(AdminActionsTest, self).setUp()
         self.flags(rpc_backend='cinder.openstack.common.rpc.impl_fake')
+        self.volume_api = volume_api.API()
 
     def test_reset_status_as_admin(self):
         # admin context
@@ -250,3 +252,40 @@ class AdminActionsTest(test.TestCase):
         # snapshot is deleted
         self.assertRaises(exception.NotFound, db.snapshot_get, ctx,
                           snapshot['id'])
+
+    def test_force_detach_volume(self):
+        # admin context
+        ctx = context.RequestContext('admin', 'fake', True)
+        # current status is available
+        volume = db.volume_create(ctx, {'status': 'available', 'host': 'test',
+                                        'provider_location': ''})
+        # start service to handle rpc messages for attach requests
+        self.start_service('volume', host='test')
+        self.volume_api.reserve_volume(ctx, volume)
+        self.volume_api.initialize_connection(ctx, volume, {})
+        mountpoint = '/dev/vbd'
+        self.volume_api.attach(ctx, volume, fakes.FAKE_UUID, mountpoint)
+        # volume is attached
+        volume = db.volume_get(ctx, volume['id'])
+        self.assertEquals(volume['status'], 'in-use')
+        self.assertEquals(volume['instance_uuid'], fakes.FAKE_UUID)
+        self.assertEquals(volume['mountpoint'], mountpoint)
+        self.assertEquals(volume['attach_status'], 'attached')
+        # build request to force detach
+        req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id'])
+        req.method = 'POST'
+        req.headers['content-type'] = 'application/json'
+        # request status of 'error'
+        req.body = jsonutils.dumps({'os-force_detach': None})
+        # attach admin context to request
+        req.environ['cinder.context'] = ctx
+        # make request
+        resp = req.get_response(app())
+        # request is accepted
+        self.assertEquals(resp.status_int, 202)
+        volume = db.volume_get(ctx, volume['id'])
+        # status changed to 'available'
+        self.assertEquals(volume['status'], 'available')
+        self.assertEquals(volume['instance_uuid'], None)
+        self.assertEquals(volume['mountpoint'], None)
+        self.assertEquals(volume['attach_status'], 'detached')
index 261bfafb4bfa3cb1d27672dd27533045bfe68889..3fe19f3b3d1331373c87741c8fd8424c58eb8293 100644 (file)
@@ -35,7 +35,7 @@ class FakeISCSIDriver(driver.ISCSIDriver):
             'data': {}
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         pass
 
     @staticmethod
index eb245073d9c197f3096038b783a9a5dfc788982d..0f4aba7f9e9afa7a2542ece0b860ac3b43526807 100644 (file)
@@ -32,6 +32,7 @@ def set_defaults(conf):
     conf.set_default('default_volume_type', def_vol_type)
     conf.set_default('volume_driver',
                      'cinder.tests.fake_driver.FakeISCSIDriver')
+    conf.set_default('iscsi_helper', 'fake')
     conf.set_default('connection_type', 'fake')
     conf.set_default('fake_rabbit', True)
     conf.set_default('rpc_backend', 'cinder.openstack.common.rpc.impl_fake')
index 3ee529c92e06d8c693571f4662a5c2ffca84950b..e1256582296d556f90c8176b10eca887e1bc4006 100644 (file)
@@ -30,6 +30,7 @@
     "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]],
     "volume_extension:volume_admin_actions:force_delete": [["rule:admin_api"]],
     "volume_extension:snapshot_admin_actions:force_delete": [["rule:admin_api"]],
+    "volume_extension:volume_admin_actions:force_detach": [["rule:admin_api"]],
     "volume_extension:volume_actions:upload_image": [],
     "volume_extension:types_manage": [],
     "volume_extension:types_extra_specs": [],
index 49400b73d3c88d68f1d1ee5093540d0ae3fe354a..3e281390f007946abed80742c4f1372398d7a0b7 100644 (file)
@@ -407,14 +407,14 @@ class API(base.Base):
                                   "connector": connector}})
 
     @wrap_check_policy
-    def terminate_connection(self, context, volume, connector):
+    def terminate_connection(self, context, volume, connector, force=False):
         self.unreserve_volume(context, volume)
         host = volume['host']
         queue = rpc.queue_get_for(context, FLAGS.volume_topic, host)
         return rpc.call(context, queue,
                         {"method": "terminate_connection",
                          "args": {"volume_id": volume['id'],
-                                  "connector": connector}})
+                                  "connector": connector, 'force': force}})
 
     def _create_snapshot(self, context, volume, name, description,
                          force=False):
index 4eb45eaddd7f39e31df817dedfc24159efa9b4bb..98ef8f0e1da5b8b96736dd332d8850eeccade77a 100644 (file)
@@ -226,7 +226,7 @@ class VolumeDriver(object):
         """Allow connection to connector and return connection info."""
         raise NotImplementedError()
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, force=False, **kwargs):
         """Disallow connection from connector"""
         raise NotImplementedError()
 
@@ -580,7 +580,7 @@ class ISCSIDriver(VolumeDriver):
             'data': iscsi_properties
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         pass
 
     def copy_image_to_volume(self, context, volume, image_service, image_id):
@@ -598,6 +598,32 @@ class ISCSIDriver(VolumeDriver):
                 image_service.update(context, image_id, {}, volume_file)
 
 
+class FakeISCSIDriver(ISCSIDriver):
+    """Logs calls instead of executing."""
+    def __init__(self, *args, **kwargs):
+        super(FakeISCSIDriver, self).__init__(execute=self.fake_execute,
+                                              *args, **kwargs)
+
+    def check_for_setup_error(self):
+        """No setup necessary in fake mode."""
+        pass
+
+    def initialize_connection(self, volume, connector):
+        return {
+            'driver_volume_type': 'iscsi',
+            'data': {}
+        }
+
+    def terminate_connection(self, volume, connector, **kwargs):
+        pass
+
+    @staticmethod
+    def fake_execute(cmd, *_args, **_kwargs):
+        """Execute that simply logs the command."""
+        LOG.debug(_("FAKE ISCSI: %s"), cmd)
+        return (None, None)
+
+
 def _iscsi_location(ip, target, iqn, lun=None):
     return "%s:%s,%s %s %s" % (ip, FLAGS.iscsi_port, target, iqn, lun)
 
index 3e02dfc4cca657929465bd5a75763c2feccceb48..60be2eba23b461a550a913058cc889db7adda02c 100644 (file)
@@ -169,7 +169,7 @@ class RBDDriver(driver.VolumeDriver):
                 }
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         pass
 
     def _parse_location(self, location):
index b256e975b52be1c913239935324fa0e6803c8f63..3b615b6e78058a3f637ecbb7ccd3af2f77404ab8 100644 (file)
@@ -96,5 +96,5 @@ class SheepdogDriver(driver.VolumeDriver):
             }
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         pass
index cc7d520639b83588f1a630d2889c0d6e163a03f4..925921c4864045d5345ed5dba0d89a12580f0084 100644 (file)
@@ -255,8 +255,23 @@ class IetAdm(TargetAdmin):
                   **kwargs)
 
 
+class FakeIscsiHelper(object):
+
+    def __init__(self):
+        self.tid = 1
+
+    def set_execute(self, execute):
+        self._execute = execute
+
+    def create_iscsi_target(self, *args, **kwargs):
+        self.tid += 1
+        return self.tid
+
+
 def get_target_admin():
     if FLAGS.iscsi_helper == 'tgtadm':
         return TgtAdm()
+    elif FLAGS.iscsi_helper == 'fake':
+        return FakeIscsiHelper()
     else:
         return IetAdm()
index 06ac7187a54f7452527488381c8256f81a319aa5..7b4e2b8e289a6b7045d8cd32d2a7085ec4827056 100644 (file)
@@ -405,13 +405,13 @@ class VolumeManager(manager.SchedulerDependentManager):
         volume_ref = self.db.volume_get(context, volume_id)
         return self.driver.initialize_connection(volume_ref, connector)
 
-    def terminate_connection(self, context, volume_id, connector):
+    def terminate_connection(self, context, volume_id, connector, force=False):
         """Cleanup connection from host represented by connector.
 
         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)
+        self.driver.terminate_connection(volume_ref, connector, force=force)
 
     def _volume_stats_changed(self, stat1, stat2):
         if FLAGS.volume_force_update_capabilities:
index c6f1fbf1d0ff82b1717afc8cd5c1dc92a802c171..9d1cc47517fa11d47f8bc019d69f2014f2a02141 100644 (file)
@@ -802,7 +802,7 @@ class NetAppISCSIDriver(driver.ISCSIDriver):
             'data': properties,
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Driver entry point to unattach a volume from an instance.
 
         Unmask the LUN on the storage system so the given intiator can no
@@ -1181,7 +1181,7 @@ class NetAppCmodeISCSIDriver(driver.ISCSIDriver):
             'data': properties,
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Driver entry point to unattach a volume from an instance.
 
         Unmask the LUN on the storage system so the given intiator can no
index 2d83ba6eed04f2f4d7cdfd89014737984210fec4..deec01672440de93ee8b29efed72fa7cb4f85ec6 100644 (file)
@@ -132,7 +132,7 @@ class NfsDriver(driver.VolumeDriver):
             'data': data
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Disallow connection from connector"""
         pass
 
index e2ec46ccec9c2e0ee5ce74ffcb6eb5106e8e200f..bb452eb14ff988d4213776dc2b30c42da0d0402f 100644 (file)
@@ -260,7 +260,7 @@ class HpSanISCSIDriver(SanISCSIDriver):
             'data': iscsi_properties
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Unassign the volume from the host."""
         cliq_args = {}
         cliq_args['volumeName'] = volume['name']
index 5054197f18b4a810648bdb869db4db01348cf2c4..c5df7f0af360130cd77f07bf6bb1dea7bbe1b047 100644 (file)
@@ -569,7 +569,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
 
         return {'driver_volume_type': 'iscsi', 'data': properties, }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Cleanup after an iSCSI connection has been terminated.
 
         When we clean up a terminated connection between a given iSCSI name
index 28dbdbcdc8fd928cf64649df19717757813cc254..970ac7ccae05e96a8983d8cc7c136bfbc50b8718 100644 (file)
@@ -111,7 +111,7 @@ class WindowsDriver(driver.ISCSIDriver):
             'data': properties,
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Driver entry point to unattach a volume from an instance.
 
         Unmask the LUN on the storage system so the given intiator can no
index b7f324f1c42e39bb9e4f5ec97bb8dbb31dcb31ea..4068ca3121fa2e49419faf6b5baebe462e70375c 100644 (file)
@@ -243,5 +243,5 @@ class XenSMDriver(cinder.volume.driver.VolumeDriver):
             'data': xensm_properties
         }
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         pass
index 553d32a5b3e58add048f337460d3fae1f720a63c..f4066bb9b594e2aa6620442142a131f2dd281852 100644 (file)
@@ -98,7 +98,7 @@ class XIVDriver(san.SanISCSIDriver):
                 volume,
                 connector)
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """Terminate a connection to a volume."""
 
         return self.xiv_proxy.terminate_connection(
index 03f34ca548416d7930ae4b6681ae28ed324d7d43..053b7b5e8946c980f69b247ca6c321262f450230 100644 (file)
@@ -449,7 +449,7 @@ class ZadaraVPSAISCSIDriver(driver.ISCSIDriver):
         return {'driver_volume_type': 'iscsi',
                 'data': properties}
 
-    def terminate_connection(self, volume, connector):
+    def terminate_connection(self, volume, connector, **kwargs):
         """
         Detach volume from the initiator.
         """