From d877b61c5e37ff012341194b8349af6f38155e5d Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Mon, 7 Jan 2019 14:57:18 -0200 Subject: [PATCH] Add manage/unmanage of shares in DHSS=True This patch adds Manage/Unmanage of share servers in Manila. It also updates the Manage Share API to accept a "share_server_id" parameter, and updates Unmanage of Share and Snapshots API to allow unmanaging of shares and snapshots in DHSS=True. Managed share servers are not deleted automatically by manila, and if a single share is unmanaged in DHSS=True, the respective share server will not be deleted automatically as well. Managing share servers require that the driver implements 2 functions: - get_share_server_network_info: obtain IPs from share server. - manage_server: perform required operations to manage and return dict of backend_details. Unmanaging share servers require that the driver overrides unmanage_server function. The IPs obtained from the backend are validated by the Network plugin, so ports with the exact IPs must exist in the subnet and admin subnet associated with the share network specified when managing the share server (unless StandaloneNetworkPlugin is used). It is recommended to rename the backend resource if possible when managing the share server, to prevent issues with re-managing a share server that has already been managed. This patch bumps the API microversion to 2.49. APIImpact DocImpact Depends-On: I17c74b2aa242918188eeff368232c762a4b31093 Partially-implements: bp manage-unmanage-with-share-servers Change-Id: I108961e7436ba13550ef2b8f02079c6e599a6166 --- contrib/ci/post_test_hook.sh | 9 +- manila/api/openstack/api_version_request.py | 5 +- .../openstack/rest_api_version_history.rst | 5 + manila/api/v1/share_manage.py | 24 +- manila/api/v1/share_servers.py | 18 +- manila/api/v1/share_unmanage.py | 10 +- manila/api/v2/router.py | 10 +- manila/api/v2/share_servers.py | 175 ++++ manila/api/v2/share_snapshots.py | 18 +- manila/api/v2/shares.py | 31 +- manila/api/views/share_servers.py | 25 +- manila/db/api.py | 17 +- ...and_identifier_fields_for_share_servers.py | 72 ++ manila/db/sqlalchemy/api.py | 56 +- manila/db/sqlalchemy/models.py | 13 +- manila/exception.py | 4 + manila/network/__init__.py | 9 + .../network/neutron/neutron_network_plugin.py | 123 ++- manila/network/standalone_network_plugin.py | 43 + manila/policies/share_server.py | 30 + manila/share/api.py | 89 +++ manila/share/driver.py | 169 ++++ manila/share/manager.py | 323 ++++++-- manila/share/rpcapi.py | 19 +- manila/tests/api/fakes.py | 18 + manila/tests/api/v1/test_share_manage.py | 56 ++ manila/tests/api/v1/test_share_servers.py | 81 +- manila/tests/api/v1/test_share_unmanage.py | 20 + manila/tests/api/v2/test_share_servers.py | 390 +++++++++ manila/tests/api/v2/test_share_snapshots.py | 45 ++ manila/tests/api/v2/test_shares.py | 26 + .../alembic/migrations_data_checks.py | 41 + manila/tests/db/sqlalchemy/test_api.py | 149 ++++ manila/tests/fake_driver.py | 4 +- manila/tests/fake_share.py | 18 + .../network/neutron/test_neutron_plugin.py | 217 ++++- .../network/test_standalone_network_plugin.py | 64 ++ manila/tests/share/drivers/dummy.py | 90 ++- manila/tests/share/test_api.py | 312 ++++++++ manila/tests/share/test_manager.py | 751 +++++++++++++----- manila/tests/share/test_rpcapi.py | 20 + manila/tests/test_exception.py | 10 + manila/tests/test_network.py | 48 ++ ...manage-share-servers-cd4a6523d8e9fbdf.yaml | 24 + 44 files changed, 3326 insertions(+), 355 deletions(-) create mode 100644 manila/api/v2/share_servers.py create mode 100644 manila/db/migrations/alembic/versions/6a3fd2984bc31_add_is_auto_deletable_and_identifier_fields_for_share_servers.py create mode 100644 manila/tests/api/v2/test_share_servers.py create mode 100644 releasenotes/notes/manage-unmanage-share-servers-cd4a6523d8e9fbdf.yaml diff --git a/contrib/ci/post_test_hook.sh b/contrib/ci/post_test_hook.sh index 873efed6bf..de41ab1bde 100755 --- a/contrib/ci/post_test_hook.sh +++ b/contrib/ci/post_test_hook.sh @@ -163,6 +163,12 @@ if [[ "$DRIVER" == "generic"* ]]; then RUN_MANILA_HOST_ASSISTED_MIGRATION_TESTS=True RUN_MANILA_MANAGE_SNAPSHOT_TESTS=True RUN_MANILA_CG_TESTS=False + if [[ "$MULTITENANCY_ENABLED" == "True" ]]; then + # NOTE(ganso): The generic driver has not implemented support for + # Manage/unmanage shares/snapshots in DHSS=True + RUN_MANILA_MANAGE_SNAPSHOT_TESTS=False + RUN_MANILA_MANAGE_TESTS=False + fi if [[ "$POSTGRES_ENABLED" == "True" ]]; then # Run only CIFS tests on PostgreSQL DB backend # to reduce amount of tests per job using 'generic' share driver. @@ -241,7 +247,8 @@ elif [[ "$DRIVER" == "dummy" ]]; then MANILA_TEMPEST_CONCURRENCY=24 MANILA_CONFIGURE_DEFAULT_TYPES=False RUN_MANILA_SG_TESTS=True - RUN_MANILA_MANAGE_TESTS=False + RUN_MANILA_MANAGE_TESTS=True + RUN_MANILA_MANAGE_SNAPSHOT_TESTS=True RUN_MANILA_DRIVER_ASSISTED_MIGRATION_TESTS=True RUN_MANILA_REVERT_TO_SNAPSHOT_TESTS=True RUN_MANILA_MOUNT_SNAPSHOT_TESTS=True diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 506a632e02..49a72e11d9 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -131,13 +131,16 @@ REST_API_VERSION_HISTORY = """ replica export locations if available. * 2.48 - Added support for extra-spec "availability_zones" within Share types along with validation in the API. + * 2.49 - Added Manage/Unmanage Share Server APIs. Updated Manage/Unmanage + Shares and Snapshots APIs to work in + ``driver_handles_shares_servers`` enabled mode. """ # 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.48" +_MAX_API_VERSION = "2.49" 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 62313ead0f..7230437d42 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -270,3 +270,8 @@ user documentation. 'availability_zones' within share types to allow provisioning of shares only within specific availability zones. The extra-spec allows using comma separated names of one or more availability zones. + +2.49 +---- + Added Manage/Unmanage Share Server APIs. Updated Manage/Unmanage Shares and + Snapshots APIs to work in ``driver_handles_shares_servers`` enabled mode. diff --git a/manila/api/v1/share_manage.py b/manila/api/v1/share_manage.py index 90e812c8a3..cd0872bf5e 100644 --- a/manila/api/v1/share_manage.py +++ b/manila/api/v1/share_manage.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import six from webob import exc from manila.api import common @@ -29,7 +28,7 @@ from manila import utils class ShareManageMixin(object): @wsgi.Controller.authorize('manage') - def _manage(self, req, body): + def _manage(self, req, body, allow_dhss_true=False): context = req.environ['manila.context'] share_data = self._validate_manage_parameters(context, body) share_data = common.validate_public_share_policy(context, share_data) @@ -56,12 +55,17 @@ class ShareManageMixin(object): driver_options = share_data.get('driver_options', {}) + if allow_dhss_true: + share['share_server_id'] = share_data.get('share_server_id') + try: share_ref = self.share_api.manage(context, share, driver_options) except exception.PolicyNotAuthorized as e: - raise exc.HTTPForbidden(explanation=six.text_type(e)) - except exception.InvalidShare as e: - raise exc.HTTPConflict(explanation=six.text_type(e)) + raise exc.HTTPForbidden(explanation=e) + except (exception.InvalidShare, exception.InvalidShareServer) as e: + raise exc.HTTPConflict(explanation=e) + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e) return self._view_builder.detail(req, share_ref) @@ -90,13 +94,13 @@ class ShareManageMixin(object): utils.validate_service_host( context, share_utils.extract_host(data['service_host'])) except exception.ServiceNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) except exception.PolicyNotAuthorized as e: - raise exc.HTTPForbidden(explanation=six.text_type(e)) + raise exc.HTTPForbidden(explanation=e) except exception.AdminRequired as e: - raise exc.HTTPForbidden(explanation=six.text_type(e)) + raise exc.HTTPForbidden(explanation=e) except exception.ServiceIsDown as e: - raise exc.HTTPBadRequest(explanation=six.text_type(e)) + raise exc.HTTPBadRequest(explanation=e) data['share_type_id'] = self._get_share_type_id( context, data.get('share_type')) @@ -110,7 +114,7 @@ class ShareManageMixin(object): share_type) return stype['id'] except exception.ShareTypeNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) class ShareManageController(ShareManageMixin, wsgi.Controller): diff --git a/manila/api/v1/share_servers.py b/manila/api/v1/share_servers.py index eb0c841038..d24a7ad7ec 100644 --- a/manila/api/v1/share_servers.py +++ b/manila/api/v1/share_servers.py @@ -14,7 +14,6 @@ # under the License. from oslo_log import log -import six from six.moves import http_client import webob from webob import exc @@ -33,10 +32,11 @@ LOG = log.getLogger(__name__) class ShareServerController(wsgi.Controller): """The Share Server API controller for the OpenStack API.""" + _view_builder_class = share_servers_views.ViewBuilder + resource_name = 'share_server' + def __init__(self): self.share_api = share.API() - self._view_builder_class = share_servers_views.ViewBuilder - self.resource_name = 'share_server' super(ShareServerController, self).__init__() @wsgi.Controller.authorize @@ -62,7 +62,7 @@ class ShareServerController(wsgi.Controller): s[k] == v or k == 'share_network' and v in [s.share_network['name'], s.share_network['id']])] - return self._view_builder.build_share_servers(share_servers) + return self._view_builder.build_share_servers(req, share_servers) @wsgi.Controller.authorize def show(self, req, id): @@ -76,8 +76,8 @@ class ShareServerController(wsgi.Controller): else: server.share_network_name = server.share_network_id except exception.ShareServerNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) - return self._view_builder.build_share_server(server) + raise exc.HTTPNotFound(explanation=e) + return self._view_builder.build_share_server(req, server) @wsgi.Controller.authorize def details(self, req, id): @@ -86,7 +86,7 @@ class ShareServerController(wsgi.Controller): try: share_server = db_api.share_server_get(context, id) except exception.ShareServerNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) return self._view_builder.build_share_server_details( share_server['backend_details']) @@ -98,7 +98,7 @@ class ShareServerController(wsgi.Controller): try: share_server = db_api.share_server_get(context, id) except exception.ShareServerNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) allowed_statuses = [constants.STATUS_ERROR, constants.STATUS_ACTIVE] if share_server['status'] not in allowed_statuses: data = { @@ -112,7 +112,7 @@ class ShareServerController(wsgi.Controller): try: self.share_api.delete_share_server(context, share_server) except exception.ShareServerInUse as e: - raise exc.HTTPConflict(explanation=six.text_type(e)) + raise exc.HTTPConflict(explanation=e) return webob.Response(status_int=http_client.ACCEPTED) diff --git a/manila/api/v1/share_unmanage.py b/manila/api/v1/share_unmanage.py index 529975f17d..d414b256fb 100644 --- a/manila/api/v1/share_unmanage.py +++ b/manila/api/v1/share_unmanage.py @@ -13,7 +13,6 @@ # under the License. from oslo_log import log -import six from six.moves import http_client import webob from webob import exc @@ -30,7 +29,7 @@ LOG = log.getLogger(__name__) class ShareUnmanageMixin(object): @wsgi.Controller.authorize("unmanage") - def _unmanage(self, req, id, body=None): + def _unmanage(self, req, id, body=None, allow_dhss_true=False): """Unmanage a share.""" context = req.environ['manila.context'] @@ -42,7 +41,8 @@ class ShareUnmanageMixin(object): msg = _("Share %s has replicas. It cannot be unmanaged " "until all replicas are removed.") % share['id'] raise exc.HTTPConflict(explanation=msg) - if share['instance'].get('share_server_id'): + if (not allow_dhss_true and + share['instance'].get('share_server_id')): msg = _("Operation 'unmanage' is not supported for shares " "that are created on top of share servers " "(created with share-networks).") @@ -61,9 +61,9 @@ class ShareUnmanageMixin(object): raise exc.HTTPForbidden(explanation=msg) self.share_api.unmanage(context, share) except exception.NotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) except (exception.InvalidShare, exception.PolicyNotAuthorized) as e: - raise exc.HTTPForbidden(explanation=six.text_type(e)) + raise exc.HTTPForbidden(explanation=e) return webob.Response(status_int=http_client.ACCEPTED) diff --git a/manila/api/v2/router.py b/manila/api/v2/router.py index 7db6fe92ff..310a33d26c 100644 --- a/manila/api/v2/router.py +++ b/manila/api/v2/router.py @@ -27,7 +27,6 @@ from manila.api.v1 import scheduler_stats from manila.api.v1 import security_service from manila.api.v1 import share_manage from manila.api.v1 import share_metadata -from manila.api.v1 import share_servers from manila.api.v1 import share_types_extra_specs from manila.api.v1 import share_unmanage from manila.api.v2 import availability_zones @@ -47,6 +46,7 @@ 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_servers from manila.api.v2 import share_snapshot_export_locations from manila.api.v2 import share_snapshot_instance_export_locations from manila.api.v2 import share_snapshot_instances @@ -299,12 +299,18 @@ class APIRouter(manila.api.openstack.APIRouter): self.resources["share_servers"] = share_servers.create_resource() mapper.resource("share_server", "share-servers", - controller=self.resources["share_servers"]) + controller=self.resources["share_servers"], + member={"action": "POST"}) mapper.connect("details", "/{project_id}/share-servers/{id}/details", controller=self.resources["share_servers"], action="details", conditions={"method": ["GET"]}) + mapper.connect("share_servers", + "/{project_id}/share-servers/manage", + controller=self.resources["share_servers"], + action="manage", + conditions={"method": ["POST"]}) self.resources["types"] = share_types.create_resource() mapper.resource("type", "types", diff --git a/manila/api/v2/share_servers.py b/manila/api/v2/share_servers.py new file mode 100644 index 0000000000..19e46da2ac --- /dev/null +++ b/manila/api/v2/share_servers.py @@ -0,0 +1,175 @@ +# Copyright 2019 NetApp, Inc. +# 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. + +from oslo_log import log +from six.moves import http_client +import webob +from webob import exc + +from manila.api.openstack import wsgi +from manila.api.v1 import share_servers +from manila.common import constants +from manila.db import api as db_api +from manila import exception +from manila.i18n import _ +from manila.share import utils as share_utils +from manila import utils + +LOG = log.getLogger(__name__) + + +class ShareServerController(share_servers.ShareServerController, + wsgi.AdminActionsMixin): + """The Share Server API V2 controller for the OpenStack API.""" + + valid_statuses = { + 'status': { + constants.STATUS_ACTIVE, + constants.STATUS_ERROR, + constants.STATUS_DELETING, + constants.STATUS_CREATING, + constants.STATUS_MANAGING, + constants.STATUS_UNMANAGING, + constants.STATUS_UNMANAGE_ERROR, + constants.STATUS_MANAGE_ERROR, + } + } + + def _update(self, context, id, update): + db_api.share_server_update(context, id, update) + + @wsgi.Controller.api_version('2.49') + @wsgi.action('reset_status') + def share_server_reset_status(self, req, id, body): + return self._reset_status(req, id, body) + + @wsgi.Controller.api_version("2.49") + @wsgi.Controller.authorize('manage_share_server') + @wsgi.response(202) + def manage(self, req, body): + """Manage a share server.""" + context = req.environ['manila.context'] + identifier, host, share_network, driver_opts = ( + self._validate_manage_share_server_parameters(context, body)) + + try: + result = self.share_api.manage_share_server( + context, identifier, host, share_network, driver_opts) + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e) + + result.project_id = share_network["project_id"] + if result.share_network['name']: + result.share_network_name = result.share_network['name'] + else: + result.share_network_name = result.share_network_id + return self._view_builder.build_share_server(req, result) + + @wsgi.Controller.authorize('unmanage_share_server') + def _unmanage(self, req, id, body=None): + context = req.environ['manila.context'] + + LOG.debug("Unmanage Share Server with id: %s", id) + + # force's default value is False + # force will be True if body is {'unmanage': {'force': True}} + force = (body.get('unmanage') or {}).get('force', False) or False + + try: + share_server = db_api.share_server_get( + context, id) + except exception.ShareServerNotFound as e: + raise exc.HTTPNotFound(explanation=e) + + allowed_statuses = [constants.STATUS_ERROR, constants.STATUS_ACTIVE, + constants.STATUS_MANAGE_ERROR, + constants.STATUS_UNMANAGE_ERROR] + if share_server['status'] not in allowed_statuses: + data = { + 'status': share_server['status'], + 'allowed_statuses': ', '.join(allowed_statuses), + } + msg = _("Share server's actual status is %(status)s, allowed " + "statuses for unmanaging are " + "%(allowed_statuses)s.") % data + raise exc.HTTPBadRequest(explanation=msg) + + try: + self.share_api.unmanage_share_server( + context, share_server, force=force) + except (exception.ShareServerInUse, + exception.PolicyNotAuthorized) as e: + raise exc.HTTPBadRequest(explanation=e) + + return webob.Response(status_int=http_client.ACCEPTED) + + @wsgi.Controller.api_version("2.49") + @wsgi.action('unmanage') + def unmanage(self, req, id, body=None): + """Unmanage a share server.""" + return self._unmanage(req, id, body) + + def _validate_manage_share_server_parameters(self, context, body): + + if not (body and self.is_valid_body(body, 'share_server')): + msg = _("Share Server entity not found in request body") + raise exc.HTTPUnprocessableEntity(explanation=msg) + + required_parameters = ('host', 'share_network_id', 'identifier') + data = body['share_server'] + + for parameter in required_parameters: + if parameter not in data: + msg = _("Required parameter %s not found") % parameter + raise exc.HTTPBadRequest(explanation=msg) + if not data.get(parameter): + msg = _("Required parameter %s is empty") % parameter + raise exc.HTTPBadRequest(explanation=msg) + + identifier = data['identifier'] + host, share_network_id = data['host'], data['share_network_id'] + + if share_utils.extract_host(host, 'pool'): + msg = _("Host parameter should not contain pool.") + raise exc.HTTPBadRequest(explanation=msg) + + try: + utils.validate_service_host( + context, share_utils.extract_host(host)) + except exception.ServiceNotFound as e: + raise exc.HTTPBadRequest(explanation=e) + except exception.PolicyNotAuthorized as e: + raise exc.HTTPForbidden(explanation=e) + except exception.AdminRequired as e: + raise exc.HTTPForbidden(explanation=e) + except exception.ServiceIsDown as e: + raise exc.HTTPBadRequest(explanation=e) + + try: + share_network = db_api.share_network_get( + context, share_network_id) + except exception.ShareNetworkNotFound as e: + raise exc.HTTPBadRequest(explanation=e) + + driver_opts = data.get('driver_options') + if driver_opts is not None and not isinstance(driver_opts, dict): + msg = _("Driver options must be in dictionary format.") + raise exc.HTTPBadRequest(explanation=msg) + + return identifier, host, share_network, driver_opts + + +def create_resource(): + return wsgi.Resource(ShareServerController()) diff --git a/manila/api/v2/share_snapshots.py b/manila/api/v2/share_snapshots.py index 5257707f17..1db4ec5999 100644 --- a/manila/api/v2/share_snapshots.py +++ b/manila/api/v2/share_snapshots.py @@ -17,7 +17,6 @@ """The share snapshots api.""" from oslo_log import log -import six from six.moves import http_client import webob from webob import exc @@ -47,7 +46,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin, self.share_api = share.API() @wsgi.Controller.authorize('unmanage_snapshot') - def _unmanage(self, req, id, body=None): + def _unmanage(self, req, id, body=None, allow_dhss_true=False): """Unmanage a share snapshot.""" context = req.environ['manila.context'] @@ -57,7 +56,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin, snapshot = self.share_api.get_snapshot(context, id) share = self.share_api.get(context, snapshot['share_id']) - if share.get('share_server_id'): + if not allow_dhss_true and share.get('share_server_id'): msg = _("Operation 'unmanage_snapshot' is not supported for " "snapshots of shares that are created with share" " servers (created with share-networks).") @@ -76,7 +75,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin, self.share_api.unmanage_snapshot(context, snapshot, share['host']) except (exception.ShareSnapshotNotFound, exception.ShareNotFound) as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) return webob.Response(status_int=http_client.ACCEPTED) @@ -128,10 +127,10 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin, snapshot_ref = self.share_api.manage_snapshot(context, snapshot, driver_options) except (exception.ShareNotFound, exception.ShareSnapshotNotFound) as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) except (exception.InvalidShare, exception.ManageInvalidShareSnapshot) as e: - raise exc.HTTPConflict(explanation=six.text_type(e)) + raise exc.HTTPConflict(explanation=e) return self._view_builder.detail(req, snapshot_ref) @@ -266,11 +265,16 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin, def manage(self, req, body): return self._manage(req, body) - @wsgi.Controller.api_version('2.12') + @wsgi.Controller.api_version('2.12', '2.48') @wsgi.action('unmanage') def unmanage(self, req, id, body=None): return self._unmanage(req, id, body) + @wsgi.Controller.api_version('2.49') # noqa + @wsgi.action('unmanage') # pylint: disable=function-redefined + def unmanage(self, req, id, body=None): + return self._unmanage(req, id, body, allow_dhss_true=True) + @wsgi.Controller.api_version('2.32') @wsgi.action('allow_access') @wsgi.response(202) diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index 3188fda541..91cf829896 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -14,7 +14,6 @@ # under the License. from oslo_log import log -import six from six.moves import http_client import webob from webob import exc @@ -150,13 +149,13 @@ class ShareController(shares.ShareMixin, self.share_api.revert_to_snapshot(context, share, snapshot) except exception.ShareNotFound as e: - raise exc.HTTPNotFound(explanation=six.text_type(e)) + raise exc.HTTPNotFound(explanation=e) except exception.ShareSnapshotNotFound as e: - raise exc.HTTPBadRequest(explanation=six.text_type(e)) + raise exc.HTTPBadRequest(explanation=e) except exception.ShareSizeExceedsAvailableQuota as e: - raise exc.HTTPForbidden(explanation=six.text_type(e)) + raise exc.HTTPForbidden(explanation=e) except exception.ReplicationException as e: - raise exc.HTTPBadRequest(explanation=six.text_type(e)) + raise exc.HTTPBadRequest(explanation=e) return webob.Response(status_int=http_client.ACCEPTED) @@ -278,7 +277,7 @@ class ShareController(shares.ShareMixin, new_share_network=new_share_network, new_share_type=new_share_type) except exception.Conflict as e: - raise exc.HTTPConflict(explanation=six.text_type(e)) + raise exc.HTTPConflict(explanation=e) return webob.Response(status_int=return_code) @@ -407,18 +406,28 @@ class ShareController(shares.ShareMixin, @wsgi.Controller.api_version('2.7', '2.7') def manage(self, req, body): body.get('share', {}).pop('is_public', None) - detail = self._manage(req, body) + detail = self._manage(req, body, allow_dhss_true=False) return detail - @wsgi.Controller.api_version("2.8") # noqa + @wsgi.Controller.api_version("2.8", "2.48") # noqa def manage(self, req, body): # pylint: disable=function-redefined - detail = self._manage(req, body) + detail = self._manage(req, body, allow_dhss_true=False) return detail - @wsgi.Controller.api_version('2.7') + @wsgi.Controller.api_version("2.49") # noqa + def manage(self, req, body): # pylint: disable=function-redefined + detail = self._manage(req, body, allow_dhss_true=True) + return detail + + @wsgi.Controller.api_version('2.7', '2.48') @wsgi.action('unmanage') def unmanage(self, req, id, body=None): - return self._unmanage(req, id, body) + return self._unmanage(req, id, body, allow_dhss_true=False) + + @wsgi.Controller.api_version('2.49') # noqa + @wsgi.action('unmanage') # pylint: disable=function-redefined + def unmanage(self, req, id, body=None): + return self._unmanage(req, id, body, allow_dhss_true=True) @wsgi.Controller.api_version('2.27') @wsgi.action('revert') diff --git a/manila/api/views/share_servers.py b/manila/api/views/share_servers.py index 73e786ce52..e0c3252c7a 100644 --- a/manila/api/views/share_servers.py +++ b/manila/api/views/share_servers.py @@ -20,25 +20,29 @@ class ViewBuilder(common.ViewBuilder): """Model a server API response as a python dictionary.""" _collection_name = 'share_servers' + _detail_version_modifiers = [ + "add_is_auto_deletable_and_identifier_fields", + ] - def build_share_server(self, share_server): + def build_share_server(self, request, share_server): """View of a share server.""" return { 'share_server': - self._build_share_server_view(share_server, detailed=True) + self._build_share_server_view( + request, share_server, detailed=True) } - def build_share_servers(self, share_servers): + def build_share_servers(self, request, share_servers): return { 'share_servers': - [self._build_share_server_view(share_server) + [self._build_share_server_view(request, share_server) for share_server in share_servers] } def build_share_server_details(self, details): return {'details': details} - def _build_share_server_view(self, share_server, detailed=False): + def _build_share_server_view(self, request, share_server, detailed=False): share_server_dict = { 'id': share_server.id, 'project_id': share_server.project_id, @@ -51,4 +55,15 @@ class ViewBuilder(common.ViewBuilder): if detailed: share_server_dict['created_at'] = share_server.created_at share_server_dict['backend_details'] = share_server.backend_details + + self.update_versioned_resource_dict( + request, share_server_dict, share_server) + return share_server_dict + + @common.ViewBuilder.versioned_method("2.49") + def add_is_auto_deletable_and_identifier_fields( + self, context, share_server_dict, share_server): + share_server_dict['is_auto_deletable'] = ( + share_server['is_auto_deletable']) + share_server_dict['identifier'] = share_server['identifier'] diff --git a/manila/db/api.py b/manila/db/api.py index ace3f801bd..213c697bf6 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -859,9 +859,16 @@ def network_allocation_delete(context, id): return IMPL.network_allocation_delete(context, id) -def network_allocation_update(context, id, values): +def network_allocation_update(context, id, values, read_deleted=None): """Update a network allocation DB record.""" - return IMPL.network_allocation_update(context, id, values) + return IMPL.network_allocation_update(context, id, values, + read_deleted=read_deleted) + + +def network_allocation_get(context, id, session=None, read_deleted=None): + """Get a network allocation DB record.""" + return IMPL.network_allocation_get(context, id, session, + read_deleted=read_deleted) def network_allocations_get_for_share_server(context, share_server_id, @@ -899,6 +906,12 @@ def share_server_get(context, id, session=None): return IMPL.share_server_get(context, id, session=session) +def share_server_search_by_identifier(context, identifier, session=None): + """Search for share servers based on given identifier.""" + return IMPL.share_server_search_by_identifier( + context, identifier, session=session) + + def share_server_get_all_by_host_and_share_net_valid(context, host, share_net_id, session=None): diff --git a/manila/db/migrations/alembic/versions/6a3fd2984bc31_add_is_auto_deletable_and_identifier_fields_for_share_servers.py b/manila/db/migrations/alembic/versions/6a3fd2984bc31_add_is_auto_deletable_and_identifier_fields_for_share_servers.py new file mode 100644 index 0000000000..f7e27ad8ac --- /dev/null +++ b/manila/db/migrations/alembic/versions/6a3fd2984bc31_add_is_auto_deletable_and_identifier_fields_for_share_servers.py @@ -0,0 +1,72 @@ +# Copyright 2019 NetApp, Inc. +# 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. + +"""Add is_auto_deletable and identifier fields for share servers + +Revision ID: 6a3fd2984bc31 +Revises: 11ee96se625f3 +Create Date: 2018-10-29 11:27:44.194732 + +""" + +# revision identifiers, used by Alembic. +revision = '6a3fd2984bc31' +down_revision = '11ee96se625f3' + +from alembic import op +from oslo_log import log +import sqlalchemy as sa + +from manila.db.migrations import utils + + +LOG = log.getLogger(__name__) + + +def upgrade(): + + try: + op.add_column('share_servers', sa.Column( + 'is_auto_deletable', sa.Boolean, default=True)) + op.add_column('share_servers', sa.Column( + 'identifier', sa.String(length=255), default=None)) + except Exception: + LOG.error("Columns share_servers.is_auto_deletable " + "and/or share_servers.identifier not created!") + raise + + try: + connection = op.get_bind() + share_servers_table = utils.load_table('share_servers', connection) + for server in connection.execute(share_servers_table.select()): + connection.execute( + share_servers_table.update().where( + share_servers_table.c.id == server.id, + ).values({"identifier": server.id, "is_auto_deletable": True})) + except Exception: + LOG.error( + "Could not initialize share_servers.is_auto_deletable to True" + " and share_servers.identifier with the share server ID!") + raise + + +def downgrade(): + try: + op.drop_column('share_servers', 'is_auto_deletable') + op.drop_column('share_servers', 'identifier') + except Exception: + LOG.error("Columns share_servers.is_auto_deletable and/or " + "share_servers.identifier not dropped!") + raise diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index efa1408ac5..48ec90c704 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -43,6 +43,7 @@ import six from sqlalchemy import MetaData from sqlalchemy import or_ from sqlalchemy.orm import joinedload +from sqlalchemy.sql.expression import literal from sqlalchemy.sql.expression import true from sqlalchemy.sql import func @@ -3554,6 +3555,50 @@ def share_server_get(context, server_id, session=None): return result +@require_context +def share_server_search_by_identifier(context, identifier, session=None): + + identifier_field = models.ShareServer.identifier + + # try if given identifier is a substring of existing entry's identifier + result = (_server_get_query(context, session).filter( + identifier_field.like('%{}%'.format(identifier))).all()) + + if not result: + # repeat it with underscores instead of hyphens + result = (_server_get_query(context, session).filter( + identifier_field.like('%{}%'.format( + identifier.replace("-", "_")))).all()) + + if not result: + # repeat it with hypens instead of underscores + result = (_server_get_query(context, session).filter( + identifier_field.like('%{}%'.format( + identifier.replace("_", "-")))).all()) + + if not result: + # try if an existing identifier is a substring of given identifier + result = (_server_get_query(context, session).filter( + literal(identifier).contains(identifier_field)).all()) + + if not result: + # repeat it with underscores instead of hyphens + result = (_server_get_query(context, session).filter( + literal(identifier.replace("-", "_")).contains( + identifier_field)).all()) + + if not result: + # repeat it with hypens instead of underscores + result = (_server_get_query(context, session).filter( + literal(identifier.replace("_", "-")).contains( + identifier_field)).all()) + + if not result: + raise exception.ShareServerNotFound(share_server_id=identifier) + + return result + + @require_context def share_server_get_all_by_host_and_share_net_valid(context, host, share_net_id, @@ -3595,6 +3640,7 @@ def share_server_get_all_unused_deletable(context, host, updated_before): constants.STATUS_ERROR, ) result = (_server_get_query(context) + .filter_by(is_auto_deletable=True) .filter_by(host=host) .filter(~models.ShareServer.share_groups.any()) .filter(~models.ShareServer.share_instances.any()) @@ -3747,10 +3793,11 @@ def network_allocation_delete(context, id): @require_context -def network_allocation_get(context, id, session=None): +def network_allocation_get(context, id, session=None, read_deleted="no"): if session is None: session = get_session() - result = (model_query(context, models.NetworkAllocation, session=session). + result = (model_query(context, models.NetworkAllocation, session=session, + read_deleted=read_deleted). filter_by(id=id).first()) if result is None: raise exception.NotFound() @@ -3791,10 +3838,11 @@ def network_allocations_get_for_share_server(context, share_server_id, @require_context -def network_allocation_update(context, id, values): +def network_allocation_update(context, id, values, read_deleted=None): session = get_session() with session.begin(): - alloc_ref = network_allocation_get(context, id, session=session) + alloc_ref = network_allocation_get(context, id, session=session, + read_deleted=read_deleted) alloc_ref.update(values) alloc_ref.save(session=session) return alloc_ref diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 7c6e8c3ca9..de9b2567a6 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -951,10 +951,15 @@ class ShareServer(BASE, ManilaBase): share_network_id = Column(String(36), ForeignKey('share_networks.id'), nullable=True) host = Column(String(255), nullable=False) - status = Column(Enum(constants.STATUS_INACTIVE, constants.STATUS_ACTIVE, - constants.STATUS_ERROR, constants.STATUS_DELETING, - constants.STATUS_CREATING, constants.STATUS_DELETED), - default=constants.STATUS_INACTIVE) + is_auto_deletable = Column(Boolean, default=True) + identifier = Column(String(255), nullable=True) + status = Column(Enum( + constants.STATUS_INACTIVE, constants.STATUS_ACTIVE, + constants.STATUS_ERROR, constants.STATUS_DELETING, + constants.STATUS_CREATING, constants.STATUS_DELETED, + constants.STATUS_MANAGING, constants.STATUS_UNMANAGING, + constants.STATUS_UNMANAGE_ERROR, constants.STATUS_MANAGE_ERROR), + default=constants.STATUS_INACTIVE) network_allocations = orm.relationship( "NetworkAllocation", primaryjoin='and_(' diff --git a/manila/exception.py b/manila/exception.py index 9faf598ff0..11e8a9684a 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -459,6 +459,10 @@ class ManageInvalidShare(InvalidShare): "invalid share: %(reason)s") +class ManageShareServerError(ManilaException): + message = _("Manage existing share server failed due to: %(reason)s") + + class UnmanageInvalidShare(InvalidShare): message = _("Unmanage existing share failed due to " "invalid share: %(reason)s") diff --git a/manila/network/__init__.py b/manila/network/__init__.py index 31613ba812..bdc7d9173e 100644 --- a/manila/network/__init__.py +++ b/manila/network/__init__.py @@ -105,6 +105,15 @@ class NetworkBaseAPI(db_base.Base): def deallocate_network(self, context, share_server_id): pass + @abc.abstractmethod + def manage_network_allocations(self, context, allocations, share_server, + share_network=None): + pass + + @abc.abstractmethod + def unmanage_network_allocations(self, context, share_server_id): + pass + @property def enabled_ip_versions(self): if not hasattr(self, '_enabled_ip_versions'): diff --git a/manila/network/neutron/neutron_network_plugin.py b/manila/network/neutron/neutron_network_plugin.py index 9a7eb1b46e..13efb1fced 100644 --- a/manila/network/neutron/neutron_network_plugin.py +++ b/manila/network/neutron/neutron_network_plugin.py @@ -161,6 +161,108 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): return ports + def manage_network_allocations(self, context, allocations, share_server, + share_network=None): + + self._verify_share_network(share_server['id'], share_network) + self._store_neutron_net_info(context, share_network) + + # We begin matching the allocations to known neutron ports and + # finally return the non-consumed allocations + remaining_allocations = list(allocations) + + fixed_ip_filter = 'subnet_id=' + share_network['neutron_subnet_id'] + + port_list = self.neutron_api.list_ports( + network_id=share_network['neutron_net_id'], + device_owner='manila:share', + fixed_ips=fixed_ip_filter) + + selected_ports = self._get_ports_respective_to_ips( + remaining_allocations, port_list) + + LOG.debug("Found matching allocations in Neutron:" + " %s", six.text_type(selected_ports)) + + for selected_port in selected_ports: + port_dict = { + 'id': selected_port['port']['id'], + 'share_server_id': share_server['id'], + 'ip_address': selected_port['allocation'], + 'gateway': share_network['gateway'], + 'mac_address': selected_port['port']['mac_address'], + 'status': constants.STATUS_ACTIVE, + 'label': self.label, + 'network_type': share_network.get('network_type'), + 'segmentation_id': share_network.get('segmentation_id'), + 'ip_version': share_network['ip_version'], + 'cidr': share_network['cidr'], + 'mtu': share_network['mtu'], + } + + # There should not be existing allocations with the same port_id. + try: + existing_port = self.db.network_allocation_get( + context, selected_port['port']['id'], read_deleted=False) + except exception.NotFound: + pass + else: + msg = _("There were existing conflicting manila network " + "allocations found while trying to manage share " + "server %(new_ss)s. The conflicting port belongs to " + "share server %(old_ss)s.") % { + 'new_ss': share_server['id'], + 'old_ss': existing_port['share_server_id'], + } + raise exception.ManageShareServerError(reason=msg) + + # If there are previously deleted allocations, we undelete them + try: + self.db.network_allocation_get( + context, selected_port['port']['id'], read_deleted=True) + except exception.NotFound: + self.db.network_allocation_create(context, port_dict) + else: + port_dict.pop('id') + port_dict.update({ + 'deleted_at': None, + 'deleted': 'False', + }) + self.db.network_allocation_update( + context, selected_port['port']['id'], port_dict, + read_deleted=True) + + remaining_allocations.remove(selected_port['allocation']) + + return remaining_allocations + + def unmanage_network_allocations(self, context, share_server_id): + + ports = self.db.network_allocations_get_for_share_server( + context, share_server_id) + + for port in ports: + self.db.network_allocation_delete(context, port['id']) + + def _get_ports_respective_to_ips(self, allocations, port_list): + + selected_ports = [] + + for port in port_list: + for ip in port['fixed_ips']: + if ip['ip_address'] in allocations: + if not any(port['id'] == p['port']['id'] + for p in selected_ports): + selected_ports.append( + {'port': port, 'allocation': ip['ip_address']}) + else: + LOG.warning("Port %s has more than one IP that " + "matches allocations, please use ports " + "respective to only one allocation IP.", + port['id']) + + return selected_ports + def _get_matched_ip_address(self, fixed_ips, ip_version): """Get first ip address which matches the specified ip_version.""" @@ -314,8 +416,7 @@ class NeutronSingleNetworkPlugin(NeutronNetworkPlugin): self.subnet = self.neutron_api.configuration.neutron_subnet_id self._verify_net_and_subnet() - def allocate_network(self, context, share_server, share_network=None, - **kwargs): + def _select_proper_share_network(self, context, share_network): if self.label != 'admin': share_network = self._update_share_network_net_data( context, share_network) @@ -325,9 +426,27 @@ class NeutronSingleNetworkPlugin(NeutronNetworkPlugin): 'neutron_net_id': self.net, 'neutron_subnet_id': self.subnet, } + return share_network + + def allocate_network(self, context, share_server, share_network=None, + **kwargs): + + share_network = self._select_proper_share_network( + context, share_network) + return super(NeutronSingleNetworkPlugin, self).allocate_network( context, share_server, share_network, **kwargs) + def manage_network_allocations(self, context, allocations, share_server, + share_network=None): + + share_network = self._select_proper_share_network( + context, share_network) + + return super(NeutronSingleNetworkPlugin, + self).manage_network_allocations( + context, allocations, share_server, share_network) + def _verify_net_and_subnet(self): data = dict(net=self.net, subnet=self.subnet) if self.net and self.subnet: diff --git a/manila/network/standalone_network_plugin.py b/manila/network/standalone_network_plugin.py index 1d5c3c56bc..c6eca27d68 100644 --- a/manila/network/standalone_network_plugin.py +++ b/manila/network/standalone_network_plugin.py @@ -318,3 +318,46 @@ class StandaloneNetworkPlugin(network.NetworkBaseAPI): context, share_server_id) for allocation in allocations: self.db.network_allocation_delete(context, allocation['id']) + + def unmanage_network_allocations(self, context, share_server_id): + self.deallocate_network(context, share_server_id) + + def manage_network_allocations(self, context, allocations, share_server, + share_network=None): + if self.label != 'admin': + self._verify_share_network(share_server['id'], share_network) + else: + share_network = share_network or {} + self._save_network_info(context, share_network) + + # We begin matching the allocations to known neutron ports and + # finally return the non-consumed allocations + remaining_allocations = list(allocations) + + ips = [netaddr.IPAddress(allocation) for allocation + in remaining_allocations] + cidrs = [netaddr.IPNetwork(cidr) for cidr in self.allowed_cidrs] + selected_allocations = [] + + for ip in ips: + if any(ip in cidr for cidr in cidrs): + allocation = six.text_type(ip) + selected_allocations.append(allocation) + + for allocation in selected_allocations: + data = { + 'share_server_id': share_server['id'], + 'ip_address': allocation, + 'status': constants.STATUS_ACTIVE, + 'label': self.label, + 'network_type': share_network['network_type'], + 'segmentation_id': share_network['segmentation_id'], + 'cidr': share_network['cidr'], + 'gateway': share_network['gateway'], + 'ip_version': share_network['ip_version'], + 'mtu': share_network['mtu'], + } + self.db.network_allocation_create(context, data) + remaining_allocations.remove(allocation) + + return remaining_allocations diff --git a/manila/policies/share_server.py b/manila/policies/share_server.py index 0b0248cd9d..66c0c51494 100644 --- a/manila/policies/share_server.py +++ b/manila/policies/share_server.py @@ -63,6 +63,36 @@ share_server_policies = [ 'path': '/share-servers/{server_id}', } ]), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'manage_share_server', + check_str=base.RULE_ADMIN_API, + description="Manage share server.", + operations=[ + { + 'method': 'POST', + 'path': '/share-servers/manage' + } + ]), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'unmanage_share_server', + check_str=base.RULE_ADMIN_API, + description="Unmanage share server.", + operations=[ + { + 'method': 'POST', + 'path': '/share-servers/{share_server_id}/action' + } + ]), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'reset_status', + check_str=base.RULE_ADMIN_API, + description="Reset the status of a share server.", + operations=[ + { + 'method': 'POST', + 'path': '/share-servers/{share_server_id}/action' + } + ]), ] diff --git a/manila/share/api.py b/manila/share/api.py index 9037516c3f..ced4346b09 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -613,6 +613,36 @@ class API(base.Base): share_type_id = share_data['share_type_id'] share_type = share_types.get_share_type(context, share_type_id) + share_server_id = share_data.get('share_server_id') + + dhss = share_types.parse_boolean_extra_spec( + 'driver_handles_share_servers', + share_type['extra_specs']['driver_handles_share_servers']) + + if dhss and not share_server_id: + msg = _("Share Server ID parameter is required when managing a " + "share using a share type with " + "driver_handles_share_servers extra-spec set to True.") + raise exception.InvalidInput(reason=msg) + if not dhss and share_server_id: + msg = _("Share Server ID parameter is not expected when managing a" + " share using a share type with " + "driver_handles_share_servers extra-spec set to False.") + raise exception.InvalidInput(reason=msg) + + if share_server_id: + try: + share_server = self.db.share_server_get( + context, share_data['share_server_id']) + except exception.ShareServerNotFound: + msg = _("Share Server specified was not found.") + raise exception.InvalidInput(reason=msg) + + if share_server['status'] != constants.STATUS_ACTIVE: + msg = _("Share Server specified is not active.") + raise exception.InvalidShareServer(message=msg) + share_data['share_network_id'] = share_server['share_network_id'] + share_data.update({ 'user_id': context.user_id, 'project_id': context.project_id, @@ -988,6 +1018,65 @@ class API(base.Base): # and server deletion. self.share_rpcapi.delete_share_server(context, server) + def manage_share_server( + self, context, identifier, host, share_network, driver_opts): + """Manage a share server.""" + + try: + matched_servers = self.db.share_server_search_by_identifier( + context, identifier) + except exception.ShareServerNotFound: + pass + else: + msg = _("Identifier %(identifier)s specified matches existing " + "share servers: %(servers)s.") % { + 'identifier': identifier, + 'servers': ', '.join(s['identifier'] for s in matched_servers) + } + raise exception.InvalidInput(reason=msg) + + values = { + 'host': host, + 'share_network_id': share_network['id'], + 'status': constants.STATUS_MANAGING, + 'is_auto_deletable': False, + 'identifier': identifier, + } + + server = self.db.share_server_create(context, values) + + self.share_rpcapi.manage_share_server( + context, server, identifier, driver_opts) + + return self.db.share_server_get(context, server['id']) + + def unmanage_share_server(self, context, share_server, force=False): + """Unmanage a share server.""" + + shares = self.db.share_instances_get_all_by_share_server( + context, share_server['id']) + + if shares: + raise exception.ShareServerInUse( + share_server_id=share_server['id']) + + share_groups = self.db.share_group_get_all_by_share_server( + context, share_server['id']) + if share_groups: + LOG.error("share server '%(ssid)s' in use by share groups.", + {'ssid': share_server['id']}) + raise exception.ShareServerInUse( + share_server_id=share_server['id']) + + update_data = {'status': constants.STATUS_UNMANAGING, + 'terminated_at': timeutils.utcnow()} + + share_server = self.db.share_server_update( + context, share_server['id'], update_data) + + self.share_rpcapi.unmanage_share_server( + context, share_server, force=force) + def create_snapshot(self, context, share, name, description, force=False): policy.check_policy(context, 'share', 'create_snapshot', share) diff --git a/manila/share/driver.py b/manila/share/driver.py index e05e1bd2cb..ec8611d91e 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -933,6 +933,10 @@ class ShareDriver(object): If they are incompatible, raise a ManageExistingShareTypeMismatch, specifying a reason for the failure. + This method is invoked when the share is being managed with + a share type that has ``driver_handles_share_servers`` + extra-spec set to False. + :param share: Share model :param driver_options: Driver-specific options provided by admin. :return: share_update dictionary with required key 'size', @@ -940,11 +944,57 @@ class ShareDriver(object): """ raise NotImplementedError() + def manage_existing_with_server( + self, share, driver_options, share_server=None): + """Brings an existing share under Manila management. + + If the provided share is not valid, then raise a + ManageInvalidShare exception, specifying a reason for the failure. + + If the provided share is not in a state that can be managed, such as + being replicated on the backend, the driver *MUST* raise + ManageInvalidShare exception with an appropriate message. + + The share has a share_type, and the driver can inspect that and + compare against the properties of the referenced backend share. + If they are incompatible, raise a + ManageExistingShareTypeMismatch, specifying a reason for the failure. + + This method is invoked when the share is being managed with + a share type that has ``driver_handles_share_servers`` + extra-spec set to True. + + :param share: Share model + :param driver_options: Driver-specific options provided by admin. + :param share_server: Share server model or None. + :return: share_update dictionary with required key 'size', + which should contain size of the share. + """ + raise NotImplementedError() + def unmanage(self, share): """Removes the specified share from Manila management. Does not delete the underlying backend share. + For most drivers, this will not need to do anything. However, some + drivers might use this call as an opportunity to clean up any + Manila-specific configuration that they have associated with the + backend share. + + If provided share cannot be unmanaged, then raise an + UnmanageInvalidShare exception, specifying a reason for the failure. + + This method is invoked when the share is being unmanaged with + a share type that has ``driver_handles_share_servers`` + extra-spec set to False. + """ + + def unmanage_with_server(self, share, share_server=None): + """Removes the specified share from Manila management. + + Does not delete the underlying backend share. + For most drivers, this will not need to do anything. However, some drivers might use this call as an opportunity to clean up any Manila-specific configuration that they have associated with the @@ -952,6 +1002,10 @@ class ShareDriver(object): If provided share cannot be unmanaged, then raise an UnmanageInvalidShare exception, specifying a reason for the failure. + + This method is invoked when the share is being unmanaged with + a share type that has ``driver_handles_share_servers`` + extra-spec set to True. """ def manage_existing_snapshot(self, snapshot, driver_options): @@ -961,6 +1015,10 @@ class ShareDriver(object): ManageInvalidShareSnapshot exception, specifying a reason for the failure. + This method is invoked when the snapshot that is being managed + belongs to a share that has its share type with + ``driver_handles_share_servers`` extra-spec set to False. + :param snapshot: ShareSnapshotInstance model with ShareSnapshot data. Example:: @@ -988,6 +1046,46 @@ class ShareDriver(object): """ raise NotImplementedError() + def manage_existing_snapshot_with_server(self, snapshot, driver_options, + share_server=None): + """Brings an existing snapshot under Manila management. + + If provided snapshot is not valid, then raise a + ManageInvalidShareSnapshot exception, specifying a reason for + the failure. + + This method is invoked when the snapshot that is being managed + belongs to a share that has its share type with + ``driver_handles_share_servers`` extra-spec set to True. + + :param snapshot: ShareSnapshotInstance model with ShareSnapshot data. + + Example:: + { + 'id': , + 'snapshot_id': < snapshot id>, + 'provider_location': , + ... + } + + :param driver_options: Optional driver-specific options provided + by admin. + + Example:: + + { + 'key': 'value', + ... + } + + :param share_server: Share server model or None. + :return: model_update dictionary with required key 'size', + which should contain size of the share snapshot, and key + 'export_locations' containing a list of export locations, if + snapshots can be mounted. + """ + raise NotImplementedError() + def unmanage_snapshot(self, snapshot): """Removes the specified snapshot from Manila management. @@ -1001,6 +1099,29 @@ class ShareDriver(object): If provided share snapshot cannot be unmanaged, then raise an UnmanageInvalidShareSnapshot exception, specifying a reason for the failure. + + This method is invoked when the snapshot that is being unmanaged + belongs to a share that has its share type with + ``driver_handles_share_servers`` extra-spec set to False. + """ + + def unmanage_snapshot_with_server(self, snapshot, share_server=None): + """Removes the specified snapshot from Manila management. + + Does not delete the underlying backend share snapshot. + + For most drivers, this will not need to do anything. However, some + drivers might use this call as an opportunity to clean up any + Manila-specific configuration that they have associated with the + backend share snapshot. + + If provided share snapshot cannot be unmanaged, then raise an + UnmanageInvalidShareSnapshot exception, specifying a reason for + the failure. + + This method is invoked when the snapshot that is being unmanaged + belongs to a share that has its share type with + ``driver_handles_share_servers`` extra-spec set to True. """ def revert_to_snapshot(self, context, snapshot, share_access_rules, @@ -2572,3 +2693,51 @@ class ShareDriver(object): """ raise NotImplementedError() + + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + """Obtain network allocations used by share server. + + :param context: Current context. + :param share_server: Share server model. + :param identifier: A driver-specific share server identifier + :param driver_options: Dictionary of driver options to assist managing + the share server + :return: A list containing IP addresses allocated in the backend. + + Example:: + + ['10.10.10.10', 'fd11::2000', '192.168.10.10'] + + """ + raise NotImplementedError() + + def manage_server(self, context, share_server, identifier, driver_options): + """Manage the share server and return compiled back end details. + + :param context: Current context. + :param share_server: Share server model. + :param identifier: A driver-specific share server identifier + :param driver_options: Dictionary of driver options to assist managing + the share server + :return: Identifier and dictionary with back end details to be saved + in the database. + + Example:: + + 'my_new_server_identifier',{'server_name': 'my_old_server'} + + """ + raise NotImplementedError() + + def unmanage_server(self, server_details, security_services=None): + """Unmanages the share server. + + If a driver supports unmanaging of share servers, the driver must + override this method and return successfully. + + :param server_details: share server backend details. + :param security_services: list of security services configured with + this share server. + """ + raise NotImplementedError() diff --git a/manila/share/manager.py b/manila/share/manager.py index d1172b3eb2..0542000666 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -30,7 +30,6 @@ from oslo_serialization import jsonutils from oslo_service import periodic_task from oslo_utils import excutils from oslo_utils import importutils -from oslo_utils import strutils from oslo_utils import timeutils import six @@ -210,7 +209,7 @@ def add_hooks(f): class ShareManager(manager.SchedulerDependentManager): """Manages NAS storages.""" - RPC_API_VERSION = '1.18' + RPC_API_VERSION = '1.19' def __init__(self, share_driver=None, service_name=None, *args, **kwargs): """Load the driver from args, or from flags.""" @@ -597,7 +596,7 @@ class ShareManager(manager.SchedulerDependentManager): { 'host': self.host, 'share_network_id': share_network_id, - 'status': constants.STATUS_CREATING + 'status': constants.STATUS_CREATING, } ) @@ -2357,6 +2356,24 @@ class ShareManager(manager.SchedulerDependentManager): {'id': share_replica['id'], 'state': replica_state}) LOG.warning(msg) + def _validate_share_and_driver_mode(self, share_instance): + driver_dhss = self.driver.driver_handles_share_servers + + share_dhss = share_types.parse_boolean_extra_spec( + 'driver_handles_share_servers', + share_types.get_share_type_extra_specs( + share_instance['share_type_id'], + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS)) + + if driver_dhss != share_dhss: + msg = _("Driver mode of share %(share)s being managed is " + "incompatible with mode DHSS=%(dhss)s configured for" + " this backend.") % {'share': share_instance['share_id'], + 'dhss': driver_dhss} + raise exception.InvalidShare(reason=msg) + + return driver_dhss + @add_hooks @utils.require_driver_initialized def manage_share(self, context, share_id, driver_options): @@ -2366,25 +2383,23 @@ class ShareManager(manager.SchedulerDependentManager): project_id = share_ref['project_id'] try: - if self.driver.driver_handles_share_servers: - msg = _("Manage share is not supported for " - "driver_handles_share_servers=True mode.") - raise exception.InvalidDriverMode(driver_mode=msg) - driver_mode = share_types.get_share_type_extra_specs( - share_instance['share_type_id'], - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS) + driver_dhss = self._validate_share_and_driver_mode(share_instance) - if strutils.bool_from_string(driver_mode): - msg = _("%(mode)s != False") % { - 'mode': constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS - } - raise exception.ManageExistingShareTypeMismatch(reason=msg) + if driver_dhss is True: + share_server = self._get_share_server(context, share_instance) - share_update = ( - self.driver.manage_existing(share_instance, driver_options) - or {} - ) + share_update = ( + self.driver.manage_existing_with_server( + share_instance, driver_options, share_server) + or {} + ) + else: + share_update = ( + self.driver.manage_existing( + share_instance, driver_options) + or {} + ) if not share_update.get('size'): msg = _("Driver cannot calculate share size.") @@ -2436,47 +2451,34 @@ class ShareManager(manager.SchedulerDependentManager): @add_hooks @utils.require_driver_initialized def manage_snapshot(self, context, snapshot_id, driver_options): - if self.driver.driver_handles_share_servers: - msg = _("Manage snapshot is not supported for " - "driver_handles_share_servers=True mode.") - # NOTE(vponomaryov): set size as 1 because design expects size - # to be set, it also will allow us to handle delete/unmanage - # operations properly with this errored snapshot according to - # quotas. - self.db.share_snapshot_update( - context, snapshot_id, - {'status': constants.STATUS_MANAGE_ERROR, 'size': 1}) - raise exception.InvalidDriverMode(driver_mode=msg) context = context.elevated() snapshot_ref = self.db.share_snapshot_get(context, snapshot_id) - share_server = self._get_share_server(context, - snapshot_ref['share']) - - if share_server: - msg = _("Manage snapshot is not supported for " - "share snapshots with share servers.") - # NOTE(vponomaryov): set size as 1 because design expects size - # to be set, it also will allow us to handle delete/unmanage - # operations properly with this errored snapshot according to - # quotas. - self.db.share_snapshot_update( - context, snapshot_id, - {'status': constants.STATUS_MANAGE_ERROR, 'size': 1}) - raise exception.InvalidShareSnapshot(reason=msg) snapshot_instance = self.db.share_snapshot_instance_get( context, snapshot_ref.instance['id'], with_share_data=True ) project_id = snapshot_ref['project_id'] + driver_dhss = self.driver.driver_handles_share_servers + try: - snapshot_update = ( - self.driver.manage_existing_snapshot( - snapshot_instance, - driver_options) - or {} - ) + if driver_dhss is True: + + share_server = self._get_share_server(context, + snapshot_ref['share']) + + snapshot_update = ( + self.driver.manage_existing_snapshot_with_server( + snapshot_instance, driver_options, share_server) + or {} + ) + else: + snapshot_update = ( + self.driver.manage_existing_snapshot( + snapshot_instance, driver_options) + or {} + ) if not snapshot_update.get('size'): snapshot_update['size'] = snapshot_ref['share']['size'] @@ -2541,7 +2543,7 @@ class ShareManager(manager.SchedulerDependentManager): context = context.elevated() share_ref = self.db.share_get(context, share_id) share_instance = self._get_share_instance(context, share_ref) - share_server = self._get_share_server(context, share_instance) + share_server = None project_id = share_ref['project_id'] def share_manage_set_error_status(msg, exception): @@ -2549,18 +2551,14 @@ class ShareManager(manager.SchedulerDependentManager): self.db.share_update(context, share_id, status) LOG.error(msg, exception) + dhss = self.driver.driver_handles_share_servers + try: - if self.driver.driver_handles_share_servers: - msg = _("Unmanage share is not supported for " - "driver_handles_share_servers=True mode.") - raise exception.InvalidShare(reason=msg) - - if share_server: - msg = _("Unmanage share is not supported for " - "shares with share servers.") - raise exception.InvalidShare(reason=msg) - - self.driver.unmanage(share_instance) + if dhss is True: + share_server = self._get_share_server(context, share_instance) + self.driver.unmanage_with_server(share_instance, share_server) + else: + self.driver.unmanage(share_instance) except exception.InvalidShare as e: share_manage_set_error_status( @@ -2600,19 +2598,20 @@ class ShareManager(manager.SchedulerDependentManager): return self.db.share_instance_delete(context, share_instance['id']) + + # NOTE(ganso): Since we are unmanaging a share that is still within a + # share server, we need to prevent the share server from being + # auto-deleted. + if share_server and share_server['is_auto_deletable']: + self.db.share_server_update(context, share_server['id'], + {'is_auto_deletable': False}) + LOG.info("Share %s: unmanaged successfully.", share_id) @add_hooks @utils.require_driver_initialized def unmanage_snapshot(self, context, snapshot_id): status = {'status': constants.STATUS_UNMANAGE_ERROR} - if self.driver.driver_handles_share_servers: - msg = _("Unmanage snapshot is not supported for " - "driver_handles_share_servers=True mode.") - self.db.share_snapshot_update(context, snapshot_id, status) - LOG.error("Share snapshot cannot be unmanaged: %s.", - msg) - return context = context.elevated() snapshot_ref = self.db.share_snapshot_get(context, snapshot_id) @@ -2625,14 +2624,6 @@ class ShareManager(manager.SchedulerDependentManager): project_id = snapshot_ref['project_id'] - if share_server: - msg = _("Unmanage snapshot is not supported for " - "share snapshots with share servers.") - self.db.share_snapshot_update(context, snapshot_id, status) - LOG.error("Share snapshot cannot be unmanaged: %s.", - msg) - return - if self.configuration.safe_get('unmanage_remove_access_rules'): try: self.snapshot_access_helper.update_access_rules( @@ -2647,8 +2638,14 @@ class ShareManager(manager.SchedulerDependentManager): self.db.share_snapshot_update(context, snapshot_id, status) return + dhss = self.driver.driver_handles_share_servers + try: - self.driver.unmanage_snapshot(snapshot_instance) + if dhss: + self.driver.unmanage_snapshot_with_server( + snapshot_instance, share_server) + else: + self.driver.unmanage_snapshot(snapshot_instance) except exception.UnmanageInvalidShareSnapshot as e: self.db.share_snapshot_update(context, snapshot_id, status) LOG.error("Share snapshot cannot be unmanaged: %s.", e) @@ -2677,6 +2674,169 @@ class ShareManager(manager.SchedulerDependentManager): self.db.share_snapshot_instance_delete( context, snapshot_instance['id']) + @add_hooks + @utils.require_driver_initialized + def manage_share_server(self, context, share_server_id, identifier, + driver_opts): + + if self.driver.driver_handles_share_servers is False: + msg = _("Cannot manage share server %s in a " + "backend configured with driver_handles_share_servers" + " set to False.") % share_server_id + raise exception.ManageShareServerError(reason=msg) + + server = self.db.share_server_get(context, share_server_id) + + share_network = self.db.share_network_get( + context, server['share_network_id']) + + try: + + number_allocations = ( + self.driver.get_network_allocations_number()) + + if self.driver.admin_network_api: + number_allocations += ( + self.driver.get_admin_network_allocations_number()) + + if number_allocations > 0: + + # allocations obtained from the driver that still need to + # be validated + remaining_allocations = ( + self.driver.get_share_server_network_info( + context, server, identifier, driver_opts)) + + if len(remaining_allocations) > 0: + + if self.driver.admin_network_api: + remaining_allocations = ( + self.driver.admin_network_api. + manage_network_allocations( + context, remaining_allocations, server)) + + # allocations that are managed are removed from + # remaining_allocations + + remaining_allocations = ( + self.driver.network_api. + manage_network_allocations( + context, remaining_allocations, server, + share_network)) + + # We require that all allocations are managed, else we + # may have problems deleting this share server + if len(remaining_allocations) > 0: + msg = ("Failed to manage all allocations. " + "Allocations %s were not " + "managed." % six.text_type( + remaining_allocations)) + raise exception.ManageShareServerError(reason=msg) + + else: + # if there should be allocations, but the driver + # doesn't return any something is wrong + + msg = ("Driver did not return required network " + "allocations to be managed. Required number " + "of allocations is %s." % number_allocations) + raise exception.ManageShareServerError(reason=msg) + + new_identifier, backend_details = self.driver.manage_server( + context, server, identifier, driver_opts) + + if not new_identifier: + new_identifier = server['id'] + + if backend_details is None or not isinstance( + backend_details, dict): + backend_details = {} + + for security_service in share_network['security_services']: + ss_type = security_service['type'] + data = { + 'name': security_service['name'], + 'ou': security_service['ou'], + 'domain': security_service['domain'], + 'server': security_service['server'], + 'dns_ip': security_service['dns_ip'], + 'user': security_service['user'], + 'type': ss_type, + 'password': security_service['password'], + } + backend_details.update({ + 'security_service_' + ss_type: jsonutils.dumps(data) + }) + + if backend_details: + self.db.share_server_backend_details_set( + context, server['id'], backend_details) + + self.db.share_server_update( + context, share_server_id, + {'status': constants.STATUS_ACTIVE, + 'identifier': new_identifier}) + + except Exception: + msg = "Error managing share server %s" + LOG.exception(msg, share_server_id) + self.db.share_server_update( + context, share_server_id, + {'status': constants.STATUS_MANAGE_ERROR}) + raise + + LOG.info("Share server %s managed successfully.", share_server_id) + + @add_hooks + @utils.require_driver_initialized + def unmanage_share_server(self, context, share_server_id, force=False): + + server = self.db.share_server_get( + context, share_server_id) + server_details = server['backend_details'] + + security_services = [] + for ss_name in constants.SECURITY_SERVICES_ALLOWED_TYPES: + ss = server_details.get('security_service_' + ss_name) + if ss: + security_services.append(jsonutils.loads(ss)) + + try: + self.driver.unmanage_server(server_details, security_services) + except NotImplementedError: + if not force: + LOG.error("Did not unmanage share server %s since the driver " + "does not support managing share servers and no " + "``force`` option was supplied.", + share_server_id) + self.db.share_server_update( + context, share_server_id, + {'status': constants.STATUS_UNMANAGE_ERROR}) + return + + try: + + if self.driver.get_network_allocations_number() > 0: + # NOTE(ganso): This will already remove admin allocations. + self.driver.network_api.unmanage_network_allocations( + context, share_server_id) + elif (self.driver.get_admin_network_allocations_number() > 0 + and self.driver.admin_network_api): + # NOTE(ganso): This is here in case there are only admin + # allocations. + self.driver.admin_network_api.unmanage_network_allocations( + context, share_server_id) + self.db.share_server_delete(context, share_server_id) + except Exception: + msg = "Error unmanaging share server %s" + LOG.exception(msg, share_server_id) + self.db.share_server_update( + context, share_server_id, + {'status': constants.STATUS_UNMANAGE_ERROR}) + raise + + LOG.info("Share server %s unmanaged successfully.", share_server_id) + @add_hooks @utils.require_driver_initialized def revert_to_snapshot(self, context, snapshot_id, @@ -2858,7 +3018,8 @@ class ShareManager(manager.SchedulerDependentManager): if CONF.delete_share_server_with_last_share: share_server = self._get_share_server(context, share_instance) - if share_server and len(share_server.share_instances) == 0: + if (share_server and len(share_server.share_instances) == 0 + and share_server.is_auto_deletable is True): LOG.debug("Scheduled deletion of share-server " "with id '%s' automatically by " "deletion of last share.", share_server['id']) @@ -3477,7 +3638,9 @@ class ShareManager(manager.SchedulerDependentManager): context, share_server['id'], server_info) return self.db.share_server_update( context, share_server['id'], - {'status': constants.STATUS_ACTIVE}) + {'status': constants.STATUS_ACTIVE, + 'identifier': server_info.get( + 'identifier', share_server['id'])}) except Exception as e: with excutils.save_and_reraise_exception(): details = getattr(e, "detail_data", {}) diff --git a/manila/share/rpcapi.py b/manila/share/rpcapi.py index 19c33506f5..92ecfaf1fd 100644 --- a/manila/share/rpcapi.py +++ b/manila/share/rpcapi.py @@ -75,6 +75,7 @@ class ShareAPI(object): create_share_group_snapshot, and delete_share_group_snapshot 1.17 - Add snapshot_update_access() 1.18 - Remove unused "share_id" parameter from revert_to_snapshot() + 1.19 - Add manage_share_server() and unmanage_share_server() """ BASE_RPC_API_VERSION = '1.0' @@ -83,7 +84,7 @@ class ShareAPI(object): super(ShareAPI, self).__init__() target = messaging.Target(topic=CONF.share_topic, version=self.BASE_RPC_API_VERSION) - self.client = rpc.get_client(target, version_cap='1.18') + self.client = rpc.get_client(target, version_cap='1.19') def create_share_instance(self, context, share_instance, host, request_spec, filter_properties, @@ -127,6 +128,22 @@ class ShareAPI(object): 'unmanage_snapshot', snapshot_id=snapshot['id']) + def manage_share_server( + self, context, share_server, identifier, driver_opts): + host = utils.extract_host(share_server['host']) + call_context = self.client.prepare(server=host, version='1.19') + call_context.cast(context, 'manage_share_server', + share_server_id=share_server['id'], + identifier=identifier, + driver_opts=driver_opts) + + def unmanage_share_server(self, context, share_server, force=False): + host = utils.extract_host(share_server['host']) + call_context = self.client.prepare(server=host, version='1.19') + call_context.cast(context, 'unmanage_share_server', + share_server_id=share_server['id'], + force=force) + def revert_to_snapshot(self, context, share, snapshot, host, reservations): host = utils.extract_host(host) call_context = self.client.prepare(server=host, version='1.18') diff --git a/manila/tests/api/fakes.py b/manila/tests/api/fakes.py index f7c93c0908..2a4062402c 100644 --- a/manila/tests/api/fakes.py +++ b/manila/tests/api/fakes.py @@ -36,8 +36,12 @@ from manila import context from manila import exception +CONTEXT = context.get_admin_context() +driver_opts = {} FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' FAKE_UUIDS = {} +host = 'host_name' +identifier = '7cf7c200-d3af-4e05-b87e-9167c95dfcad' class Context(object): @@ -294,6 +298,20 @@ fixture_valid_reset_status_body = ( ({'reset_status': {'status': 'migrating_to'}}, '2.7'), ) +share_network_id = '5dfe0898-e2a1-4740-9177-81c7d26713b0' +share_network = { + 'name': 'share-net-fake', + 'share_network_id': '5dfe0898-e2a1-4740-9177-81c7d26713b0', + 'project_id': '5dfe0898-e2a1-4740-9177-81c7d26713b0' +} +SHARE_SERVER = { + 'share_network': share_network, + 'share_network_id': 'c5b3a865-56d0-4d88-abe5-879965e099c9', + 'host': host, + 'id': 'c39bb9ae-16a5-40f2-a24f-1cf3f549d3d3', + 'status': constants.STATUS_ACTIVE +} + def mock_fake_admin_check(context, resource_name, action, *args, **kwargs): if context.is_admin: diff --git a/manila/tests/api/v1/test_share_manage.py b/manila/tests/api/v1/test_share_manage.py index 2707f0e7bb..1dcad16d20 100644 --- a/manila/tests/api/v1/test_share_manage.py +++ b/manila/tests/api/v1/test_share_manage.py @@ -155,6 +155,36 @@ class ShareManageTest(test.TestCase): self.mock_policy_check.assert_called_once_with( self.context, self.resource_name, 'manage') + def test_share_manage_invalid_input(self): + body = get_fake_manage_body() + self._setup_manage_mocks() + error = mock.Mock(side_effect=exception.InvalidInput(message="", + reason="fake")) + self.mock_object(share_api.API, 'manage', mock.Mock(side_effect=error)) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, + self.request, + body) + self.mock_policy_check.assert_called_once_with( + self.context, self.resource_name, 'manage') + + def test_share_manage_invalid_share_server(self): + body = get_fake_manage_body() + self._setup_manage_mocks() + error = mock.Mock( + side_effect=exception.InvalidShareServer(message="", + share_server_id="") + ) + self.mock_object(share_api.API, 'manage', mock.Mock(side_effect=error)) + + self.assertRaises(webob.exc.HTTPConflict, + self.controller.create, + self.request, + body) + self.mock_policy_check.assert_called_once_with( + self.context, self.resource_name, 'manage') + @ddt.data( get_fake_manage_body(name='foo', description='bar'), get_fake_manage_body(display_name='foo', description='bar'), @@ -187,6 +217,32 @@ class ShareManageTest(test.TestCase): self.mock_policy_check.assert_called_once_with( self.context, self.resource_name, 'manage') + def test_share_manage_allow_dhss_true(self): + self._setup_manage_mocks() + data = get_fake_manage_body(name='foo', description='bar') + return_share = {'share_type_id': '', 'id': 'fake'} + self.mock_object( + share_api.API, 'manage', mock.Mock(return_value=return_share)) + share = { + 'host': data['share']['service_host'], + 'export_location': data['share']['export_path'], + 'share_proto': data['share']['protocol'].upper(), + 'share_type_id': 'fake', + 'display_name': 'foo', + 'display_description': 'bar', + 'share_server_id': 'fake' + } + data['share']['share_server_id'] = 'fake' + driver_options = data['share'].get('driver_options', {}) + + self.controller._manage(self.request, data, allow_dhss_true=True) + + share_api.API.manage.assert_called_once_with( + self.context, share, driver_options + ) + self.mock_policy_check.assert_called_once_with( + self.context, self.resource_name, 'manage') + def test_wrong_permissions(self): body = get_fake_manage_body() fake_req = fakes.HTTPRequest.blank( diff --git a/manila/tests/api/v1/test_share_servers.py b/manila/tests/api/v1/test_share_servers.py index e0882dbd78..1cbaaca9aa 100644 --- a/manila/tests/api/v1/test_share_servers.py +++ b/manila/tests/api/v1/test_share_servers.py @@ -23,7 +23,7 @@ from manila.db import api as db_api from manila import exception from manila import policy from manila import test - +from manila.tests.api import fakes fake_share_server_list = { 'share_servers': [ @@ -163,70 +163,100 @@ class ShareServerAPITest(test.TestCase): self.mock_object(db_api, 'share_server_get_all', mock.Mock(return_value=fake_share_server_get_all())) + def _prepare_request(self, url, use_admin_context): + request = fakes.HTTPRequest.blank(url, + use_admin_context=use_admin_context) + ctxt = request.environ['manila.context'] + return request, ctxt + def test_index_no_filters(self): - result = self.controller.index(FakeRequestAdmin) + request, ctxt = self._prepare_request(url='/v2/share-servers/', + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual(fake_share_server_list, result) def test_index_host_filter(self): - result = self.controller.index(FakeRequestWithHost) + request, ctxt = self._prepare_request( + url='/index?host=%s' + % fake_share_server_list['share_servers'][0]['host'], + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual([fake_share_server_list['share_servers'][0]], result['share_servers']) def test_index_status_filter(self): - result = self.controller.index(FakeRequestWithStatus) + request, ctxt = self._prepare_request(url='/index?status=%s' % + constants.STATUS_ERROR, + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual([fake_share_server_list['share_servers'][1]], result['share_servers']) def test_index_project_id_filter(self): - result = self.controller.index(FakeRequestWithProjectId) + request, ctxt = self._prepare_request( + url='/index?project_id=%s' + % fake_share_server_get_all()[0].project_id, + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual([fake_share_server_list['share_servers'][0]], result['share_servers']) def test_index_share_network_filter_by_name(self): - result = self.controller.index(FakeRequestWithShareNetworkName) + request, ctxt = self._prepare_request( + url='/index?host=%s' + % fake_share_server_list['share_servers'][0]['host'], + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual([fake_share_server_list['share_servers'][0]], result['share_servers']) def test_index_share_network_filter_by_id(self): - result = self.controller.index(FakeRequestWithShareNetworkId) + request, ctxt = self._prepare_request( + url='/index?share_network=%s' + % fake_share_server_get_all()[0].share_network['id'], + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual([fake_share_server_list['share_servers'][0]], result['share_servers']) def test_index_fake_filter(self): - result = self.controller.index(FakeRequestWithFakeFilter) + request, ctxt = self._prepare_request(url='/index?fake_key=fake_value', + use_admin_context=True) + result = self.controller.index(request) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'index') - db_api.share_server_get_all.assert_called_once_with(CONTEXT) + ctxt, self.resource_name, 'index') + db_api.share_server_get_all.assert_called_once_with(ctxt) self.assertEqual(0, len(result['share_servers'])) def test_show(self): self.mock_object(db_api, 'share_server_get', mock.Mock(return_value=fake_share_server_get())) + request, ctxt = self._prepare_request('/show', use_admin_context=True) result = self.controller.show( - FakeRequestAdmin, + request, fake_share_server_get_result['share_server']['id']) policy.check_policy.assert_called_once_with( - CONTEXT, self.resource_name, 'show') + ctxt, self.resource_name, 'show') db_api.share_server_get.assert_called_once_with( - CONTEXT, fake_share_server_get_result['share_server']['id']) + ctxt, fake_share_server_get_result['share_server']['id']) self.assertEqual(fake_share_server_get_result['share_server'], result['share_server']) @@ -235,6 +265,7 @@ class ShareServerAPITest(test.TestCase): mock.Mock(return_value=fake_share_server_get())) result = self.controller.details( FakeRequestAdmin, + fake_share_server_get_result['share_server']['id']) policy.check_policy.assert_called_once_with( CONTEXT, self.resource_name, 'details') diff --git a/manila/tests/api/v1/test_share_unmanage.py b/manila/tests/api/v1/test_share_unmanage.py index 727b3b9db9..be6c9cb8f4 100644 --- a/manila/tests/api/v1/test_share_unmanage.py +++ b/manila/tests/api/v1/test_share_unmanage.py @@ -177,6 +177,26 @@ class ShareUnmanageTest(test.TestCase): self.mock_policy_check.assert_called_once_with( self.context, self.resource_name, 'unmanage') + def test_unmanage_allow_dhss_true_with_share_server(self): + share = { + 'status': constants.STATUS_AVAILABLE, + 'id': 'foo_id', + 'instance': '', + 'share_server_id': 'fake' + } + self.mock_object(share_api.API, 'get', mock.Mock(return_value=share)) + self.mock_object(share_api.API, 'unmanage', mock.Mock()) + self.mock_object( + self.controller.share_api.db, 'share_snapshot_get_all_for_share', + mock.Mock(return_value=[])) + + actual_result = self.controller._unmanage(self.request, share['id'], + allow_dhss_true=True) + + self.assertEqual(202, actual_result.status_int) + self.mock_policy_check.assert_called_once_with( + self.context, self.resource_name, 'unmanage') + def test_wrong_permissions(self): share_id = 'fake' req = fakes.HTTPRequest.blank('/share/%s/unmanage' % share_id, diff --git a/manila/tests/api/v2/test_share_servers.py b/manila/tests/api/v2/test_share_servers.py new file mode 100644 index 0000000000..c5a3f8df44 --- /dev/null +++ b/manila/tests/api/v2/test_share_servers.py @@ -0,0 +1,390 @@ +# Copyright 2019 NetApp, Inc. +# 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 +import webob + +from manila.api.v2 import share_servers +from manila.common import constants +from manila.db import api as db_api +from manila import exception +from manila import policy +from manila.share import api as share_api +from manila import test +from manila.tests.api import fakes +from manila.tests import db_utils +from manila import utils + + +@ddt.ddt +class ShareServerControllerTest(test.TestCase): + """Share server api test""" + + def setUp(self): + super(ShareServerControllerTest, self).setUp() + self.mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=True)) + self.controller = share_servers.ShareServerController() + self.resource_name = self.controller.resource_name + + @ddt.data(constants.STATUS_ACTIVE, constants.STATUS_ERROR, + constants.STATUS_DELETING, constants.STATUS_CREATING, + constants.STATUS_MANAGING, constants.STATUS_UNMANAGING, + constants.STATUS_UNMANAGE_ERROR, constants.STATUS_MANAGE_ERROR) + def test_share_server_reset_status(self, status): + req = fakes.HTTPRequest.blank('/v2/share-servers/fake-share-server/', + use_admin_context=True) + body = {'reset_status': {'status': status}} + + context = req.environ['manila.context'] + mock_update = self.mock_object(db_api, 'share_server_update') + + result = self.controller.share_server_reset_status( + req, 'fake_server_id', body) + + self.assertEqual(202, result.status_int) + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'reset_status') + mock_update.assert_called_once_with( + context, 'fake_server_id', {'status': status}) + + def test_share_reset_server_status_invalid(self): + req = fakes.HTTPRequest.blank('/reset_status', use_admin_context=True) + body = {'reset_status': {'status': constants.STATUS_EXTENDING}} + context = req.environ['manila.context'] + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller.share_server_reset_status, + req, id='fake_server_id', body=body) + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'reset_status') + + def test_share_server_reset_status_no_body(self): + req = fakes.HTTPRequest.blank('/reset_status', use_admin_context=True) + context = req.environ['manila.context'] + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller.share_server_reset_status, + req, id='fake_server_id', body={}) + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'reset_status') + + def test_share_server_reset_status_no_status(self): + req = fakes.HTTPRequest.blank('/reset_status', use_admin_context=True) + context = req.environ['manila.context'] + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller.share_server_reset_status, + req, id='fake_server_id', body={'reset_status': {}}) + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'reset_status') + + def _setup_manage_test_request_body(self): + body = { + 'share_network_id': 'fake_net_id', + 'host': 'fake_host', + 'identifier': 'fake_identifier', + 'driver_options': {'opt1': 'fake_opt1', 'opt2': 'fake_opt2'}, + } + return body + + @ddt.data('fake_net_name', '') + def test_manage(self, share_net_name): + """Tests share server manage""" + req = fakes.HTTPRequest.blank('/v2/share-servers/', + use_admin_context=True, + version="2.49") + context = req.environ['manila.context'] + share_network = db_utils.create_share_network(name=share_net_name) + share_server = db_utils.create_share_server( + share_network_id=share_network['id'], + host='fake_host', + identifier='fake_identifier', + is_auto_deletable=False) + + self.mock_object(db_api, 'share_network_get', mock.Mock( + return_value=share_network)) + self.mock_object(utils, 'validate_service_host') + + body = { + 'share_server': self._setup_manage_test_request_body() + } + + manage_share_server_mock = self.mock_object( + share_api.API, 'manage_share_server', + mock.Mock(return_value=share_server)) + + result = self.controller.manage(req, body) + expected_result = { + 'share_server': { + 'id': share_server['id'], + 'project_id': 'fake', + 'updated_at': None, + 'status': constants.STATUS_ACTIVE, + 'host': 'fake_host', + 'share_network_id': share_server['share_network_id'], + 'created_at': share_server['created_at'], + 'backend_details': {}, + 'identifier': share_server['identifier'], + 'is_auto_deletable': share_server['is_auto_deletable'], + } + } + if share_net_name != '': + expected_result['share_server']['share_network_name'] = ( + 'fake_net_name') + else: + expected_result['share_server']['share_network_name'] = ( + share_server['share_network_id']) + + req_params = body['share_server'] + manage_share_server_mock.assert_called_once_with( + context, req_params['identifier'], req_params['host'], + share_network, req_params['driver_options']) + + self.assertEqual(expected_result, result) + + self.mock_policy_check.assert_called_once_with( + context, self.resource_name, 'manage_share_server') + + def test_manage_invalid(self): + req = fakes.HTTPRequest.blank('/manage_share_server', + use_admin_context=True) + context = req.environ['manila.context'] + share_network = db_utils.create_share_network() + + body = { + 'share_server': self._setup_manage_test_request_body() + } + self.mock_object(utils, 'validate_service_host') + self.mock_object(db_api, 'share_network_get', + mock.Mock(return_value=share_network)) + + manage_share_server_mock = self.mock_object( + share_api.API, 'manage_share_server', + mock.Mock(side_effect=exception.InvalidInput('foobar'))) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.manage, req, body) + + req_params = body['share_server'] + manage_share_server_mock.assert_called_once_with( + context, req_params['identifier'], req_params['host'], + share_network, req_params['driver_options']) + + def test_manage_forbidden(self): + """Tests share server manage without admin privileges""" + req = fakes.HTTPRequest.blank('/manage_share_server') + self.mock_object(share_api.API, 'manage_share_server', mock.Mock()) + error = mock.Mock(side_effect=exception.PolicyNotAuthorized(action='')) + self.mock_object(share_api.API, 'manage_share_server', error) + + body = { + 'share_server': self._setup_manage_test_request_body() + } + + self.assertRaises(webob.exc.HTTPForbidden, + self.controller.manage, + req, + body) + + def test__validate_manage_share_server_validate_no_body(self): + """Tests share server manage""" + req = fakes.HTTPRequest.blank('/manage') + body = {} + + self.assertRaises(webob.exc.HTTPUnprocessableEntity, + self.controller.manage, + req, + body) + + @ddt.data({'empty': False, 'key': 'host'}, + {'empty': False, 'key': 'share_network_id'}, + {'empty': False, 'key': 'identifier'}, + {'empty': True, 'key': 'host'}, + {'empty': True, 'key': 'share_network_id'}, + {'empty': True, 'key': 'identifier'}) + @ddt.unpack + def test__validate_manage_share_server_validate_without_parameters( + self, empty, key): + """Tests share server manage without some parameters""" + req = fakes.HTTPRequest.blank('/manage_share_server') + self.mock_object(share_api.API, 'manage_share_server', mock.Mock()) + + body = { + 'share_server': self._setup_manage_test_request_body(), + } + + if empty: + body['share_server'][key] = None + else: + body['share_server'].pop(key) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.manage, + req, + body) + + @ddt.data( + (webob.exc.HTTPBadRequest, exception.ServiceNotFound('foobar')), + (webob.exc.HTTPBadRequest, exception.ServiceIsDown('foobar')), + (webob.exc.HTTPForbidden, exception.PolicyNotAuthorized('foobar')), + (webob.exc.HTTPForbidden, exception.AdminRequired()) + ) + @ddt.unpack + def test__validate_manage_share_server_validate_service_host( + self, exception_to_raise, side_effect_exception): + req = fakes.HTTPRequest.blank('/manage') + context = req.environ['manila.context'] + error = mock.Mock(side_effect=side_effect_exception) + self.mock_object(utils, 'validate_service_host', error) + + self.assertRaises( + exception_to_raise, self.controller.manage, req, + {'share_server': self._setup_manage_test_request_body()}) + + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'manage_share_server') + + def test__validate_manage_share_server_share_network_not_found(self): + req = fakes.HTTPRequest.blank('/manage') + context = req.environ['manila.context'] + self.mock_object(utils, 'validate_service_host') + error = mock.Mock( + side_effect=exception.ShareNetworkNotFound(share_network_id="foo")) + self.mock_object(db_api, 'share_network_get', error) + body = self._setup_manage_test_request_body() + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.manage, + req, + {'share_server': body}) + + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'manage_share_server') + + def test__validate_manage_share_server_driver_opts_not_instance_dict(self): + req = fakes.HTTPRequest.blank('/manage') + context = req.environ['manila.context'] + self.mock_object(utils, 'validate_service_host') + self.mock_object(db_api, 'share_network_get') + body = self._setup_manage_test_request_body() + body['driver_options'] = 'incorrect' + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.manage, + req, + {'share_server': body}) + + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'manage_share_server') + + def test__validate_manage_share_server_error_extract_host(self): + req = fakes.HTTPRequest.blank('/manage') + context = req.environ['manila.context'] + body = self._setup_manage_test_request_body() + body['host'] = 'fake@backend#pool' + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.manage, + req, + {'share_server': body}) + + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'manage_share_server') + + @ddt.data(True, False) + def test_unmanage(self, force): + server = self._setup_unmanage_tests() + req = fakes.HTTPRequest.blank('/unmanage') + context = req.environ['manila.context'] + mock_get = self.mock_object( + db_api, 'share_server_get', mock.Mock(return_value=server)) + mock_unmanage = self.mock_object( + share_api.API, 'unmanage_share_server', + mock.Mock(return_value=202)) + body = {'unmanage': {'force': force}} + resp = self.controller.unmanage(req, server['id'], body) + + self.assertEqual(202, resp.status_int) + + mock_get.assert_called_once_with(context, server['id']) + mock_unmanage.assert_called_once_with(context, server, force=force) + + def test_unmanage_share_server_not_found(self): + """Tests unmanaging share servers""" + req = fakes.HTTPRequest.blank('/v2/share-servers/fake_server_id/') + context = req.environ['manila.context'] + share_server_error = mock.Mock( + side_effect=exception.ShareServerNotFound( + share_server_id='fake_server_id')) + get_mock = self.mock_object( + db_api, 'share_server_get', share_server_error) + body = {'unmanage': {'force': True}} + + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.unmanage, + req, + 'fake_server_id', + body) + + get_mock.assert_called_once_with(context, 'fake_server_id') + + @ddt.data(constants.STATUS_MANAGING, constants.STATUS_DELETING, + constants.STATUS_CREATING, constants.STATUS_UNMANAGING) + def test_unmanage_share_server_invalid_statuses(self, status): + """Tests unmanaging share servers""" + server = self._setup_unmanage_tests(status=status) + get_mock = self.mock_object(db_api, 'share_server_get', + mock.Mock(return_value=server)) + req = fakes.HTTPRequest.blank('/unmanage_share_server') + context = req.environ['manila.context'] + body = {'unmanage': {'force': True}} + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.unmanage, + req, + server['id'], + body) + + get_mock.assert_called_once_with(context, server['id']) + + def _setup_unmanage_tests(self, status=constants.STATUS_ACTIVE): + server = db_utils.create_share_server( + id='fake_server_id', status=status) + self.mock_object(db_api, 'share_server_get', + mock.Mock(return_value=server)) + return server + + @ddt.data(exception.ShareServerInUse, exception.PolicyNotAuthorized) + def test_unmanage_share_server_badrequest(self, exc): + req = fakes.HTTPRequest.blank('/unmanage') + server = self._setup_unmanage_tests() + context = req.environ['manila.context'] + error = mock.Mock(side_effect=exc('foobar')) + mock_unmanage = self.mock_object( + share_api.API, 'unmanage_share_server', error) + body = {'unmanage': {'force': True}} + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.unmanage, + req, + 'fake_server_id', + body) + + mock_unmanage.assert_called_once_with(context, server, force=True) + policy.check_policy.assert_called_once_with( + context, self.resource_name, 'unmanage_share_server') diff --git a/manila/tests/api/v2/test_share_snapshots.py b/manila/tests/api/v2/test_share_snapshots.py index 2e8ca13c43..d5abddfa85 100644 --- a/manila/tests/api/v2/test_share_snapshots.py +++ b/manila/tests/api/v2/test_share_snapshots.py @@ -799,6 +799,21 @@ class ShareSnapshotAdminActionsAPITest(test.TestCase): self.controller.manage, fake_req, body) + def test_snapshot__unmanage(self): + body = {} + snapshot = {'status': constants.STATUS_AVAILABLE, 'id': 'bar_id', + 'share_id': 'bar_id'} + fake_req = fakes.HTTPRequest.blank( + '/snapshots/unmanage', + use_admin_context=True, + version='2.49') + mock_unmanage = self.mock_object(self.controller, '_unmanage') + + self.controller.unmanage(fake_req, snapshot['id'], body) + + mock_unmanage.assert_called_once_with(fake_req, snapshot['id'], body, + allow_dhss_true=True) + def test_snapshot_unmanage_share_server(self): self.mock_policy_check = self.mock_object( policy, 'check_policy', mock.Mock(return_value=True)) @@ -939,3 +954,33 @@ class ShareSnapshotAdminActionsAPITest(test.TestCase): self.assertRaises(exception.VersionNotFoundForAPIMethod, self.controller.unmanage, fake_req, 'fake') + + def test_snapshot_unmanage_dhss_true_with_share_server(self): + self.mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=True)) + share = {'status': constants.STATUS_AVAILABLE, 'id': 'bar_id', + 'host': 'fake_host', + 'share_server_id': 'fake'} + mock_get = self.mock_object(share_api.API, 'get', + mock.Mock(return_value=share)) + snapshot = {'status': constants.STATUS_AVAILABLE, 'id': 'bar_id', + 'share_id': 'bar_id'} + self.mock_object(share_api.API, 'get_snapshot', + mock.Mock(return_value=snapshot)) + self.mock_object(share_api.API, 'unmanage_snapshot') + + actual_result = self.controller._unmanage(self.unmanage_request, + snapshot['id'], + allow_dhss_true=True) + + self.assertEqual(202, actual_result.status_int) + self.controller.share_api.get_snapshot.assert_called_once_with( + self.unmanage_request.environ['manila.context'], snapshot['id']) + share_api.API.unmanage_snapshot.assert_called_once_with( + mock.ANY, snapshot, 'fake_host') + mock_get.assert_called_once_with( + self.unmanage_request.environ['manila.context'], snapshot['id'] + ) + self.mock_policy_check.assert_called_once_with( + self.unmanage_request.environ['manila.context'], + self.resource_name, 'unmanage_snapshot') diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index e6721370d2..66a3e1069f 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -2532,6 +2532,20 @@ class ShareUnmanageTest(test.TestCase): share_api.API.unmanage.assert_called_once_with( self.request.environ['manila.context'], share) + def test__unmanage(self): + body = {} + req = fakes.HTTPRequest.blank( + '/shares/1/action', use_admin_context=False, version='2.49') + share = dict(status=constants.STATUS_AVAILABLE, id='foo_id', + instance={}) + mock_unmanage = self.mock_object(self.controller, '_unmanage') + + self.controller.unmanage(req, share['id'], body) + + mock_unmanage.assert_called_once_with( + req, share['id'], body, allow_dhss_true=True + ) + def test_unmanage_share_that_has_snapshots(self): share = dict(status=constants.STATUS_AVAILABLE, id='foo_id', instance={}) @@ -2664,6 +2678,18 @@ class ShareManageTest(test.TestCase): 'validate_service_host', mock.Mock(side_effect=exception.ServiceIsDown(service='fake'))) + def test__manage(self): + body = {} + req = fakes.HTTPRequest.blank( + '/v2/shares/manage', use_admin_context=True, version='2.49') + mock_manage = self.mock_object(self.controller, '_manage') + + self.controller.manage(req, body) + + mock_manage.assert_called_once_with( + req, body, allow_dhss_true=True + ) + @ddt.data({}, {'shares': {}}, {'share': get_fake_manage_body('', None, None)}) diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py index 5bddd23b74..6286f1a475 100644 --- a/manila/tests/db/migrations/alembic/migrations_data_checks.py +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -2757,3 +2757,44 @@ class AccessMetadataTableChecks(BaseMigrationChecks): def check_downgrade(self, engine): self.test_case.assertRaises(sa_exc.NoSuchTableError, utils.load_table, self.new_table_name, engine) + + +@map_to_migration('6a3fd2984bc31') +class ShareServerIsAutoDeletableAndIdentifierChecks(BaseMigrationChecks): + + def setup_upgrade_data(self, engine): + user_id = 'user_id' + project_id = 'project_id' + + # Create share network + share_network_data = { + 'id': 'fake_sn_id', + 'user_id': user_id, + 'project_id': project_id, + } + sn_table = utils.load_table('share_networks', engine) + engine.execute(sn_table.insert(share_network_data)) + + # Create share server + share_server_data = { + 'id': 'fake_ss_id', + 'share_network_id': share_network_data['id'], + 'host': 'fake_host', + 'status': 'active', + } + ss_table = utils.load_table('share_servers', engine) + engine.execute(ss_table.insert(share_server_data)) + + def check_upgrade(self, engine, data): + ss_table = utils.load_table('share_servers', engine) + for ss in engine.execute(ss_table.select()): + self.test_case.assertTrue(hasattr(ss, 'is_auto_deletable')) + self.test_case.assertEqual(1, ss.is_auto_deletable) + self.test_case.assertTrue(hasattr(ss, 'identifier')) + self.test_case.assertEqual(ss.id, ss.identifier) + + def check_downgrade(self, engine): + ss_table = utils.load_table('share_servers', engine) + for ss in engine.execute(ss_table.select()): + self.test_case.assertFalse(hasattr(ss, 'is_auto_deletable')) + self.test_case.assertFalse(hasattr(ss, 'identifier')) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index ba767e6fab..5e6d8c054f 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -2424,6 +2424,7 @@ class SecurityServiceDatabaseAPITestCase(BaseDatabaseAPITestCase): self._check_expected_fields(result2[0], dict2) +@ddt.ddt class ShareServerDatabaseAPITestCase(test.TestCase): def setUp(self): @@ -2600,6 +2601,70 @@ class ShareServerDatabaseAPITestCase(test.TestCase): self.assertEqual(num_records - 1, len(db_api.share_server_get_all(self.ctxt))) + @ddt.data('fake', '-fake-', 'foo_some_fake_identifier_bar', + 'foo-some-fake-identifier-bar', 'foobar') + def test_share_server_search_by_identifier(self, identifier): + + server = { + 'share_network_id': 'fake-share-net-id', + 'host': 'hostname', + 'status': constants.STATUS_ACTIVE, + 'is_auto_deletable': True, + 'updated_at': datetime.datetime(2018, 5, 1), + 'identifier': 'some_fake_identifier', + } + + server = db_utils.create_share_server(**server) + if identifier == 'foobar': + self.assertRaises(exception.ShareServerNotFound, + db_api.share_server_search_by_identifier, + self.ctxt, identifier) + else: + result = db_api.share_server_search_by_identifier( + self.ctxt, identifier) + self.assertEqual(server['id'], result[0]['id']) + + @ddt.data((True, True, True, 3), + (True, True, False, 2), + (True, False, False, 1), + (False, False, False, 0)) + @ddt.unpack + def test_share_server_get_all_unused_deletable(self, + server_1_is_auto_deletable, + server_2_is_auto_deletable, + server_3_is_auto_deletable, + expected_len): + server1 = { + 'share_network_id': 'fake-share-net-id', + 'host': 'hostname', + 'status': constants.STATUS_ACTIVE, + 'is_auto_deletable': server_1_is_auto_deletable, + 'updated_at': datetime.datetime(2018, 5, 1) + } + server2 = { + 'share_network_id': 'fake-share-net-id', + 'host': 'hostname', + 'status': constants.STATUS_ACTIVE, + 'is_auto_deletable': server_2_is_auto_deletable, + 'updated_at': datetime.datetime(2018, 5, 1) + } + server3 = { + 'share_network_id': 'fake-share-net-id', + 'host': 'hostname', + 'status': constants.STATUS_ACTIVE, + 'is_auto_deletable': server_3_is_auto_deletable, + 'updated_at': datetime.datetime(2018, 5, 1) + } + db_utils.create_share_server(**server1) + db_utils.create_share_server(**server2) + db_utils.create_share_server(**server3) + host = 'hostname' + updated_before = datetime.datetime(2019, 5, 1) + + unused_deletable = db_api.share_server_get_all_unused_deletable( + self.ctxt, host, updated_before) + self.assertEqual(expected_len, len(unused_deletable)) + class ServiceDatabaseAPITestCase(test.TestCase): @@ -2773,6 +2838,90 @@ class NetworkAllocationsDatabaseAPITestCase(test.TestCase): for na in result: self.assertIn(na.label, ('admin', 'user', None)) + def test_network_allocation_get(self): + self._setup_network_allocations_get_for_share_server() + + for allocation in self.admin_network_allocations: + result = db_api.network_allocation_get(self.ctxt, allocation['id']) + + self.assertIsInstance(result, models.NetworkAllocation) + self.assertEqual(allocation['id'], result.id) + + for allocation in self.user_network_allocations: + result = db_api.network_allocation_get(self.ctxt, allocation['id']) + + self.assertIsInstance(result, models.NetworkAllocation) + self.assertEqual(allocation['id'], result.id) + + def test_network_allocation_get_no_result(self): + self._setup_network_allocations_get_for_share_server() + + self.assertRaises(exception.NotFound, + db_api.network_allocation_get, + self.ctxt, + id='fake') + + @ddt.data(True, False) + def test_network_allocation_get_read_deleted(self, read_deleted): + self._setup_network_allocations_get_for_share_server() + + deleted_allocation = { + 'share_server_id': self.share_server_id, + 'ip_address': '1.1.1.1', + 'status': constants.STATUS_ACTIVE, + 'label': None, + 'deleted': True, + } + + new_obj = db_api.network_allocation_create(self.ctxt, + deleted_allocation) + if read_deleted: + result = db_api.network_allocation_get(self.ctxt, new_obj.id, + read_deleted=read_deleted) + self.assertIsInstance(result, models.NetworkAllocation) + self.assertEqual(new_obj.id, result.id) + else: + self.assertRaises(exception.NotFound, + db_api.network_allocation_get, + self.ctxt, + id=self.share_server_id) + + def test_network_allocation_update(self): + self._setup_network_allocations_get_for_share_server() + + for allocation in self.admin_network_allocations: + old_obj = db_api.network_allocation_get(self.ctxt, + allocation['id']) + self.assertEqual('False', old_obj.deleted) + updated_object = db_api.network_allocation_update( + self.ctxt, allocation['id'], {'deleted': 'True'}) + + self.assertEqual('True', updated_object.deleted) + + @ddt.data(True, False) + def test_network_allocation_update_read_deleted(self, read_deleted): + self._setup_network_allocations_get_for_share_server() + + db_api.network_allocation_update( + self.ctxt, + self.admin_network_allocations[0]['id'], + {'deleted': 'True'} + ) + + if read_deleted: + updated_object = db_api.network_allocation_update( + self.ctxt, self.admin_network_allocations[0]['id'], + {'deleted': 'False'}, read_deleted=read_deleted + ) + self.assertEqual('False', updated_object.deleted) + else: + self.assertRaises(exception.NotFound, + db_api.network_allocation_update, + self.ctxt, + id=self.share_server_id, + values={'deleted': read_deleted}, + read_deleted=read_deleted) + class ReservationDatabaseAPITest(test.TestCase): diff --git a/manila/tests/fake_driver.py b/manila/tests/fake_driver.py index 70af04ea84..4218e5d447 100644 --- a/manila/tests/fake_driver.py +++ b/manila/tests/fake_driver.py @@ -47,13 +47,13 @@ class FakeShareDriver(driver.ShareDriver): self.service_instance_manager = ( fake_service_instance.FakeServiceInstanceManager()) - def manage_existing(self, share, driver_options): + def manage_existing(self, share, driver_options, share_server=None): LOG.debug("Fake share driver: manage") LOG.debug("Fake share driver: driver options: %s", six.text_type(driver_options)) return {'size': 1} - def unmanage(self, share): + def unmanage(self, share, share_server=None): LOG.debug("Fake share driver: unmanage") @property diff --git a/manila/tests/fake_share.py b/manila/tests/fake_share.py index 130087d35d..05e96789ec 100644 --- a/manila/tests/fake_share.py +++ b/manila/tests/fake_share.py @@ -295,3 +295,21 @@ def fake_replica_request_spec(as_primitive=True, **kwargs): return request_spec else: return db_fakes.FakeModel(request_spec) + + +def fake_share_server_get(): + fake_share_server = { + 'status': constants.STATUS_ACTIVE, + 'updated_at': None, + 'host': 'fake_host', + 'share_network_id': 'fake_sn_id', + 'share_network_name': 'fake_sn_name', + 'project_id': 'fake_project_id', + 'id': 'fake_share_server_id', + 'backend_details': { + 'security_service_active_directory': '{"name": "fake_AD"}', + 'security_service_ldap': '{"name": "fake_LDAP"}', + 'security_service_kerberos': '{"name": "fake_kerberos"}', + } + } + return fake_share_server diff --git a/manila/tests/network/neutron/test_neutron_plugin.py b/manila/tests/network/neutron/test_neutron_plugin.py index fface87a51..24a8c02e0f 100644 --- a/manila/tests/network/neutron/test_neutron_plugin.py +++ b/manila/tests/network/neutron/test_neutron_plugin.py @@ -191,6 +191,7 @@ fake_binding_profile = { } +@ddt.ddt class NeutronNetworkPluginTest(test.TestCase): def setUp(self): @@ -323,6 +324,175 @@ class NeutronNetworkPluginTest(test.TestCase): save_subnet_data.stop() create_port.stop() + def _setup_manage_network_allocations(self): + + allocations = ['192.168.0.11', '192.168.0.12', 'fd12::2000'] + + neutron_ports = [ + copy.deepcopy(fake_neutron_port), copy.deepcopy(fake_neutron_port), + copy.deepcopy(fake_neutron_port), copy.deepcopy(fake_neutron_port), + ] + + neutron_ports[0]['fixed_ips'][0]['ip_address'] = '192.168.0.10' + neutron_ports[0]['id'] = 'fake_port_id_0' + neutron_ports[1]['fixed_ips'][0]['ip_address'] = '192.168.0.11' + neutron_ports[1]['id'] = 'fake_port_id_1' + neutron_ports[2]['fixed_ips'][0]['ip_address'] = '192.168.0.12' + neutron_ports[2]['id'] = 'fake_port_id_2' + neutron_ports[3]['fixed_ips'][0]['ip_address'] = '192.168.0.13' + neutron_ports[3]['id'] = 'fake_port_id_3' + + self.mock_object(self.plugin, '_verify_share_network') + self.mock_object(self.plugin, '_store_neutron_net_info') + + self.mock_object(self.plugin.neutron_api, 'list_ports', + mock.Mock(return_value=neutron_ports)) + + return neutron_ports, allocations + + @ddt.data({}, exception.NotFound) + def test_manage_network_allocations_create_update(self, side_effect): + + neutron_ports, allocations = self._setup_manage_network_allocations() + + self.mock_object(db_api, 'network_allocation_get', + mock.Mock( + side_effect=[exception.NotFound, side_effect, + exception.NotFound, side_effect])) + if side_effect: + self.mock_object(db_api, 'network_allocation_create') + else: + self.mock_object(db_api, 'network_allocation_update') + + result = self.plugin.manage_network_allocations( + self.fake_context, allocations, fake_share_server, + fake_share_network) + + self.assertEqual(['fd12::2000'], result) + + self.plugin.neutron_api.list_ports.assert_called_once_with( + network_id=fake_share_network['neutron_net_id'], + device_owner='manila:share', + fixed_ips='subnet_id=' + fake_share_network['neutron_subnet_id']) + + db_api.network_allocation_get.assert_has_calls([ + mock.call(self.fake_context, 'fake_port_id_1', read_deleted=False), + mock.call(self.fake_context, 'fake_port_id_1', read_deleted=True), + mock.call(self.fake_context, 'fake_port_id_2', read_deleted=False), + mock.call(self.fake_context, 'fake_port_id_2', read_deleted=True), + ]) + + port_dict_list = [{ + 'share_server_id': fake_share_server['id'], + 'ip_address': x, + 'gateway': fake_share_network['gateway'], + 'mac_address': fake_neutron_port['mac_address'], + 'status': constants.STATUS_ACTIVE, + 'label': 'user', + 'network_type': fake_share_network['network_type'], + 'segmentation_id': fake_share_network['segmentation_id'], + 'ip_version': fake_share_network['ip_version'], + 'cidr': fake_share_network['cidr'], + 'mtu': fake_share_network['mtu'], + } for x in ['192.168.0.11', '192.168.0.12']] + + if side_effect: + port_dict_list[0]['id'] = 'fake_port_id_1' + port_dict_list[1]['id'] = 'fake_port_id_2' + db_api.network_allocation_create.assert_has_calls([ + mock.call(self.fake_context, port_dict_list[0]), + mock.call(self.fake_context, port_dict_list[1]) + ]) + else: + for x in port_dict_list: + x['deleted_at'] = None + x['deleted'] = 'False' + + db_api.network_allocation_update.assert_has_calls([ + mock.call(self.fake_context, 'fake_port_id_1', + port_dict_list[0], read_deleted=True), + mock.call(self.fake_context, 'fake_port_id_2', + port_dict_list[1], read_deleted=True) + ]) + + self.plugin._verify_share_network.assert_called_once_with( + fake_share_server['id'], fake_share_network) + + self.plugin._store_neutron_net_info( + self.fake_context, fake_share_network) + + def test__get_ports_respective_to_ips_multiple_fixed_ips(self): + self.mock_object(plugin.LOG, 'warning') + + allocations = ['192.168.0.10', '192.168.0.11', '192.168.0.12'] + + neutron_ports = [ + copy.deepcopy(fake_neutron_port), copy.deepcopy(fake_neutron_port), + ] + + neutron_ports[0]['fixed_ips'][0]['ip_address'] = '192.168.0.10' + neutron_ports[0]['id'] = 'fake_port_id_0' + neutron_ports[0]['fixed_ips'].append({'ip_address': '192.168.0.11', + 'subnet_id': 'test_subnet_id'}) + neutron_ports[1]['fixed_ips'][0]['ip_address'] = '192.168.0.12' + neutron_ports[1]['id'] = 'fake_port_id_2' + + expected = [{'port': neutron_ports[0], 'allocation': '192.168.0.10'}, + {'port': neutron_ports[1], 'allocation': '192.168.0.12'}] + + result = self.plugin._get_ports_respective_to_ips(allocations, + neutron_ports) + + self.assertEqual(expected, result) + + self.assertIs(True, plugin.LOG.warning.called) + + def test_manage_network_allocations_exception(self): + + neutron_ports, allocations = self._setup_manage_network_allocations() + + fake_allocation = { + 'id': 'fake_port_id', + 'share_server_id': 'fake_server_id' + } + + self.mock_object(db_api, 'network_allocation_get', + mock.Mock(return_value=fake_allocation)) + + self.assertRaises( + exception.ManageShareServerError, + self.plugin.manage_network_allocations, self.fake_context, + allocations, fake_share_server, fake_share_network) + + db_api.network_allocation_get.assert_called_once_with( + self.fake_context, 'fake_port_id_1', read_deleted=False) + + def test_unmanage_network_allocations(self): + + neutron_ports = [ + copy.deepcopy(fake_neutron_port), copy.deepcopy(fake_neutron_port), + ] + + neutron_ports[0]['id'] = 'fake_port_id_0' + neutron_ports[1]['id'] = 'fake_port_id_1' + + get_mock = self.mock_object( + db_api, 'network_allocations_get_for_share_server', + mock.Mock(return_value=neutron_ports)) + + self.mock_object(db_api, 'network_allocation_delete') + + self.plugin.unmanage_network_allocations( + self.fake_context, fake_share_server['id']) + + get_mock.assert_called_once_with( + self.fake_context, fake_share_server['id']) + + db_api.network_allocation_delete.assert_has_calls([ + mock.call(self.fake_context, 'fake_port_id_0'), + mock.call(self.fake_context, 'fake_port_id_1') + ]) + @mock.patch.object(db_api, 'network_allocation_delete', mock.Mock()) @mock.patch.object(db_api, 'share_network_update', mock.Mock()) @mock.patch.object(db_api, 'network_allocations_get_for_share_server', @@ -547,7 +717,8 @@ class NeutronSingleNetworkPluginTest(test.TestCase): plugin.NeutronSingleNetworkPlugin) neutron_api.API.get_network.assert_called_once_with(fake_net_id) - def _get_neutron_network_plugin_instance(self, config_data=None): + def _get_neutron_network_plugin_instance( + self, config_data=None, label=None): if not config_data: fake_subnet_id = 'fake_subnet_id' config_data = { @@ -561,7 +732,7 @@ class NeutronSingleNetworkPluginTest(test.TestCase): neutron_api.API, 'get_network', mock.Mock(return_value=fake_net)) with test_utils.create_temp_config_with_opts(config_data): - instance = plugin.NeutronSingleNetworkPlugin() + instance = plugin.NeutronSingleNetworkPlugin(label=label) return instance def test___update_share_network_net_data_same_values(self): @@ -640,6 +811,48 @@ class NeutronSingleNetworkPluginTest(test.TestCase): self.context, share_server, share_network_upd, count=count, device_owner=device_owner) + def test_manage_network_allocations(self): + allocations = ['192.168.10.10', 'fd12::2000'] + instance = self._get_neutron_network_plugin_instance() + parent = self.mock_object( + plugin.NeutronNetworkPlugin, 'manage_network_allocations', + mock.Mock(return_value=['fd12::2000'])) + self.mock_object( + instance, '_update_share_network_net_data', + mock.Mock(return_value=fake_share_network)) + + result = instance.manage_network_allocations( + self.context, allocations, fake_share_server, fake_share_network) + + self.assertEqual(['fd12::2000'], result) + + instance._update_share_network_net_data.assert_called_once_with( + self.context, fake_share_network) + + parent.assert_called_once_with( + self.context, allocations, fake_share_server, fake_share_network) + + def test_manage_network_allocations_admin(self): + allocations = ['192.168.10.10', 'fd12::2000'] + instance = self._get_neutron_network_plugin_instance(label='admin') + parent = self.mock_object( + plugin.NeutronNetworkPlugin, 'manage_network_allocations', + mock.Mock(return_value=['fd12::2000'])) + + share_network_dict = { + 'project_id': instance.neutron_api.admin_project_id, + 'neutron_net_id': 'fake_net_id', + 'neutron_subnet_id': 'fake_subnet_id', + } + + result = instance.manage_network_allocations( + self.context, allocations, fake_share_server, None) + + self.assertEqual(['fd12::2000'], result) + + parent.assert_called_once_with( + self.context, allocations, fake_share_server, share_network_dict) + @ddt.ddt class NeutronBindNetworkPluginTest(test.TestCase): diff --git a/manila/tests/network/test_standalone_network_plugin.py b/manila/tests/network/test_standalone_network_plugin.py index ec7410e086..8f25084893 100644 --- a/manila/tests/network/test_standalone_network_plugin.py +++ b/manila/tests/network/test_standalone_network_plugin.py @@ -465,3 +465,67 @@ class StandaloneNetworkPluginTest(test.TestCase): mtu=1500)) instance.db.network_allocations_get_by_ip_address.assert_has_calls( [mock.call(fake_context, '10.0.0.2')]) + + def _setup_manage_network_allocations(self, label=None): + data = { + 'DEFAULT': { + 'standalone_network_plugin_gateway': '192.168.0.1', + 'standalone_network_plugin_mask': '24', + }, + } + with test_utils.create_temp_config_with_opts(data): + instance = plugin.StandaloneNetworkPlugin(label=label) + + return instance + + @ddt.data('admin', None) + def test_manage_network_allocations(self, label): + allocations = ['192.168.0.11', '192.168.0.12', 'fd12::2000'] + + instance = self._setup_manage_network_allocations(label=label) + if not label: + self.mock_object(instance, '_verify_share_network') + self.mock_object(instance.db, 'share_network_update') + self.mock_object(instance.db, 'network_allocation_create') + + result = instance.manage_network_allocations( + fake_context, allocations, fake_share_server, fake_share_network) + + self.assertEqual(['fd12::2000'], result) + + network_data = { + 'network_type': instance.network_type, + 'segmentation_id': instance.segmentation_id, + 'cidr': six.text_type(instance.net.cidr), + 'gateway': six.text_type(instance.gateway), + 'ip_version': instance.ip_version, + 'mtu': instance.mtu, + } + + data_list = [{ + 'share_server_id': fake_share_server['id'], + 'ip_address': x, + 'status': constants.STATUS_ACTIVE, + 'label': instance.label, + } for x in ['192.168.0.11', '192.168.0.12']] + + data_list[0].update(network_data) + data_list[1].update(network_data) + + if not label: + instance.db.share_network_update.assert_called_once_with( + fake_context, fake_share_network['id'], network_data) + instance._verify_share_network.assert_called_once_with( + fake_share_server['id'], fake_share_network) + + instance.db.network_allocation_create.assert_has_calls([ + mock.call(fake_context, data_list[0]), + mock.call(fake_context, data_list[1]) + ]) + + def test_unmanage_network_allocations(self): + instance = self._setup_manage_network_allocations() + self.mock_object(instance, 'deallocate_network') + instance.unmanage_network_allocations('context', 'server_id') + instance.deallocate_network.assert_called_once_with( + 'context', 'server_id') diff --git a/manila/tests/share/drivers/dummy.py b/manila/tests/share/drivers/dummy.py index b2514973f4..f22f54b106 100644 --- a/manila/tests/share/drivers/dummy.py +++ b/manila/tests/share/drivers/dummy.py @@ -303,21 +303,80 @@ class DummyDriver(driver.ShareDriver): @slow_me_down def manage_existing(self, share, driver_options): """Brings an existing share under Manila management.""" - return {"size": 1, "export_locations": self._create_share(share)} + new_export = share['export_location'] + old_share_id = self._get_share_id_from_export(new_export) + old_export = self.private_storage.get( + old_share_id, key='export_location') + if old_export.split(":/")[-1] == new_export.split(":/")[-1]: + result = {"size": 1, "export_locations": self._create_share(share)} + self.private_storage.delete(old_share_id) + return result + else: + msg = ("Invalid export specified, existing share %s" + " could not be found" % old_share_id) + raise exception.ShareBackendException(msg=msg) + + @slow_me_down + def manage_existing_with_server( + self, share, driver_options, share_server=None): + return self.manage_existing(share, driver_options) + + def _get_share_id_from_export(self, export_location): + values = export_location.split('share_') + if len(values) > 1: + return values[1][37:].replace("_", "-") + else: + return export_location @slow_me_down def unmanage(self, share): """Removes the specified share from Manila management.""" + self.private_storage.update( + share['id'], {'export_location': share['export_location']}) + + @slow_me_down + def unmanage_with_server(self, share, share_server=None): + self.unmanage(share) + + @slow_me_down + def manage_existing_snapshot_with_server(self, snapshot, driver_options, + share_server=None): + return self.manage_existing_snapshot(snapshot, driver_options) @slow_me_down def manage_existing_snapshot(self, snapshot, driver_options): """Brings an existing snapshot under Manila management.""" - self._create_snapshot(snapshot) - return {"size": 1, "provider_location": snapshot["provider_location"]} + old_snap_id = self._get_snap_id_from_provider_location( + snapshot['provider_location']) + old_provider_location = self.private_storage.get( + old_snap_id, key='provider_location') + if old_provider_location == snapshot['provider_location']: + self._create_snapshot(snapshot) + self.private_storage.delete(old_snap_id) + return {"size": 1, + "provider_location": snapshot["provider_location"]} + else: + msg = ("Invalid provider location specified, existing snapshot %s" + " could not be found" % old_snap_id) + raise exception.ShareBackendException(msg=msg) + + def _get_snap_id_from_provider_location(self, provider_location): + values = provider_location.split('snapshot_') + if len(values) > 1: + return values[1][37:].replace("_", "-") + else: + return provider_location @slow_me_down def unmanage_snapshot(self, snapshot): """Removes the specified snapshot from Manila management.""" + self.private_storage.update( + snapshot['id'], + {'provider_location': snapshot['provider_location']}) + + @slow_me_down + def unmanage_snapshot_with_server(self, snapshot, share_server=None): + self.unmanage_snapshot(snapshot) @slow_me_down def revert_to_snapshot(self, context, snapshot, share_access_rules, @@ -354,6 +413,7 @@ class DummyDriver(driver.ShareDriver): "service_ip": network_info[ "admin_network_allocations"][0]["ip_address"], "username": "fake_username", + "server_id": network_info['server_id'] } return server_details @@ -639,3 +699,27 @@ class DummyDriver(driver.ShareDriver): 'used_size': 1, 'gathered_at': gathered_at}) return share_updates + + @slow_me_down + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + try: + server_details = self.private_storage.get(identifier) + except Exception: + msg = ("Unable to find share server %s in " + "private storage." % identifier) + raise exception.ShareBackendException(msg=msg) + + return [server_details['primary_public_ip'], + server_details['secondary_public_ip'], + server_details['service_ip']] + + @slow_me_down + def manage_server(self, context, share_server, identifier, driver_options): + server_details = self.private_storage.get(identifier) + self.private_storage.delete(identifier) + return identifier, server_details + + def unmanage_server(self, server_details, security_services=None): + self.private_storage.update(server_details['server_id'], + server_details) diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 643c625475..a967a482db 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -891,6 +891,7 @@ class ShareAPITestCase(test.TestCase): 'create_share_from_snapshot_support': False, 'revert_to_snapshot_support': False, 'mount_snapshot_support': False, + 'driver_handles_share_servers': False, }, } @@ -938,6 +939,163 @@ class ShareAPITestCase(test.TestCase): self.scheduler_rpcapi.manage_share.assert_called_once_with( self.context, share['id'], driver_options, expected_request_spec) + @ddt.data((True, exception.InvalidInput, True), + (True, exception.InvalidInput, False), + (False, exception.InvalidInput, True), + (True, exception.InvalidInput, True)) + @ddt.unpack + def test_manage_new_dhss_true_and_false(self, dhss, exception_type, + has_share_server_id): + share_data = { + 'host': 'fake', + 'export_location': 'fake', + 'share_proto': 'fake', + 'share_type_id': 'fake', + } + if has_share_server_id: + share_data['share_server_id'] = 'fake' + + driver_options = {} + date = datetime.datetime(1, 1, 1, 1, 1, 1) + timeutils.utcnow.return_value = date + fake_type = { + 'id': 'fake_type_id', + 'extra_specs': { + 'snapshot_support': False, + 'create_share_from_snapshot_support': False, + 'revert_to_snapshot_support': False, + 'mount_snapshot_support': False, + 'driver_handles_share_servers': dhss, + }, + } + + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=fake_type)) + self.mock_object(self.api, 'get_all', mock.Mock(return_value=[])) + + self.assertRaises(exception_type, + self.api.manage, + self.context, + share_data=share_data, + driver_options=driver_options + ) + share_types.get_share_type.assert_called_once_with( + self.context, share_data['share_type_id'] + ) + self.api.get_all.assert_called_once_with( + self.context, { + 'host': share_data['host'], + 'export_location': share_data['export_location'], + 'share_proto': share_data['share_proto'], + 'share_type_id': share_data['share_type_id'] + } + ) + + def test_manage_new_share_server_not_found(self): + share_data = { + 'host': 'fake', + 'export_location': 'fake', + 'share_proto': 'fake', + 'share_type_id': 'fake', + 'share_server_id': 'fake' + + } + driver_options = {} + date = datetime.datetime(1, 1, 1, 1, 1, 1) + timeutils.utcnow.return_value = date + + fake_type = { + 'id': 'fake_type_id', + 'extra_specs': { + 'snapshot_support': False, + 'replication_type': 'dr', + 'create_share_from_snapshot_support': False, + 'revert_to_snapshot_support': False, + 'mount_snapshot_support': False, + 'driver_handles_share_servers': True, + }, + } + + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=fake_type)) + self.mock_object(self.api, 'get_all', mock.Mock(return_value=[])) + + self.assertRaises(exception.InvalidInput, + self.api.manage, + self.context, + share_data=share_data, + driver_options=driver_options + ) + share_types.get_share_type.assert_called_once_with( + self.context, share_data['share_type_id'] + ) + self.api.get_all.assert_called_once_with( + self.context, { + 'host': share_data['host'], + 'export_location': share_data['export_location'], + 'share_proto': share_data['share_proto'], + 'share_type_id': share_data['share_type_id'] + } + ) + + def test_manage_new_share_server_not_active(self): + share_data = { + 'host': 'fake', + 'export_location': 'fake', + 'share_proto': 'fake', + 'share_type_id': 'fake', + 'share_server_id': 'fake' + + } + fake_share_data = { + 'id': 'fakeid', + 'status': constants.STATUS_ERROR, + } + driver_options = {} + date = datetime.datetime(1, 1, 1, 1, 1, 1) + timeutils.utcnow.return_value = date + + fake_type = { + 'id': 'fake_type_id', + 'extra_specs': { + 'snapshot_support': False, + 'replication_type': 'dr', + 'create_share_from_snapshot_support': False, + 'revert_to_snapshot_support': False, + 'mount_snapshot_support': False, + 'driver_handles_share_servers': True, + }, + } + + share = db_api.share_create(self.context, fake_share_data) + + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=fake_type)) + self.mock_object(self.api, 'get_all', mock.Mock(return_value=[])) + self.mock_object(db_api, 'share_server_get', + mock.Mock(return_value=share)) + + self.assertRaises(exception.InvalidShareServer, + self.api.manage, + self.context, + share_data=share_data, + driver_options=driver_options + ) + share_types.get_share_type.assert_called_once_with( + self.context, share_data['share_type_id'] + ) + self.api.get_all.assert_called_once_with( + self.context, { + 'host': share_data['host'], + 'export_location': share_data['export_location'], + 'share_proto': share_data['share_proto'], + 'share_type_id': share_data['share_type_id'] + } + ) + db_api.share_server_get.assert_called_once_with( + self.context, share_data['share_server_id'] + ) + @ddt.data(constants.STATUS_MANAGE_ERROR, constants.STATUS_AVAILABLE) def test_manage_duplicate(self, status): share_data = { @@ -952,6 +1110,7 @@ class ShareAPITestCase(test.TestCase): 'extra_specs': { 'snapshot_support': False, 'create_share_from_snapshot_support': False, + 'driver_handles_share_servers': False, }, } shares = [{'id': 'fake', 'status': status}] @@ -1222,6 +1381,82 @@ class ShareAPITestCase(test.TestCase): mock_rpc_call.assert_called_once_with( self.context, snapshot, share_ref['host'], {}) + def test_manage_share_server(self): + """Tests manage share server""" + host = 'fake_host' + fake_share_network = { + 'id': 'fake_net_id' + } + identifier = 'fake_identifier' + values = { + 'host': host, + 'share_network_id': fake_share_network['id'], + 'status': constants.STATUS_MANAGING, + 'is_auto_deletable': False, + 'identifier': identifier, + } + + server_managing = { + 'id': 'fake_server_id', + 'status': constants.STATUS_MANAGING, + 'host': host, + 'share_network_id': fake_share_network['id'], + 'is_auto_deletable': False, + 'identifier': identifier, + } + + mock_share_server_search = self.mock_object( + db_api, 'share_server_search_by_identifier', + mock.Mock(side_effect=exception.ShareServerNotFound('fake'))) + + mock_share_server_get = self.mock_object( + db_api, 'share_server_get', + mock.Mock( + return_value=server_managing) + ) + mock_share_server_create = self.mock_object( + db_api, 'share_server_create', + mock.Mock(return_value=server_managing) + ) + result = self.api.manage_share_server( + self.context, 'fake_identifier', host, fake_share_network, + {'opt1': 'val1', 'opt2': 'val2'} + ) + + mock_share_server_create.assert_called_once_with( + self.context, values) + + mock_share_server_get.assert_called_once_with( + self.context, 'fake_server_id') + + mock_share_server_search.assert_called_once_with( + self.context, 'fake_identifier') + + result_dict = { + 'host': result['host'], + 'share_network_id': result['share_network_id'], + 'status': result['status'], + 'is_auto_deletable': result['is_auto_deletable'], + 'identifier': result['identifier'], + } + self.assertEqual(values, result_dict) + + def test_manage_share_server_invalid(self): + + server = {'identifier': 'fake_server'} + + mock_share_server_search = self.mock_object( + db_api, 'share_server_search_by_identifier', + mock.Mock(return_value=[server])) + + self.assertRaises( + exception.InvalidInput, self.api.manage_share_server, + self.context, 'invalid_identifier', 'fake_host', 'fake_share_net', + {}) + + mock_share_server_search.assert_called_once_with( + self.context, 'invalid_identifier') + def test_unmanage_snapshot(self): fake_host = 'fake_host' snapshot_data = { @@ -1244,6 +1479,83 @@ class ShareAPITestCase(test.TestCase): mock_rpc_call.assert_called_once_with( self.context, snapshot, fake_host) + def test_unmanage_share_server(self): + shr1 = {} + share_server = db_utils.create_share_server(**shr1) + update_data = {'status': constants.STATUS_UNMANAGING, + 'terminated_at': timeutils.utcnow()} + + mock_share_instances_get_all = self.mock_object( + db_api, 'share_instances_get_all_by_share_server', + mock.Mock(return_value={})) + mock_share_group_get_all = self.mock_object( + db_api, 'share_group_get_all_by_share_server', + mock.Mock(return_value={})) + mock_share_server_update = self.mock_object( + db_api, 'share_server_update', + mock.Mock(return_value=share_server)) + + mock_rpc = self.mock_object( + self.api.share_rpcapi, 'unmanage_share_server') + + self.api.unmanage_share_server(self.context, share_server, True) + + mock_share_instances_get_all.assert_called_once_with( + self.context, share_server['id'] + ) + mock_share_group_get_all.assert_called_once_with( + self.context, share_server['id'] + ) + mock_share_server_update.assert_called_once_with( + self.context, share_server['id'], update_data + ) + + mock_rpc.assert_called_once_with( + self.context, share_server, force=True) + + def test_unmanage_share_server_in_use(self): + fake_share = db_utils.create_share() + fake_share_server = db_utils.create_share_server() + + fake_share_instance = db_utils.create_share_instance( + share_id=fake_share['id']) + share_instance_get_all_mock = self.mock_object( + db_api, 'share_instances_get_all_by_share_server', + mock.Mock(return_value=fake_share_instance) + ) + + self.assertRaises(exception.ShareServerInUse, + self.api.unmanage_share_server, + self.context, + fake_share_server, True) + share_instance_get_all_mock.assert_called_once_with( + self.context, fake_share_server['id'] + ) + + def test_unmanage_share_server_in_use_share_groups(self): + fake_share_server = db_utils.create_share_server() + fake_share_groups = db_utils.create_share_group() + + share_instance_get_all_mock = self.mock_object( + db_api, 'share_instances_get_all_by_share_server', + mock.Mock(return_value={}) + ) + group_get_all_mock = self.mock_object( + db_api, 'share_group_get_all_by_share_server', + mock.Mock(return_value=fake_share_groups) + ) + + self.assertRaises(exception.ShareServerInUse, + self.api.unmanage_share_server, + self.context, + fake_share_server, True) + share_instance_get_all_mock.assert_called_once_with( + self.context, fake_share_server['id'] + ) + group_get_all_mock.assert_called_once_with( + self.context, fake_share_server['id'] + ) + @ddt.data(True, False) def test_revert_to_snapshot(self, has_replicas): diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 0818e44481..48c6b64d62 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -2447,46 +2447,10 @@ class ShareManagerTestCase(test.TestCase): self.share_manager._provide_share_server_for_share_group, self.context, None, None) - def test_manage_share_invalid_driver(self): - self.mock_object(self.share_manager, 'driver', mock.Mock()) - self.share_manager.driver.driver_handles_share_servers = True - self.mock_object(share_types, - 'get_share_type_extra_specs', - mock.Mock(return_value='False')) - self.mock_object(self.share_manager.db, 'share_update', mock.Mock()) - share = db_utils.create_share() - share_id = share['id'] - - self.assertRaises( - exception.InvalidDriverMode, - self.share_manager.manage_share, self.context, share_id, {}) - - self.share_manager.db.share_update.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), share_id, - {'status': constants.STATUS_MANAGE_ERROR, 'size': 1}) - - def test_manage_share_invalid_share_type(self): - self.mock_object(self.share_manager, 'driver', mock.Mock()) - self.share_manager.driver.driver_handles_share_servers = False - self.mock_object(share_types, - 'get_share_type_extra_specs', - mock.Mock(return_value='True')) - self.mock_object(self.share_manager.db, 'share_update', mock.Mock()) - share = db_utils.create_share() - share_id = share['id'] - - self.assertRaises( - exception.ManageExistingShareTypeMismatch, - self.share_manager.manage_share, self.context, share_id, {}) - - self.share_manager.db.share_update.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), share_id, - {'status': constants.STATUS_MANAGE_ERROR, 'size': 1}) - def test_manage_share_driver_exception(self): - CustomException = type('CustomException', (Exception,), dict()) - self.mock_object(self.share_manager, 'driver', mock.Mock()) + self.mock_object(self.share_manager, 'driver') self.share_manager.driver.driver_handles_share_servers = False + CustomException = type('CustomException', (Exception,), dict()) self.mock_object(self.share_manager.driver, 'manage_existing', mock.Mock(side_effect=CustomException)) @@ -2562,38 +2526,62 @@ class ShareManagerTestCase(test.TestCase): mock.ANY, share_id, {'status': constants.STATUS_MANAGE_ERROR, 'size': 1}) - @ddt.data( - {'size': 1, 'replication_type': None}, - {'size': 2, 'name': 'fake', 'replication_type': 'dr'}, - {'size': 3, 'export_locations': ['foo', 'bar', 'quuz'], - 'replication_type': 'writable'}, - ) - def test_manage_share_valid_share(self, driver_data): + def test_manage_share_incompatible_dhss(self): + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False + share = db_utils.create_share() + self.mock_object(share_types, + 'get_share_type_extra_specs', + mock.Mock(return_value="True")) + self.assertRaises( + exception.InvalidShare, self.share_manager.manage_share, + self.context, share['id'], {}) + + @ddt.data({'dhss': True, + 'driver_data': {'size': 1, 'replication_type': None}}, + {'dhss': False, + 'driver_data': {'size': 2, 'name': 'fake', + 'replication_type': 'dr'}}, + {'dhss': False, + 'driver_data': {'size': 3, + 'export_locations': ['foo', 'bar', 'quuz'], + 'replication_type': 'writable'}}) + @ddt.unpack + def test_manage_share_valid_share(self, dhss, driver_data): + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = dhss replication_type = driver_data.pop('replication_type') export_locations = driver_data.get('export_locations') self.mock_object(self.share_manager.db, 'share_update', mock.Mock()) - self.mock_object(self.share_manager, 'driver', mock.Mock()) self.mock_object(quota.QUOTAS, 'reserve', mock.Mock()) self.mock_object( self.share_manager.db, 'share_export_locations_update', mock.Mock(side_effect=( self.share_manager.db.share_export_locations_update))) - self.share_manager.driver.driver_handles_share_servers = False self.mock_object(share_types, 'get_share_type_extra_specs', - mock.Mock(return_value='False')) - self.mock_object(self.share_manager.driver, - "manage_existing", - mock.Mock(return_value=driver_data)) + mock.Mock(return_value=six.text_type(dhss))) + if dhss: + mock_manage = self.mock_object( + self.share_manager.driver, + "manage_existing_with_server", + mock.Mock(return_value=driver_data)) + else: + mock_manage = self.mock_object( + self.share_manager.driver, + "manage_existing", + mock.Mock(return_value=driver_data)) share = db_utils.create_share(replication_type=replication_type) share_id = share['id'] driver_options = {'fake': 'fake'} self.share_manager.manage_share(self.context, share_id, driver_options) - (self.share_manager.driver.manage_existing. - assert_called_once_with(mock.ANY, driver_options)) + if dhss: + mock_manage.assert_called_once_with(mock.ANY, driver_options, None) + else: + mock_manage.assert_called_once_with(mock.ANY, driver_options) if export_locations: (self.share_manager.db.share_export_locations_update. assert_called_once_with( @@ -2646,35 +2634,26 @@ class ShareManagerTestCase(test.TestCase): self.share_manager.db.quota_usage_create.assert_called_once_with( mock.ANY, project_id, mock.ANY, resource_name, usage) - def _setup_unmanage_mocks(self, mock_driver=True, mock_unmanage=None): + def _setup_unmanage_mocks(self, mock_driver=True, mock_unmanage=None, + dhss=False): if mock_driver: self.mock_object(self.share_manager, 'driver') if mock_unmanage: - self.mock_object(self.share_manager.driver, "unmanage", - mock_unmanage) + if dhss: + self.mock_object( + self.share_manager.driver, "unmanage_with_share_server", + mock_unmanage) + else: + self.mock_object(self.share_manager.driver, "unmanage", + mock_unmanage) self.mock_object(self.share_manager.db, 'share_update') self.mock_object(self.share_manager.db, 'share_instance_delete') - @ddt.data(True, False) - def test_unmanage_share_invalid_driver(self, driver_handles_share_servers): - self._setup_unmanage_mocks() - self.share_manager.driver.driver_handles_share_servers = ( - driver_handles_share_servers - ) - share_net = db_utils.create_share_network() - share_srv = db_utils.create_share_server( - share_network_id=share_net['id'], host=self.share_manager.host) - share = db_utils.create_share(share_network_id=share_net['id'], - share_server_id=share_srv['id']) - - self.share_manager.unmanage_share(self.context, share['id']) - - self.share_manager.db.share_update.assert_called_once_with( - mock.ANY, share['id'], {'status': constants.STATUS_UNMANAGE_ERROR}) - def test_unmanage_share_invalid_share(self): + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False unmanage = mock.Mock(side_effect=exception.InvalidShare(reason="fake")) self._setup_unmanage_mocks(mock_driver=False, mock_unmanage=unmanage) share = db_utils.create_share() @@ -2685,22 +2664,52 @@ class ShareManagerTestCase(test.TestCase): mock.ANY, share['id'], {'status': constants.STATUS_UNMANAGE_ERROR}) def test_unmanage_share_valid_share(self): - manager.CONF.set_default('driver_handles_share_servers', False) + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False self._setup_unmanage_mocks(mock_driver=False, mock_unmanage=mock.Mock()) share = db_utils.create_share() share_id = share['id'] share_instance_id = share.instance['id'] + self.mock_object(self.share_manager.db, 'share_instance_get', + mock.Mock(return_value=share.instance)) self.share_manager.unmanage_share(self.context, share_id) (self.share_manager.driver.unmanage. - assert_called_once_with(mock.ANY)) + assert_called_once_with(share.instance)) self.share_manager.db.share_instance_delete.assert_called_once_with( mock.ANY, share_instance_id) + def test_unmanage_share_valid_share_with_share_server(self): + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = True + self._setup_unmanage_mocks(mock_driver=False, + mock_unmanage=mock.Mock(), + dhss=True) + server = db_utils.create_share_server(id='fake_server_id') + share = db_utils.create_share(share_server_id='fake_server_id') + self.mock_object(self.share_manager.db, 'share_server_update') + self.mock_object(self.share_manager.db, 'share_server_get', + mock.Mock(return_value=server)) + self.mock_object(self.share_manager.db, 'share_instance_get', + mock.Mock(return_value=share.instance)) + + share_id = share['id'] + share_instance_id = share.instance['id'] + + self.share_manager.unmanage_share(self.context, share_id) + + (self.share_manager.driver.unmanage_with_server. + assert_called_once_with(share.instance, server)) + self.share_manager.db.share_instance_delete.assert_called_once_with( + mock.ANY, share_instance_id) + self.share_manager.db.share_server_update.assert_called_once_with( + mock.ANY, server['id'], {'is_auto_deletable': False}) + def test_unmanage_share_valid_share_with_quota_error(self): - manager.CONF.set_default('driver_handles_share_servers', False) + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False self._setup_unmanage_mocks(mock_driver=False, mock_unmanage=mock.Mock()) self.mock_object(quota.QUOTAS, 'reserve', @@ -2710,13 +2719,13 @@ class ShareManagerTestCase(test.TestCase): self.share_manager.unmanage_share(self.context, share['id']) - (self.share_manager.driver.unmanage. - assert_called_once_with(mock.ANY)) + self.share_manager.driver.unmanage.assert_called_once_with(mock.ANY) self.share_manager.db.share_instance_delete.assert_called_once_with( mock.ANY, share_instance_id) def test_unmanage_share_remove_access_rules_error(self): - manager.CONF.set_default('driver_handles_share_servers', False) + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False manager.CONF.unmanage_remove_access_rules = True self._setup_unmanage_mocks(mock_driver=False, mock_unmanage=mock.Mock()) @@ -2734,7 +2743,8 @@ class ShareManagerTestCase(test.TestCase): mock.ANY, share['id'], {'status': constants.STATUS_UNMANAGE_ERROR}) def test_unmanage_share_valid_share_remove_access_rules(self): - manager.CONF.set_default('driver_handles_share_servers', False) + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = False manager.CONF.unmanage_remove_access_rules = True self._setup_unmanage_mocks(mock_driver=False, mock_unmanage=mock.Mock()) @@ -3010,7 +3020,8 @@ class ShareManagerTestCase(test.TestCase): ])) self.share_manager.db.share_server_update.assert_called_once_with( self.context, share_server['id'], - {'status': constants.STATUS_ACTIVE}) + {'status': constants.STATUS_ACTIVE, + 'identifier': share_server['id']}) def test_setup_server_server_info_not_present(self): # Setup required test data @@ -3053,7 +3064,8 @@ class ShareManagerTestCase(test.TestCase): network_info, metadata=metadata) self.share_manager.db.share_server_update.assert_called_once_with( self.context, share_server['id'], - {'status': constants.STATUS_ACTIVE}) + {'status': constants.STATUS_ACTIVE, + 'identifier': share_server['id']}) self.share_manager.driver.allocate_network.assert_called_once_with( self.context, share_server, share_network) @@ -5527,53 +5539,237 @@ class ShareManagerTestCase(test.TestCase): (self.share_manager._create_share_server_in_backend. assert_called_once_with(self.context, server)) - def test_manage_snapshot_invalid_driver_mode(self): - self.mock_object(self.share_manager, 'driver') - self.share_manager.driver.driver_handles_share_servers = True - share = db_utils.create_share() - snapshot = db_utils.create_snapshot(share_id=share['id']) - driver_options = {'fake': 'fake'} + @ddt.data({'admin_network_api': mock.Mock(), + 'driver_return': ('new_identifier', {'some_id': 'some_value'})}, + {'admin_network_api': None, + 'driver_return': (None, None)}) + @ddt.unpack + def test_manage_share_server(self, admin_network_api, driver_return): + driver_opts = {} + fake_share_server = fakes.fake_share_server_get() + fake_list_network_info = [{}, {}] + fake_list_empty_network_info = [] + identifier = 'fake_id' + ss_data = { + 'name': 'fake_name', + 'ou': 'fake_ou', + 'domain': 'fake_domain', + 'server': 'fake_server', + 'dns_ip': 'fake_dns_ip', + 'user': 'fake_user', + 'type': 'FAKE', + 'password': 'fake_pass', + } + mock_manage_admin_network_allocations = mock.Mock() + share_server = db_utils.create_share_server(**fake_share_server) + security_service = db_utils.create_security_service(**ss_data) + share_network = db_utils.create_share_network() + db.share_network_add_security_service(context.get_admin_context(), + share_network['id'], + security_service['id']) + share_network = db.share_network_get(context.get_admin_context(), + share_network['id']) + self.share_manager.driver._admin_network_api = admin_network_api - self.assertRaises( - exception.InvalidDriverMode, - self.share_manager.manage_snapshot, self.context, - snapshot['id'], driver_options) + mock_share_server_update = self.mock_object( + db, 'share_server_update') + mock_share_server_get = self.mock_object( + db, 'share_server_get', mock.Mock(return_value=share_server)) + mock_share_network_get = self.mock_object( + db, 'share_network_get', mock.Mock(return_value=share_network)) + mock_network_allocations_get = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1)) + mock_share_server_net_info = self.mock_object( + self.share_manager.driver, 'get_share_server_network_info', + mock.Mock(return_value=fake_list_network_info)) + mock_manage_network_allocations = self.mock_object( + self.share_manager.driver.network_api, + 'manage_network_allocations', + mock.Mock(return_value=fake_list_empty_network_info)) + mock_manage_server = self.mock_object( + self.share_manager.driver, 'manage_server', + mock.Mock(return_value=driver_return)) + mock_set_backend_details = self.mock_object( + db, 'share_server_backend_details_set') - def test_manage_snapshot_invalid_snapshot(self): - fake_share_server = 'fake_share_server' + ss_from_db = share_network['security_services'][0] + ss_data_from_db = { + 'name': ss_from_db['name'], + 'ou': ss_from_db['ou'], + 'domain': ss_from_db['domain'], + 'server': ss_from_db['server'], + 'dns_ip': ss_from_db['dns_ip'], + 'user': ss_from_db['user'], + 'type': ss_from_db['type'], + 'password': ss_from_db['password'], + } + + expected_backend_details = { + 'security_service_FAKE': jsonutils.dumps(ss_data_from_db), + } + if driver_return[1]: + expected_backend_details.update(driver_return[1]) + + if admin_network_api is not None: + mock_manage_admin_network_allocations = self.mock_object( + self.share_manager.driver.admin_network_api, + 'manage_network_allocations', + mock.Mock(return_value=fake_list_network_info)) + + self.share_manager.manage_share_server(self.context, + fake_share_server['id'], + identifier, + driver_opts) + + mock_share_server_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_network_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_share_server['share_network_id'] + ) + mock_network_allocations_get.assert_called_once_with() + mock_share_server_net_info.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), share_server, identifier, + driver_opts + ) + mock_manage_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_list_network_info, share_server, share_network + ) + mock_manage_server.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), share_server, identifier, + driver_opts + ) + mock_share_server_update.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'], + {'status': constants.STATUS_ACTIVE, + 'identifier': driver_return[0] or share_server['id']} + ) + mock_set_backend_details.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), share_server['id'], + expected_backend_details + ) + if admin_network_api is not None: + mock_manage_admin_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_list_network_info, share_server + ) + + def test_manage_share_server_dhss_false(self): self.mock_object(self.share_manager, 'driver') self.share_manager.driver.driver_handles_share_servers = False - mock_get_share_server = self.mock_object( - self.share_manager, - '_get_share_server', - mock.Mock(return_value=fake_share_server)) - share = db_utils.create_share() - snapshot = db_utils.create_snapshot(share_id=share['id']) - driver_options = {'fake': 'fake'} - mock_get = self.mock_object(self.share_manager.db, - 'share_snapshot_get', - mock.Mock(return_value=snapshot)) - self.assertRaises( - exception.InvalidShareSnapshot, - self.share_manager.manage_snapshot, self.context, - snapshot['id'], driver_options) + exception.ManageShareServerError, + self.share_manager.manage_share_server, + self.context, "fake_id", "foo", {}) - mock_get.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['id']) - mock_get_share_server.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['share']) + def test_manage_share_server_without_allocations(self): + + driver_opts = {} + fake_share_server = fakes.fake_share_server_get() + fake_list_empty_network_info = [] + identifier = 'fake_id' + share_server = db_utils.create_share_server(**fake_share_server) + share_network = db_utils.create_share_network() + self.share_manager.driver._admin_network_api = mock.Mock() + + mock_share_server_get = self.mock_object( + db, 'share_server_get', mock.Mock(return_value=share_server)) + mock_share_network_get = self.mock_object( + db, 'share_network_get', mock.Mock(return_value=share_network)) + mock_network_allocations_get = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1)) + mock_get_share_network_info = self.mock_object( + self.share_manager.driver, 'get_share_server_network_info', + mock.Mock(return_value=fake_list_empty_network_info)) + + self.assertRaises(exception.ManageShareServerError, + self.share_manager.manage_share_server, + context=self.context, + share_server_id=fake_share_server['id'], + identifier=identifier, + driver_opts=driver_opts) + mock_share_server_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_network_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_share_server['share_network_id'] + ) + mock_network_allocations_get.assert_called_once_with() + mock_get_share_network_info.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), share_server, identifier, + driver_opts + ) + + def test_manage_share_server_allocations_not_managed(self): + driver_opts = {} + fake_share_server = fakes.fake_share_server_get() + fake_list_network_info = [{}, {}] + identifier = 'fake_id' + share_server = db_utils.create_share_server(**fake_share_server) + share_network = db_utils.create_share_network() + self.share_manager.driver._admin_network_api = mock.Mock() + + mock_share_server_get = self.mock_object( + db, 'share_server_get', mock.Mock(return_value=share_server)) + mock_share_network_get = self.mock_object( + db, 'share_network_get', mock.Mock(return_value=share_network)) + mock_network_allocations_get = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1)) + mock_get_share_network_info = self.mock_object( + self.share_manager.driver, 'get_share_server_network_info', + mock.Mock(return_value=fake_list_network_info)) + mock_manage_admin_network_allocations = self.mock_object( + self.share_manager.driver.admin_network_api, + 'manage_network_allocations', + mock.Mock(return_value=fake_list_network_info)) + mock_manage_network_allocations = self.mock_object( + self.share_manager.driver.network_api, + 'manage_network_allocations', + mock.Mock(return_value=fake_list_network_info)) + + self.assertRaises(exception.ManageShareServerError, + self.share_manager.manage_share_server, + context=self.context, + share_server_id=fake_share_server['id'], + identifier=identifier, + driver_opts=driver_opts) + mock_share_server_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_network_get.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_share_server['share_network_id'] + ) + mock_network_allocations_get.assert_called_once_with() + mock_get_share_network_info.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), share_server, identifier, + driver_opts + ) + mock_manage_admin_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_list_network_info, share_server + ) + mock_manage_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + fake_list_network_info, share_server, share_network + ) def test_manage_snapshot_driver_exception(self): CustomException = type('CustomException', (Exception,), {}) self.mock_object(self.share_manager, 'driver') self.share_manager.driver.driver_handles_share_servers = False + self.mock_object(share_types, + 'get_share_type_extra_specs', + mock.Mock(return_value="False")) mock_manage = self.mock_object(self.share_manager.driver, 'manage_existing_snapshot', mock.Mock(side_effect=CustomException)) - mock_get_share_server = self.mock_object(self.share_manager, - '_get_share_server', - mock.Mock(return_value=None)) share = db_utils.create_share() snapshot = db_utils.create_snapshot(share_id=share['id']) driver_options = {} @@ -5589,35 +5785,259 @@ class ShareManagerTestCase(test.TestCase): mock_manage.assert_called_once_with(mock.ANY, driver_options) mock_get.assert_called_once_with( utils.IsAMatcher(context.RequestContext), snapshot['id']) - mock_get_share_server.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['share']) - @ddt.data({'driver_data': {'size': 1}, 'mount_snapshot_support': False}, - {'driver_data': {'size': 2, 'name': 'fake'}, + def test_unmanage_share_server_no_allocations(self): + + fake_share_server = fakes.fake_share_server_get() + + ss_list = [ + {'name': 'fake_AD'}, + {'name': 'fake_LDAP'}, + {'name': 'fake_kerberos'} + ] + + db_utils.create_share_server(**fake_share_server) + self.mock_object(self.share_manager.driver, 'unmanage_server', + mock.Mock(side_effect=NotImplementedError())) + self.mock_object(self.share_manager.db, 'share_server_delete') + + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=0) + ) + mock_admin_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_admin_network_allocations_number', + mock.Mock(return_value=0) + ) + + self.share_manager.unmanage_share_server( + self.context, fake_share_server['id'], True) + + mock_network_allocations_number.assert_called_once_with() + mock_admin_network_allocations_number.assert_called_once_with() + + self.share_manager.driver.unmanage_server.assert_called_once_with( + fake_share_server['backend_details'], ss_list) + self.share_manager.db.share_server_delete.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id']) + + def test_unmanage_share_server_no_allocations_driver_not_implemented(self): + + fake_share_server = fakes.fake_share_server_get() + fake_share_server['status'] = constants.STATUS_UNMANAGING + ss_list = [ + {'name': 'fake_AD'}, + {'name': 'fake_LDAP'}, + {'name': 'fake_kerberos'} + ] + db_utils.create_share_server(**fake_share_server) + self.mock_object(self.share_manager.driver, 'unmanage_server', + mock.Mock(side_effect=NotImplementedError())) + self.mock_object(self.share_manager.db, 'share_server_update') + + self.share_manager.unmanage_share_server( + self.context, fake_share_server['id'], False) + + self.share_manager.driver.unmanage_server.assert_called_once_with( + fake_share_server['backend_details'], ss_list) + + self.share_manager.db.share_server_update.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'], + {'status': constants.STATUS_UNMANAGE_ERROR}) + + def test_unmanage_share_server_with_network_allocations(self): + + fake_share_server = fakes.fake_share_server_get() + db_utils.create_share_server(**fake_share_server) + + mock_unmanage_network_allocations = self.mock_object( + self.share_manager.driver.network_api, + 'unmanage_network_allocations' + ) + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1) + ) + + self.share_manager.unmanage_share_server( + self.context, fake_share_server['id'], True) + mock_network_allocations_number.assert_called_once_with() + mock_unmanage_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id']) + + def test_unmanage_share_server_with_admin_network_allocations(self): + + fake_share_server = fakes.fake_share_server_get() + db_utils.create_share_server(**fake_share_server) + + mock_admin_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_admin_network_allocations_number', + mock.Mock(return_value=1) + ) + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=0) + ) + + self.share_manager.driver._admin_network_api = mock.Mock() + self.share_manager.unmanage_share_server( + self.context, fake_share_server['id'], True) + + mock_admin_network_allocations_number.assert_called_once_with() + mock_network_allocations_number.assert_called_once_with() + + def test_unmanage_share_server_error(self): + + fake_share_server = fakes.fake_share_server_get() + db_utils.create_share_server(**fake_share_server) + + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1) + ) + error = mock.Mock( + side_effect=exception.ShareServerNotFound(share_server_id="fake")) + + mock_share_server_delete = self.mock_object( + db, 'share_server_delete', error + ) + mock_share_server_update = self.mock_object( + db, 'share_server_update' + ) + self.share_manager.driver._admin_network_api = mock.Mock() + + self.assertRaises(exception.ShareServerNotFound, + self.share_manager.unmanage_share_server, + self.context, + fake_share_server['id'], + True) + mock_network_allocations_number.assert_called_once_with() + mock_share_server_delete.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_server_update.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'], + {'status': constants.STATUS_UNMANAGE_ERROR} + ) + + def test_unmanage_share_server_network_allocations_error(self): + + fake_share_server = fakes.fake_share_server_get() + db_utils.create_share_server(**fake_share_server) + + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=1) + ) + error = mock.Mock( + side_effect=exception.ShareNetworkNotFound(share_network_id="fake") + ) + mock_unmanage_network_allocations = self.mock_object( + self.share_manager.driver.network_api, + 'unmanage_network_allocations', error) + + mock_share_server_update = self.mock_object( + db, 'share_server_update' + ) + self.share_manager.driver._admin_network_api = mock.Mock() + + self.assertRaises(exception.ShareNetworkNotFound, + self.share_manager.unmanage_share_server, + self.context, + fake_share_server['id'], + True) + + mock_network_allocations_number.assert_called_once_with() + mock_unmanage_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_server_update.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'], + {'status': constants.STATUS_UNMANAGE_ERROR} + ) + + def test_unmanage_share_server_admin_network_allocations_error(self): + + fake_share_server = fakes.fake_share_server_get() + db_utils.create_share_server(**fake_share_server) + self.share_manager.driver._admin_network_api = mock.Mock() + + mock_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_network_allocations_number', + mock.Mock(return_value=0) + ) + mock_admin_network_allocations_number = self.mock_object( + self.share_manager.driver, 'get_admin_network_allocations_number', + mock.Mock(return_value=1) + ) + error = mock.Mock( + side_effect=exception.ShareNetworkNotFound(share_network_id="fake") + ) + mock_unmanage_admin_network_allocations = self.mock_object( + self.share_manager.driver._admin_network_api, + 'unmanage_network_allocations', error + ) + mock_unmanage_network_allocations = self.mock_object( + self.share_manager.driver.network_api, + 'unmanage_network_allocations', error) + + mock_share_server_update = self.mock_object( + db, 'share_server_update' + ) + + self.assertRaises(exception.ShareNetworkNotFound, + self.share_manager.unmanage_share_server, + self.context, + fake_share_server['id'], + True) + mock_network_allocations_number.assert_called_once_with() + mock_admin_network_allocations_number.assert_called_once_with() + mock_unmanage_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_unmanage_admin_network_allocations.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'] + ) + mock_share_server_update.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), fake_share_server['id'], + {'status': constants.STATUS_UNMANAGE_ERROR} + ) + + @ddt.data({'dhss': True, 'driver_data': {'size': 1}, 'mount_snapshot_support': False}, - {'driver_data': {'size': 3}, 'mount_snapshot_support': False}, - {'driver_data': {'size': 3, 'export_locations': [ + {'dhss': True, 'driver_data': {'size': 2, 'name': 'fake'}, + 'mount_snapshot_support': False}, + {'dhss': False, 'driver_data': {'size': 3}, + 'mount_snapshot_support': False}, + {'dhss': False, 'driver_data': {'size': 3, 'export_locations': [ {'path': '/path1', 'is_admin_only': True}, {'path': '/path2', 'is_admin_only': False} ]}, 'mount_snapshot_support': False}, - {'driver_data': {'size': 3, 'export_locations': [ + {'dhss': False, 'driver_data': {'size': 3, 'export_locations': [ {'path': '/path1', 'is_admin_only': True}, {'path': '/path2', 'is_admin_only': False} ]}, 'mount_snapshot_support': True}) @ddt.unpack def test_manage_snapshot_valid_snapshot( - self, driver_data, mount_snapshot_support): + self, driver_data, mount_snapshot_support, dhss): mock_get_share_server = self.mock_object(self.share_manager, '_get_share_server', mock.Mock(return_value=None)) self.mock_object(self.share_manager.db, 'share_snapshot_update') self.mock_object(self.share_manager, 'driver') self.mock_object(quota.QUOTAS, 'reserve', mock.Mock()) - self.share_manager.driver.driver_handles_share_servers = False - mock_manage = self.mock_object( - self.share_manager.driver, - "manage_existing_snapshot", - mock.Mock(return_value=driver_data)) + self.share_manager.driver.driver_handles_share_servers = dhss + + if dhss: + mock_manage = self.mock_object( + self.share_manager.driver, + "manage_existing_snapshot_with_server", + mock.Mock(return_value=driver_data)) + else: + mock_manage = self.mock_object( + self.share_manager.driver, + "manage_existing_snapshot", + mock.Mock(return_value=driver_data)) size = driver_data['size'] export_locations = driver_data.get('export_locations') share = db_utils.create_share( @@ -5636,15 +6056,19 @@ class ShareManagerTestCase(test.TestCase): self.share_manager.manage_snapshot(self.context, snapshot_id, driver_options) - mock_manage.assert_called_once_with(mock.ANY, driver_options) + if dhss: + mock_manage.assert_called_once_with(mock.ANY, driver_options, None) + else: + mock_manage.assert_called_once_with(mock.ANY, driver_options) valid_snapshot_data = { 'status': constants.STATUS_AVAILABLE} valid_snapshot_data.update(driver_data) self.share_manager.db.share_snapshot_update.assert_called_once_with( utils.IsAMatcher(context.RequestContext), snapshot_id, valid_snapshot_data) - mock_get_share_server.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['share']) + if dhss: + mock_get_share_server.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), snapshot['share']) mock_get.assert_called_once_with( utils.IsAMatcher(context.RequestContext), snapshot_id) if mount_snapshot_support and export_locations: @@ -5660,48 +6084,6 @@ class ShareManagerTestCase(test.TestCase): else: mock_export_update.assert_not_called() - def test_unmanage_snapshot_invalid_driver_mode(self): - self.mock_object(self.share_manager, 'driver') - self.share_manager.driver.driver_handles_share_servers = True - share = db_utils.create_share() - snapshot = db_utils.create_snapshot(share_id=share['id']) - self.mock_object(self.share_manager.db, 'share_snapshot_update') - - ret = self.share_manager.unmanage_snapshot(self.context, - snapshot['id']) - self.assertIsNone(ret) - self.share_manager.db.share_snapshot_update.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), - snapshot['id'], - {'status': constants.STATUS_UNMANAGE_ERROR}) - - def test_unmanage_snapshot_invalid_snapshot(self): - self.mock_object(self.share_manager, 'driver') - self.share_manager.driver.driver_handles_share_servers = False - mock_get_share_server = self.mock_object( - self.share_manager, - '_get_share_server', - mock.Mock(return_value='fake_share_server')) - self.mock_object(self.share_manager.db, 'share_snapshot_update') - share = db_utils.create_share() - snapshot = db_utils.create_snapshot(share_id=share['id']) - mock_get = self.mock_object(self.share_manager.db, - 'share_snapshot_get', - mock.Mock(return_value=snapshot)) - - ret = self.share_manager.unmanage_snapshot(self.context, - snapshot['id']) - - self.assertIsNone(ret) - self.share_manager.db.share_snapshot_update.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), - snapshot['id'], - {'status': constants.STATUS_UNMANAGE_ERROR}) - mock_get.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['id']) - mock_get_share_server.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), snapshot['share']) - def test_unmanage_snapshot_invalid_share(self): manager.CONF.unmanage_remove_access_rules = False self.mock_object(self.share_manager, 'driver') @@ -5733,18 +6115,27 @@ class ShareManagerTestCase(test.TestCase): mock_get_share_server.assert_called_once_with( utils.IsAMatcher(context.RequestContext), snapshot['share']) - @ddt.data(False, True) - def test_unmanage_snapshot_valid_snapshot(self, quota_error): + @ddt.data({'dhss': False, 'quota_error': False}, + {'dhss': True, 'quota_error': False}, + {'dhss': False, 'quota_error': True}, + {'dhss': True, 'quota_error': True}) + @ddt.unpack + def test_unmanage_snapshot_valid_snapshot(self, dhss, quota_error): if quota_error: self.mock_object(quota.QUOTAS, 'reserve', mock.Mock( side_effect=exception.ManilaException(message='error'))) manager.CONF.unmanage_remove_access_rules = True mock_log_warning = self.mock_object(manager.LOG, 'warning') self.mock_object(self.share_manager, 'driver') - self.share_manager.driver.driver_handles_share_servers = False + self.share_manager.driver.driver_handles_share_servers = dhss mock_update_access = self.mock_object( self.share_manager.snapshot_access_helper, "update_access_rules") - self.mock_object(self.share_manager.driver, "unmanage_snapshot") + if dhss: + mock_unmanage = self.mock_object( + self.share_manager.driver, "unmanage_snapshot_with_server") + else: + mock_unmanage = self.mock_object( + self.share_manager.driver, "unmanage_snapshot") mock_get_share_server = self.mock_object( self.share_manager, '_get_share_server', @@ -5762,8 +6153,10 @@ class ShareManagerTestCase(test.TestCase): self.share_manager.unmanage_snapshot(self.context, snapshot['id']) - self.share_manager.driver.unmanage_snapshot.assert_called_once_with( - snapshot.instance) + if dhss: + mock_unmanage.assert_called_once_with(snapshot.instance, None) + else: + mock_unmanage.assert_called_once_with(snapshot.instance) mock_update_access.assert_called_once_with( utils.IsAMatcher(context.RequestContext), snapshot.instance['id'], delete_all_rules=True, share_server=None) diff --git a/manila/tests/share/test_rpcapi.py b/manila/tests/share/test_rpcapi.py index c6c91fcc16..7ae8fb931b 100644 --- a/manila/tests/share/test_rpcapi.py +++ b/manila/tests/share/test_rpcapi.py @@ -115,6 +115,11 @@ class ShareRpcAPITestCase(test.TestCase): if 'snapshot_instance' in expected_msg: snapshot_instance = expected_msg.pop('snapshot_instance', None) expected_msg['snapshot_instance_id'] = snapshot_instance['id'] + if ('share_server' in expected_msg + and (method == 'manage_share_server') + or method == 'unmanage_share_server'): + share_server = expected_msg.pop('share_server', None) + expected_msg['share_server_id'] = share_server['id'] if 'host' in kwargs: host = kwargs['host'] @@ -322,6 +327,21 @@ class ShareRpcAPITestCase(test.TestCase): snapshot=self.fake_snapshot, host='fake_host') + def test_manage_share_server(self): + self._test_share_api('manage_share_server', + rpc_method='cast', + version='1.19', + share_server=self.fake_share_server, + identifier='fake', + driver_opts={}) + + def test_unmanage_share_server(self): + self._test_share_api('unmanage_share_server', + rpc_method='cast', + version='1.19', + share_server=self.fake_share_server, + force='fake_force') + def test_revert_to_snapshot(self): self._test_share_api('revert_to_snapshot', rpc_method='cast', diff --git a/manila/tests/test_exception.py b/manila/tests/test_exception.py index 8cfdcc1af7..e357f43d7a 100644 --- a/manila/tests/test_exception.py +++ b/manila/tests/test_exception.py @@ -135,6 +135,16 @@ class ManilaExceptionTestCase(test.TestCase): self.assertIn(access_type, e.msg) self.assertIn(access, e.msg) + def test_manage_share_server_error(self): + # Verify response code for exception.ManageShareServerError + reason = 'Invalid share server id.' + share_server_id = 'fake' + e = exception.ManageShareServerError(reason=reason, + share_server_id=share_server_id) + + self.assertEqual(500, e.code) + self.assertIn(reason, e.msg) + class ManilaExceptionResponseCode400(test.TestCase): diff --git a/manila/tests/test_network.py b/manila/tests/test_network.py index 6795764755..3a40746886 100644 --- a/manila/tests/test_network.py +++ b/manila/tests/test_network.py @@ -80,6 +80,14 @@ class NetworkBaseAPITestCase(test.TestCase): def allocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + self.assertRaises(TypeError, FakeNetworkAPI) def test_inherit_network_base_api_allocate_not_redefined(self): @@ -87,6 +95,14 @@ class NetworkBaseAPITestCase(test.TestCase): def deallocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + self.assertRaises(TypeError, FakeNetworkAPI) def test_inherit_network_base_api(self): @@ -97,6 +113,14 @@ class NetworkBaseAPITestCase(test.TestCase): def deallocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + result = FakeNetworkAPI() self.assertTrue(hasattr(result, '_verify_share_network')) @@ -111,6 +135,14 @@ class NetworkBaseAPITestCase(test.TestCase): def deallocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + result = FakeNetworkAPI() result._verify_share_network('foo_id', {'id': 'bar_id'}) @@ -123,6 +155,14 @@ class NetworkBaseAPITestCase(test.TestCase): def deallocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + result = FakeNetworkAPI() self.assertRaises( @@ -142,6 +182,14 @@ class NetworkBaseAPITestCase(test.TestCase): def deallocate_network(self, *args, **kwargs): pass + def manage_network_allocations( + self, context, allocations, share_server, + share_network=None): + pass + + def unmanage_network_allocations(self, context, share_server_id): + pass + network.CONF.set_default('network_plugin_ipv6_enabled', network_plugin_ipv6_enabled) network.CONF.set_default('network_plugin_ipv4_enabled', diff --git a/releasenotes/notes/manage-unmanage-share-servers-cd4a6523d8e9fbdf.yaml b/releasenotes/notes/manage-unmanage-share-servers-cd4a6523d8e9fbdf.yaml new file mode 100644 index 0000000000..cb3e57a0b5 --- /dev/null +++ b/releasenotes/notes/manage-unmanage-share-servers-cd4a6523d8e9fbdf.yaml @@ -0,0 +1,24 @@ +--- +features: + - Added APIs with default policy set to + 'rule:admin_api' that allow managing and + unmanaging share servers. Managing Share servers + is useful for importing pre-existing shares and + snapshots into Manila's management when the driver + is configured in ``driver_handles_share_servers`` + enabled mode. Unmanaging removes manila share + servers from the database without removing them + from the back end. Managed share servers, or share + servers that have had one or more shares unmanaged will + not be deleted automatically when they do not have + any shares managed by Manila, even if the config options + [DEFAULT]/delete_share_server_with_last_share or + [DEFAULT]/automatic_share_server_cleanup have been + set to True. + - Updated Manage Share API to be able to manage shares + in ``driver_handles_share_servers`` enabled driver + mode by supplying the Share Server ID. + - Updated Unmanage Share and Unmanage Snapshot APIs + to allow unmanaging shares and snapshots in + ``driver_handles_share_servers`` enabled driver + mode.