openstack/manila
Code Issues Proposed changes

Update export location retrieval APIs

Browse Source 
Update the export location retrieval APIs for shares and share
instances to do the following:

1. Restore the API-to-view-builder calling convention of
index-->summary and show-->detail. In so doing, modify which
values are returned by the list commands (currently, all but
the timestamps). The admin context from the request determines
whether the admin-only values are returned.

2. Report the UUID field from the export location table as
'id' to be consistent will all other objects returned via the
Manila REST API.

3. Add the preferred flag to the output of the API. Drivers
can report preferred:True or preferred:False in their export
location metadata, and this standard flag will be returned
via the REST interface, like this:

+-------------------+--------------------------------------+
| Property          | Value                                |
+-------------------+--------------------------------------+
| is_admin_only     | False                                |
| uuid              | df828d44-0b04-47fa-8ee5-516ffc199ca7 |
| share_instance_id | 1b40e873-331e-4e1c-ab53-38ec95b3bfcc |
| path              | 10.0.0.100:/share_1b40e873           |
| created_at        | 2016-02-18T21:12:51.000000           |
| updated_at        | 2016-02-18T21:12:51.000000           |
| preferred         | True                                 |
+-------------------+--------------------------------------+

APIImpact
Implements: blueprint update-export-location-retrieval-apis
Change-Id: Ia63477d4f3e28ab4f53d7b9d51756cc798c977b9
This commit is contained in:
Clinton Knight 2016-02-18 16:43:17 -05:00
parent f4e62f3348
commit 55777cfb4e
10 changed files with 270 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -60,13 +60,15 @@ REST_API_VERSION_HISTORY = """
* 2.11 - Share Replication support
* 2.12 - Manage/unmanage snapshot API.
* 2.13 - Add "cephx" auth type to allow_access
* 2.14 - 'Preferred' attribute in export location metadata
"""
# 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.13"
_MAX_API_VERSION = "2.14"
DEFAULT_API_VERSION = _MIN_API_VERSION

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

@ -93,3 +93,10 @@ user documentation.
2.13
----
Add 'cephx' authentication type for the CephFS Native driver.
2.14
____
Added attribute 'preferred' to export locations. Drivers may use this
field to identify which export locations are most efficient and should be
used preferentially by clients. Also, change 'uuid' field to 'id', move
timestamps to detail view, and return all non-admin fields to users.

21
manila/api/v2/share_export_locations.py
View File

