From 101becf9b4e7326d1079bdba43c7d5bc537f5737 Mon Sep 17 00:00:00 2001 From: Carlos Eduardo Date: Tue, 16 Jul 2024 16:01:34 -0300 Subject: [PATCH] API for ensure shares Introduces a new API to allow OpenStack administrators to start the share manager's ensure shares operation. Also, introduced a new configuration option named `update_shares_status_on_ensure`, so administrators can define whether the shares status should be updated during the ensure shares operation or not. A new column was added to the services table, named `ensuring`. Through this field, we can identify whether there is an undergoing ensure shares operation or not. Closes-Bug: #1996793 Partially-Implements: bp ensure-shares-api Change-Id: If7bf059eb8581f20a3ceb7c1af93558774f4ef5e --- manila/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 4 + manila/api/v2/router.py | 7 + manila/api/v2/services.py | 48 ++++++- manila/api/views/services.py | 5 + manila/common/config.py | 5 + manila/common/constants.py | 2 + ...a6287df8_add_ensuring_field_to_services.py | 48 +++++++ manila/db/sqlalchemy/models.py | 1 + manila/policies/service.py | 19 +++ manila/services/__init__.py | 0 manila/services/api.py | 38 +++++ manila/share/manager.py | 79 +++++++---- manila/share/rpcapi.py | 12 +- manila/tests/api/v2/test_services.py | 98 +++++++++++++ manila/tests/services/__init__.py | 0 manila/tests/services/test_api.py | 61 ++++++++ manila/tests/share/test_manager.py | 134 ++++++++++++++++++ manila/tests/share/test_rpcapi.py | 10 ++ ...dd-ensure-shares-api-9ac10877a99ab0c5.yaml | 16 +++ 20 files changed, 556 insertions(+), 34 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/cdefa6287df8_add_ensuring_field_to_services.py create mode 100644 manila/services/__init__.py create mode 100644 manila/services/api.py create mode 100644 manila/tests/services/__init__.py create mode 100644 manila/tests/services/test_api.py create mode 100644 releasenotes/notes/add-ensure-shares-api-9ac10877a99ab0c5.yaml diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 0fbcd5e9f4..716889ab16 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -203,13 +203,14 @@ REST_API_VERSION_HISTORY = """ * 2.83 - Added 'disabled_reason' field to services. * 2.84 - Added mount_point_name to shares. * 2.85 - Added backup_type field to share backups. + * 2.86 - Add ensure share API. """ # 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.85" +_MAX_API_VERSION = "2.86" 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 d1023ae879..eb045c77f0 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -458,3 +458,7 @@ user documentation. 2.85 ---- Added ``backup_type`` field to share backup object. + +2.86 +---- + Added ensure shares API. diff --git a/manila/api/v2/router.py b/manila/api/v2/router.py index b4f5a485ce..5c277055f2 100644 --- a/manila/api/v2/router.py +++ b/manila/api/v2/router.py @@ -102,6 +102,13 @@ class APIRouter(manila.api.openstack.APIRouter): mapper.resource("service", "services", controller=self.resources["services"]) + for path_prefix in ['/{project_id}', '']: + # project_id is optional + mapper.connect("services", + "%s/services/ensure-shares" % path_prefix, + controller=self.resources["services"], + action="ensure_shares", + conditions={"method": ["POST"]}) self.resources["quota_sets_legacy"] = ( quota_sets.create_resource_legacy()) diff --git a/manila/api/v2/services.py b/manila/api/v2/services.py index 8ff40bfb63..48a09f8d13 100644 --- a/manila/api/v2/services.py +++ b/manila/api/v2/services.py @@ -14,13 +14,17 @@ # License for the specific language governing permissions and limitations # under the License. +import http.client as http_client from oslo_utils import strutils import webob.exc from manila.api.openstack import wsgi from manila.api.views import services as services_views from manila import db +from manila import exception from manila.i18n import _ +from manila import policy +from manila.services import api as service_api class ServiceMixin(object): @@ -34,7 +38,7 @@ class ServiceMixin(object): _view_builder_class = services_views.ViewBuilder @wsgi.Controller.authorize("index") - def _index(self, req): + def _index(self, req, support_ensure_shares=False): """Return a list of all running services.""" context = req.environ['manila.context'] @@ -42,7 +46,7 @@ class ServiceMixin(object): services = [] for service in all_services: - service = { + service_data = { 'id': service['id'], 'binary': service['binary'], 'host': service['host'], @@ -52,7 +56,9 @@ class ServiceMixin(object): 'state': service['state'], 'updated_at': service['updated_at'], } - services.append(service) + if support_ensure_shares: + service_data['ensuring'] = service['ensuring'] + services.append(service_data) search_opts = [ 'host', @@ -141,10 +147,18 @@ class ServiceController(ServiceMixin, wsgi.Controller): Registered under API URL 'services'. """ - @wsgi.Controller.api_version('2.7') + def __init__(self): + super().__init__() + self.service_api = service_api.API() + + @wsgi.Controller.api_version('2.7', '2.85') def index(self, req): return self._index(req) + @wsgi.Controller.api_version('2.86') # noqa + def index(self, req): # pylint: disable=function-redefined # noqa F811 + return self._index(req, support_ensure_shares=True) + @wsgi.Controller.api_version('2.7', '2.82') def update(self, req, id, body): return self._update(req, id, body, support_disabled_reason=False) @@ -153,6 +167,32 @@ class ServiceController(ServiceMixin, wsgi.Controller): def update(self, req, id, body): # pylint: disable=function-redefined # noqa F811 return self._update(req, id, body) + @wsgi.Controller.api_version('2.86') + @wsgi.Controller.authorize + def ensure_shares(self, req, body): + """Starts ensure shares for a given manila-share binary.""" + context = req.environ['manila.context'] + + policy.check_policy(context, 'service', 'ensure_shares') + host = body.get('host', None) + if not host: + raise webob.exc.HTTPBadRequest('Missing host parameter.') + + try: + # The only binary supported is Manila share. + service = db.service_get_by_args(context, host, 'manila-share') + except exception.NotFound: + raise webob.exc.HTTPNotFound( + "manila-share binary for '%s' host not found" % id + ) + + try: + self.service_api.ensure_shares(context, service, host) + except webob.exc.HTTPConflict: + raise + + return webob.Response(status_int=http_client.ACCEPTED) + def create_resource_legacy(): return wsgi.Resource(ServiceControllerLegacy()) diff --git a/manila/api/views/services.py b/manila/api/views/services.py index d0db7df3c9..3769c83101 100644 --- a/manila/api/views/services.py +++ b/manila/api/views/services.py @@ -21,6 +21,7 @@ class ViewBuilder(common.ViewBuilder): _collection_name = "services" _detail_version_modifiers = [ "add_disabled_reason_field", + "add_ensuring_field", ] def summary(self, request, service): @@ -49,3 +50,7 @@ class ViewBuilder(common.ViewBuilder): service_dict.pop('disabled', None) service_dict['status'] = service.get('status') service_dict['disabled_reason'] = service.get('disabled_reason') + + @common.ViewBuilder.versioned_method("2.86") + def add_ensuring_field(self, context, service_dict, service): + service_dict['ensuring'] = service.get('ensuring') diff --git a/manila/common/config.py b/manila/common/config.py index 75a0300126..879a6f9979 100644 --- a/manila/common/config.py +++ b/manila/common/config.py @@ -147,6 +147,11 @@ global_opts = [ '(element of the list is , ' 'i.e max_files) can be passed to share drivers as part ' 'of metadata create/update operations.'), + cfg.BoolOpt('update_shares_status_on_ensure', + default=True, + help='Whether Manila should update the status of all shares ' + 'within a backend during ongoing ensure_shares ' + 'run.'), ] CONF.register_opts(global_opts) diff --git a/manila/common/constants.py b/manila/common/constants.py index 1aafe41191..bb09e29cd0 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -53,6 +53,7 @@ STATUS_AWAITING_TRANSFER = 'awaiting_transfer' STATUS_BACKUP_CREATING = 'backup_creating' STATUS_BACKUP_RESTORING = 'backup_restoring' STATUS_BACKUP_RESTORING_ERROR = 'backup_restoring_error' +STATUS_ENSURING = 'ensuring' # Transfer resource type SHARE_RESOURCE_TYPE = 'share' @@ -144,6 +145,7 @@ TRANSITIONAL_STATUSES = ( STATUS_RESTORING, STATUS_REVERTING, STATUS_SERVER_MIGRATING, STATUS_SERVER_MIGRATING_TO, STATUS_BACKUP_RESTORING, STATUS_BACKUP_CREATING, + STATUS_ENSURING, ) INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES = ( diff --git a/manila/db/migrations/alembic/versions/cdefa6287df8_add_ensuring_field_to_services.py b/manila/db/migrations/alembic/versions/cdefa6287df8_add_ensuring_field_to_services.py new file mode 100644 index 0000000000..3ac69aa53a --- /dev/null +++ b/manila/db/migrations/alembic/versions/cdefa6287df8_add_ensuring_field_to_services.py @@ -0,0 +1,48 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""add-ensuring-field-to-services + +Revision ID: cdefa6287df8 +Revises: 2f27d904214c +Create Date: 2024-07-15 14:29:16.733696 + +""" + +# revision identifiers, used by Alembic. +revision = 'cdefa6287df8' +down_revision = '2f27d904214c' + +from alembic import op +from oslo_log import log +import sqlalchemy as sa + + +LOG = log.getLogger(__name__) + + +def upgrade(): + try: + op.add_column('services', sa.Column( + 'ensuring', sa.Boolean, + nullable=False, server_default=sa.sql.false())) + except Exception: + LOG.error("Column services.ensuring not created!") + raise + + +def downgrade(): + try: + op.drop_column('services', 'ensuring') + except Exception: + LOG.error("Column shares.ensuring not dropped!") + raise diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 9e166dfc55..bfb114bb53 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -72,6 +72,7 @@ class Service(BASE, ManilaBase): availability_zone_id = Column(String(36), ForeignKey('availability_zones.id'), nullable=True) + ensuring = Column(Boolean, default=False) availability_zone = orm.relationship( "AvailabilityZone", diff --git a/manila/policies/service.py b/manila/policies/service.py index f22772b13f..6946eb1ce2 100644 --- a/manila/policies/service.py +++ b/manila/policies/service.py @@ -34,6 +34,12 @@ deprecated_service_update = policy.DeprecatedRule( deprecated_reason=DEPRECATED_REASON, deprecated_since=versionutils.deprecated.WALLABY ) +deprecated_service_ensure = policy.DeprecatedRule( + name=BASE_POLICY_NAME % 'ensure_shares', + check_str=base.RULE_ADMIN_API, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='2024.2/Dalmatian' +) service_policies = [ @@ -79,6 +85,19 @@ service_policies = [ ], deprecated_rule=deprecated_service_update ), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'ensure_shares', + check_str=base.ADMIN, + scope_types=['project'], + description="Run ensure shares for a manila-share binary.", + operations=[ + { + 'method': 'POST', + 'path': '/services/ensure', + } + ], + deprecated_rule=deprecated_service_ensure + ), ] diff --git a/manila/services/__init__.py b/manila/services/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/manila/services/api.py b/manila/services/api.py new file mode 100644 index 0000000000..77f0e9b193 --- /dev/null +++ b/manila/services/api.py @@ -0,0 +1,38 @@ +# 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_config import cfg + +from webob import exc + +from manila.db import base +from manila.share import rpcapi as share_rpcapi + +CONF = cfg.CONF + + +class API(base.Base): + """API for handling service actions.""" + + def __init__(self): + super(API, self).__init__() + self.share_rpcapi = share_rpcapi.ShareAPI() + + def ensure_shares(self, context, service, host): + """Start the ensure shares in a given host.""" + + if service['state'] != "up": + raise exc.HTTPConflict( + "The service must have its state set to 'up' prior to running " + "ensure shares.") + + self.share_rpcapi.ensure_driver_resources(context, host) diff --git a/manila/share/manager.py b/manila/share/manager.py index 7fabfd694f..2e56a14119 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -264,7 +264,7 @@ def add_hooks(f): class ShareManager(manager.SchedulerDependentManager): """Manages NAS storages.""" - RPC_API_VERSION = '1.28' + RPC_API_VERSION = '1.29' def __init__(self, share_driver=None, service_name=None, *args, **kwargs): """Load the driver from args, or from flags.""" @@ -401,7 +401,8 @@ class ShareManager(manager.SchedulerDependentManager): """ return self.driver.initialized - def ensure_driver_resources(self, ctxt): + def ensure_driver_resources(self, ctxt, skip_backend_info_check=False): + update_instances_status = CONF.update_shares_status_on_ensure old_backend_info = self.db.backend_info_get(ctxt, self.host) old_backend_info_hash = (old_backend_info.get('info_hash') if old_backend_info is not None else None) @@ -409,31 +410,33 @@ class ShareManager(manager.SchedulerDependentManager): new_backend_info_hash = None backend_info_implemented = True update_share_instances = [] - try: - new_backend_info = self.driver.get_backend_info(ctxt) - except Exception as e: - if not isinstance(e, NotImplementedError): - LOG.exception( - "The backend %(host)s could not get backend info.", - {'host': self.host}) - raise - else: - backend_info_implemented = False - LOG.debug( - ("The backend %(host)s does not support get backend" - " info method."), - {'host': self.host}) + if not skip_backend_info_check: + try: + new_backend_info = self.driver.get_backend_info(ctxt) + except Exception as e: + if not isinstance(e, NotImplementedError): + LOG.exception( + "The backend %(host)s could not get backend info.", + {'host': self.host}) + raise + else: + backend_info_implemented = False + LOG.debug( + ("The backend %(host)s does not support get backend" + " info method."), + {'host': self.host}) - if new_backend_info: - new_backend_info_hash = hashlib.sha1(str( - sorted(new_backend_info.items())).encode('utf-8')).hexdigest() - if (old_backend_info_hash == new_backend_info_hash and - backend_info_implemented): - LOG.debug( - ("Ensure shares is being skipped because the %(host)s's old " - "backend info is the same as its new backend info."), - {'host': self.host}) - return + if new_backend_info: + new_backend_info_hash = hashlib.sha1( + str(sorted(new_backend_info.items())).encode( + 'utf-8')).hexdigest() + if ((old_backend_info_hash == new_backend_info_hash and + backend_info_implemented) and not skip_backend_info_check): + LOG.debug( + ("Ensure shares is being skipped because the %(host)s's " + "old backend info is the same as its new backend info."), + {'host': self.host}) + return share_instances = self.db.share_instance_get_all_by_host( ctxt, self.host) @@ -467,7 +470,19 @@ class ShareManager(manager.SchedulerDependentManager): ctxt, share_instance) update_share_instances.append(share_instance_dict) + do_service_status_update = False if update_share_instances: + # No reason to update the shares status if nothing will be done. + do_service_status_update = True + service = self.db.service_get_by_args( + ctxt, self.host, 'manila-share') + self.db.service_update(ctxt, service['id'], {'ensuring': True}) + if update_instances_status: + for instance in update_share_instances: + self.db.share_instance_update( + ctxt, instance['id'], + {'status': constants.STATUS_ENSURING} + ) try: update_share_instances = self.driver.ensure_shares( ctxt, update_share_instances) or {} @@ -494,10 +509,11 @@ class ShareManager(manager.SchedulerDependentManager): share_instance_update_dict = ( update_share_instances[share_instance['id']] ) - if share_instance_update_dict.get('status'): + backend_provided_status = share_instance_update_dict.get('status') + if backend_provided_status: self.db.share_instance_update( ctxt, share_instance['id'], - {'status': share_instance_update_dict.get('status'), + {'status': backend_provided_status, 'host': share_instance['host']} ) metadata_updates = share_instance_update_dict.get('metadata') @@ -568,6 +584,13 @@ class ShareManager(manager.SchedulerDependentManager): "Unexpected error occurred while updating " "access rules for snapshot instance %s.", snap_instance['id']) + if not backend_provided_status and update_instances_status: + self.db.share_instance_update( + ctxt, share_instance['id'], + {'status': constants.STATUS_AVAILABLE} + ) + if do_service_status_update: + self.db.service_update(ctxt, service['id'], {'ensuring': False}) def _ensure_share(self, ctxt, share_instance): export_locations = None diff --git a/manila/share/rpcapi.py b/manila/share/rpcapi.py index c842fd2fbc..c1dbeb611c 100644 --- a/manila/share/rpcapi.py +++ b/manila/share/rpcapi.py @@ -89,6 +89,7 @@ class ShareAPI(object): restore_backup() methods 1.27 - Update delete_share_instance() and delete_snapshot() methods 1.28 - Add update_share_from_metadata() method + 1.29 - Add ensure_shares() """ BASE_RPC_API_VERSION = '1.0' @@ -97,7 +98,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.28') + self.client = rpc.get_client(target, version_cap='1.29') def create_share_instance(self, context, share_instance, host, request_spec, filter_properties, @@ -540,3 +541,12 @@ class ShareAPI(object): 'update_share_from_metadata', share_id=share['id'], metadata=metadata) + + def ensure_driver_resources(self, context, host): + host = utils.extract_host(host) + call_context = self.client.prepare(server=host, version='1.29') + return call_context.cast( + context, + 'ensure_driver_resources', + skip_backend_info_check=True + ) diff --git a/manila/tests/api/v2/test_services.py b/manila/tests/api/v2/test_services.py index 97141d637c..f1e03ac0ca 100644 --- a/manila/tests/api/v2/test_services.py +++ b/manila/tests/api/v2/test_services.py @@ -17,6 +17,7 @@ import datetime from unittest import mock +import webob import ddt from oslo_utils import timeutils @@ -158,6 +159,7 @@ fake_response_service_list_with_disabled_reason = {'services': [ 'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38), }, ]} +ENSURE_SHARES_VERSION = "2.86" def fake_service_get_all(context): @@ -386,3 +388,99 @@ class ServicesTest(test.TestCase): self.assertRaises( exception.VersionNotFoundForAPIMethod, controller().index, req) + + def test_ensure_shares_no_host_param(self): + req = fakes.HTTPRequest.blank( + '/fooproject/services/ensure', version=ENSURE_SHARES_VERSION) + body = {} + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller.ensure_shares, + req, + body + ) + + def test_ensure_shares_host_not_found(self): + req = fakes.HTTPRequest.blank( + '/fooproject/services/ensure', version=ENSURE_SHARES_VERSION) + req_context = req.environ['manila.context'] + body = {'host': 'host1'} + + mock_service_get = self.mock_object( + db, 'service_get_by_args', + mock.Mock(side_effect=exception.NotFound()) + ) + + self.assertRaises( + webob.exc.HTTPNotFound, + self.controller.ensure_shares, + req, + body + ) + mock_service_get.assert_called_once_with( + req_context, + body['host'], + 'manila-share' + ) + + def test_ensure_shares_conflict(self): + req = fakes.HTTPRequest.blank( + '/fooproject/services/ensure', version=ENSURE_SHARES_VERSION) + req_context = req.environ['manila.context'] + body = {'host': 'host1'} + fake_service = {'id': 'fake_service_id'} + + mock_service_get = self.mock_object( + db, + 'service_get_by_args', + mock.Mock(return_value=fake_service) + ) + mock_ensure = self.mock_object( + self.controller.service_api, + 'ensure_shares', + mock.Mock(side_effect=webob.exc.HTTPConflict) + ) + + self.assertRaises( + webob.exc.HTTPConflict, + self.controller.ensure_shares, + req, + body + ) + mock_service_get.assert_called_once_with( + req_context, + body['host'], + 'manila-share' + ) + mock_ensure.assert_called_once_with( + req_context, fake_service, body['host'] + ) + + def test_ensure_shares(self): + req = fakes.HTTPRequest.blank( + '/fooproject/services/ensure', version=ENSURE_SHARES_VERSION) + req_context = req.environ['manila.context'] + body = {'host': 'host1'} + fake_service = {'id': 'fake_service_id'} + + mock_service_get = self.mock_object( + db, + 'service_get_by_args', + mock.Mock(return_value=fake_service) + ) + mock_ensure = self.mock_object( + self.controller.service_api, 'ensure_shares', + ) + + response = self.controller.ensure_shares(req, body) + + self.assertEqual(202, response.status_int) + mock_service_get.assert_called_once_with( + req_context, + body['host'], + 'manila-share' + ) + mock_ensure.assert_called_once_with( + req_context, fake_service, body['host'] + ) diff --git a/manila/tests/services/__init__.py b/manila/tests/services/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/manila/tests/services/test_api.py b/manila/tests/services/test_api.py new file mode 100644 index 0000000000..7ea489e03c --- /dev/null +++ b/manila/tests/services/test_api.py @@ -0,0 +1,61 @@ +# 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 unittest import mock +from webob import exc + +from manila import context +from manila.services import api as service_api +from manila import test + + +class ServicesApiTest(test.TestCase): + + def setUp(self): + super(ServicesApiTest, self).setUp() + self.context = context.get_admin_context() + self.share_rpcapi = mock.Mock() + self.share_rpcapi.ensure_shares = mock.Mock() + self.services_api = service_api.API() + self.mock_object( + self.services_api, 'share_rpcapi', self.share_rpcapi + ) + + def test_ensure_shares(self): + host = 'fake_host@fakebackend' + fake_service = { + 'id': 'fake_service_id', + 'state': 'up' + } + + self.services_api.ensure_shares(self.context, fake_service, host) + + self.share_rpcapi.ensure_driver_resources.assert_called_once_with( + self.context, host + ) + + def test_ensure_shares_host_down(self): + host = 'fake_host@fakebackend' + fake_service = { + 'id': 'fake_service_id', + 'state': 'down' + } + + self.assertRaises( + exc.HTTPConflict, + self.services_api.ensure_shares, + self.context, + fake_service, + host + ) + + self.share_rpcapi.ensure_shares.assert_not_called() diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index cb0a033a45..a2d2380368 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -208,6 +208,11 @@ class ShareManagerTestCase(test.TestCase): 'reapply_access_rules': driver_needs_to_reapply_rules, }, } + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') mock_backend_info_update = self.mock_object( self.share_manager.db, 'backend_info_update') mock_share_get_all_by_host = self.mock_object( @@ -263,6 +268,23 @@ class ShareManagerTestCase(test.TestCase): mock.call(utils.IsAMatcher(context.RequestContext), instances[2]), ]) + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': False} + ) + ]) if driver_needs_to_reapply_rules: # don't care if share_instance['access_rules_status'] is "syncing" mock_reset_rules_method.assert_has_calls([ @@ -301,6 +323,11 @@ class ShareManagerTestCase(test.TestCase): 'metadata': metadata_updates, }, } + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') mock_backend_info_update = self.mock_object( self.share_manager.db, 'backend_info_update') mock_share_get_all_by_host = self.mock_object( @@ -359,6 +386,23 @@ class ShareManagerTestCase(test.TestCase): ]) # none of the share instances in the fake data have syncing rules mock_reset_rules_method.assert_not_called() + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': False} + ) + ]) def test_init_host_with_no_shares(self): self.mock_object(self.share_manager.db, @@ -520,6 +564,7 @@ class ShareManagerTestCase(test.TestCase): } instances[0]['access_rules_status'] = '' instances[2]['access_rules_status'] = '' + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} self.mock_object(self.share_manager.db, 'backend_info_get', mock.Mock(return_value=old_backend_info)) @@ -530,6 +575,10 @@ class ShareManagerTestCase(test.TestCase): mock_share_get_all_by_host = self.mock_object( self.share_manager.db, 'share_instance_get_all_by_host', mock.Mock(return_value=instances)) + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') self.mock_object(self.share_manager.db, 'share_instance_get', mock.Mock(side_effect=[instances[0], instances[2], instances[4]])) @@ -607,6 +656,23 @@ class ShareManagerTestCase(test.TestCase): mock.call(mock.ANY, instances[2]['id'], share_server=share_server), ])) + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': False} + ) + ]) @ddt.data(("some_hash", {"db_version": "test_version"}), ("ddd86ec90923b686597501e2f2431f3af59238c0", @@ -623,6 +689,7 @@ class ShareManagerTestCase(test.TestCase): new_backend_info else None) mock_backend_info_update = self.mock_object( self.share_manager.db, 'backend_info_update') + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} self.mock_object( self.share_manager.db, 'backend_info_get', mock.Mock(return_value=old_backend_info)) @@ -630,6 +697,10 @@ class ShareManagerTestCase(test.TestCase): mock.Mock(return_value=new_backend_info)) self.mock_object(self.share_manager, 'publish_service_capabilities', mock.Mock()) + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') mock_ensure_shares = self.mock_object( self.share_manager.driver, 'ensure_shares') mock_share_instance_get_all_by_host = self.mock_object( @@ -665,6 +736,7 @@ class ShareManagerTestCase(test.TestCase): instances = self._setup_init_mocks(setup_access_rules=False) share_server = fakes.fake_share_server_get() + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} self.mock_object(self.share_manager.db, 'share_instance_get_all_by_host', mock.Mock(return_value=instances)) @@ -683,6 +755,10 @@ class ShareManagerTestCase(test.TestCase): self.mock_object(self.share_manager, '_get_share_server_dict', mock.Mock(return_value=share_server)) self.mock_object(self.share_manager, 'publish_service_capabilities') + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') self.mock_object(manager.LOG, 'error') self.mock_object(manager.LOG, 'info') @@ -730,6 +806,23 @@ class ShareManagerTestCase(test.TestCase): mock.ANY, {'id': instances[1]['id'], 'status': instances[1]['status']}, ) + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': False} + ) + ]) def _get_share_instance_dict(self, share_instance, **kwargs): # TODO(gouthamr): remove method when the db layer returns primitives @@ -775,11 +868,16 @@ class ShareManagerTestCase(test.TestCase): raise exception.ManilaException(message="Fake raise") instances = self._setup_init_mocks(setup_access_rules=False) + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} mock_ensure_share = self.mock_object( self.share_manager.driver, 'ensure_share') self.mock_object(self.share_manager.db, 'share_instance_get_all_by_host', mock.Mock(return_value=instances)) + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') self.mock_object(self.share_manager.db, 'share_instance_get', mock.Mock(side_effect=[instances[0], instances[2], instances[3]])) @@ -808,6 +906,20 @@ class ShareManagerTestCase(test.TestCase): mock.call(utils.IsAMatcher(context.RequestContext), instances[0]), mock.call(utils.IsAMatcher(context.RequestContext), instances[2]), ]) + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), fake_service['id'], + {'ensuring': False} + ) + ]) self.share_manager.driver.ensure_shares.assert_called_once_with( utils.IsAMatcher(context.RequestContext), [dict_instances[0], dict_instances[2], dict_instances[3]]) @@ -854,11 +966,16 @@ class ShareManagerTestCase(test.TestCase): instances[4]['id']: {'status': 'available'} } smanager = self.share_manager + fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'} self.mock_object(smanager.db, 'share_instance_get_all_by_host', mock.Mock(return_value=instances)) self.mock_object(self.share_manager.db, 'share_instance_get', mock.Mock(side_effect=[instances[0], instances[2], instances[4]])) + self.mock_object(self.share_manager.db, + 'service_get_by_args', + mock.Mock(return_value=fake_service)) + self.mock_object(self.share_manager.db, 'service_update') self.mock_object(self.share_manager.driver, 'ensure_share', mock.Mock(return_value=None)) self.mock_object(self.share_manager.driver, 'ensure_shares', @@ -915,6 +1032,23 @@ class ShareManagerTestCase(test.TestCase): manager.LOG.exception.assert_has_calls([ mock.call(mock.ANY, mock.ANY), ]) + self.share_manager.db.service_get_by_args.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + self.share_manager.host, + 'manila-share' + ) + self.share_manager.db.service_update.assert_has_calls([ + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': True} + ), + mock.call( + utils.IsAMatcher(context.RequestContext), + fake_service['id'], + {'ensuring': False} + ) + ]) def test_create_share_instance_from_snapshot_with_server(self): """Test share can be created from snapshot if server exists.""" diff --git a/manila/tests/share/test_rpcapi.py b/manila/tests/share/test_rpcapi.py index cef55344ad..52fa1ecf45 100644 --- a/manila/tests/share/test_rpcapi.py +++ b/manila/tests/share/test_rpcapi.py @@ -147,6 +147,8 @@ class ShareRpcAPITestCase(test.TestCase): and method in src_dest_share_server_methods): share_server = expected_msg.pop('dest_share_server', None) expected_msg['dest_share_server_id'] = share_server['id'] + if method == 'ensure_driver_resources': + expected_msg['skip_backend_info_check'] = True if 'host' in kwargs: host = kwargs['host'] @@ -527,3 +529,11 @@ class ShareRpcAPITestCase(test.TestCase): dest_host=self.fake_host, share_network_id='fake_net_id', new_share_network_subnet_id='new_share_network_subnet_id') + + def test_ensure_driver_resources(self): + self._test_share_api( + 'ensure_driver_resources', + rpc_method='cast', + version='1.29', + host=self.fake_host, + ) diff --git a/releasenotes/notes/add-ensure-shares-api-9ac10877a99ab0c5.yaml b/releasenotes/notes/add-ensure-shares-api-9ac10877a99ab0c5.yaml new file mode 100644 index 0000000000..0ff0bb93e0 --- /dev/null +++ b/releasenotes/notes/add-ensure-shares-api-9ac10877a99ab0c5.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + A new API to start the ensure shares procedure for Manila has been added. + Through this API, OpenStack administrators will be able to recalculate the + shares' export location without restarting the shares manager service. + Additionally, a new configuration option named + `update_shares_status_on_ensure` is now available to help OpenStack + administrators determine whether the shares' status should be modified + during the ensure shares procedure or not. +upgrade: + - | + When restarting the service on an upgrade, when ensure shares is being run + it will automatically transition the shares status to `ensuring`. In case + you would like to prevent it, please change the value of the + `update_shares_status_on_ensure` configuration option.