From: Clay Gerrard Date: Mon, 27 Aug 2012 07:48:32 +0000 (+0000) Subject: Add admin actions extension X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c191d0d10c6cb5730a4fb6540198ca3be6595b02;p=openstack-build%2Fcinder-build.git Add admin actions extension The optional os-admin-actions extension adds new wsgi_actions to the volumes/action resource and a new snapshots/action endpoint. With this extension both controllers will support an os-reset_status action to force a database update of a volume or snapshot that is stuck in a failed/incorrect status. The os-reset_status action works similarly to the compute api's os-reset_state action for instances. The os-force_delete action behaves similarly to the "cinder-manage volume delete" command and allows operators/admins to retry the delete operation after it has gone into an error_deleting status with an admin api call. The os-admin-actions extension is enabled by default, but limited to the admin api by the default policy.json rules. Individual admin actions can be disabled with policy rules as well. Example of os-reset_status action on a volume: curl http://localhost:8776/v1/${PROJECT_ID}/volumes/${VOLUME_ID}/action \ -H 'x-auth-token: ${ADMIN_AUTH_TOKEN}' \ -H 'content-type: application/json' \ -d '{"os-reset_status": {"status": "error"}}' The new admin only api can assist deployers who encounter bugs or operational issues that result in failed actions. It can also be used by future storage backends to support async callback style status updates from long running actions or operations which have encountered an error will be retried. Also updates the api.openstack.wsgi.ControllerMetaclass to support sub-classing wsgi.Controllers that define wsgi_actions. Partial fix for bug #1039706 Change-Id: I29f4b892a99108b6c24eebc3eb58033a9e01e679 --- diff --git a/cinder/api/openstack/extensions.py b/cinder/api/openstack/extensions.py index 9c397e45b..a615f2a08 100644 --- a/cinder/api/openstack/extensions.py +++ b/cinder/api/openstack/extensions.py @@ -216,11 +216,12 @@ class ExtensionManager(object): controller_exts = [] for ext in self.extensions.values(): try: - controller_exts.extend(ext.get_controller_extensions()) + get_ext_method = ext.get_controller_extensions except AttributeError: # NOTE(Vek): Extensions aren't required to have # controller extensions - pass + continue + controller_exts.extend(get_ext_method()) return controller_exts def _check_extension(self, extension): diff --git a/cinder/api/openstack/volume/__init__.py b/cinder/api/openstack/volume/__init__.py index 4bfe9938d..9a9945100 100644 --- a/cinder/api/openstack/volume/__init__.py +++ b/cinder/api/openstack/volume/__init__.py @@ -58,10 +58,11 @@ class APIRouter(cinder.api.openstack.APIRouter): mapper.resource("type", "types", controller=self.resources['types']) - self.resources['snapshots'] = snapshots.create_resource() + self.resources['snapshots'] = snapshots.create_resource(ext_mgr) mapper.resource("snapshot", "snapshots", controller=self.resources['snapshots'], - collection={'detail': 'GET'}) + collection={'detail': 'GET'}, + member={'action': 'POST'}) self.resources['limits'] = limits.create_resource() mapper.resource("limit", "limits", diff --git a/cinder/api/openstack/volume/contrib/admin_actions.py b/cinder/api/openstack/volume/contrib/admin_actions.py new file mode 100644 index 000000000..3351ecab0 --- /dev/null +++ b/cinder/api/openstack/volume/contrib/admin_actions.py @@ -0,0 +1,129 @@ +# Copyright 2012 OpenStack, LLC. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import webob +from webob import exc + +from cinder.api.openstack import extensions +from cinder.api.openstack import wsgi +from cinder import db +from cinder import exception +from cinder import volume +from cinder.openstack.common import log as logging + + +LOG = logging.getLogger(__name__) + + +class AdminController(wsgi.Controller): + """Abstract base class for AdminControllers.""" + + collection = None # api collection to extend + + # FIXME(clayg): this will be hard to keep up-to-date + # Concrete classes can expand or over-ride + valid_status = set([ + 'creating', + 'available', + 'deleting', + 'error', + 'error_deleting', + ]) + + def __init__(self, *args, **kwargs): + super(AdminController, self).__init__(*args, **kwargs) + # singular name of the resource + self.resource_name = self.collection.rstrip('s') + self.volume_api = volume.API() + + def _update(self, *args, **kwargs): + raise NotImplementedError() + + def _validate_status(self, status): + if status not in self.valid_status: + raise exc.HTTPBadRequest("Must specify a valid status") + + def authorize(self, context, action_name): + # e.g. "snapshot_admin_actions:reset_status" + action = '%s_admin_actions:%s' % (self.resource_name, action_name) + extensions.extension_authorizer('volume', action)(context) + + @wsgi.action('os-reset_status') + def _reset_status(self, req, id, body): + """Reset status on the resource.""" + context = req.environ['cinder.context'] + self.authorize(context, 'reset_status') + try: + new_status = body['os-reset_status']['status'] + except (TypeError, KeyError): + raise exc.HTTPBadRequest("Must specify 'status'") + self._validate_status(new_status) + msg = _("Updating status of %(resource)s '%(id)s' to '%(status)s'") + LOG.debug(msg, {'resource': self.resource_name, 'id': id, + 'status': new_status}) + try: + self._update(context, id, {'status': new_status}) + except exception.NotFound, e: + raise exc.HTTPNotFound(e) + return webob.Response(status_int=202) + + +class VolumeAdminController(AdminController): + """AdminController for Volumes.""" + + collection = 'volumes' + valid_status = AdminController.valid_status.union( + set(['attaching', 'in-use', 'detaching'])) + + def _update(self, *args, **kwargs): + db.volume_update(*args, **kwargs) + + @wsgi.action('os-force_delete') + def _force_delete(self, req, id, body): + """Delete a resource, bypassing the check that it must be available.""" + context = req.environ['cinder.context'] + self.authorize(context, 'force_delete') + try: + volume = self.volume_api.get(context, id) + except exception.NotFound: + raise exc.HTTPNotFound() + self.volume_api.delete(context, volume, force=True) + return webob.Response(status_int=202) + + +class SnapshotAdminController(AdminController): + """AdminController for Snapshots.""" + + collection = 'snapshots' + + def _update(self, *args, **kwargs): + db.snapshot_update(*args, **kwargs) + + +class Admin_actions(extensions.ExtensionDescriptor): + """Enable admin actions.""" + + name = "AdminActions" + alias = "os-admin-actions" + namespace = "http://docs.openstack.org/volume/ext/admin-actions/api/v1.1" + updated = "2012-08-25T00:00:00+00:00" + + def get_controller_extensions(self): + exts = [] + for class_ in (VolumeAdminController, SnapshotAdminController): + controller = class_() + extension = extensions.ControllerExtension( + self, class_.collection, controller) + exts.append(extension) + return exts diff --git a/cinder/api/openstack/volume/snapshots.py b/cinder/api/openstack/volume/snapshots.py index 9fe84a7fd..4a26e9c99 100644 --- a/cinder/api/openstack/volume/snapshots.py +++ b/cinder/api/openstack/volume/snapshots.py @@ -86,8 +86,9 @@ class SnapshotsTemplate(xmlutil.TemplateBuilder): class SnapshotsController(object): """The Volumes API controller for the OpenStack API.""" - def __init__(self): + def __init__(self, ext_mgr=None): self.volume_api = volume.API() + self.ext_mgr = ext_mgr super(SnapshotsController, self).__init__() @wsgi.serializers(xml=SnapshotTemplate) @@ -169,5 +170,5 @@ class SnapshotsController(object): return {'snapshot': retval} -def create_resource(): - return wsgi.Resource(SnapshotsController()) +def create_resource(ext_mgr): + return wsgi.Resource(SnapshotsController(ext_mgr)) diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index f420d41b3..f86341d14 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -969,6 +969,9 @@ class ControllerMetaclass(type): # Find all actions actions = {} extensions = [] + # start with wsgi actions from base classes + for base in bases: + actions.update(getattr(base, 'wsgi_actions', {})) for key, value in cls_dict.items(): if not callable(value): continue diff --git a/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py b/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py new file mode 100644 index 000000000..25e1d5192 --- /dev/null +++ b/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py @@ -0,0 +1,176 @@ +import webob + +from cinder import context +from cinder import db +from cinder import exception +from cinder import test +from cinder.openstack.common import jsonutils +from cinder.tests.api.openstack import fakes + + +def app(): + # no auth, just let environ['cinder.context'] pass through + api = fakes.volume.APIRouter() + mapper = fakes.urlmap.URLMap() + mapper['/v1'] = api + return mapper + + +class AdminActionsTest(test.TestCase): + + def test_reset_status_as_admin(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available'}) + 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-reset_status': {'status': 'error'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # request is accepted + self.assertEquals(resp.status_int, 202) + volume = db.volume_get(ctx, volume['id']) + # status changed to 'error' + self.assertEquals(volume['status'], 'error') + + def test_reset_status_as_non_admin(self): + # current status is 'error' + volume = db.volume_create(context.get_admin_context(), + {'status': 'error'}) + req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # request changing status to available + req.body = jsonutils.dumps({'os-reset_status': {'status': + 'available'}}) + # non-admin context + req.environ['cinder.context'] = context.RequestContext('fake', 'fake') + resp = req.get_response(app()) + # request is not authorized + self.assertEquals(resp.status_int, 403) + volume = db.volume_get(context.get_admin_context(), volume['id']) + # status is still 'error' + self.assertEquals(volume['status'], 'error') + + def test_malformed_reset_status_body(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available'}) + req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # malformed request body + req.body = jsonutils.dumps({'os-reset_status': {'x-status': 'bad'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # bad request + self.assertEquals(resp.status_int, 400) + volume = db.volume_get(ctx, volume['id']) + # status is still 'available' + self.assertEquals(volume['status'], 'available') + + def test_invalid_status_for_volume(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available'}) + req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # 'invalid' is not a valid status + req.body = jsonutils.dumps({'os-reset_status': {'status': 'invalid'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # bad request + self.assertEquals(resp.status_int, 400) + volume = db.volume_get(ctx, volume['id']) + # status is still 'available' + self.assertEquals(volume['status'], 'available') + + def test_reset_status_for_missing_volume(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # missing-volume-id + req = webob.Request.blank('/v1/fake/volumes/%s/action' % + 'missing-volume-id') + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # malformed request body + req.body = jsonutils.dumps({'os-reset_status': {'status': + 'available'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # not found + self.assertEquals(resp.status_int, 404) + self.assertRaises(exception.NotFound, db.volume_get, ctx, + 'missing-volume-id') + + def test_snapshot_reset_status(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # snapshot in 'error_deleting' + volume = db.volume_create(ctx, {}) + snapshot = db.snapshot_create(ctx, {'status': 'error_deleting', + 'volume_id': volume['id']}) + req = webob.Request.blank('/v1/fake/snapshots/%s/action' % + snapshot['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # request status of 'error' + req.body = jsonutils.dumps({'os-reset_status': {'status': 'error'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # request is accepted + self.assertEquals(resp.status_int, 202) + snapshot = db.snapshot_get(ctx, snapshot['id']) + # status changed to 'error' + self.assertEquals(snapshot['status'], 'error') + + def test_invalid_status_for_snapshot(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # snapshot in 'available' + volume = db.volume_create(ctx, {}) + snapshot = db.snapshot_create(ctx, {'status': 'available', + 'volume_id': volume['id']}) + req = webob.Request.blank('/v1/fake/snapshots/%s/action' % + snapshot['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # 'attaching' is not a valid status for snapshots + req.body = jsonutils.dumps({'os-reset_status': {'status': + 'attaching'}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # request is accepted + self.assertEquals(resp.status_int, 400) + snapshot = db.snapshot_get(ctx, snapshot['id']) + # status is still 'available' + self.assertEquals(snapshot['status'], 'available') + + def test_force_delete(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is creating + volume = db.volume_create(ctx, {'status': 'creating'}) + req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = jsonutils.dumps({'os-force_delete': {}}) + # attach admin context to request + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + # request is accepted + self.assertEquals(resp.status_int, 202) + # volume is deleted + self.assertRaises(exception.NotFound, db.volume_get, ctx, volume['id']) diff --git a/cinder/tests/api/openstack/volume/test_router.py b/cinder/tests/api/openstack/volume/test_router.py index f6a1c12ae..6be12e14d 100644 --- a/cinder/tests/api/openstack/volume/test_router.py +++ b/cinder/tests/api/openstack/volume/test_router.py @@ -40,11 +40,7 @@ class FakeController(object): return {} -def create_resource(): - return wsgi.Resource(FakeController()) - - -def create_volume_resource(ext_mgr): +def create_resource(ext_mgr): return wsgi.Resource(FakeController(ext_mgr)) @@ -53,7 +49,7 @@ class VolumeRouterTestCase(test.TestCase): super(VolumeRouterTestCase, self).setUp() # NOTE(vish): versions is just returning text so, no need to stub. self.stubs.Set(snapshots, 'create_resource', create_resource) - self.stubs.Set(volumes, 'create_resource', create_volume_resource) + self.stubs.Set(volumes, 'create_resource', create_resource) self.app = volume.APIRouter() def test_versions(self): diff --git a/cinder/tests/policy.json b/cinder/tests/policy.json index 43ec1a19a..30b2b9ce9 100644 --- a/cinder/tests/policy.json +++ b/cinder/tests/policy.json @@ -1,4 +1,6 @@ { + "admin_api": [["role:admin"]], + "volume:create": [], "volume:get": [], "volume:get_all": [], @@ -22,6 +24,9 @@ "volume:get_snapshot": [], "volume:get_all_snapshots": [], + "volume_extension:volume_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:volume_admin_actions:force_delete": [["rule:admin_api"]], "volume_extension:volume_actions:upload_image": [], "volume_extension:types_manage": [], "volume_extension:types_extra_specs": [], diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 896a4e025..d243c3397 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -282,8 +282,53 @@ class VolumeTestCase(test.TestCase): snapshot_id) self.volume.delete_volume(self.context, volume['id']) + def test_cant_delete_volume_in_use(self): + """Test volume can't be deleted in invalid stats.""" + # create a volume and assign to host + volume = self._create_volume() + self.volume.create_volume(self.context, volume['id']) + volume['status'] = 'in-use' + volume['host'] = 'fakehost' + + volume_api = cinder.volume.api.API() + + # 'in-use' status raises InvalidVolume + self.assertRaises(exception.InvalidVolume, + volume_api.delete, + self.context, + volume) + + # clean up + self.volume.delete_volume(self.context, volume['id']) + + def test_force_delete_volume(self): + """Test volume can be forced to delete.""" + # create a volume and assign to host + volume = self._create_volume() + self.volume.create_volume(self.context, volume['id']) + volume['status'] = 'error_deleting' + volume['host'] = 'fakehost' + + volume_api = cinder.volume.api.API() + + # 'error_deleting' volumes can't be deleted + self.assertRaises(exception.InvalidVolume, + volume_api.delete, + self.context, + volume) + + # delete with force + volume_api.delete(self.context, volume, force=True) + + # status is deleting + volume = db.volume_get(context.get_admin_context(), volume['id']) + self.assertEquals(volume['status'], 'deleting') + + # clean up + self.volume.delete_volume(self.context, volume['id']) + def test_cant_delete_volume_with_snapshots(self): - """Test snapshot can be created and deleted.""" + """Test volume can't be deleted with dependent snapshots.""" volume = self._create_volume() self.volume.create_volume(self.context, volume['id']) snapshot_id = self._create_snapshot(volume['id']) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index a13d801de..b561e103a 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -203,13 +203,13 @@ class API(base.Base): "reservations": reservations}}) @wrap_check_policy - def delete(self, context, volume): + def delete(self, context, volume, force=False): volume_id = volume['id'] if not volume['host']: # NOTE(vish): scheduling failed, so delete it self.db.volume_destroy(context, volume_id) return - if volume['status'] not in ["available", "error"]: + if not force and volume['status'] not in ["available", "error"]: msg = _("Volume status must be available or error") raise exception.InvalidVolume(reason=msg) diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index 9f7383a4c..d1f920a19 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -17,6 +17,9 @@ "volume_extension:quotas:show": [], "volume_extension:quotas:update_for_project": [["rule:admin_api"]], "volume_extension:quotas:update_for_user": [["rule:admin_or_projectadmin"]], - "volume_extension:quota_classes": [] + "volume_extension:quota_classes": [], + "volume_extension:volume_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:volume_admin_actions:force_delete": [["rule:admin_api"]] }