From e15d8e7f0920cb7cd5719d2861dea886bd6f9cb0 Mon Sep 17 00:00:00 2001 From: Yuriy Nesenenko Date: Thu, 2 Jun 2016 16:34:33 +0300 Subject: [PATCH] Deleting volume metadata keys with a single request Deleting multiple volume metadata keys with a single request to improve performance. To delete multiple metadata items without affecting the remaining ones, just update the metadata items with the updated complete list of ones (without items to delete) in the body of the request. This patch uses etags to avoid the lost update problem with volume metadata. The command isn't changed: $ cinder metadata volume_id unset k1 k2 k3 Co-Authored-By: Ivan Kolodyazhny Depends-On: I575635258c10f299181b8e4cdb51a7ad1f1be764 Implements: blueprint delete-multiple-metadata-keys Change-Id: I8e18133ffee87c240a7af4b8177683ab99330d9e --- cinderclient/api_versions.py | 2 +- cinderclient/base.py | 2 +- .../openstack/common/apiclient/base.py | 2 ++ cinderclient/tests/unit/test_base.py | 2 ++ cinderclient/tests/unit/v2/fakes.py | 4 ++++ cinderclient/tests/unit/v2/test_volumes.py | 9 ++++++--- cinderclient/tests/unit/v3/test_shell.py | 15 ++++++++++++++ cinderclient/v3/shell.py | 8 ++++++++ cinderclient/v3/volumes.py | 20 ++++++++++++++++++- 9 files changed, 58 insertions(+), 6 deletions(-) diff --git a/cinderclient/api_versions.py b/cinderclient/api_versions.py index bdd975fa3..383d10fe2 100644 --- a/cinderclient/api_versions.py +++ b/cinderclient/api_versions.py @@ -32,7 +32,7 @@ if not LOG.handlers: # key is a deprecated version and value is an alternative version. DEPRECATED_VERSIONS = {"1": "2"} -MAX_VERSION = "3.14" +MAX_VERSION = "3.15" _SUBSTITUTIONS = {} diff --git a/cinderclient/base.py b/cinderclient/base.py index 613a066f9..d30a08e45 100644 --- a/cinderclient/base.py +++ b/cinderclient/base.py @@ -328,7 +328,7 @@ class Manager(common_base.HookableMixin): def _update(self, url, body, response_key=None, **kwargs): self.run_hooks('modify_body_for_update', body, **kwargs) - resp, body = self.api.client.put(url, body=body) + resp, body = self.api.client.put(url, body=body, **kwargs) if response_key: return self.resource_class(self, body[response_key], loaded=True, resp=resp) diff --git a/cinderclient/openstack/common/apiclient/base.py b/cinderclient/openstack/common/apiclient/base.py index bea1520ce..95ae8b957 100644 --- a/cinderclient/openstack/common/apiclient/base.py +++ b/cinderclient/openstack/common/apiclient/base.py @@ -467,6 +467,8 @@ class Resource(RequestIdMixin): self._info = info self._add_details(info) self._loaded = loaded + if resp and hasattr(resp, "headers"): + self._checksum = resp.headers.get("Etag") self.setup() self.append_request_ids(resp) diff --git a/cinderclient/tests/unit/test_base.py b/cinderclient/tests/unit/test_base.py index 613d09b91..587925aa3 100644 --- a/cinderclient/tests/unit/test_base.py +++ b/cinderclient/tests/unit/test_base.py @@ -33,6 +33,8 @@ REQUEST_ID = 'req-test-request-id' def create_response_obj_with_header(): resp = Response() resp.headers['x-openstack-request-id'] = REQUEST_ID + resp.headers['Etag'] = 'd5103bf7b26ff0310200d110da3ed186' + resp.status_code = 200 return resp diff --git a/cinderclient/tests/unit/v2/fakes.py b/cinderclient/tests/unit/v2/fakes.py index a70d63cdc..3c9d02843 100644 --- a/cinderclient/tests/unit/v2/fakes.py +++ b/cinderclient/tests/unit/v2/fakes.py @@ -474,6 +474,10 @@ class FakeHTTPClient(base_client.HTTPClient): r = {'volume': self.get_volumes_detail(id=5678)[2]['volumes'][0]} return (200, {}, r) + def get_volumes_1234_metadata(self, **kw): + r = {"metadata": {'k1': 'v1', 'k2': 'v2', 'k3': 'v3'}} + return (200, {}, r) + def get_volumes_1234_encryption(self, **kw): r = {'encryption_key_id': 'id'} return (200, {}, r) diff --git a/cinderclient/tests/unit/v2/test_volumes.py b/cinderclient/tests/unit/v2/test_volumes.py index 0fb54cee9..fbc85aa90 100644 --- a/cinderclient/tests/unit/v2/test_volumes.py +++ b/cinderclient/tests/unit/v2/test_volumes.py @@ -177,9 +177,12 @@ class VolumesTest(utils.TestCase): self._assert_request_id(vol) def test_delete_metadata(self): - keys = ['key1'] - vol = cs.volumes.delete_metadata(1234, keys) - cs.assert_called('DELETE', '/volumes/1234/metadata/key1') + volume = Volume(self, {'id': '1234', 'metadata': { + 'k1': 'v1', 'k2': 'v2', 'k3': 'v3'}}) + keys = ['k1', 'k3'] + vol = cs.volumes.delete_metadata(volume, keys) + cs.assert_called('PUT', '/volumes/1234/metadata', + {'metadata': {'k2': 'v2'}}) self._assert_request_id(vol) def test_extend(self): diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py index a3718fa2b..e6a1ecaba 100644 --- a/cinderclient/tests/unit/v3/test_shell.py +++ b/cinderclient/tests/unit/v3/test_shell.py @@ -21,6 +21,7 @@ from requests_mock.contrib import fixture as requests_mock_fixture from cinderclient import client from cinderclient import exceptions from cinderclient import shell +from cinderclient.v3 import volumes from cinderclient.tests.unit import utils from cinderclient.tests.unit.v3 import fakes from cinderclient.tests.unit.fixture_data import keystone_client @@ -376,3 +377,17 @@ class ShellTest(utils.TestCase): '--os-volume-api-version 3.3 message-delete 1234 12345') self.assert_called_anytime('DELETE', '/messages/1234') self.assert_called_anytime('DELETE', '/messages/12345') + + @mock.patch('cinderclient.utils.find_volume') + def test_delete_metadata(self, mock_find_volume): + mock_find_volume.return_value = volumes.Volume(self, + {'id': '1234', + 'metadata': + {'k1': 'v1', + 'k2': 'v2', + 'k3': 'v3'}}, + loaded = True) + expected = {'metadata': {'k2': 'v2'}} + self.run_command('--os-volume-api-version 3.15 ' + 'metadata 1234 unset k1 k3') + self.assert_called('PUT', '/volumes/1234/metadata', body=expected) diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py index 13097d232..d0ed94c8e 100644 --- a/cinderclient/v3/shell.py +++ b/cinderclient/v3/shell.py @@ -617,8 +617,16 @@ def do_rename(cs, args): metavar='', nargs='+', default=[], + end_version='3.14', help='Metadata key and value pair to set or unset. ' 'For unset, specify only the key.') +@utils.arg('metadata', + metavar='', + nargs='+', + default=[], + start_version='3.15', + help='Metadata key and value pair to set or unset. ' + 'For unset, specify only the key(s): ') @utils.service_type('volumev3') def do_metadata(cs, args): """Sets or deletes volume metadata.""" diff --git a/cinderclient/v3/volumes.py b/cinderclient/v3/volumes.py index 6862a183e..6dd0f9c96 100644 --- a/cinderclient/v3/volumes.py +++ b/cinderclient/v3/volumes.py @@ -435,6 +435,7 @@ class VolumeManager(base.ManagerWithFind): return self._create("/volumes/%s/metadata" % base.getid(volume), body, "metadata") + @api_versions.wraps("2.0") def delete_metadata(self, volume, keys): """Delete specified keys from volumes metadata. @@ -444,11 +445,28 @@ class VolumeManager(base.ManagerWithFind): response_list = [] for k in keys: resp, body = self._delete("/volumes/%s/metadata/%s" % - (base.getid(volume), k)) + (base.getid(volume), k)) response_list.append(resp) return common_base.ListWithMeta([], response_list) + @api_versions.wraps("3.15") + def delete_metadata(self, volume, keys): + """Delete specified keys from volumes metadata. + + :param volume: The :class:`Volume`. + :param keys: A list of keys to be removed. + """ + data = self._get("/volumes/%s/metadata" % base.getid(volume)) + metadata = data._info.get("metadata", {}) + if set(keys).issubset(metadata.keys()): + for k in keys: + metadata.pop(k) + body = {'metadata': metadata} + kwargs = {'headers': {'If-Match': data._checksum}} + return self._update("/volumes/%s/metadata" % base.getid(volume), + body, **kwargs) + def set_image_metadata(self, volume, metadata): """Set a volume's image metadata.