From 4334a067f0ba11c2bd23d9c4c8e6b4fe5276e270 Mon Sep 17 00:00:00 2001 From: zhongjun Date: Thu, 21 Apr 2016 16:19:33 +0800 Subject: [PATCH] Add user_id echo in manila show/create/manage API Add "user_id" detail when we run command "manila show/create/manage ...". Make the operator know which user created this share. APIImpact Closes-Bug: #1562846 Change-Id: I2858c7f63182288f354b96448f0970d3642d4bf7 --- manila/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 4 + manila/api/views/shares.py | 5 + manila/tests/api/v2/test_shares.py | 119 +++++++++++++++++- manila_tempest_tests/config.py | 2 +- .../tests/api/admin/test_share_manage.py | 21 +++- manila_tempest_tests/tests/api/test_shares.py | 6 + .../tests/api/test_shares_actions.py | 15 ++- .../add-user-id-echo-8f42db469b27ff14.yaml | 3 + 9 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index fef44435..02be4bb1 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -65,13 +65,14 @@ REST_API_VERSION_HISTORY = """ 'migration_get_progress', 'migration_complete' APIs, renamed 'migrate_share' to 'migration_start' and added notify parameter to 'migration_start'. + * 2.16 - Add user_id in share show/create/manage API. """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # the minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.15" +_MAX_API_VERSION = "2.16" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 59d2d419..e687b90f 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -106,3 +106,7 @@ ____ Added Share migration 'migration_cancel', 'migration_get_progress', 'migration_complete' APIs, renamed 'migrate_share' to 'migration_start' and added notify parameter to 'migration_start'. + +2.16 +---- + Add user_id in share show/create/manage API. diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index f4606a9d..ed67ddec 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -28,6 +28,7 @@ class ViewBuilder(common.ViewBuilder): "remove_export_locations", "add_access_rules_status_field", "add_replication_fields", + "add_user_id", ] def summary_list(self, request, shares): @@ -144,6 +145,10 @@ class ViewBuilder(common.ViewBuilder): share_dict['replication_type'] = share.get('replication_type') share_dict['has_replicas'] = share['has_replicas'] + @common.ViewBuilder.versioned_method("2.16") + def add_user_id(self, share_dict, share): + share_dict['user_id'] = share.get('user_id') + def _list_view(self, func, request, shares): """Provide a view for a list of shares.""" shares_list = [func(request, share)['share'] for share in shares] diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index d24efca2..5d0780ce 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -258,6 +258,32 @@ class ShareAPITest(test.TestCase): self.assertEqual("fakenetid", create_mock.call_args[1]['share_network_id']) + @ddt.data("2.15", "2.16") + def test_share_create_original_with_user_id(self, microversion): + self.mock_object(share_api.API, 'create', self.create_mock) + body = {"share": copy.deepcopy(self.share)} + req = fakes.HTTPRequest.blank('/shares', version=microversion) + + res_dict = self.controller.create(req, body) + + expected = self._get_expected_share_detailed_response(self.share) + if api_version.APIVersionRequest(microversion) >= ( + api_version.APIVersionRequest("2.16")): + expected['share']['user_id'] = 'fakeuser' + else: + self.assertNotIn('user_id', expected['share']) + expected['share']['task_state'] = None + expected['share']['consistency_group_id'] = None + expected['share']['source_cgsnapshot_member_id'] = None + expected['share']['replication_type'] = None + expected['share']['share_type_name'] = None + expected['share']['has_replicas'] = False + expected['share']['access_rules_status'] = 'active' + expected['share'].pop('export_location') + expected['share'].pop('export_locations') + + self.assertEqual(expected, res_dict) + @ddt.data('2.6', '2.7', '2.14', '2.15') def test_migration_start(self, version): share = db_utils.create_share() @@ -789,6 +815,30 @@ class ShareAPITest(test.TestCase): expected['share']['task_state'] = None self.assertEqual(expected, res_dict) + @ddt.data("2.15", "2.16") + def test_share_show_with_user_id(self, microversion): + req = fakes.HTTPRequest.blank('/shares/1', version=microversion) + + res_dict = self.controller.show(req, '1') + + expected = self._get_expected_share_detailed_response() + if api_version.APIVersionRequest(microversion) >= ( + api_version.APIVersionRequest("2.16")): + expected['share']['user_id'] = 'fakeuser' + else: + self.assertNotIn('user_id', expected['share']) + expected['share']['consistency_group_id'] = None + expected['share']['source_cgsnapshot_member_id'] = None + expected['share']['share_type_name'] = None + expected['share']['task_state'] = None + expected['share']['access_rules_status'] = 'active' + expected['share'].pop('export_location') + expected['share'].pop('export_locations') + expected['share']['replication_type'] = None + expected['share']['has_replicas'] = False + + self.assertEqual(expected, res_dict) + def test_share_show_admin(self): req = fakes.HTTPRequest.blank('/shares/1', use_admin_context=True) expected = self._get_expected_share_detailed_response(admin=True) @@ -1865,14 +1915,52 @@ class ShareManageTest(test.TestCase): def test_share_manage_with_is_public(self, data): self._test_share_manage(data, "2.8") + def test_share_manage_with_user_id(self): + self._test_share_manage(get_fake_manage_body( + name='foo', description='bar', is_public=True), "2.16") + def _test_share_manage(self, data, version): - self._setup_manage_mocks() - return_share = { - 'share_type_id': '', 'id': 'fake', - 'instance': {}, + expected = { + 'share': { + 'status': 'fakestatus', + 'description': 'displaydesc', + 'availability_zone': 'fakeaz', + 'name': 'displayname', + 'share_proto': 'FAKEPROTO', + 'metadata': {}, + 'project_id': 'fakeproject', + 'host': 'fakehost', + 'id': 'fake', + 'snapshot_id': '2', + 'share_network_id': None, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'size': 1, + 'share_type_name': None, + 'share_server_id': 'fake_share_server_id', + 'share_type': '1', + 'volume_type': '1', + 'is_public': False, + 'consistency_group_id': None, + 'source_cgsnapshot_member_id': None, + 'snapshot_support': True, + 'task_state': None, + 'links': [ + { + 'href': 'http://localhost/v1/fake/shares/fake', + 'rel': 'self' + }, + { + 'href': 'http://localhost/fake/shares/fake', + 'rel': 'bookmark' + } + ], + } } + self._setup_manage_mocks() + return_share = mock.Mock( + return_value=stubs.stub_share('fake', instance={})) self.mock_object( - share_api.API, 'manage', mock.Mock(return_value=return_share)) + share_api.API, 'manage', return_share) share = { 'host': data['share']['service_host'], 'export_location': data['share']['export_path'], @@ -1883,6 +1971,25 @@ class ShareManageTest(test.TestCase): } driver_options = data['share'].get('driver_options', {}) + if (api_version.APIVersionRequest(version) <= + api_version.APIVersionRequest('2.8')): + expected['share']['export_location'] = 'fake_location' + expected['share']['export_locations'] = ( + ['fake_location', 'fake_location2']) + + if (api_version.APIVersionRequest(version) >= + api_version.APIVersionRequest('2.10')): + expected['share']['access_rules_status'] = ( + constants.STATUS_ACTIVE) + if (api_version.APIVersionRequest(version) >= + api_version.APIVersionRequest('2.11')): + expected['share']['has_replicas'] = False + expected['share']['replication_type'] = None + + if (api_version.APIVersionRequest(version) >= + api_version.APIVersionRequest('2.16')): + expected['share']['user_id'] = 'fakeuser' + if (api_version.APIVersionRequest(version) >= api_version.APIVersionRequest('2.8')): share['is_public'] = data['share']['is_public'] @@ -1894,7 +2001,9 @@ class ShareManageTest(test.TestCase): share_api.API.manage.assert_called_once_with( mock.ANY, share, driver_options) + self.assertIsNotNone(actual_result) + self.assertEqual(expected, actual_result) self.mock_policy_check.assert_called_once_with( req.environ['manila.context'], self.resource_name, 'manage') diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py index 94ffb5f3..4e51ad58 100644 --- a/manila_tempest_tests/config.py +++ b/manila_tempest_tests/config.py @@ -36,7 +36,7 @@ ShareGroup = [ help="The minimum api microversion is configured to be the " "value of the minimum microversion supported by Manila."), cfg.StrOpt("max_api_microversion", - default="2.15", + default="2.16", help="The maximum api microversion is configured to be the " "value of the latest microversion supported by Manila."), cfg.StrOpt("region", diff --git a/manila_tempest_tests/tests/api/admin/test_share_manage.py b/manila_tempest_tests/tests/api/admin/test_share_manage.py index f1ec4024..31269a70 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_manage.py +++ b/manila_tempest_tests/tests/api/admin/test_share_manage.py @@ -83,13 +83,22 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): data.append(creation_data) if utils.is_microversion_ge(CONF.share.max_api_microversion, "2.8"): data.append(creation_data) + if utils.is_microversion_ge(CONF.share.max_api_microversion, "2.16"): + data.append(creation_data) shares_created = cls.create_shares(data) cls.shares = [] # Load all share data (host, etc.) for share in shares_created: # Unmanage shares from manila - cls.shares.append(cls.shares_client.get_share(share['id'])) + get_share = cls.shares_v2_client.get_share(share['id']) + if utils.is_microversion_ge( + CONF.share.max_api_microversion, "2.9"): + get_share["export_locations"] = ( + cls.shares_v2_client.list_share_export_locations( + share["id"]) + ) + cls.shares.append(get_share) cls.shares_client.unmanage_share(share['id']) cls.shares_client.wait_for_resource_deletion( share_id=share['id']) @@ -138,6 +147,11 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): else: self.assertFalse(managed_share['is_public']) + if utils.is_microversion_ge(version, "2.16"): + self.assertEqual(share['user_id'], managed_share['user_id']) + else: + self.assertNotIn('user_id', managed_share) + # Delete share self.shares_v2_client.delete_share(managed_share['id']) self.shares_v2_client.wait_for_resource_deletion( @@ -156,6 +170,11 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): def test_manage_with_is_public_True(self): self._test_manage(share=self.shares[3], is_public=True, version="2.8") + @test.attr(type=["gate", "smoke"]) + @base.skip_if_microversion_not_supported("2.16") + def test_manage_show_user_id(self): + self._test_manage(share=self.shares[4], version="2.16") + @test.attr(type=["gate", "smoke"]) def test_manage(self): # After 'unmanage' operation, share instance should be deleted. diff --git a/manila_tempest_tests/tests/api/test_shares.py b/manila_tempest_tests/tests/api/test_shares.py index 2057d220..760f2f07 100644 --- a/manila_tempest_tests/tests/api/test_shares.py +++ b/manila_tempest_tests/tests/api/test_shares.py @@ -84,6 +84,12 @@ class SharesNFSTest(base.BaseSharesTest): detailed_elements.add('replication_type') self.assertTrue(detailed_elements.issubset(share.keys()), msg) + # In v 2.16 and beyond, we add user_id in show/create/manage + # share echo. + if utils.is_microversion_supported('2.16'): + detailed_elements.add('user_id') + self.assertTrue(detailed_elements.issubset(share.keys()), msg) + # Delete share self.shares_v2_client.delete_share(share['id']) self.shares_v2_client.wait_for_resource_deletion(share_id=share['id']) diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py index 1f37c2a4..b00cef4f 100644 --- a/manila_tempest_tests/tests/api/test_shares_actions.py +++ b/manila_tempest_tests/tests/api/test_shares_actions.py @@ -99,6 +99,8 @@ class SharesActionsTest(base.BaseSharesTest): expected_keys.append("access_rules_status") if utils.is_microversion_ge(version, '2.11'): expected_keys.append("replication_type") + if utils.is_microversion_ge(version, '2.16'): + expected_keys.append("user_id") actual_keys = list(share.keys()) [self.assertIn(key, actual_keys) for key in expected_keys] @@ -150,6 +152,11 @@ class SharesActionsTest(base.BaseSharesTest): def test_get_share_with_replication_type_key(self): self._get_share('2.11') + @test.attr(type=["gate", ]) + @utils.skip_if_microversion_not_supported('2.16') + def test_get_share_with_user_id(self): + self._get_share('2.16') + @test.attr(type=["gate", ]) def test_list_shares(self): @@ -192,7 +199,8 @@ class SharesActionsTest(base.BaseSharesTest): keys.append("access_rules_status") if utils.is_microversion_ge(version, '2.11'): keys.append("replication_type") - + if utils.is_microversion_ge(version, '2.16'): + keys.append("user_id") [self.assertIn(key, sh.keys()) for sh in shares for key in keys] # our shares in list and have no duplicates @@ -234,6 +242,11 @@ class SharesActionsTest(base.BaseSharesTest): def test_list_shares_with_detail_replication_type_key(self): self._list_shares_with_detail('2.11') + @test.attr(type=["gate", ]) + @utils.skip_if_microversion_not_supported('2.16') + def test_list_shares_with_user_id(self): + self._list_shares_with_detail('2.16') + @test.attr(type=["gate", ]) def test_list_shares_with_detail_filter_by_metadata(self): filters = {'metadata': self.metadata} diff --git a/releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml b/releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml new file mode 100644 index 00000000..e6caa034 --- /dev/null +++ b/releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml @@ -0,0 +1,3 @@ +--- +features: + - User ID is added to the JSON response of the /shares APIs.