From 53918308c8ff1ba42926aa7f390f89b6787ecbed Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Thu, 27 Dec 2018 01:01:08 -0800 Subject: [PATCH] Separate APIs for share & replica export locations Users of replicated shares expect to see primary export locations when viewing information regarding the share. Because we collate exports of all replicas within the export locations APIs, it becomes hard for users to discern which exports belong to the primary share. For secondary replicas, users would also need additional information (availability zone, state of the replication) to work with. Introduce micro-version 2.47 from which the export locations API (GET /v2/{tenant_id}/shares/{share_id}/export_locations) no longer provides export locations of non-active share replicas. A new API has been introduced to provide export location details for share replicas, both active and non-active. (GET /v2/{tenant_id}/share-replicas/{share_replica_id}/export-locations) The new API provides the replica's state and availability zone in addition to the export location information. APIImpact Implements: bp export-locations-az Change-Id: I0a1d9dd00b4c13ac01988e30ca2b7d7ce4a747d1 --- api-ref/source/index.rst | 1 + api-ref/source/parameters.yaml | 21 +- ...replica-export-location-list-response.json | 22 ++ ...replica-export-location-show-response.json | 13 ++ api-ref/source/share-export-locations.inc | 6 +- .../source/share-replica-export-locations.inc | 106 ++++++++++ manila/api/openstack/api_version_request.py | 10 +- .../openstack/rest_api_version_history.rst | 11 + manila/api/v2/router.py | 17 ++ manila/api/v2/share_export_locations.py | 47 +++-- .../api/v2/share_replica_export_locations.py | 70 ++++++ manila/api/views/export_locations.py | 38 ++-- manila/db/api.py | 17 +- manila/db/sqlalchemy/api.py | 28 ++- manila/db/sqlalchemy/models.py | 7 +- manila/policies/__init__.py | 2 + .../policies/share_replica_export_location.py | 48 +++++ .../api/v2/test_share_export_locations.py | 59 ++++++ .../v2/test_share_replica_export_locations.py | 199 ++++++++++++++++++ ...tions-az-api-changes-c8aa1a3a5bc86312.yaml | 22 ++ 20 files changed, 699 insertions(+), 45 deletions(-) create mode 100644 api-ref/source/samples/share-replica-export-location-list-response.json create mode 100644 api-ref/source/samples/share-replica-export-location-show-response.json create mode 100644 api-ref/source/share-replica-export-locations.inc create mode 100644 manila/api/v2/share_replica_export_locations.py create mode 100644 manila/policies/share_replica_export_location.py create mode 100644 manila/tests/api/v2/test_share_replica_export_locations.py create mode 100644 releasenotes/notes/bp-export-locations-az-api-changes-c8aa1a3a5bc86312.yaml diff --git a/api-ref/source/index.rst b/api-ref/source/index.rst index 3ed2707063..891a3c0666 100644 --- a/api-ref/source/index.rst +++ b/api-ref/source/index.rst @@ -38,6 +38,7 @@ Shared File Systems API (EXPERIMENTAL) .. include:: experimental.inc .. include:: share-migration.inc .. include:: share-replicas.inc +.. include:: share-replica-export-locations.inc .. include:: share-groups.inc .. include:: share-group-types.inc .. include:: share-group-snapshots.inc diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 6ef90dc34e..85da812a0b 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1178,6 +1178,12 @@ export_location: required: false type: string max_version: 2.8 +export_location_availability_zone: + description: | + The name of the availability zone that the export location belongs to. + in: body + required: true + type: string export_location_created_at: description: | The date and time stamp when the share export location was @@ -1208,7 +1214,9 @@ export_location_is_admin_only: Defines purpose of an export location. If set to ``true``, then it is expected to be used for service needs and by administrators only. If it is set to ``false``, then this export - location can be used by end users. + location can be used by end users. This parameter is only available to + users with an "administrator" role, and cannot be controlled via policy + .json. in: body required: true type: boolean @@ -1227,10 +1235,19 @@ export_location_preferred: required: true type: boolean min_version: 2.14 +export_location_preferred_replicas: + description: | + Drivers may use this field to identify which export locations + are most efficient and should be used preferentially by clients. + By default it is set to ``false`` value. + in: body + required: true + type: boolean export_location_share_instance_id: description: | The UUID of the share instance that this - export location belongs to. + export location belongs to. This parameter is only available to users + with an "administrator" role, and cannot be controlled via policy.json. in: body required: true type: string diff --git a/api-ref/source/samples/share-replica-export-location-list-response.json b/api-ref/source/samples/share-replica-export-location-list-response.json new file mode 100644 index 0000000000..a6782a8085 --- /dev/null +++ b/api-ref/source/samples/share-replica-export-location-list-response.json @@ -0,0 +1,22 @@ +{ + "export_locations": [ + { + "path": "10.254.0.3:/shares/share-e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "share_instance_id": "e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "is_admin_only": false, + "id": "b6bd76ce-12a2-42a9-a30a-8a43b503867d", + "preferred": false, + "replica_state": "in_sync", + "availability_zone": "paris" + }, + { + "path": "10.0.0.3:/shares/share-e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "share_instance_id": "e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "is_admin_only": true, + "id": "6921e862-88bc-49a5-a2df-efeed9acd583", + "preferred": false, + "replica_state": "in_sync", + "availability_zone": "paris" + } + ] +} diff --git a/api-ref/source/samples/share-replica-export-location-show-response.json b/api-ref/source/samples/share-replica-export-location-show-response.json new file mode 100644 index 0000000000..635f9671e7 --- /dev/null +++ b/api-ref/source/samples/share-replica-export-location-show-response.json @@ -0,0 +1,13 @@ +{ + "export_location": { + "created_at": "2016-03-24T14:20:47.000000", + "updated_at": "2016-03-24T14:20:47.000000", + "preferred": false, + "is_admin_only": true, + "share_instance_id": "e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "path": "10.0.0.3:/shares/share-e1c2d35e-fe67-4028-ad7a-45f668732b1d", + "id": "6921e862-88bc-49a5-a2df-efeed9acd583", + "replica_state": "in_sync", + "availability_zone": "paris" + } +} diff --git a/api-ref/source/share-export-locations.inc b/api-ref/source/share-export-locations.inc index 170af05bcb..9fb15f3b59 100644 --- a/api-ref/source/share-export-locations.inc +++ b/api-ref/source/share-export-locations.inc @@ -6,8 +6,10 @@ Share export locations (since API v2.9) Set of APIs used for viewing export locations of shares. -By default, these APIs are admin-only. Use the ``policy.json`` file -to grant permissions for these actions to other roles. +These APIs allow retrieval of export locations belonging to non-active share +replicas until API version 2.46. In and beyond API version 2.47, export +locations of non-active share replicas can only be retrieved using the +:ref:`Share Replica Export Locations APIs `. List export locations diff --git a/api-ref/source/share-replica-export-locations.inc b/api-ref/source/share-replica-export-locations.inc new file mode 100644 index 0000000000..1fbced135d --- /dev/null +++ b/api-ref/source/share-replica-export-locations.inc @@ -0,0 +1,106 @@ +.. -*- rst -*- + +.. _share_replica_export_locations: + +================================================ +Share replica export locations (since API v2.47) +================================================ + +Set of APIs used to view export locations of share replicas. + +List export locations +===================== + +.. rest_method:: GET /v2/{tenant_id}/share-replicas/{share_replica_id}/export-locations + +Response codes +-------------- + +.. rest_status_code:: success status.yaml + + - 200 + +.. rest_status_code:: error status.yaml + + - 400 + - 401 + - 403 + - 404 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - tenant_id: tenant_id_path + - share_replica_id: share_replica_id_path + +Response parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - id: export_location_id + - share_instance_id: export_location_share_instance_id + - path: export_location_path + - is_admin_only: export_location_is_admin_only + - preferred: export_location_preferred_replicas + - availability_zone: export_location_availability_zone + - replica_state: share_replica_replica_state + +Response example +---------------- + +.. literalinclude:: samples/share-replica-export-location-list-response.json + :language: javascript + + +Show single export location +=========================== + +.. rest_method:: GET /v2/{tenant_id}/share-replicas/{share_replica_id}/export-locations/{export-location-id} + + +Response codes +-------------- + +.. rest_status_code:: success status.yaml + + - 200 + +.. rest_status_code:: error status.yaml + + - 400 + - 401 + - 403 + - 404 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - tenant_id: tenant_id_path + - share_replica_id: share_replica_id_path + - export_location_id: export_location_id_path + +Response parameters +------------------- + +.. rest_parameters:: parameters.yaml + + - id: export_location_id + - share_instance_id: export_location_share_instance_id + - path: export_location_path + - is_admin_only: export_location_is_admin_only + - preferred: export_location_preferred_replicas + - availability_zone: export_location_availability_zone + - replica_state: share_replica_replica_state + - created_at: export_location_created_at + - updated_at: export_location_updated_at + +Response example +---------------- + +.. literalinclude:: samples/share-replica-export-location-show-response.json + :language: javascript diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 656bc97b96..c68e810bc7 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -121,13 +121,21 @@ REST_API_VERSION_HISTORY = """ access rules will not work with API version >=2.45. * 2.46 - Added 'is_default' field to 'share_type' and 'share_group_type' objects. + * 2.47 - Export locations for non-active share replicas are no longer + retrievable through the export locations APIs: + GET /v2/{tenant_id}/shares/{share_id}/export_locations and + GET /v2/{tenant_id}/shares/{share_id}/export_locations/{ + export_location_id}. A new API is introduced at this + version: GET /v2/{tenant_id}/share-replicas/{ + replica_id}/export-locations to allow retrieving individual + replica export locations if available. """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.46" +_MAX_API_VERSION = "2.47" 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 1c439e1795..a82fd356f5 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -253,3 +253,14 @@ user documentation. ----------------------- Added 'is_default' field to 'share_type' and 'share_group_type' objects. + +2.47 +---- + + Export locations for non-active share replicas are no longer retrievable + through the export locations APIs: ``GET + /v2/{tenant_id}/shares/{share_id}/export_locations`` and ``GET + /v2/{tenant_id}/shares/{share_id}/export_locations/{export_location_id}``. + A new API is introduced at this version: ``GET + /v2/{tenant_id}/share-replicas/{replica_id}/export-locations`` to allow + retrieving export locations of share replicas if available. diff --git a/manila/api/v2/router.py b/manila/api/v2/router.py index 571383c698..7db6fe92ff 100644 --- a/manila/api/v2/router.py +++ b/manila/api/v2/router.py @@ -45,6 +45,7 @@ from manila.api.v2 import share_groups from manila.api.v2 import share_instance_export_locations from manila.api.v2 import share_instances from manila.api.v2 import share_networks +from manila.api.v2 import share_replica_export_locations from manila.api.v2 import share_replicas from manila.api.v2 import share_snapshot_export_locations from manila.api.v2 import share_snapshot_instance_export_locations @@ -413,6 +414,22 @@ class APIRouter(manila.api.openstack.APIRouter): controller=self.resources['share-replicas'], collection={'detail': 'GET'}, member={'action': 'POST'}) + self.resources["share-replica-export-locations"] = ( + share_replica_export_locations.create_resource()) + mapper.connect("share-replicas", + ("/{project_id}/share-replicas/{share_replica_id}/" + "export-locations"), + controller=self.resources[ + "share-replica-export-locations"], + action="index", + conditions={"method": ["GET"]}) + mapper.connect("share-replicas", + ("/{project_id}/share-replicas/{share_replica_id}/" + "export-locations/{export_location_uuid}"), + controller=self.resources[ + "share-replica-export-locations"], + action="show", + conditions={"method": ["GET"]}) self.resources['messages'] = messages.create_resource() mapper.resource("message", "messages", diff --git a/manila/api/v2/share_export_locations.py b/manila/api/v2/share_export_locations.py index babbb55e72..3f5b5e0e9d 100644 --- a/manila/api/v2/share_export_locations.py +++ b/manila/api/v2/share_export_locations.py @@ -37,27 +37,28 @@ class ShareExportLocationController(wsgi.Controller): msg = _("Share '%s' not found.") % share_id raise exc.HTTPNotFound(explanation=msg) - @wsgi.Controller.api_version('2.9') - @wsgi.Controller.authorize - def index(self, req, share_id): - """Return a list of export locations for share.""" - + @wsgi.Controller.authorize('index') + def _index(self, req, share_id, ignore_secondary_replicas=False): context = req.environ['manila.context'] self._verify_share(context, share_id) + kwargs = { + 'include_admin_only': context.is_admin, + 'ignore_migration_destination': True, + 'ignore_secondary_replicas': ignore_secondary_replicas, + } export_locations = db_api.share_export_locations_get_by_share_id( - context, share_id, include_admin_only=context.is_admin, - ignore_migration_destination=True) + context, share_id, **kwargs) return self._view_builder.summary_list(req, export_locations) - @wsgi.Controller.api_version('2.9') - @wsgi.Controller.authorize - def show(self, req, share_id, export_location_uuid): - """Return data about the requested export location.""" + @wsgi.Controller.authorize('show') + def _show(self, req, share_id, export_location_uuid, + ignore_secondary_replicas=False): context = req.environ['manila.context'] self._verify_share(context, share_id) try: export_location = db_api.share_export_location_get_by_uuid( - context, export_location_uuid) + context, export_location_uuid, + ignore_secondary_replicas=ignore_secondary_replicas) except exception.ExportLocationNotFound: msg = _("Export location '%s' not found.") % export_location_uuid raise exc.HTTPNotFound(explanation=msg) @@ -67,6 +68,28 @@ class ShareExportLocationController(wsgi.Controller): return self._view_builder.detail(req, export_location) + @wsgi.Controller.api_version('2.9', '2.46') + def index(self, req, share_id): + """Return a list of export locations for share.""" + return self._index(req, share_id) + + @wsgi.Controller.api_version('2.47') # noqa: F811 + def index(self, req, share_id): + """Return a list of export locations for share.""" + return self._index(req, share_id, + ignore_secondary_replicas=True) + + @wsgi.Controller.api_version('2.9', '2.46') + def show(self, req, share_id, export_location_uuid): + """Return data about the requested export location.""" + return self._show(req, share_id, export_location_uuid) + + @wsgi.Controller.api_version('2.47') # noqa: F811 + def show(self, req, share_id, export_location_uuid): + """Return data about the requested export location.""" + return self._show(req, share_id, export_location_uuid, + ignore_secondary_replicas=True) + def create_resource(): return wsgi.Resource(ShareExportLocationController()) diff --git a/manila/api/v2/share_replica_export_locations.py b/manila/api/v2/share_replica_export_locations.py new file mode 100644 index 0000000000..7bdacb3088 --- /dev/null +++ b/manila/api/v2/share_replica_export_locations.py @@ -0,0 +1,70 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import six +from webob import exc + +from manila.api.openstack import wsgi +from manila.api.views import export_locations as export_locations_views +from manila.db import api as db_api +from manila import exception +from manila.i18n import _ + + +class ShareReplicaExportLocationController(wsgi.Controller): + """The Share Instance Export Locations API controller.""" + + def __init__(self): + self._view_builder_class = export_locations_views.ViewBuilder + self.resource_name = 'share_replica_export_location' + super(ShareReplicaExportLocationController, self).__init__() + + def _verify_share_replica(self, context, share_replica_id): + try: + db_api.share_replica_get(context, share_replica_id) + except exception.NotFound: + msg = _("Share replica '%s' not found.") % share_replica_id + raise exc.HTTPNotFound(explanation=msg) + + @wsgi.Controller.api_version('2.47', experimental=True) + @wsgi.Controller.authorize + def index(self, req, share_replica_id): + """Return a list of export locations for the share instance.""" + context = req.environ['manila.context'] + self._verify_share_replica(context, share_replica_id) + export_locations = ( + db_api.share_export_locations_get_by_share_instance_id( + context, share_replica_id, + include_admin_only=context.is_admin) + ) + return self._view_builder.summary_list(req, export_locations, + replica=True) + + @wsgi.Controller.api_version('2.47', experimental=True) + @wsgi.Controller.authorize + def show(self, req, share_replica_id, export_location_uuid): + """Return data about the requested export location.""" + context = req.environ['manila.context'] + self._verify_share_replica(context, share_replica_id) + try: + export_location = db_api.share_export_location_get_by_uuid( + context, export_location_uuid) + return self._view_builder.detail(req, export_location, + replica=True) + except exception.ExportLocationNotFound as e: + raise exc.HTTPNotFound(explanation=six.text_type(e)) + + +def create_resource(): + return wsgi.Resource(ShareReplicaExportLocationController()) diff --git a/manila/api/views/export_locations.py b/manila/api/views/export_locations.py index 8bc96de019..ea71d031c5 100644 --- a/manila/api/views/export_locations.py +++ b/manila/api/views/export_locations.py @@ -28,7 +28,7 @@ class ViewBuilder(common.ViewBuilder): ] def _get_export_location_view(self, request, export_location, - detail=False): + detail=False, replica=False): context = request.environ['manila.context'] @@ -38,43 +38,49 @@ class ViewBuilder(common.ViewBuilder): } self.update_versioned_resource_dict(request, view, export_location) if context.is_admin: - view['share_instance_id'] = export_location[ - 'share_instance_id'] + 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'] + if replica: + share_instance = export_location['share_instance'] + view['replica_state'] = share_instance['replica_state'] + view['availability_zone'] = share_instance['availability_zone'] + return {'export_location': view} - def summary(self, request, export_location): + def summary(self, request, export_location, replica=False): """Summary view of a single export location.""" - return self._get_export_location_view(request, export_location, - detail=False) + return self._get_export_location_view( + request, export_location, detail=False, replica=replica) - def detail(self, request, export_location): + def detail(self, request, export_location, replica=False): """Detailed view of a single export location.""" - return self._get_export_location_view(request, export_location, - detail=True) + return self._get_export_location_view( + request, export_location, detail=True, replica=replica) - def _list_export_locations(self, request, export_locations, detail=False): + def _list_export_locations(self, req, export_locations, + detail=False, replica=False): """View of export locations list.""" view_method = self.detail if detail else self.summary - return {self._collection_name: [ - view_method(request, export_location)['export_location'] - for export_location in export_locations - ]} + return { + self._collection_name: [ + view_method(req, elocation, replica=replica)['export_location'] + for elocation in export_locations + ]} def detail_list(self, request, export_locations): """Detailed View of export locations list.""" return self._list_export_locations(request, export_locations, detail=True) - def summary_list(self, request, export_locations): + def summary_list(self, request, export_locations, replica=False): """Summary View of export locations list.""" return self._list_export_locations(request, export_locations, - detail=False) + detail=False, replica=replica) @common.ViewBuilder.versioned_method('2.14') def add_preferred_path_attribute(self, context, view_dict, diff --git a/manila/db/api.py b/manila/db/api.py index 20bdfca910..ace3f801bd 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -728,10 +728,12 @@ def share_metadata_update(context, share, metadata, delete): ################### -def share_export_location_get_by_uuid(context, export_location_uuid): +def share_export_location_get_by_uuid(context, export_location_uuid, + ignore_secondary_replicas=False): """Get specific export location of a share.""" return IMPL.share_export_location_get_by_uuid( - context, export_location_uuid) + context, export_location_uuid, + ignore_secondary_replicas=ignore_secondary_replicas) def share_export_locations_get(context, share_id): @@ -741,18 +743,21 @@ def share_export_locations_get(context, share_id): def share_export_locations_get_by_share_id(context, share_id, include_admin_only=True, - ignore_migration_destination=False): + ignore_migration_destination=False, + ignore_secondary_replicas=False): """Get all export locations of a share by its ID.""" return IMPL.share_export_locations_get_by_share_id( context, share_id, include_admin_only=include_admin_only, - ignore_migration_destination=ignore_migration_destination) + ignore_migration_destination=ignore_migration_destination, + ignore_secondary_replicas=ignore_secondary_replicas) def share_export_locations_get_by_share_instance_id(context, - share_instance_id): + share_instance_id, + include_admin_only=True): """Get all export locations of a share instance by its ID.""" return IMPL.share_export_locations_get_by_share_instance_id( - context, share_instance_id) + context, share_instance_id, include_admin_only=include_admin_only) def share_export_locations_update(context, share_instance_id, export_locations, diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index d73289b66e..08a4c6f649 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -3005,7 +3005,8 @@ def _share_metadata_get_item(context, share_id, key, session=None): ############################ def _share_export_locations_get(context, share_instance_ids, - include_admin_only=True, session=None): + include_admin_only=True, + ignore_secondary_replicas=False, session=None): session = session or get_session() if not isinstance(share_instance_ids, (set, list, tuple)): @@ -3027,6 +3028,13 @@ def _share_export_locations_get(context, share_instance_ids, if not include_admin_only: query = query.filter_by(is_admin_only=False) + + if ignore_secondary_replicas: + replica_state_attr = models.ShareInstance.replica_state + query = query.join("share_instance").filter( + or_(replica_state_attr == None, # noqa + replica_state_attr == constants.REPLICA_STATE_ACTIVE)) + return query.all() @@ -3034,7 +3042,8 @@ def _share_export_locations_get(context, share_instance_ids, @require_share_exists def share_export_locations_get_by_share_id(context, share_id, include_admin_only=True, - ignore_migration_destination=False): + ignore_migration_destination=False, + ignore_secondary_replicas=False): share = share_get(context, share_id) if ignore_migration_destination: ids = [instance.id for instance in share.instances @@ -3042,16 +3051,18 @@ def share_export_locations_get_by_share_id(context, share_id, else: ids = [instance.id for instance in share.instances] rows = _share_export_locations_get( - context, ids, include_admin_only=include_admin_only) + context, ids, include_admin_only=include_admin_only, + ignore_secondary_replicas=ignore_secondary_replicas) return rows @require_context @require_share_instance_exists def share_export_locations_get_by_share_instance_id(context, - share_instance_id): + share_instance_id, + include_admin_only=True): rows = _share_export_locations_get( - context, [share_instance_id], include_admin_only=True) + context, [share_instance_id], include_admin_only=include_admin_only) return rows @@ -3070,6 +3081,7 @@ def share_export_locations_get(context, share_id): @require_context def share_export_location_get_by_uuid(context, export_location_uuid, + ignore_secondary_replicas=False, session=None): session = session or get_session() @@ -3084,6 +3096,12 @@ def share_export_location_get_by_uuid(context, export_location_uuid, joinedload("_el_metadata_bare"), ) + if ignore_secondary_replicas: + replica_state_attr = models.ShareInstance.replica_state + query = query.join("share_instance").filter( + or_(replica_state_attr == None, # noqa + replica_state_attr == constants.REPLICA_STATE_ACTIVE)) + result = query.first() if not result: raise exception.ExportLocationNotFound(uuid=export_location_uuid) diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index d3f28382b3..0d2fa62f8b 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -400,7 +400,8 @@ class ShareInstance(BASE, ManilaBase): export_locations = orm.relationship( "ShareInstanceExportLocations", - lazy='immediate', + lazy='joined', + backref=orm.backref('share_instance', lazy='joined'), primaryjoin=( 'and_(' 'ShareInstance.id == ' @@ -434,6 +435,10 @@ class ShareInstanceExportLocations(BASE, ManilaBase): el_metadata[meta['key']] = meta['value'] return el_metadata + @property + def replica_state(self): + return self.share_instance['replica_state'] + id = Column(Integer, primary_key=True) uuid = Column(String(36), nullable=False, unique=True) share_instance_id = Column( diff --git a/manila/policies/__init__.py b/manila/policies/__init__.py index 3310b8fc83..83ee1636c5 100644 --- a/manila/policies/__init__.py +++ b/manila/policies/__init__.py @@ -35,6 +35,7 @@ from manila.policies import share_instance from manila.policies import share_instance_export_location from manila.policies import share_network from manila.policies import share_replica +from manila.policies import share_replica_export_location from manila.policies import share_server from manila.policies import share_snapshot from manila.policies import share_snapshot_export_location @@ -67,6 +68,7 @@ def list_rules(): share_group_snapshot.list_rules(), share_group.list_rules(), share_replica.list_rules(), + share_replica_export_location.list_rules(), share_network.list_rules(), security_service.list_rules(), share_export_location.list_rules(), diff --git a/manila/policies/share_replica_export_location.py b/manila/policies/share_replica_export_location.py new file mode 100644 index 0000000000..30ced21ded --- /dev/null +++ b/manila/policies/share_replica_export_location.py @@ -0,0 +1,48 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_policy import policy + +from manila.policies import base + + +BASE_POLICY_NAME = 'share_replica_export_location:%s' + + +share_replica_export_location_policies = [ + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'index', + check_str=base.RULE_DEFAULT, + description="Get all export locations of a given share replica.", + operations=[ + { + 'method': 'GET', + 'path': '/share-replicas/{share_replica_id}/export-locations', + } + ]), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'show', + check_str=base.RULE_DEFAULT, + description="Get details about the requested share replica export " + "location.", + operations=[ + { + 'method': 'GET', + 'path': ('/share-replicas/{share_replica_id}/export-locations/' + '{export_location_id}'), + } + ]), +] + + +def list_rules(): + return share_replica_export_location_policies diff --git a/manila/tests/api/v2/test_share_export_locations.py b/manila/tests/api/v2/test_share_export_locations.py index cad62dd315..501cb99cf7 100644 --- a/manila/tests/api/v2/test_share_export_locations.py +++ b/manila/tests/api/v2/test_share_export_locations.py @@ -17,7 +17,9 @@ import ddt import mock from webob import exc +from manila.api.openstack import api_version_request as api_version from manila.api.v2 import share_export_locations as export_locations +from manila.common import constants from manila import context from manila import db from manila import exception @@ -169,6 +171,63 @@ class ShareExportLocationsAPITest(test.TestCase): for k, v in el.items(): self.assertEqual(v, el[k]) + @ddt.data(*set(('2.46', '2.47', api_version._MAX_API_VERSION))) + def test_list_export_locations_replicated_share(self, version): + """Test the export locations API changes between 2.46 and 2.47 + + For API version <= 2.46, non-active replica export locations are + included in the API response. They are not included in and beyond + version 2.47. + """ + # Setup data + share = db_utils.create_share( + replication_type=constants.REPLICATION_TYPE_READABLE, + replica_state=constants.REPLICA_STATE_ACTIVE) + active_replica_id = share.instance.id + exports = [ + {'path': 'myshare.mydomain/active-replica-exp1', + 'is_admin_only': False}, + {'path': 'myshare.mydomain/active-replica-exp2', + 'is_admin_only': False}, + ] + db.share_export_locations_update( + self.ctxt['user'], active_replica_id, exports) + + # Replicas + share_replica2 = db_utils.create_share_replica( + share_id=share.id, replica_state=constants.REPLICA_STATE_IN_SYNC) + share_replica3 = db_utils.create_share_replica( + share_id=share.id, + replica_state=constants.REPLICA_STATE_OUT_OF_SYNC) + replica2_exports = [ + {'path': 'myshare.mydomain/insync-replica-exp', + 'is_admin_only': False} + ] + replica3_exports = [ + {'path': 'myshare.mydomain/outofsync-replica-exp', + 'is_admin_only': False} + ] + db.share_export_locations_update( + self.ctxt['user'], share_replica2.id, replica2_exports) + db.share_export_locations_update( + self.ctxt['user'], share_replica3.id, replica3_exports) + + req = self._get_request(version=version) + index_result = self.controller.index(req, share['id']) + + actual_paths = [el['path'] for el in index_result['export_locations']] + if self.is_microversion_ge(version, '2.47'): + self.assertEqual(2, len(index_result['export_locations'])) + self.assertNotIn( + 'myshare.mydomain/insync-replica-exp', actual_paths) + self.assertNotIn( + 'myshare.mydomain/outofsync-replica-exp', actual_paths) + else: + self.assertEqual(4, len(index_result['export_locations'])) + self.assertIn('myshare.mydomain/insync-replica-exp', actual_paths) + self.assertIn( + 'myshare.mydomain/outofsync-replica-exp', actual_paths) + @ddt.data('1.0', '2.0', '2.8') def test_list_with_unsupported_version(self, version): self.assertRaises( diff --git a/manila/tests/api/v2/test_share_replica_export_locations.py b/manila/tests/api/v2/test_share_replica_export_locations.py new file mode 100644 index 0000000000..622b33cd08 --- /dev/null +++ b/manila/tests/api/v2/test_share_replica_export_locations.py @@ -0,0 +1,199 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import ddt +import mock +from webob import exc + +from manila.api.v2 import share_replica_export_locations as export_locations +from manila.common import constants +from manila import context +from manila import db +from manila import exception +from manila import policy +from manila import test +from manila.tests.api import fakes +from manila.tests import db_utils + + +@ddt.ddt +class ShareReplicaExportLocationsAPITest(test.TestCase): + + def _get_request(self, version="2.47", use_admin_context=False): + req = fakes.HTTPRequest.blank( + '/v2/share-replicas/%s/export-locations' % self.active_replica_id, + version=version, use_admin_context=use_admin_context, + experimental=True) + return req + + def setUp(self): + super(ShareReplicaExportLocationsAPITest, self).setUp() + self.controller = ( + export_locations.ShareReplicaExportLocationController()) + self.resource_name = 'share_replica_export_location' + self.ctxt = context.RequestContext('fake', 'fake') + self.mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=True)) + self.share = db_utils.create_share( + replication_type=constants.REPLICATION_TYPE_READABLE, + replica_state=constants.REPLICA_STATE_ACTIVE) + self.active_replica_id = self.share.instance.id + self.req = self._get_request() + exports = [ + {'path': 'myshare.mydomain/active-replica-exp1', + 'is_admin_only': False}, + {'path': 'myshare.mydomain/active-replica-exp2', + 'is_admin_only': False}, + ] + db.share_export_locations_update( + self.ctxt, self.active_replica_id, exports) + + # Replicas + self.share_replica2 = db_utils.create_share_replica( + share_id=self.share.id, + replica_state=constants.REPLICA_STATE_IN_SYNC) + self.share_replica3 = db_utils.create_share_replica( + share_id=self.share.id, + replica_state=constants.REPLICA_STATE_OUT_OF_SYNC) + replica2_exports = [ + {'path': 'myshare.mydomain/insync-replica-exp', + 'is_admin_only': False}, + {'path': 'myshare.mydomain/insync-replica-exp2', + 'is_admin_only': False} + ] + replica3_exports = [ + {'path': 'myshare.mydomain/outofsync-replica-exp', + 'is_admin_only': False}, + {'path': 'myshare.mydomain/outofsync-replica-exp2', + 'is_admin_only': False} + ] + db.share_export_locations_update( + self.ctxt, self.share_replica2.id, replica2_exports) + db.share_export_locations_update( + self.ctxt, self.share_replica3.id, replica3_exports) + + @ddt.data('user', 'admin') + def test_list_and_show(self, role): + summary_keys = [ + 'id', 'path', 'replica_state', 'availability_zone', '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, summary_keys, detail_keys, + admin_summary_keys, admin_detail_keys) + + def _test_list_and_show(self, role, summary_keys, detail_keys, + admin_summary_keys, admin_detail_keys): + + req = self._get_request(use_admin_context=(role == 'admin')) + for replica_id in (self.active_replica_id, self.share_replica2.id, + self.share_replica3.id): + index_result = self.controller.index(req, replica_id) + + self.assertIn('export_locations', index_result) + self.assertEqual(1, len(index_result)) + self.assertEqual(2, len(index_result['export_locations'])) + + for index_el in index_result['export_locations']: + self.assertIn('id', index_el) + show_result = self.controller.show( + req, replica_id, index_el['id']) + self.assertIn('export_location', show_result) + self.assertEqual(1, len(show_result)) + + show_el = show_result['export_location'] + + # Check summary keys in index result & detail keys in show + 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 have matching values + for key in summary_keys: + self.assertEqual(index_el[key], show_el[key]) + + def test_list_and_show_with_non_replicas(self): + non_replicated_share = db_utils.create_share() + instance_id = non_replicated_share.instance.id + exports = [ + {'path': 'myshare.mydomain/non-replicated-share', + 'is_admin_only': False}, + {'path': 'myshare.mydomain/non-replicated-share-2', + 'is_admin_only': False}, + ] + db.share_export_locations_update(self.ctxt, instance_id, exports) + updated_exports = db.share_export_locations_get_by_share_id( + self.ctxt, non_replicated_share.id) + + self.assertRaises(exc.HTTPNotFound, self.controller.index, self.req, + instance_id) + + for export in updated_exports: + self.assertRaises(exc.HTTPNotFound, self.controller.show, self.req, + instance_id, export['id']) + + def test_list_export_locations_share_replica_not_found(self): + self.assertRaises( + exc.HTTPNotFound, + self.controller.index, + self.req, 'non-existent-share-replica-id') + + def test_show_export_location_share_replica_not_found(self): + index_result = self.controller.index(self.req, self.active_replica_id) + el_id = index_result['export_locations'][0]['id'] + + self.assertRaises( + exc.HTTPNotFound, + self.controller.show, + self.req, 'non-existent-share-replica-id', el_id) + + self.assertRaises( + exc.HTTPNotFound, + self.controller.show, + self.req, self.active_replica_id, + 'non-existent-export-location-id') + + @ddt.data('1.0', '2.0', '2.46') + def test_list_with_unsupported_version(self, version): + self.assertRaises( + exception.VersionNotFoundForAPIMethod, + self.controller.index, + self._get_request(version), + self.active_replica_id) + + @ddt.data('1.0', '2.0', '2.46') + def test_show_with_unsupported_version(self, version): + index_result = self.controller.index(self.req, self.active_replica_id) + + self.assertRaises( + exception.VersionNotFoundForAPIMethod, + self.controller.show, + self._get_request(version), + self.active_replica_id, + index_result['export_locations'][0]['id']) diff --git a/releasenotes/notes/bp-export-locations-az-api-changes-c8aa1a3a5bc86312.yaml b/releasenotes/notes/bp-export-locations-az-api-changes-c8aa1a3a5bc86312.yaml new file mode 100644 index 0000000000..459b0d8dd4 --- /dev/null +++ b/releasenotes/notes/bp-export-locations-az-api-changes-c8aa1a3a5bc86312.yaml @@ -0,0 +1,22 @@ +--- +features: + - | + New experimental APIs were introduced version ``2.47`` to retrieve + export locations of share replicas. With API versions ``2.46`` and + prior, export locations of non-active or secondary share replicas are + included in the share export locations APIs, albeit these APIs do not + provide all the necessary distinguishing information (availability zone, + replica state and replica ID). See the `API reference + `_ for more + information regarding these API changes. +deprecations: + - | + In API version ``2.47``, export locations APIs: ``GET + /v2/{tenant_id}/shares/{share_id}/export_locations`` and ``GET + /v2/{tenant_id}/shares/{share_id}/export_locations/​{export_location_id + }`` no longer provide export locations of non-active or secondary share + replicas where available. Use the newly introduced share replica export + locations APIs to gather this information: ``GET + /v2/{tenant_id}/share-replicas/{share_replica_id}/export-locations`` and + ``GET /v2/{tenant_id}/share-replicas/{share_replica_id}/export + -locations/{export_location_id}``.