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 919c01da255bc7eae70be16a940472ea35f4d79c..2c4d645fd43c76f6c5c5e1172d5a39b522e1edd7 100644 (file)
--- 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 b0a4c6dfaa77deb767f44ebfb7bbb748df0fa76e..290e0e2d3fa88bcb852e9c430c79a135143c09c9 100644 (file)
--- 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 2c1426c331fa4f8afc7a8e1a7ec478fea3ccbb82..b8555339731807616b7e92aa8c70b610fcd26ca8 100644 (file)
--- 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 07d57057b0ff08321b59156d6ee69fefc4ec3239..bfd965eb013c1d5ccf3301d3126feb9d3782b499 100644 (file)
--- 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 4fcf449795b60862f04ac5a2a1855b06c47d7811..86c417e3f727d83a3a75a87295eb0abd93f76835 100644 (file)
--- 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 a5c1b19623e2e78f0b522a60c5682aefce50c42b..7a7e5a931689cf0bb4f560008e95802140b3d373 100644 (file)
--- 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 0b82bde99306b7b8326956285f829443da8c6d1b..6c4476114b1cc124dd5891b45486caa7626c75a7 100644 (file)
--- 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 30b2b9ce98663f140712ffee708111a8be345fb4..cf1a6af5d9a769fd6c6abdc6c7526e7ab8a0d34d 100644 (file)
--- 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 4d752b108e4dd226582d54799901db7814eed0a5..5903fd4d2eb629c8057aeb524153a9712434fb8c 100644 (file)
--- 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 0d340bc4aa31527fb0d6c44719a5f29dc32dcb3f..8890821742382cdc93e5fb0bf20ba3dba628dc1a 100644 (file)
--- 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."""