openstack/manila
Code Issues Proposed changes

Add user_id echo in manila show/create/manage API

Browse Source 
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
This commit is contained in:
zhongjun 2016-04-21 16:19:33 +08:00
parent 5fe5fa8b2f
commit 4334a067f0
9 changed files with 169 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
manila/api/openstack/api_version_request.py
View File

@ -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

4
manila/api/openstack/rest_api_version_history.rst
View File

@ -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.

5
manila/api/views/shares.py
View File

@ -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]

119
manila/tests/api/v2/test_shares.py
View File

@ -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')

2
manila_tempest_tests/config.py
View File

@ -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",

21
manila_tempest_tests/tests/api/admin/test_share_manage.py
View File

@ -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.

6
manila_tempest_tests/tests/api/test_shares.py
View File

@ -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'])

15
manila_tempest_tests/tests/api/test_shares_actions.py
View File

@ -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}

3
releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml Normal file
View File

@ -0,0 +1,3 @@
---
features:
- User ID is added to the JSON response of the /shares APIs.