From 956731973ee872d38eebacfb519ed62d6767a319 Mon Sep 17 00:00:00 2001 From: clayg Date: Thu, 25 Oct 2012 02:55:13 -0700 Subject: [PATCH] Add admin only action for force detach 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 --- .../openstack/volume/contrib/admin_actions.py | 17 ++++++++ .../volume/contrib/test_admin_actions.py | 39 +++++++++++++++++++ cinder/tests/fake_driver.py | 2 +- cinder/tests/fake_flags.py | 1 + cinder/tests/policy.json | 1 + cinder/volume/api.py | 4 +- cinder/volume/driver.py | 30 +++++++++++++- cinder/volume/drivers/rbd.py | 2 +- cinder/volume/drivers/sheepdog.py | 2 +- cinder/volume/iscsi.py | 15 +++++++ cinder/volume/manager.py | 4 +- cinder/volume/netapp.py | 4 +- cinder/volume/nfs.py | 2 +- cinder/volume/san/hp_lefthand.py | 2 +- cinder/volume/storwize_svc.py | 2 +- cinder/volume/windows.py | 2 +- cinder/volume/xensm.py | 2 +- cinder/volume/xiv.py | 2 +- cinder/volume/zadara.py | 2 +- 19 files changed, 117 insertions(+), 18 deletions(-) diff --git a/cinder/api/openstack/volume/contrib/admin_actions.py b/cinder/api/openstack/volume/contrib/admin_actions.py index 8aa6863c0..c427f3e45 100644 --- a/cinder/api/openstack/volume/contrib/admin_actions.py +++ b/cinder/api/openstack/volume/contrib/admin_actions.py @@ -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.""" diff --git a/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py b/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py index 91b6b73be..e8fcc688d 100644 --- a/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py +++ b/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py @@ -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') diff --git a/cinder/tests/fake_driver.py b/cinder/tests/fake_driver.py index 261bfafb4..3fe19f3b3 100644 --- a/cinder/tests/fake_driver.py +++ b/cinder/tests/fake_driver.py @@ -35,7 +35,7 @@ class FakeISCSIDriver(driver.ISCSIDriver): 'data': {} } - def terminate_connection(self, volume, connector): + def terminate_connection(self, volume, connector, **kwargs): pass @staticmethod diff --git a/cinder/tests/fake_flags.py b/cinder/tests/fake_flags.py index eb245073d..0f4aba7f9 100644 --- a/cinder/tests/fake_flags.py +++ b/cinder/tests/fake_flags.py @@ -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') diff --git a/cinder/tests/policy.json b/cinder/tests/policy.json index 3ee529c92..e12565822 100644 --- a/cinder/tests/policy.json +++ b/cinder/tests/policy.json @@ -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": [], diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 49400b73d..3e281390f 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -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): diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 4eb45eadd..98ef8f0e1 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -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) diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 3e02dfc4c..60be2eba2 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -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): diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index b256e975b..3b615b6e7 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -96,5 +96,5 @@ class SheepdogDriver(driver.VolumeDriver): } } - def terminate_connection(self, volume, connector): + def terminate_connection(self, volume, connector, **kwargs): pass diff --git a/cinder/volume/iscsi.py b/cinder/volume/iscsi.py index cc7d52063..925921c48 100644 --- a/cinder/volume/iscsi.py +++ b/cinder/volume/iscsi.py @@ -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() diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 06ac7187a..7b4e2b8e2 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -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: diff --git a/cinder/volume/netapp.py b/cinder/volume/netapp.py index c6f1fbf1d..9d1cc4751 100644 --- a/cinder/volume/netapp.py +++ b/cinder/volume/netapp.py @@ -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 diff --git a/cinder/volume/nfs.py b/cinder/volume/nfs.py index 2d83ba6ee..deec01672 100644 --- a/cinder/volume/nfs.py +++ b/cinder/volume/nfs.py @@ -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 diff --git a/cinder/volume/san/hp_lefthand.py b/cinder/volume/san/hp_lefthand.py index e2ec46cce..bb452eb14 100644 --- a/cinder/volume/san/hp_lefthand.py +++ b/cinder/volume/san/hp_lefthand.py @@ -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'] diff --git a/cinder/volume/storwize_svc.py b/cinder/volume/storwize_svc.py index 5054197f1..c5df7f0af 100644 --- a/cinder/volume/storwize_svc.py +++ b/cinder/volume/storwize_svc.py @@ -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 diff --git a/cinder/volume/windows.py b/cinder/volume/windows.py index 28dbdbcdc..970ac7cca 100644 --- a/cinder/volume/windows.py +++ b/cinder/volume/windows.py @@ -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 diff --git a/cinder/volume/xensm.py b/cinder/volume/xensm.py index b7f324f1c..4068ca312 100644 --- a/cinder/volume/xensm.py +++ b/cinder/volume/xensm.py @@ -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 diff --git a/cinder/volume/xiv.py b/cinder/volume/xiv.py index 553d32a5b..f4066bb9b 100644 --- a/cinder/volume/xiv.py +++ b/cinder/volume/xiv.py @@ -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( diff --git a/cinder/volume/zadara.py b/cinder/volume/zadara.py index 03f34ca54..053b7b5e8 100644 --- a/cinder/volume/zadara.py +++ b/cinder/volume/zadara.py @@ -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. """ -- 2.45.2