From 350dfb8b4a376607bcdbe7fb3d10a214dc0a3f94 Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Mon, 14 Sep 2015 14:51:44 -0300 Subject: [PATCH] Fix task_state field shown on API < 2.5 After Share Migration was included, the task_state field was added to be displayed on GET requests, but API versions prior to 2.5 should not see this field. This patch fixes that by using microversions. Closes-bug: #1494746 Change-Id: Ie755ae53fe8efdf1702a0ecbabf022a5fe4beb93 --- manila/api/views/shares.py | 6 +- manila/tests/api/v1/test_shares.py | 79 +++++++------------ .../services/share/v2/json/shares_client.py | 15 +++- 3 files changed, 43 insertions(+), 57 deletions(-) diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index 4bbaf6f4c2..e124ff9652 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -22,6 +22,7 @@ class ViewBuilder(common.ViewBuilder): _collection_name = 'shares' _detail_version_modifiers = [ "add_consistency_group_fields", + "add_task_state_field", "modify_share_type_field", ] @@ -65,7 +66,6 @@ class ViewBuilder(common.ViewBuilder): 'availability_zone': share.get('availability_zone'), 'created_at': share.get('created_at'), 'status': share.get('status'), - 'task_state': share.get('task_state'), 'name': share.get('display_name'), 'description': share.get('display_description'), 'project_id': share.get('project_id'), @@ -95,6 +95,10 @@ class ViewBuilder(common.ViewBuilder): share_dict['source_cgsnapshot_member_id'] = share.get( 'source_cgsnapshot_member_id') + @common.ViewBuilder.versioned_method("2.5", None, True) + def add_task_state_field(self, share_dict, share): + share_dict['task_state'] = share.get('task_state') + @common.ViewBuilder.versioned_method("2.6") def modify_share_type_field(self, share_dict, share): share_type = share.get('share_type_id') diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index a788b0f616..0f88b79745 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -101,7 +101,6 @@ class ShareApiTest(test.TestCase): 'snapshot_id': '2', 'share_network_id': None, 'status': 'fakestatus', - 'task_state': None, 'share_type': '1', 'volume_type': '1', 'is_public': False, @@ -691,17 +690,11 @@ class ShareApiTest(test.TestCase): def test_share_list_detail_with_search_opts_by_admin(self): self._share_list_detail_with_search_opts(use_admin_context=True) - def test_share_list_detail(self): - self.mock_object(share_api.API, 'get_all', - stubs.stub_share_get_all_by_project) - env = {'QUERY_STRING': 'name=Share+Test+Name'} - req = fakes.HTTPRequest.blank('/shares/detail', environ=env) - res_dict = self.controller.detail(req) - expected = { + def _list_detail_common_expected(self): + return { 'shares': [ { 'status': 'fakestatus', - 'task_state': None, 'description': 'displaydesc', 'export_location': 'fake_location', 'export_locations': ['fake_location', 'fake_location2'], @@ -732,57 +725,39 @@ class ShareApiTest(test.TestCase): } ] } + + def _list_detail_test_common(self, req, expected): + self.mock_object(share_api.API, 'get_all', + stubs.stub_share_get_all_by_project) + res_dict = self.controller.detail(req) self.assertEqual(expected, res_dict) self.assertEqual(res_dict['shares'][0]['volume_type'], res_dict['shares'][0]['share_type']) + def test_share_list_detail(self): + env = {'QUERY_STRING': 'name=Share+Test+Name'} + req = fakes.HTTPRequest.blank('/shares/detail', environ=env) + expected = self._list_detail_common_expected() + self._list_detail_test_common(req, expected) + def test_share_list_detail_with_consistency_group(self): - self.mock_object(share_api.API, 'get_all', - stubs.stub_share_get_all_by_project) env = {'QUERY_STRING': 'name=Share+Test+Name'} req = fakes.HTTPRequest.blank('/shares/detail', environ=env, version="2.4") - res_dict = self.controller.detail(req) - expected = { - 'shares': [ - { - 'status': 'fakestatus', - 'task_state': None, - 'description': 'displaydesc', - 'export_location': 'fake_location', - 'export_locations': ['fake_location', 'fake_location2'], - 'availability_zone': 'fakeaz', - 'name': 'displayname', - 'share_proto': 'FAKEPROTO', - 'metadata': {}, - 'project_id': 'fakeproject', - 'host': 'fakehost', - 'id': '1', - 'snapshot_id': '2', - 'share_network_id': None, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'size': 1, - 'share_type': '1', - 'volume_type': '1', - 'is_public': False, - 'consistency_group_id': None, - 'source_cgsnapshot_member_id': None, - 'links': [ - { - 'href': 'http://localhost/v1/fake/shares/1', - 'rel': 'self' - }, - { - 'href': 'http://localhost/fake/shares/1', - 'rel': 'bookmark' - } - ], - } - ] - } - self.assertEqual(expected, res_dict) - self.assertEqual(res_dict['shares'][0]['volume_type'], - res_dict['shares'][0]['share_type']) + expected = self._list_detail_common_expected() + expected['shares'][0]['consistency_group_id'] = None + expected['shares'][0]['source_cgsnapshot_member_id'] = None + self._list_detail_test_common(req, expected) + + def test_share_list_detail_with_task_state(self): + env = {'QUERY_STRING': 'name=Share+Test+Name'} + req = fakes.HTTPRequest.blank('/shares/detail', environ=env, + version="2.5", experimental=True) + expected = self._list_detail_common_expected() + expected['shares'][0]['consistency_group_id'] = None + expected['shares'][0]['source_cgsnapshot_member_id'] = None + expected['shares'][0]['task_state'] = None + self._list_detail_test_common(req, expected) def test_remove_invalid_options(self): ctx = context.RequestContext('fakeuser', 'fakeproject', is_admin=False) diff --git a/manila_tempest_tests/services/share/v2/json/shares_client.py b/manila_tempest_tests/services/share/v2/json/shares_client.py index da72ffc114..3387a4f1fe 100644 --- a/manila_tempest_tests/services/share/v2/json/shares_client.py +++ b/manila_tempest_tests/services/share/v2/json/shares_client.py @@ -220,8 +220,15 @@ class SharesV2Client(shares_client.SharesClient): """Get detailed list of shares w/o filters.""" return self.list_shares(detailed=True, params=params, version=version) - def get_share(self, share_id, version=LATEST_MICROVERSION): - resp, body = self.get("shares/%s" % share_id, version=version) + def get_share(self, share_id, version=LATEST_MICROVERSION, + experimental=False): + headers = None + extra_headers = False + if experimental: + headers = EXPERIMENTAL + extra_headers = True + resp, body = self.get("shares/%s" % share_id, version=version, + headers=headers, extra_headers=extra_headers) self.expected_success(200, resp.status) return self._parse_resp(body) @@ -493,12 +500,12 @@ class SharesV2Client(shares_client.SharesClient): def wait_for_migration_completed(self, share_id, dest_host): """Waits for a share to migrate to a certain host.""" - share = self.get_share(share_id) + share = self.get_share(share_id, "2.5", True) migration_timeout = CONF.share.migration_timeout start = int(time.time()) while share['task_state'] != 'migration_success': time.sleep(self.build_interval) - share = self.get_share(share_id) + share = self.get_share(share_id, "2.5", True) if share['task_state'] == 'migration_success': return share elif share['task_state'] == 'migration_error':