diff --git a/cinder/db/api.py b/cinder/db/api.py index 05298c29ca2..8af6c557a9f 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -340,7 +340,7 @@ def volume_metadata_delete(context, volume_id, key): def volume_metadata_update(context, volume_id, metadata, delete): """Update metadata if it exists, otherwise create it.""" - IMPL.volume_metadata_update(context, volume_id, metadata, delete) + return IMPL.volume_metadata_update(context, volume_id, metadata, delete) ################## diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index ca7186bba14..54a11e0a7b6 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1316,7 +1316,7 @@ def _volume_x_metadata_update(context, volume_id, metadata, delete, meta_ref.update(item) meta_ref.save(session=session) - return metadata + return _volume_x_metadata_get(context, volume_id, model) def _volume_user_metadata_get_query(context, volume_id, session=None): diff --git a/cinder/tests/api/v1/test_volume_metadata.py b/cinder/tests/api/v1/test_volume_metadata.py index 1f1b76ec9b3..c6a6c1c6f2e 100644 --- a/cinder/tests/api/v1/test_volume_metadata.py +++ b/cinder/tests/api/v1/test_volume_metadata.py @@ -42,6 +42,15 @@ def return_create_volume_metadata(context, volume_id, metadata, delete): return stub_volume_metadata() +def return_new_volume_metadata(context, volume_id, metadata, delete): + return stub_new_volume_metadata() + + +def return_create_volume_metadata_insensitive(context, snapshot_id, + metadata, delete): + return stub_volume_metadata_insensitive() + + def return_volume_metadata(context, volume_id): if not isinstance(volume_id, str) or not len(volume_id) == 36: msg = 'id %s must be a uuid in return volume metadata' % volume_id @@ -53,6 +62,10 @@ def return_empty_volume_metadata(context, volume_id): return {} +def return_empty_container_metadata(context, volume_id, metadata, delete): + return {} + + def delete_volume_metadata(context, volume_id, key): pass @@ -66,6 +79,25 @@ def stub_volume_metadata(): return metadata +def stub_new_volume_metadata(): + metadata = { + 'key10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + } + return metadata + + +def stub_volume_metadata_insensitive(): + metadata = { + "key1": "value1", + "key2": "value2", + "key3": "value3", + "KEY4": "value4", + } + return metadata + + def stub_max_volume_metadata(): metadata = {"metadata": {}} for num in range(CONF.quota_metadata_items): @@ -202,11 +234,38 @@ class volumeMetaDataTest(test.TestCase): req = fakes.HTTPRequest.blank('/v1/volume_metadata') req.method = 'POST' req.content_type = "application/json" - body = {"metadata": {"key9": "value9"}} + body = {"metadata": {"key1": "value1", + "key2": "value2", + "key3": "value3", }} req.body = jsonutils.dumps(body) res_dict = self.controller.create(req, self.req_id, body) self.assertEqual(body, res_dict) + def test_create_with_keys_in_uppercase_and_lowercase(self): + # if the keys in uppercase_and_lowercase, should return the one + # which server added + self.stubs.Set(cinder.db, 'volume_metadata_get', + return_empty_volume_metadata) + self.stubs.Set(cinder.db, 'volume_metadata_update', + return_create_volume_metadata_insensitive) + + req = fakes.HTTPRequest.blank('/v1/volume_metadata') + req.method = 'POST' + req.content_type = "application/json" + body = {"metadata": {"key1": "value1", + "KEY1": "value1", + "key2": "value2", + "KEY2": "value2", + "key3": "value3", + "KEY4": "value4"}} + expected = {"metadata": {"key1": "value1", + "key2": "value2", + "key3": "value3", + "KEY4": "value4"}} + req.body = jsonutils.dumps(body) + res_dict = self.controller.create(req, self.req_id, body) + self.assertEqual(expected, res_dict) + def test_create_empty_body(self): self.stubs.Set(cinder.db, 'volume_metadata_update', return_create_volume_metadata) @@ -260,7 +319,7 @@ class volumeMetaDataTest(test.TestCase): def test_update_all(self): self.stubs.Set(cinder.db, 'volume_metadata_update', - return_create_volume_metadata) + return_new_volume_metadata) req = fakes.HTTPRequest.blank(self.url) req.method = 'PUT' req.content_type = "application/json" @@ -268,6 +327,7 @@ class volumeMetaDataTest(test.TestCase): 'metadata': { 'key10': 'value10', 'key99': 'value99', + 'KEY20': 'value20', }, } req.body = jsonutils.dumps(expected) @@ -275,9 +335,37 @@ class volumeMetaDataTest(test.TestCase): self.assertEqual(expected, res_dict) + def test_update_all_with_keys_in_uppercase_and_lowercase(self): + self.stubs.Set(cinder.db, 'volume_metadata_get', + return_create_volume_metadata) + self.stubs.Set(cinder.db, 'volume_metadata_update', + return_new_volume_metadata) + req = fakes.HTTPRequest.blank(self.url) + req.method = 'PUT' + req.content_type = "application/json" + body = { + 'metadata': { + 'key10': 'value10', + 'KEY10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + }, + } + expected = { + 'metadata': { + 'key10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + }, + } + req.body = jsonutils.dumps(expected) + res_dict = self.controller.update_all(req, self.req_id, body) + + self.assertEqual(expected, res_dict) + def test_update_all_empty_container(self): self.stubs.Set(cinder.db, 'volume_metadata_update', - return_create_volume_metadata) + return_empty_container_metadata) req = fakes.HTTPRequest.blank(self.url) req.method = 'PUT' req.content_type = "application/json" diff --git a/cinder/tests/api/v2/test_snapshot_metadata.py b/cinder/tests/api/v2/test_snapshot_metadata.py index ba250484778..bcbbaa49b1a 100644 --- a/cinder/tests/api/v2/test_snapshot_metadata.py +++ b/cinder/tests/api/v2/test_snapshot_metadata.py @@ -265,7 +265,7 @@ class SnapshotMetaDataTest(test.TestCase): self.stubs.Set(cinder.db, 'snapshot_metadata_update', return_create_snapshot_metadata_insensitive) - req = fakes.HTTPRequest.blank('/v1/snapshot_metadata') + req = fakes.HTTPRequest.blank('/v2/snapshot_metadata') req.method = 'POST' req.content_type = "application/json" body = {"metadata": {"key1": "value1", diff --git a/cinder/tests/api/v2/test_volume_metadata.py b/cinder/tests/api/v2/test_volume_metadata.py index 16ea3b7ce82..1ef807ba2be 100644 --- a/cinder/tests/api/v2/test_volume_metadata.py +++ b/cinder/tests/api/v2/test_volume_metadata.py @@ -43,6 +43,15 @@ def return_create_volume_metadata(context, volume_id, metadata, delete): return stub_volume_metadata() +def return_new_volume_metadata(context, volume_id, metadata, delete): + return stub_new_volume_metadata() + + +def return_create_volume_metadata_insensitive(context, snapshot_id, + metadata, delete): + return stub_volume_metadata_insensitive() + + def return_volume_metadata(context, volume_id): if not isinstance(volume_id, str) or not len(volume_id) == 36: msg = 'id %s must be a uuid in return volume metadata' % volume_id @@ -54,6 +63,10 @@ def return_empty_volume_metadata(context, volume_id): return {} +def return_empty_container_metadata(context, volume_id, metadata, delete): + return {} + + def delete_volume_metadata(context, volume_id, key): pass @@ -67,6 +80,25 @@ def stub_volume_metadata(): return metadata +def stub_new_volume_metadata(): + metadata = { + 'key10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + } + return metadata + + +def stub_volume_metadata_insensitive(): + metadata = { + "key1": "value1", + "key2": "value2", + "key3": "value3", + "KEY4": "value4", + } + return metadata + + def stub_max_volume_metadata(): metadata = {"metadata": {}} for num in range(CONF.quota_metadata_items): @@ -203,11 +235,38 @@ class volumeMetaDataTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/volume_metadata') req.method = 'POST' req.content_type = "application/json" - body = {"metadata": {"key9": "value9"}} + body = {"metadata": {"key1": "value1", + "key2": "value2", + "key3": "value3", }} req.body = jsonutils.dumps(body) res_dict = self.controller.create(req, self.req_id, body) self.assertEqual(body, res_dict) + def test_create_with_keys_in_uppercase_and_lowercase(self): + # if the keys in uppercase_and_lowercase, should return the one + # which server added + self.stubs.Set(db, 'volume_metadata_get', + return_empty_volume_metadata) + self.stubs.Set(db, 'volume_metadata_update', + return_create_volume_metadata_insensitive) + + req = fakes.HTTPRequest.blank('/v2/volume_metadata') + req.method = 'POST' + req.content_type = "application/json" + body = {"metadata": {"key1": "value1", + "KEY1": "value1", + "key2": "value2", + "KEY2": "value2", + "key3": "value3", + "KEY4": "value4"}} + expected = {"metadata": {"key1": "value1", + "key2": "value2", + "key3": "value3", + "KEY4": "value4"}} + req.body = jsonutils.dumps(body) + res_dict = self.controller.create(req, self.req_id, body) + self.assertEqual(expected, res_dict) + def test_create_empty_body(self): self.stubs.Set(db, 'volume_metadata_update', return_create_volume_metadata) @@ -261,7 +320,7 @@ class volumeMetaDataTest(test.TestCase): def test_update_all(self): self.stubs.Set(db, 'volume_metadata_update', - return_create_volume_metadata) + return_new_volume_metadata) req = fakes.HTTPRequest.blank(self.url) req.method = 'PUT' req.content_type = "application/json" @@ -269,6 +328,7 @@ class volumeMetaDataTest(test.TestCase): 'metadata': { 'key10': 'value10', 'key99': 'value99', + 'KEY20': 'value20', }, } req.body = jsonutils.dumps(expected) @@ -276,9 +336,37 @@ class volumeMetaDataTest(test.TestCase): self.assertEqual(expected, res_dict) + def test_update_all_with_keys_in_uppercase_and_lowercase(self): + self.stubs.Set(db, 'volume_metadata_get', + return_create_volume_metadata) + self.stubs.Set(db, 'volume_metadata_update', + return_new_volume_metadata) + req = fakes.HTTPRequest.blank(self.url) + req.method = 'PUT' + req.content_type = "application/json" + body = { + 'metadata': { + 'key10': 'value10', + 'KEY10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + }, + } + expected = { + 'metadata': { + 'key10': 'value10', + 'key99': 'value99', + 'KEY20': 'value20', + }, + } + req.body = jsonutils.dumps(expected) + res_dict = self.controller.update_all(req, self.req_id, body) + + self.assertEqual(expected, res_dict) + def test_update_all_empty_container(self): self.stubs.Set(db, 'volume_metadata_update', - return_create_volume_metadata) + return_empty_container_metadata) req = fakes.HTTPRequest.blank(self.url) req.method = 'PUT' req.content_type = "application/json" diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index 8d2ba7b2f60..b2def7a81af 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -343,7 +343,7 @@ class VolumeApiTest(test.TestCase): "display_name": "Updated Test Name", } body = {"volume": updates} - req = fakes.HTTPRequest.blank('/v1/volumes/1') + req = fakes.HTTPRequest.blank('/v2/volumes/1') admin_ctx = context.RequestContext('admin', 'fake', True) req.environ['cinder.context'] = admin_ctx res_dict = self.controller.update(req, '1', body) diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py index 0e87483bff6..20c59c69d4b 100644 --- a/cinder/tests/test_db_api.py +++ b/cinder/tests/test_db_api.py @@ -442,9 +442,9 @@ class DBAPIVolumeTestCase(BaseTest): should_be = {'a': '3', 'c': '2', 'd': '5'} db.volume_create(self.ctxt, {'id': 1, 'metadata': metadata1}) - db.volume_metadata_update(self.ctxt, 1, metadata2, False) + db_meta = db.volume_metadata_update(self.ctxt, 1, metadata2, False) - self.assertEqual(should_be, db.volume_metadata_get(self.ctxt, 1)) + self.assertEqual(should_be, db_meta) def test_volume_metadata_update_delete(self): metadata1 = {'a': '1', 'c': '2'} @@ -452,9 +452,9 @@ class DBAPIVolumeTestCase(BaseTest): should_be = metadata2 db.volume_create(self.ctxt, {'id': 1, 'metadata': metadata1}) - db.volume_metadata_update(self.ctxt, 1, metadata2, True) + db_meta = db.volume_metadata_update(self.ctxt, 1, metadata2, True) - self.assertEqual(should_be, db.volume_metadata_get(self.ctxt, 1)) + self.assertEqual(should_be, db_meta) def test_volume_metadata_delete(self): metadata = {'a': 'b', 'c': 'd'} diff --git a/cinder/volume/api.py b/cinder/volume/api.py index cdcefedf870..f6ca4724124 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -610,12 +610,12 @@ class API(base.Base): self._check_metadata_properties(context, _metadata) - self.db.volume_metadata_update(context, volume['id'], - _metadata, delete) + db_meta = self.db.volume_metadata_update(context, volume['id'], + _metadata, delete) # TODO(jdg): Implement an RPC call for drivers that may use this info - return _metadata + return db_meta def get_volume_metadata_value(self, volume, key): """Get value of particular metadata key."""