From 7d8369099180bb65449685755b0f69f1378bc8ae Mon Sep 17 00:00:00 2001 From: Sheel Rana Date: Thu, 4 Feb 2016 00:23:16 +0530 Subject: [PATCH] Use of metadata id instead of metadata name If multiple image-metadata entries are present with same name but different cases, 'image-metadata unset' command deletes all of them irrespective of case. For ex, "image-metadata unset abc=123" will delete all image-metadata entries with name abc, ABC, Abc etc. This is fixed by using id instead of name to fetch image-metadata during 'image-metadata unset' command execution flow. Closes-Bug: #1538418 Change-Id: I9fb29667964a4180af822b811345fa1148dbcd66 --- cinder/db/sqlalchemy/api.py | 14 +++++++- cinder/tests/unit/test_db_api.py | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index e82edd890..445378153 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1881,6 +1881,16 @@ def _volume_image_metadata_update(context, volume_id, metadata, delete, session=session) +@require_context +def _volume_glance_metadata_key_to_id(context, volume_id, key): + db_data = volume_glance_metadata_get(context, volume_id) + metadata = {meta_entry.key: meta_entry.id + for meta_entry in db_data + if meta_entry.key == key} + metadata_id = metadata[key] + return metadata_id + + @require_context @require_volume_exists def volume_metadata_get(context, volume_id): @@ -1898,8 +1908,10 @@ def volume_metadata_delete(context, volume_id, key, meta_type): 'deleted_at': timeutils.utcnow(), 'updated_at': literal_column('updated_at')})) elif meta_type == common.METADATA_TYPES.image: + metadata_id = _volume_glance_metadata_key_to_id(context, + volume_id, key) (_volume_image_metadata_get_query(context, volume_id). - filter_by(key=key). + filter_by(id=metadata_id). update({'deleted': True, 'deleted_at': timeutils.utcnow(), 'updated_at': literal_column('updated_at')})) diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index ed3215378..b2dd9fade 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -967,6 +967,66 @@ class DBAPIVolumeTestCase(BaseTest): self.assertEqual(should_be, db_meta) + @mock.patch.object(db.sqlalchemy.api, + '_volume_glance_metadata_key_to_id', + return_value = '1') + def test_volume_glance_metadata_key_to_id_called(self, + metadata_key_to_id_mock): + image_metadata = {'abc': '123'} + + # create volume with metadata. + db.volume_create(self.ctxt, {'id': 1, + 'metadata': image_metadata}) + + # delete metadata associated with the volume. + db.volume_metadata_delete(self.ctxt, + 1, + 'abc', + meta_type=common.METADATA_TYPES.image) + + # assert _volume_glance_metadata_key_to_id() was called exactly once + metadata_key_to_id_mock.assert_called_once_with(self.ctxt, 1, 'abc') + + def test_case_sensitive_glance_metadata_delete(self): + user_metadata = {'a': '1', 'c': '2'} + image_metadata = {'abc': '123', 'ABC': '123'} + + # create volume with metadata. + db.volume_create(self.ctxt, {'id': 1, + 'metadata': user_metadata}) + + # delete user metadata associated with the volume. + db.volume_metadata_delete(self.ctxt, 1, 'c', + meta_type=common.METADATA_TYPES.user) + user_metadata.pop('c') + + self.assertEqual(user_metadata, + db.volume_metadata_get(self.ctxt, 1)) + + # create image metadata associated with the volume. + db.volume_metadata_update( + self.ctxt, + 1, + image_metadata, + False, + meta_type=common.METADATA_TYPES.image) + + # delete image metadata associated with the volume. + db.volume_metadata_delete( + self.ctxt, + 1, + 'abc', + meta_type=common.METADATA_TYPES.image) + + image_metadata.pop('abc') + + # parse the result to build the dict. + rows = db.volume_glance_metadata_get(self.ctxt, 1) + result = {} + for row in rows: + result[row['key']] = row['value'] + self.assertEqual(image_metadata, result) + def test_volume_metadata_update_with_metatype(self): user_metadata1 = {'a': '1', 'c': '2'} user_metadata2 = {'a': '3', 'd': '5'}