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 4dbf736d867764489239cfbd969f3d6fc7a5e78c..15ad489c042b8a0c29f01ef97cb8e88b89841fee 100644 (file)
--- 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 0d9e4d1cedb05a547f8214bd27b4f859a6c780a1..e14cfd58ac978a3c45c1609ebbef566df12600d0 100644 (file)
--- 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):