@ -44,14 +44,9 @@ class ShareExportLocationController(wsgi.Controller):
context = req.environ['manila.context']
self._verify_share(context, share_id)
if context.is_admin:
export_locations = db_api.share_export_locations_get_by_share_id(
context, share_id, include_admin_only=True)
return self._view_builder.detail_list(export_locations)
else:
export_locations = db_api.share_export_locations_get_by_share_id(
context, share_id, include_admin_only=False)
return self._view_builder.summary_list(export_locations)
export_locations = db_api.share_export_locations_get_by_share_id(
context, share_id, include_admin_only=context.is_admin)
return self._view_builder.summary_list(req, export_locations)
@wsgi.Controller.api_version('2.9')
@wsgi.Controller.authorize
@ -60,19 +55,17 @@ class ShareExportLocationController(wsgi.Controller):
context = req.environ['manila.context']
self._verify_share(context, share_id)
try:
el = db_api.share_export_location_get_by_uuid(
export_location = db_api.share_export_location_get_by_uuid(
context, export_location_uuid)
except exception.ExportLocationNotFound:
msg = _("Export location '%s' not found.") % export_location_uuid
raise exc.HTTPNotFound(explanation=msg)
if context.is_admin:
return self._view_builder.detail(el)
else:
if not el.is_admin_only:
return self._view_builder.summary(el)
if export_location.is_admin_only and not context.is_admin:
raise exc.HTTPForbidden()
return self._view_builder.detail(req, export_location)
def create_resource():
return wsgi.Resource(ShareExportLocationController())

6
manila/api/v2/share_instance_export_locations.py
View File

@ -47,7 +47,7 @@ class ShareInstanceExportLocationController(wsgi.Controller):
export_locations = (
db_api.share_export_locations_get_by_share_instance_id(
context, share_instance_id))
return self._view_builder.detail_list(export_locations)
return self._view_builder.summary_list(req, export_locations)
@wsgi.Controller.api_version('2.9')
@wsgi.Controller.authorize
@ -56,9 +56,9 @@ class ShareInstanceExportLocationController(wsgi.Controller):
context = req.environ['manila.context']
self._verify_share_instance(context, share_instance_id)
try:
el = db_api.share_export_location_get_by_uuid(
export_location = db_api.share_export_location_get_by_uuid(
context, export_location_uuid)
return self._view_builder.detail(el)
return self._view_builder.detail(req, export_location)
except exception.ExportLocationNotFound as e:
raise exc.HTTPNotFound(explanation=six.text_type(e))

69
manila/api/views/export_locations.py
View File

@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from oslo_utils import strutils
from manila.api import common
@ -21,51 +23,60 @@ class ViewBuilder(common.ViewBuilder):
_collection_name = "export_locations"
def _get_export_location_view(self, export_location, detail=False):
_detail_version_modifiers = [
'add_preferred_path_attribute',
]
def _get_export_location_view(self, request, export_location,
detail=False):
context = request.environ['manila.context']
view = {
'uuid': export_location['uuid'],
'id': export_location['uuid'],
'path': export_location['path'],
'created_at': export_location['created_at'],
'updated_at': export_location['updated_at'],
}
# TODO(vponomaryov): include metadata keys here as export location
# attributes when such appear.
#
# Example having export_location['el_metadata'] as following:
#
# {'speed': '1Gbps', 'access': 'rw'}
#
# or
#
# {'speed': '100Mbps', 'access': 'ro'}
#
# view['speed'] = export_location['el_metadata'].get('speed')
# view['access'] = export_location['el_metadata'].get('access')
if detail:
view['share_instance_id'] = export_location['share_instance_id']
self.update_versioned_resource_dict(request, view, export_location)
if context.is_admin:
view['share_instance_id'] = export_location[
'share_instance_id']
view['is_admin_only'] = export_location['is_admin_only']
if detail:
view['created_at'] = export_location['created_at']
view['updated_at'] = export_location['updated_at']
return {'export_location': view}
def summary(self, export_location):
def summary(self, request, export_location):
"""Summary view of a single export location."""
return self._get_export_location_view(export_location, detail=False)
return self._get_export_location_view(request, export_location,
detail=False)
def detail(self, export_location):
def detail(self, request, export_location):
"""Detailed view of a single export location."""
return self._get_export_location_view(export_location, detail=True)
return self._get_export_location_view(request, export_location,
detail=True)
def _list_export_locations(self, export_locations, detail=False):
def _list_export_locations(self, request, export_locations, detail=False):
"""View of export locations list."""
view_method = self.detail if detail else self.summary
return {self._collection_name: [
view_method(export_location)['export_location']
view_method(request, export_location)['export_location']
for export_location in export_locations
]}
def detail_list(self, export_locations):
def detail_list(self, request, export_locations):
"""Detailed View of export locations list."""
return self._list_export_locations(export_locations, detail=True)
return self._list_export_locations(request, export_locations,
detail=True)
def summary_list(self, export_locations):
def summary_list(self, request, export_locations):
"""Summary View of export locations list."""
return self._list_export_locations(export_locations, detail=False)
return self._list_export_locations(request, export_locations,
detail=False)
@common.ViewBuilder.versioned_method('2.14')
def add_preferred_path_attribute(self, view_dict, export_location):
view_dict['preferred'] = strutils.bool_from_string(
export_location['el_metadata'].get('preferred'))

83
manila/tests/api/v2/test_share_export_locations.py
View File

@ -54,9 +54,39 @@ class ShareExportLocationsAPITest(test.TestCase):
db.share_export_locations_update(
self.ctxt['admin'], self.share_instance_id, paths, False)
@ddt.data({'role': 'admin', 'version': '2.9'},
{'role': 'user', 'version': '2.9'},
{'role': 'admin', 'version': '2.13'},
{'role': 'user', 'version': '2.13'})
@ddt.unpack
def test_list_and_show(self, role, version):
summary_keys = ['id', 'path']
admin_summary_keys = summary_keys + [
'share_instance_id', 'is_admin_only']
detail_keys = summary_keys + ['created_at', 'updated_at']
admin_detail_keys = admin_summary_keys + ['created_at', 'updated_at']
self._test_list_and_show(role, version, summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys)
@ddt.data('admin', 'user')
def test_list_and_show(self, role):
req = self._get_request(use_admin_context=(role == 'admin'))
def test_list_and_show_with_preferred_flag(self, role):
summary_keys = ['id', 'path', 'preferred']
admin_summary_keys = summary_keys + [
'share_instance_id', 'is_admin_only']
detail_keys = summary_keys + ['created_at', 'updated_at']
admin_detail_keys = admin_summary_keys + ['created_at', 'updated_at']
self._test_list_and_show(role, '2.14', summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys)
def _test_list_and_show(self, role, version, summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys):
req = self._get_request(version=version,
use_admin_context=(role == 'admin'))
index_result = self.controller.index(req, self.share['id'])
self.assertIn('export_locations', index_result)
@ -64,24 +94,33 @@ class ShareExportLocationsAPITest(test.TestCase):
self.assertEqual(3, len(index_result['export_locations']))
for index_el in index_result['export_locations']:
self.assertIn('uuid', index_el)
self.assertIn('id', index_el)
show_result = self.controller.show(
req, self.share['id'], index_el['uuid'])
req, self.share['id'], index_el['id'])
self.assertIn('export_location', show_result)
self.assertEqual(1, len(show_result))
expected_keys = [
'created_at', 'updated_at', 'uuid', 'path',
]
if role == 'admin':
expected_keys.extend(['share_instance_id', 'is_admin_only'])
for el in (index_el, show_result['export_location']):
self.assertEqual(len(expected_keys), len(el))
for key in expected_keys:
self.assertIn(key, el)
for key in expected_keys:
self.assertEqual(
index_el[key], show_result['export_location'][key])
show_el = show_result['export_location']
# Check summary keys in index result & detail keys in show result
if role == 'admin':
self.assertEqual(len(admin_summary_keys), len(index_el))
for key in admin_summary_keys:
self.assertIn(key, index_el)
self.assertEqual(len(admin_detail_keys), len(show_el))
for key in admin_detail_keys:
self.assertIn(key, show_el)
else:
self.assertEqual(len(summary_keys), len(index_el))
for key in summary_keys:
self.assertIn(key, index_el)
self.assertEqual(len(detail_keys), len(show_el))
for key in detail_keys:
self.assertIn(key, show_el)
# Ensure keys common to index & show results have matching values
for key in summary_keys:
self.assertEqual(index_el[key], show_el[key])
def test_list_export_locations_share_not_found(self):
self.assertRaises(
@ -92,11 +131,11 @@ class ShareExportLocationsAPITest(test.TestCase):
def test_show_export_location_share_not_found(self):
index_result = self.controller.index(self.req, self.share['id'])
el_uuid = index_result['export_locations'][0]['uuid']
el_id = index_result['export_locations'][0]['id']
self.assertRaises(
exc.HTTPNotFound,
self.controller.show,
self.req, 'inexistent_share_id', el_uuid,
self.req, 'inexistent_share_id', el_id,
)
def test_show_export_location_not_found(self):
@ -115,18 +154,18 @@ class ShareExportLocationsAPITest(test.TestCase):
db.share_export_locations_update(
self.ctxt['admin'], self.share_instance_id, el_data, True)
index_result = self.controller.index(self.req, self.share['id'])
el_uuid = index_result['export_locations'][0]['uuid']
el_id = index_result['export_locations'][0]['id']
# Not found for member
member_req = self._get_request(use_admin_context=False)
self.assertRaises(
exc.HTTPForbidden,
self.controller.show,
member_req, self.share['id'], el_uuid,
member_req, self.share['id'], el_id,
)
# Ok for admin
el = self.controller.show(self.req, self.share['id'], el_uuid)
el = self.controller.show(self.req, self.share['id'], el_id)
for k, v in el.items():
self.assertEqual(v, el[k])
@ -148,5 +187,5 @@ class ShareExportLocationsAPITest(test.TestCase):
self.controller.show,
self._get_request(version),
self.share['id'],
index_result['export_locations'][0]['uuid']
index_result['export_locations'][0]['id']
)

76
manila/tests/api/v2/test_share_instance_export_locations.py
View File

@ -54,9 +54,39 @@ class ShareInstanceExportLocationsAPITest(test.TestCase):
db.share_export_locations_update(
self.ctxt['admin'], self.share_instance_id, paths, False)
@ddt.data({'role': 'admin', 'version': '2.9'},
{'role': 'user', 'version': '2.9'},
{'role': 'admin', 'version': '2.13'},
{'role': 'user', 'version': '2.13'})
@ddt.unpack
def test_list_and_show(self, role, version):
summary_keys = ['id', 'path']
admin_summary_keys = summary_keys + [
'share_instance_id', 'is_admin_only']
detail_keys = summary_keys + ['created_at', 'updated_at']
admin_detail_keys = admin_summary_keys + ['created_at', 'updated_at']
self._test_list_and_show(role, version, summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys)
@ddt.data('admin', 'user')
def test_list_and_show(self, role):
req = self._get_request(use_admin_context=(role == 'admin'))
def test_list_and_show_with_preferred_flag(self, role):
summary_keys = ['id', 'path', 'preferred']
admin_summary_keys = summary_keys + [
'share_instance_id', 'is_admin_only']
detail_keys = summary_keys + ['created_at', 'updated_at']
admin_detail_keys = admin_summary_keys + ['created_at', 'updated_at']
self._test_list_and_show(role, '2.14', summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys)
def _test_list_and_show(self, role, version, summary_keys, detail_keys,
admin_summary_keys, admin_detail_keys):
req = self._get_request(version=version,
use_admin_context=(role == 'admin'))
index_result = self.controller.index(req, self.share_instance_id)
self.assertIn('export_locations', index_result)
@ -64,23 +94,33 @@ class ShareInstanceExportLocationsAPITest(test.TestCase):
self.assertEqual(3, len(index_result['export_locations']))
for index_el in index_result['export_locations']:
self.assertIn('uuid', index_el)
self.assertIn('id', index_el)
show_result = self.controller.show(
req, self.share_instance_id, index_el['uuid'])
req, self.share_instance_id, index_el['id'])
self.assertIn('export_location', show_result)
self.assertEqual(1, len(show_result))
expected_keys = (
'created_at', 'updated_at', 'uuid', 'path',
'share_instance_id', 'is_admin_only',
)
for el in (index_el, show_result['export_location']):
self.assertEqual(len(expected_keys), len(el))
for key in expected_keys:
self.assertIn(key, el)
for key in expected_keys:
self.assertEqual(
index_el[key], show_result['export_location'][key])
show_el = show_result['export_location']
# Check summary keys in index result & detail keys in show result
if role == 'admin':
self.assertEqual(len(admin_summary_keys), len(index_el))
for key in admin_summary_keys:
self.assertIn(key, index_el)
self.assertEqual(len(admin_detail_keys), len(show_el))
for key in admin_detail_keys:
self.assertIn(key, show_el)
else:
self.assertEqual(len(summary_keys), len(index_el))
for key in summary_keys:
self.assertIn(key, index_el)
self.assertEqual(len(detail_keys), len(show_el))
for key in detail_keys:
self.assertIn(key, show_el)
# Ensure keys common to index & show results have matching values
for key in summary_keys:
self.assertEqual(index_el[key], show_el[key])
def test_list_export_locations_share_instance_not_found(self):
self.assertRaises(
@ -91,12 +131,12 @@ class ShareInstanceExportLocationsAPITest(test.TestCase):
def test_show_export_location_share_instance_not_found(self):
index_result = self.controller.index(self.req, self.share_instance_id)
el_uuid = index_result['export_locations'][0]['uuid']
el_id = index_result['export_locations'][0]['id']
self.assertRaises(
exc.HTTPNotFound,
self.controller.show,
self.req, 'inexistent_share_id', el_uuid,
self.req, 'inexistent_share_id', el_id,
)
@ddt.data('1.0', '2.0', '2.8')
@ -117,5 +157,5 @@ class ShareInstanceExportLocationsAPITest(test.TestCase):
self.controller.show,
self._get_request(version),
self.share_instance_id,
index_result['export_locations'][0]['uuid']
index_result['export_locations'][0]['id']
)

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.13",
default="2.14",
help="The maximum api microversion is configured to be the "
"value of the latest microversion supported by Manila."),
cfg.StrOpt("region",

121
manila_tempest_tests/tests/api/admin/test_export_locations.py
View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
from oslo_utils import timeutils
from oslo_utils import uuidutils
import six
@ -21,11 +22,14 @@ from tempest import test
from manila_tempest_tests import clients_share as clients
from manila_tempest_tests.tests.api import base
from manila_tempest_tests import utils
CONF = config.CONF
LATEST_MICROVERSION = CONF.share.max_api_microversion
@base.skip_if_microversion_not_supported("2.9")
@ddt.ddt
class ExportLocationsTest(base.BaseSharesAdminTest):
@classmethod
@ -38,42 +42,82 @@ class ExportLocationsTest(base.BaseSharesAdminTest):
cls.share_instances = cls.shares_v2_client.get_instances_of_share(
cls.share['id'])
def _verify_export_location_structure(self, export_locations,
role='admin'):
expected_keys = [
'created_at', 'updated_at', 'path', 'uuid',
]
if role == 'admin':
expected_keys.extend(['is_admin_only', 'share_instance_id'])
def _verify_export_location_structure(
self, export_locations, role='admin', version=LATEST_MICROVERSION,
format='summary'):
# Determine which keys to expect based on role, version and format
summary_keys = ['id', 'path']
if utils.is_microversion_ge(version, '2.14'):
summary_keys += ['preferred']
admin_summary_keys = summary_keys + [
'share_instance_id', 'is_admin_only']
detail_keys = summary_keys + ['created_at', 'updated_at']
admin_detail_keys = admin_summary_keys + ['created_at', 'updated_at']
if format == 'summary':
if role == 'admin':
expected_keys = admin_summary_keys
else:
expected_keys = summary_keys
else:
if role == 'admin':
expected_keys = admin_detail_keys
else:
expected_keys = detail_keys
if not isinstance(export_locations, (list, tuple, set)):
export_locations = (export_locations, )
for export_location in export_locations:
# Check that the correct keys are present
self.assertEqual(len(expected_keys), len(export_location))
for key in expected_keys:
self.assertIn(key, export_location)
# Check the format of ever-present summary keys
self.assertTrue(uuidutils.is_uuid_like(export_location['id']))
self.assertTrue(isinstance(export_location['path'],
six.string_types))
if utils.is_microversion_ge(version, '2.14'):
self.assertIn(export_location['preferred'], (True, False))
if role == 'admin':
self.assertIn(export_location['is_admin_only'], (True, False))
self.assertTrue(
uuidutils.is_uuid_like(
export_location['share_instance_id']))
self.assertTrue(uuidutils.is_uuid_like(export_location['uuid']))
self.assertTrue(
isinstance(export_location['path'], six.string_types))
for time in (export_location['created_at'],
export_location['updated_at']):
# If var 'time' has incorrect value then ValueError exception
# is expected to be raised. So, just try parse it making
# assertion that it has proper date value.
timeutils.parse_strtime(time)
self.assertTrue(uuidutils.is_uuid_like(
export_location['share_instance_id']))
# Check the format of the detail keys
if format == 'detail':
for time in (export_location['created_at'],
export_location['updated_at']):
# If var 'time' has incorrect value then ValueError
# exception is expected to be raised. So, just try parse
# it making assertion that it has proper date value.
timeutils.parse_strtime(time)
@test.attr(type=["gate", ])
@utils.skip_if_microversion_not_supported('2.13')
def test_list_share_export_locations(self):
export_locations = self.admin_client.list_share_export_locations(
self.share['id'])
self.share['id'], version='2.13')
self._verify_export_location_structure(export_locations)
self._verify_export_location_structure(export_locations,
version='2.13')
@test.attr(type=["gate", ])
@utils.skip_if_microversion_not_supported('2.14')
def test_list_share_export_locations_with_preferred_flag(self):
export_locations = self.admin_client.list_share_export_locations(
self.share['id'], version='2.14')
self._verify_export_location_structure(export_locations,
version='2.14')
@test.attr(type=["gate", ])
def test_get_share_export_location(self):
@ -82,15 +126,15 @@ class ExportLocationsTest(base.BaseSharesAdminTest):
for export_location in export_locations:
el = self.admin_client.get_share_export_location(
self.share['id'], export_location['uuid'])
self._verify_export_location_structure(el)
self.share['id'], export_location['id'])
self._verify_export_location_structure(el, format='detail')
@test.attr(type=["gate", ])
def test_list_share_export_locations_by_member(self):
export_locations = self.member_client.list_share_export_locations(
self.share['id'])
self._verify_export_location_structure(export_locations, 'member')
self._verify_export_location_structure(export_locations, role='member')
@test.attr(type=["gate", ])
def test_get_share_export_location_by_member(self):
@ -101,16 +145,29 @@ class ExportLocationsTest(base.BaseSharesAdminTest):
if export_location['is_admin_only']:
continue
el = self.member_client.get_share_export_location(
self.share['id'], export_location['uuid'])
self._verify_export_location_structure(el, 'member')
self.share['id'], export_location['id'])
self._verify_export_location_structure(el, role='member',
format='detail')
@test.attr(type=["gate", ])
@utils.skip_if_microversion_not_supported('2.13')
def test_list_share_instance_export_locations(self):
for share_instance in self.share_instances:
export_locations = (
self.admin_client.list_share_instance_export_locations(
share_instance['id']))
self._verify_export_location_structure(export_locations)
share_instance['id'], version='2.13'))
self._verify_export_location_structure(export_locations,
version='2.13')
@test.attr(type=["gate", ])
@utils.skip_if_microversion_not_supported('2.14')
def test_list_share_instance_export_locations_with_preferred_flag(self):
for share_instance in self.share_instances:
export_locations = (
self.admin_client.list_share_instance_export_locations(
share_instance['id'], version='2.14'))
self._verify_export_location_structure(export_locations,
version='2.14')
@test.attr(type=["gate", ])
def test_get_share_instance_export_location(self):
@ -120,8 +177,8 @@ class ExportLocationsTest(base.BaseSharesAdminTest):
share_instance['id']))
for el in export_locations:
el = self.admin_client.get_share_instance_export_location(
share_instance['id'], el['uuid'])
self._verify_export_location_structure(el)
share_instance['id'], el['id'])
self._verify_export_location_structure(el, format='detail')
@test.attr(type=["gate", ])
def test_share_contains_all_export_locations_of_all_share_instances(self):
@ -140,6 +197,6 @@ class ExportLocationsTest(base.BaseSharesAdminTest):
len(share_instances_export_locations)
)
self.assertEqual(
sorted(share_export_locations, key=lambda el: el['uuid']),
sorted(share_instances_export_locations, key=lambda el: el['uuid'])
sorted(share_export_locations, key=lambda el: el['id']),
sorted(share_instances_export_locations, key=lambda el: el['id'])
)

2
manila_tempest_tests/tests/api/admin/test_export_locations_negative.py
View File

@ -90,5 +90,5 @@ class ExportLocationsNegativeTest(base.BaseSharesAdminTest):
self.assertRaises(
lib_exc.Forbidden,
self.member_client.get_share_instance_export_location,
share_instance['id'], el['uuid'],
share_instance['id'], el['id'],
)