From: john-griffith Date: Wed, 16 Apr 2014 21:03:18 +0000 (-0600) Subject: Allow reset-state on attach and migration fields X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=aabb0fa1f68974e2fa4828cde1462dc47c429655;p=openstack-build%2Fcinder-build.git Allow reset-state on attach and migration fields The reset-state API call only sets that main status column on the volume object. There's also a method to set attach status, however it turns out that due to the implementation of the status validation method in the parent class, this was impossible to use because it only checked and accepted "status" NOT "attach_status". This patch fixes that attach_status problem, it also implements the ability to update the migration status. A cinderclient change will be needed to expose these calls as well. DocImpact Change-Id: I59e1bb2522f033c944fa07acf4876ca71c8c3d3f Closes-Bug: #1308253 --- diff --git a/cinder/api/contrib/admin_actions.py b/cinder/api/contrib/admin_actions.py index 4dbf736d8..15ad489c0 100644 --- a/cinder/api/contrib/admin_actions.py +++ b/cinder/api/contrib/admin_actions.py @@ -35,13 +35,11 @@ class AdminController(wsgi.Controller): # 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', - ]) + valid_status = set(['creating', + 'available', + 'deleting', + 'error', + 'error_deleting', ]) def __init__(self, *args, **kwargs): super(AdminController, self).__init__(*args, **kwargs) @@ -61,7 +59,7 @@ class AdminController(wsgi.Controller): def validate_update(self, body): update = {} try: - update['status'] = body['status'] + update['status'] = body['status'].lower() except (TypeError, KeyError): raise exc.HTTPBadRequest("Must specify 'status'") if update['status'] not in self.valid_status: @@ -115,9 +113,21 @@ class VolumeAdminController(AdminController): """AdminController for Volumes.""" collection = 'volumes' + + # FIXME(jdg): We're appending additional valid status + # entries to the set we declare in the parent class + # this doesn't make a ton of sense, we should probably + # look at the structure of this whole process again + # Perhaps we don't even want any definitions in the abstract + # parent class? valid_status = AdminController.valid_status.union( set(['attaching', 'in-use', 'detaching'])) + valid_attach_status = set(['detached', 'attached', ]) + valid_migration_status = set(['migrating', 'error', + 'completing', 'none', + 'starting', ]) + def _update(self, *args, **kwargs): db.volume_update(*args, **kwargs) @@ -128,11 +138,34 @@ class VolumeAdminController(AdminController): 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'] + update = {} + status = body.get('status', None) + attach_status = body.get('attach_status', None) + migration_status = body.get('migration_status', None) + + valid = False + if status: + valid = True + update = super(VolumeAdminController, self).validate_update(body) + + if attach_status: + valid = True + update['attach_status'] = attach_status.lower() + if update['attach_status'] not in self.valid_attach_status: + raise exc.HTTPBadRequest("Must specify a valid attach status") + + if migration_status: + valid = True + update['migration_status'] = migration_status.lower() + if update['migration_status'] not in self.valid_migration_status: + raise exc.HTTPBadRequest("Must specify a valid migration " + "status") + if update['migration_status'] == 'none': + update['migration_status'] = None + + if not valid: + raise exc.HTTPBadRequest("Must specify 'status', 'attach_status' " + "or 'migration_status' for update.") return update @wsgi.action('os-force_detach') diff --git a/cinder/tests/api/contrib/test_admin_actions.py b/cinder/tests/api/contrib/test_admin_actions.py index 0d9e4d1ce..e14cfd58a 100644 --- a/cinder/tests/api/contrib/test_admin_actions.py +++ b/cinder/tests/api/contrib/test_admin_actions.py @@ -16,6 +16,7 @@ import webob from oslo.config import cfg +from cinder.api.contrib import admin_actions from cinder.brick.local_dev import lvm as brick_lvm from cinder import context from cinder import db @@ -53,38 +54,114 @@ class AdminActionsTest(test.TestCase): cast_as_call.mock_cast_as_call(self.volume_api.scheduler_rpcapi.client) self.stubs.Set(brick_lvm.LVM, '_vg_exists', lambda x: True) - 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'}) + def _issue_volume_reset(self, ctx, volume, updated_status): req = webob.Request.blank('/v2/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.body = \ + jsonutils.dumps({'os-reset_status': updated_status}) req.environ['cinder.context'] = ctx resp = req.get_response(app()) - # request is accepted + return resp + + def _issue_snapshot_reset(self, ctx, snapshot, updated_status): + req = webob.Request.blank('/v2/fake/snapshots/%s/action' % + snapshot['id']) + req.method = 'POST' + req.headers['content-type'] = 'application/json' + req.body = \ + jsonutils.dumps({'os-reset_status': updated_status}) + req.environ['cinder.context'] = ctx + resp = req.get_response(app()) + return resp + + def test_valid_updates(self): + vac = admin_actions.VolumeAdminController() + + vac.validate_update({'status': 'creating'}) + vac.validate_update({'status': 'available'}) + vac.validate_update({'status': 'deleting'}) + vac.validate_update({'status': 'error'}) + vac.validate_update({'status': 'error_deleting'}) + + vac.validate_update({'attach_status': 'detached'}) + vac.validate_update({'attach_status': 'attached'}) + + vac.validate_update({'migration_status': 'migrating'}) + vac.validate_update({'migration_status': 'error'}) + vac.validate_update({'migration_status': 'completing'}) + vac.validate_update({'migration_status': 'none'}) + vac.validate_update({'migration_status': 'starting'}) + + def test_reset_attach_status(self): + ctx = context.RequestContext('admin', 'fake', True) + volume = db.volume_create(ctx, {'attach_status': 'detached'}) + + resp = self._issue_volume_reset(ctx, + volume, + {'attach_status': 'attached'}) + + self.assertEqual(resp.status_int, 202) + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['attach_status'], 'attached') + + def test_reset_attach_invalid_status(self): + ctx = context.RequestContext('admin', 'fake', True) + volume = db.volume_create(ctx, {'attach_status': 'detached'}) + + resp = self._issue_volume_reset(ctx, + volume, + {'attach_status': 'bogus-status'}) + + self.assertEqual(resp.status_int, 400) + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['attach_status'], 'detached') + + def test_reset_migration_invalid_status(self): + ctx = context.RequestContext('admin', 'fake', True) + volume = db.volume_create(ctx, {'migration_status': None}) + + resp = self._issue_volume_reset(ctx, + volume, + {'migration_status': 'bogus-status'}) + + self.assertEqual(resp.status_int, 400) + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['migration_status'], None) + + def test_reset_migration_status(self): + ctx = context.RequestContext('admin', 'fake', True) + volume = db.volume_create(ctx, {'migration_status': None}) + + resp = self._issue_volume_reset(ctx, + volume, + {'migration_status': 'migrating'}) + + self.assertEqual(resp.status_int, 202) + volume = db.volume_get(ctx, volume['id']) + self.assertEqual(volume['migration_status'], 'migrating') + + def test_reset_status_as_admin(self): + ctx = context.RequestContext('admin', 'fake', True) + volume = db.volume_create(ctx, {'status': 'available'}) + + resp = self._issue_volume_reset(ctx, + volume, + {'status': 'error'}) + self.assertEqual(resp.status_int, 202) volume = db.volume_get(ctx, volume['id']) - # status changed to 'error' self.assertEqual(volume['status'], 'error') def test_reset_status_as_non_admin(self): - # current status is 'error' + ctx = context.RequestContext('fake', 'fake') volume = db.volume_create(context.get_admin_context(), {'status': 'error', 'size': 1}) - req = webob.Request.blank('/v2/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()) + + resp = self._issue_volume_reset(ctx, + volume, + {'status': 'error'}) + # request is not authorized self.assertEqual(resp.status_int, 403) volume = db.volume_get(context.get_admin_context(), volume['id']) @@ -92,156 +169,100 @@ class AdminActionsTest(test.TestCase): self.assertEqual(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', 'size': 1}) - req = webob.Request.blank('/v2/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 + + resp = self._issue_volume_reset(ctx, + volume, + {'x-status': 'bad'}) + self.assertEqual(resp.status_int, 400) volume = db.volume_get(ctx, volume['id']) - # status is still 'available' self.assertEqual(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', 'size': 1}) - req = webob.Request.blank('/v2/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 + resp = self._issue_volume_reset(ctx, + volume, + {'status': 'invalid'}) + self.assertEqual(resp.status_int, 400) volume = db.volume_get(ctx, volume['id']) - # status is still 'available' self.assertEqual(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('/v2/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.assertEqual(resp.status_int, 404) 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', 'host': 'test', 'provider_location': '', 'size': 1, 'attach_status': 'attached'}) - req = webob.Request.blank('/v2/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 + + resp = self._issue_volume_reset(ctx, + volume, + {'status': 'available', + 'attach_status': 'detached'}) + self.assertEqual(resp.status_int, 202) volume = db.volume_get(ctx, volume['id']) - # attach_status changed to 'detached' self.assertEqual(volume['attach_status'], 'detached') - # status un-modified self.assertEqual(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', 'host': 'test', 'provider_location': '', 'size': 1, 'attach_status': 'detached'}) - req = webob.Request.blank('/v2/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 + resp = self._issue_volume_reset(ctx, + volume, + {'status': 'available', + 'attach_status': 'invalid'}) self.assertEqual(resp.status_int, 400) volume = db.volume_get(ctx, volume['id']) - # status and attach_status un-modified self.assertEqual(volume['status'], 'available') self.assertEqual(volume['attach_status'], 'detached') def test_snapshot_reset_status(self): - # admin context ctx = context.RequestContext('admin', 'fake', True) - # snapshot in 'error_deleting' volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', 'provider_location': '', 'size': 1}) snapshot = db.snapshot_create(ctx, {'status': 'error_deleting', 'volume_id': volume['id']}) - req = webob.Request.blank('/v2/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 + + resp = self._issue_snapshot_reset(ctx, + snapshot, + {'status': 'error'}) + self.assertEqual(resp.status_int, 202) snapshot = db.snapshot_get(ctx, snapshot['id']) - # status changed to 'error' self.assertEqual(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, {'status': 'available', 'host': 'test', 'provider_location': '', 'size': 1}) snapshot = db.snapshot_create(ctx, {'status': 'available', 'volume_id': volume['id']}) - req = webob.Request.blank('/v2/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 + + resp = self._issue_snapshot_reset(ctx, + snapshot, + {'status': 'attaching'}) + self.assertEqual(resp.status_int, 400) snapshot = db.snapshot_get(ctx, snapshot['id']) - # status is still 'available' self.assertEqual(snapshot['status'], 'available') def test_force_delete(self):