From 6750acc73420490172d557302d715433351f1373 Mon Sep 17 00:00:00 2001 From: clayg Date: Thu, 4 Oct 2012 02:31:28 -0500 Subject: [PATCH] Add snapshot force delete admin action Change-Id: I289030cf0db42aa78cd2d8441d89610dad6cbeaa --- .../openstack/volume/contrib/admin_actions.py | 70 ++++++++++++----- .../volume/contrib/test_admin_actions.py | 76 +++++++++++++++++++ cinder/tests/policy.json | 1 + cinder/volume/api.py | 4 +- etc/cinder/policy.json | 3 +- 5 files changed, 130 insertions(+), 24 deletions(-) diff --git a/cinder/api/openstack/volume/contrib/admin_actions.py b/cinder/api/openstack/volume/contrib/admin_actions.py index 3351ecab0..8aa6863c0 100644 --- a/cinder/api/openstack/volume/contrib/admin_actions.py +++ b/cinder/api/openstack/volume/contrib/admin_actions.py @@ -50,9 +50,21 @@ class AdminController(wsgi.Controller): def _update(self, *args, **kwargs): raise NotImplementedError() - def _validate_status(self, status): - if status not in self.valid_status: + def _get(self, *args, **kwargs): + raise NotImplementedError() + + def _delete(self, *args, **kwargs): + raise NotImplementedError() + + def validate_update(self, body): + update = {} + try: + update['status'] = body['status'] + except (TypeError, KeyError): + raise exc.HTTPBadRequest("Must specify 'status'") + if update['status'] not in self.valid_status: raise exc.HTTPBadRequest("Must specify a valid status") + return update def authorize(self, context, action_name): # e.g. "snapshot_admin_actions:reset_status" @@ -64,20 +76,28 @@ class AdminController(wsgi.Controller): """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'") + update = self.validate_update(body['os-reset_status']) + msg = _("Updating %(resource)s '%(id)s' with '%(update)r'") LOG.debug(msg, {'resource': self.resource_name, 'id': id, - 'status': new_status}) + 'update': update}) try: - self._update(context, id, {'status': new_status}) + self._update(context, id, update) except exception.NotFound, e: raise exc.HTTPNotFound(e) return webob.Response(status_int=202) + @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: + resource = self._get(context, id) + except exception.NotFound: + raise exc.HTTPNotFound() + self._delete(context, resource, force=True) + return webob.Response(status_int=202) + class VolumeAdminController(AdminController): """AdminController for Volumes.""" @@ -89,17 +109,19 @@ class VolumeAdminController(AdminController): 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) + def _get(self, *args, **kwargs): + return self.volume_api.get(*args, **kwargs) + + def _delete(self, *args, **kwargs): + return self.volume_api.delete(*args, **kwargs) + + def validate_update(self, body): + update = super(VolumeAdminController, self).validate_update(body) + if 'attach_status' in body: + if body['attach_status'] not in ('detached', 'attached'): + raise exc.HTTPBadRequest("Must specify a valid attach_status") + update['attach_status'] = body['attach_status'] + return update class SnapshotAdminController(AdminController): @@ -110,6 +132,12 @@ class SnapshotAdminController(AdminController): def _update(self, *args, **kwargs): db.snapshot_update(*args, **kwargs) + def _get(self, *args, **kwargs): + return self.volume_api.get_snapshot(*args, **kwargs) + + def _delete(self, *args, **kwargs): + return self.volume_api.delete_snapshot(*args, **kwargs) + class Admin_actions(extensions.ExtensionDescriptor): """Enable admin actions.""" 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 25e1d5192..91b6b73be 100644 --- a/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py +++ b/cinder/tests/api/openstack/volume/contrib/test_admin_actions.py @@ -18,6 +18,10 @@ def app(): class AdminActionsTest(test.TestCase): + def setUp(self): + super(AdminActionsTest, self).setUp() + self.flags(rpc_backend='cinder.openstack.common.rpc.impl_fake') + def test_reset_status_as_admin(self): # admin context ctx = context.RequestContext('admin', 'fake', True) @@ -113,6 +117,53 @@ class AdminActionsTest(test.TestCase): self.assertRaises(exception.NotFound, db.volume_get, ctx, 'missing-volume-id') + def test_reset_attached_status(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available', + 'attach_status': 'attached'}) + req = webob.Request.blank('/v1/fake/volumes/%s/action' % volume['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + # request update attach_status to detached + body = {'os-reset_status': {'status': 'available', + 'attach_status': 'detached'}} + req.body = jsonutils.dumps(body) + # 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']) + # attach_status changed to 'detached' + self.assertEquals(volume['attach_status'], 'detached') + # status un-modified + self.assertEquals(volume['status'], 'available') + + def test_invalid_reset_attached_status(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is available + volume = db.volume_create(ctx, {'status': 'available', + 'attach_status': 'detached'}) + 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 attach_status + body = {'os-reset_status': {'status': 'available', + 'attach_status': 'invalid'}} + req.body = jsonutils.dumps(body) + # 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 and attach_status un-modified + self.assertEquals(volume['status'], 'available') + self.assertEquals(volume['attach_status'], 'detached') + def test_snapshot_reset_status(self): # admin context ctx = context.RequestContext('admin', 'fake', True) @@ -174,3 +225,28 @@ class AdminActionsTest(test.TestCase): self.assertEquals(resp.status_int, 202) # volume is deleted self.assertRaises(exception.NotFound, db.volume_get, ctx, volume['id']) + + def test_force_delete_snapshot(self): + # admin context + ctx = context.RequestContext('admin', 'fake', True) + # current status is creating + volume = db.volume_create(ctx, {'host': 'test'}) + snapshot = db.snapshot_create(ctx, {'status': 'creating', + 'volume_size': 1, + 'volume_id': volume['id']}) + path = '/v1/fake/snapshots/%s/action' % snapshot['id'] + req = webob.Request.blank(path) + 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 + # start service to handle rpc.cast for 'delete snapshot' + self.start_service('volume', host='test') + # make request + resp = req.get_response(app()) + # request is accepted + self.assertEquals(resp.status_int, 202) + # snapshot is deleted + self.assertRaises(exception.NotFound, db.snapshot_get, ctx, + snapshot['id']) diff --git a/cinder/tests/policy.json b/cinder/tests/policy.json index cf1a6af5d..a18b0c00d 100644 --- a/cinder/tests/policy.json +++ b/cinder/tests/policy.json @@ -28,6 +28,7 @@ "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:snapshot_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/volume/api.py b/cinder/volume/api.py index e25fb81e3..ece1c9682 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -442,8 +442,8 @@ class API(base.Base): True) @wrap_check_policy - def delete_snapshot(self, context, snapshot): - if snapshot['status'] not in ["available", "error"]: + def delete_snapshot(self, context, snapshot, force=False): + if not force and snapshot['status'] not in ["available", "error"]: msg = _("Volume Snapshot status must be available or error") raise exception.InvalidVolume(reason=msg) self.db.snapshot_update(context, snapshot['id'], diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index d1f920a19..0fc4fd783 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -21,5 +21,6 @@ "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_admin_actions:force_delete": [["rule:admin_api"]], + "volume_extension:snapshot_admin_actions:force_delete": [["rule:admin_api"]] } -- 2.45.2