From: Clay Gerrard Date: Mon, 27 Aug 2012 21:38:25 +0000 (+0000) Subject: Add update to volume and snapshot controllers X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=04ca527aa394376b20e1fc09b1cdd1ca8a0a105a;p=openstack-build%2Fcinder-build.git Add update to volume and snapshot controllers Add update method to api.openstack.volume.volumes and snapshots controllers. Url mapping automatically routes a PUT on the resource to the update method. Implementation closely matches updates to server resource for the openstack compute api. e.g. curl http://localhost:8776/v1/${PROJECT_ID}/volumes/${VOLUME_ID} \ -H 'x-auth-token: ${ADMIN_AUTH_TOKEN}' \ -H 'content-type: application/json' \ -XPUT -d '{"volume": {"display_name": "new-name"}}' Add volume.api.API.update_snapshot method to avoid making db calls directly from the SnapshotsController. Add tests for new methods. Update return value of test_volume.VolumeTestCase._create_snapshot to return the whole model like _create_volume fixes lp bug #1042904 Change-Id: I887178f9b7834cc7afd54bba8fb902b630faf8c0 --- diff --git a/cinder/api/openstack/volume/snapshots.py b/cinder/api/openstack/volume/snapshots.py index 919c01da2..2c4d645fd 100644 --- a/cinder/api/openstack/volume/snapshots.py +++ b/cinder/api/openstack/volume/snapshots.py @@ -173,6 +173,39 @@ class SnapshotsController(object): return {'snapshot': retval} + @wsgi.serializers(xml=SnapshotTemplate) + def update(self, req, id, body): + """Update a snapshot.""" + context = req.environ['cinder.context'] + + if not body: + raise exc.HTTPUnprocessableEntity() + + if not 'snapshot' in body: + raise exc.HTTPUnprocessableEntity() + + snapshot = body['snapshot'] + update_dict = {} + + valid_update_keys = ( + 'display_name', + 'display_description', + ) + + for key in valid_update_keys: + if key in snapshot: + update_dict[key] = snapshot[key] + + try: + snapshot = self.volume_api.get_snapshot(context, id) + self.volume_api.update_snapshot(context, snapshot, update_dict) + except exception.NotFound: + raise exc.HTTPNotFound() + + snapshot.update(update_dict) + + return {'snapshot': _translate_snapshot_detail_view(context, snapshot)} + def create_resource(ext_mgr): return wsgi.Resource(SnapshotsController(ext_mgr)) diff --git a/cinder/api/openstack/volume/volumes.py b/cinder/api/openstack/volume/volumes.py index b0a4c6dfa..290e0e2d3 100644 --- a/cinder/api/openstack/volume/volumes.py +++ b/cinder/api/openstack/volume/volumes.py @@ -339,6 +339,39 @@ class VolumeController(object): """Return volume search options allowed by non-admin.""" return ('display_name', 'status') + @wsgi.serializers(xml=VolumeTemplate) + def update(self, req, id, body): + """Update a volume.""" + context = req.environ['cinder.context'] + + if not body: + raise exc.HTTPUnprocessableEntity() + + if not 'volume' in body: + raise exc.HTTPUnprocessableEntity() + + volume = body['volume'] + update_dict = {} + + valid_update_keys = ( + 'display_name', + 'display_description', + ) + + for key in valid_update_keys: + if key in volume: + update_dict[key] = volume[key] + + try: + volume = self.volume_api.get(context, id) + self.volume_api.update(context, volume, update_dict) + except exception.NotFound: + raise exc.HTTPNotFound() + + volume.update(update_dict) + + return {'volume': _translate_volume_detail_view(context, volume)} + def create_resource(ext_mgr): return wsgi.Resource(VolumeController(ext_mgr)) diff --git a/cinder/tests/api/openstack/fakes.py b/cinder/tests/api/openstack/fakes.py index 2c1426c33..b85553397 100644 --- a/cinder/tests/api/openstack/fakes.py +++ b/cinder/tests/api/openstack/fakes.py @@ -277,3 +277,7 @@ def stub_snapshot_get_all(self): def stub_snapshot_get_all_by_project(self, context): return [stub_snapshot(1)] + + +def stub_snapshot_update(self, context, *args, **param): + pass diff --git a/cinder/tests/api/openstack/volume/test_snapshots.py b/cinder/tests/api/openstack/volume/test_snapshots.py index 07d57057b..bfd965eb0 100644 --- a/cinder/tests/api/openstack/volume/test_snapshots.py +++ b/cinder/tests/api/openstack/volume/test_snapshots.py @@ -118,6 +118,49 @@ class SnapshotApiTest(test.TestCase): self.assertEqual(resp_dict['snapshot']['display_description'], snapshot['display_description']) + def test_snapshot_update(self): + self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) + self.stubs.Set(volume.api.API, "update_snapshot", + fakes.stub_snapshot_update) + updates = { + "display_name": "Updated Test Name", + } + body = {"snapshot": updates} + req = fakes.HTTPRequest.blank('/v1/snapshots/%s' % UUID) + res_dict = self.controller.update(req, UUID, body) + expected = {'snapshot': { + 'id': UUID, + 'volume_id': 12, + 'status': 'available', + 'size': 100, + 'created_at': None, + 'display_name': 'Updated Test Name', + 'display_description': 'Default description', + }} + self.assertEquals(expected, res_dict) + + def test_snapshot_update_missing_body(self): + body = {} + req = fakes.HTTPRequest.blank('/v1/snapshots/%s' % UUID) + self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.controller.update, req, UUID, body) + + def test_snapshot_update_invalid_body(self): + body = {'display_name': 'missing top level snapshot key'} + req = fakes.HTTPRequest.blank('/v1/snapshots/%s' % UUID) + self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.controller.update, req, UUID, body) + + def test_snapshot_update_not_found(self): + self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) + updates = { + "display_name": "Updated Test Name", + } + body = {"snapshot": updates} + req = fakes.HTTPRequest.blank('/v1/snapshots/not-the-uuid') + self.assertRaises(webob.exc.HTTPNotFound, self.controller.update, req, + 'not-the-uuid', body) + def test_snapshot_delete(self): self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete) diff --git a/cinder/tests/api/openstack/volume/test_volumes.py b/cinder/tests/api/openstack/volume/test_volumes.py index 4fcf44979..86c417e3f 100644 --- a/cinder/tests/api/openstack/volume/test_volumes.py +++ b/cinder/tests/api/openstack/volume/test_volumes.py @@ -187,6 +187,59 @@ class VolumeApiTest(test.TestCase): req, body) + def test_volume_update(self): + self.stubs.Set(volume_api.API, "update", fakes.stub_volume_update) + updates = { + "display_name": "Updated Test Name", + } + body = {"volume": updates} + req = fakes.HTTPRequest.blank('/v1/volumes/1') + res_dict = self.controller.update(req, '1', body) + expected = {'volume': { + 'status': 'fakestatus', + 'display_description': 'displaydesc', + 'availability_zone': 'fakeaz', + 'display_name': 'Updated Test Name', + 'attachments': [{ + 'id': '1', + 'volume_id': '1', + 'server_id': 'fakeuuid', + 'device': '/', + }], + 'volume_type': 'vol_type_name', + 'snapshot_id': None, + 'metadata': {}, + 'id': '1', + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'size': 1, + }} + self.assertEquals(res_dict, expected) + + def test_update_empty_body(self): + body = {} + req = fakes.HTTPRequest.blank('/v1/volumes/1') + self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.controller.update, + req, '1', body) + + def test_update_invalid_body(self): + body = {'display_name': 'missing top level volume key'} + req = fakes.HTTPRequest.blank('/v1/volumes/1') + self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.controller.update, + req, '1', body) + + def test_update_not_found(self): + self.stubs.Set(volume_api.API, "get", fakes.stub_volume_get_notfound) + updates = { + "display_name": "Updated Test Name", + } + body = {"volume": updates} + req = fakes.HTTPRequest.blank('/v1/volumes/1') + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.update, + req, '1', body) + def test_volume_list(self): self.stubs.Set(volume_api.API, 'get_all', fakes.stub_volume_get_all_by_project) diff --git a/cinder/tests/integrated/api/client.py b/cinder/tests/integrated/api/client.py index a5c1b1962..7a7e5a931 100644 --- a/cinder/tests/integrated/api/client.py +++ b/cinder/tests/integrated/api/client.py @@ -214,3 +214,6 @@ class TestOpenStackClient(object): def delete_volume(self, volume_id): return self.api_delete('/volumes/%s' % volume_id) + + def put_volume(self, volume_id, volume): + return self.api_put('/volumes/%s' % volume_id, volume)['volume'] diff --git a/cinder/tests/integrated/test_volumes.py b/cinder/tests/integrated/test_volumes.py index 0b82bde99..6c4476114 100644 --- a/cinder/tests/integrated/test_volumes.py +++ b/cinder/tests/integrated/test_volumes.py @@ -177,5 +177,22 @@ class VolumesTest(integrated_helpers._IntegratedTestBase): self.assertEqual(created_volume_id, found_volume['id']) self.assertEqual(availability_zone, found_volume['availability_zone']) + def test_create_and_update_volume(self): + # Create vol1 + created_volume = self.api.post_volume({'volume': { + 'size': 1, 'display_name': 'vol1'}}) + self.assertEqual(created_volume['display_name'], 'vol1') + created_volume_id = created_volume['id'] + + # update volume + body = {'volume': {'display_name': 'vol-one'}} + updated_volume = self.api.put_volume(created_volume_id, body) + self.assertEqual(updated_volume['display_name'], 'vol-one') + + # check for update + found_volume = self.api.get_volume(created_volume_id) + self.assertEqual(created_volume_id, found_volume['id']) + self.assertEqual(found_volume['display_name'], 'vol-one') + if __name__ == "__main__": unittest.main() diff --git a/cinder/tests/policy.json b/cinder/tests/policy.json index 30b2b9ce9..cf1a6af5d 100644 --- a/cinder/tests/policy.json +++ b/cinder/tests/policy.json @@ -23,6 +23,7 @@ "volume:delete_snapshot": [], "volume:get_snapshot": [], "volume:get_all_snapshots": [], + "volume:update_snapshot": [], "volume_extension:volume_admin_actions:reset_status": [["rule:admin_api"]], "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 4d752b108..5903fd4d2 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -158,7 +158,7 @@ class VolumeTestCase(test.TestCase): """Test volume can be created from a snapshot.""" volume_src = self._create_volume() self.volume.create_volume(self.context, volume_src['id']) - snapshot_id = self._create_snapshot(volume_src['id']) + snapshot_id = self._create_snapshot(volume_src['id'])['id'] self.volume.create_snapshot(self.context, volume_src['id'], snapshot_id) volume_dst = self._create_volume(0, snapshot_id) @@ -250,13 +250,13 @@ class VolumeTestCase(test.TestCase): snap['project_id'] = 'fake' snap['volume_id'] = volume_id snap['status'] = "creating" - return db.snapshot_create(context.get_admin_context(), snap)['id'] + return db.snapshot_create(context.get_admin_context(), snap) def test_create_delete_snapshot(self): """Test snapshot can be created and deleted.""" volume = self._create_volume() self.volume.create_volume(self.context, volume['id']) - snapshot_id = self._create_snapshot(volume['id']) + snapshot_id = self._create_snapshot(volume['id'])['id'] self.volume.create_snapshot(self.context, volume['id'], snapshot_id) self.assertEqual(snapshot_id, db.snapshot_get(context.get_admin_context(), @@ -318,7 +318,7 @@ class VolumeTestCase(test.TestCase): """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']) + snapshot_id = self._create_snapshot(volume['id'])['id'] self.volume.create_snapshot(self.context, volume['id'], snapshot_id) self.assertEqual(snapshot_id, db.snapshot_get(context.get_admin_context(), @@ -340,7 +340,7 @@ class VolumeTestCase(test.TestCase): """Test snapshot can be created and deleted.""" volume = self._create_volume() self.volume.create_volume(self.context, volume['id']) - snapshot_id = self._create_snapshot(volume['id']) + snapshot_id = self._create_snapshot(volume['id'])['id'] self.volume.create_snapshot(self.context, volume['id'], snapshot_id) snapshot = db.snapshot_get(context.get_admin_context(), snapshot_id) @@ -388,7 +388,7 @@ class VolumeTestCase(test.TestCase): volume = self._create_volume() volume_id = volume['id'] self.volume.create_volume(self.context, volume_id) - snapshot_id = self._create_snapshot(volume_id) + snapshot_id = self._create_snapshot(volume_id)['id'] self.volume.create_snapshot(self.context, volume_id, snapshot_id) self.mox.StubOutWithMock(self.volume.driver, 'delete_snapshot') @@ -717,6 +717,30 @@ class VolumeTestCase(test.TestCase): volume = db.volume_get(self.context, volume['id']) self.assertEqual(volume['status'], "in-use") + def test_volume_api_update(self): + # create a raw vol + volume = self._create_volume() + # use volume.api to update name + volume_api = cinder.volume.api.API() + update_dict = {'display_name': 'test update name'} + volume_api.update(self.context, volume, update_dict) + # read changes from db + vol = db.volume_get(context.get_admin_context(), volume['id']) + self.assertEquals(vol['display_name'], 'test update name') + + def test_volume_api_update_snapshot(self): + # create raw snapshot + volume = self._create_volume() + snapshot = self._create_snapshot(volume['id']) + self.assertEquals(snapshot['display_name'], None) + # use volume.api to update name + volume_api = cinder.volume.api.API() + update_dict = {'display_name': 'test update name'} + volume_api.update_snapshot(self.context, snapshot, update_dict) + # read changes from db + snap = db.snapshot_get(context.get_admin_context(), snapshot['id']) + self.assertEquals(snap['display_name'], 'test update name') + class DriverTestCase(test.TestCase): """Base Test class for Drivers.""" diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 0d340bc4a..889082174 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -439,6 +439,10 @@ class API(base.Base): {"method": "delete_snapshot", "args": {"snapshot_id": snapshot['id']}}) + @wrap_check_policy + def update_snapshot(self, context, snapshot, fields): + self.db.snapshot_update(context, snapshot['id'], fields) + @wrap_check_policy def get_volume_metadata(self, context, volume): """Get all metadata associated with a volume."""