diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index c68e810bc7..506a632e02 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -129,13 +129,15 @@ REST_API_VERSION_HISTORY = """ version: GET /v2/{tenant_id}/share-replicas/{ replica_id}/export-locations to allow retrieving individual replica export locations if available. + * 2.48 - Added support for extra-spec "availability_zones" within Share + types along with validation in the 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.47" +_MAX_API_VERSION = "2.48" 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 a82fd356f5..62313ead0f 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -256,7 +256,6 @@ user documentation. 2.47 ---- - Export locations for non-active share replicas are no longer retrievable through the export locations APIs: ``GET /v2/{tenant_id}/shares/{share_id}/export_locations`` and ``GET @@ -264,3 +263,10 @@ user documentation. A new API is introduced at this version: ``GET /v2/{tenant_id}/share-replicas/{replica_id}/export-locations`` to allow retrieving export locations of share replicas if available. + +2.48 +---- + Administrators can now use the common, user-visible extra-spec + '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. diff --git a/manila/api/v1/share_types_extra_specs.py b/manila/api/v1/share_types_extra_specs.py index ea66d1a30b..f593826e56 100644 --- a/manila/api/v1/share_types_extra_specs.py +++ b/manila/api/v1/share_types_extra_specs.py @@ -99,6 +99,7 @@ class ShareTypeExtraSpecsController(wsgi.Controller): raise webob.exc.HTTPBadRequest(e.message) self._check_key_names(specs.keys()) + specs = share_types.sanitize_extra_specs(specs) db.share_type_extra_specs_update_or_create(context, type_id, specs) notifier_info = dict(type_id=type_id, specs=specs) notifier = rpc.get_notifier('shareTypeExtraSpecs') @@ -119,11 +120,12 @@ class ShareTypeExtraSpecsController(wsgi.Controller): expl = _('Request body contains too many items') raise webob.exc.HTTPBadRequest(explanation=expl) self._verify_extra_specs(body, False) - db.share_type_extra_specs_update_or_create(context, type_id, body) + specs = share_types.sanitize_extra_specs(body) + db.share_type_extra_specs_update_or_create(context, type_id, specs) notifier_info = dict(type_id=type_id, id=id) notifier = rpc.get_notifier('shareTypeExtraSpecs') notifier.info(context, 'share_type_extra_specs.update', notifier_info) - return body + return specs @wsgi.Controller.authorize def show(self, req, type_id, id): diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index ed713ed242..2c859289f2 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -233,7 +233,8 @@ class ShareMixin(object): return share def _create(self, req, body, - check_create_share_from_snapshot_support=False): + check_create_share_from_snapshot_support=False, + check_availability_zones_extra_spec=False): """Creates a new share.""" context = req.environ['manila.context'] @@ -300,6 +301,7 @@ class ShareMixin(object): share_network_id = share.get('share_network_id') + parent_share_type = {} if snapshot: # Need to check that share_network_id from snapshot's # parents share equals to share_network_id from args. @@ -307,6 +309,8 @@ class ShareMixin(object): # share_network_id of parent share. parent_share = self.share_api.get(context, snapshot['share_id']) parent_share_net_id = parent_share.instance['share_network_id'] + parent_share_type = share_types.get_share_type( + context, parent_share.instance['share_type_id']) if share_network_id: if share_network_id != parent_share_net_id: msg = ("Share network ID should be the same as snapshot's" @@ -371,6 +375,24 @@ class ShareMixin(object): 'driver_handles_share_servers is true.') raise exc.HTTPBadRequest(explanation=msg) + type_chosen = share_type or parent_share_type + if type_chosen and check_availability_zones_extra_spec: + type_azs = type_chosen.get( + 'extra_specs', {}).get('availability_zones', '') + type_azs = type_azs.split(',') if type_azs else [] + kwargs['availability_zones'] = type_azs + if (availability_zone and type_azs and + availability_zone not in type_azs): + msg = _("Share type %(type)s is not supported within the " + "availability zone chosen %(az)s.") + type_chosen = ( + req_share_type or "%s (from source snapshot)" % ( + parent_share_type.get('name') or + parent_share_type.get('id')) + ) + payload = {'type': type_chosen, 'az': availability_zone} + raise exc.HTTPBadRequest(explanation=msg % payload) + if share_type: kwargs['share_type'] = share_type new_share = self.share_api.create(context, diff --git a/manila/api/v2/share_groups.py b/manila/api/v2/share_groups.py index 8aefa59709..fd279b7061 100644 --- a/manila/api/v2/share_groups.py +++ b/manila/api/v2/share_groups.py @@ -212,8 +212,9 @@ class ShareGroupController(wsgi.Controller, wsgi.AdminActionsMixin): "availability zone is inherited from the source.") raise exc.HTTPBadRequest(explanation=msg) try: - az_id = db.availability_zone_get(context, availability_zone).id - kwargs['availability_zone_id'] = az_id + az = db.availability_zone_get(context, availability_zone) + kwargs['availability_zone_id'] = az.id + kwargs['availability_zone'] = az.name except exception.AvailabilityZoneNotFound as e: raise exc.HTTPNotFound(explanation=six.text_type(e)) diff --git a/manila/api/v2/share_types.py b/manila/api/v2/share_types.py index a9ced209d3..ffcfb0303f 100644 --- a/manila/api/v2/share_types.py +++ b/manila/api/v2/share_types.py @@ -126,17 +126,19 @@ class ShareTypesController(wsgi.Controller): else: filters['is_public'] = True - if (req.api_version_request < api_version.APIVersionRequest("2.43")): - extra_specs = req.params.get('extra_specs') - if extra_specs: - msg = _("Filter by 'extra_specs' is not supported by this " - "microversion. Use 2.43 or greater microversion to " - "be able to use filter search by 'extra_specs.") - raise webob.exc.HTTPBadRequest(explanation=msg) - else: - extra_specs = req.params.get('extra_specs') - if extra_specs: - filters['extra_specs'] = ast.literal_eval(extra_specs) + extra_specs = req.params.get('extra_specs', {}) + extra_specs_disallowed = (req.api_version_request < + api_version.APIVersionRequest("2.43")) + + if extra_specs and extra_specs_disallowed: + msg = _("Filter by 'extra_specs' is not supported by this " + "microversion. Use 2.43 or greater microversion to " + "be able to use filter search by 'extra_specs.") + raise webob.exc.HTTPBadRequest(explanation=msg) + elif extra_specs: + extra_specs = ast.literal_eval(extra_specs) + filters['extra_specs'] = share_types.sanitize_extra_specs( + extra_specs) limited_types = share_types.get_all_types( context, search_opts=filters).values() diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index a50ed68064..5e8f7304ff 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -178,8 +178,14 @@ class ShareController(shares.ShareMixin, return data - @wsgi.Controller.api_version("2.31") + @wsgi.Controller.api_version("2.48") def create(self, req, body): + return self._create(req, body, + check_create_share_from_snapshot_support=True, + check_availability_zones_extra_spec=True) + + @wsgi.Controller.api_version("2.31", "2.47") # noqa + def create(self, req, body): # pylint: disable=E0102 return self._create( req, body, check_create_share_from_snapshot_support=True) diff --git a/manila/common/constants.py b/manila/common/constants.py index c826832a7d..ec331f54e2 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -196,6 +196,7 @@ class ExtraSpecs(object): CREATE_SHARE_FROM_SNAPSHOT_SUPPORT = "create_share_from_snapshot_support" REVERT_TO_SNAPSHOT_SUPPORT = "revert_to_snapshot_support" MOUNT_SNAPSHOT_SUPPORT = "mount_snapshot_support" + AVAILABILITY_ZONES = "availability_zones" # Extra specs containers REQUIRED = ( @@ -208,6 +209,7 @@ class ExtraSpecs(object): REVERT_TO_SNAPSHOT_SUPPORT, REPLICATION_TYPE_SPEC, MOUNT_SNAPSHOT_SUPPORT, + AVAILABILITY_ZONES, ) # NOTE(cknight): Some extra specs are necessary parts of the Manila API and diff --git a/manila/scheduler/filters/availability_zone.py b/manila/scheduler/filters/availability_zone.py index f18d6e4995..21991a34bc 100644 --- a/manila/scheduler/filters/availability_zone.py +++ b/manila/scheduler/filters/availability_zone.py @@ -25,10 +25,17 @@ class AvailabilityZoneFilter(base_host.BaseHostFilter): def host_passes(self, host_state, filter_properties): spec = filter_properties.get('request_spec', {}) props = spec.get('resource_properties', {}) - availability_zone_id = props.get( - 'availability_zone_id', spec.get('availability_zone_id')) + request_az_id = props.get('availability_zone_id', + spec.get('availability_zone_id')) + request_azs = spec.get('availability_zones') + host_az_id = host_state.service['availability_zone_id'] + host_az = host_state.service['availability_zone']['name'] - if availability_zone_id: - return (availability_zone_id == - host_state.service['availability_zone_id']) - return True + host_satisfied = True + if request_az_id is not None: + host_satisfied = request_az_id == host_az_id + + if request_azs: + host_satisfied = host_satisfied and host_az in request_azs + + return host_satisfied diff --git a/manila/scheduler/utils.py b/manila/scheduler/utils.py index afc6622223..481117531b 100644 --- a/manila/scheduler/utils.py +++ b/manila/scheduler/utils.py @@ -129,7 +129,16 @@ def thin_provisioning(host_state_thin_provisioning): def capabilities_satisfied(capabilities, extra_specs): + # These extra-specs are not capabilities for matching hosts + ignored_extra_specs = ( + 'availability_zones', 'capabilities:availability_zones', + ) + for key, req in extra_specs.items(): + # Ignore some extra_specs if told to + if key in ignored_extra_specs: + continue + # Either not scoped format, or in capabilities scope scope = key.split(':') diff --git a/manila/share/api.py b/manila/share/api.py index 68692194e8..bb4c8076f5 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -69,7 +69,8 @@ class API(base.Base): def create(self, context, share_proto, size, name, description, snapshot_id=None, availability_zone=None, metadata=None, share_network_id=None, share_type=None, is_public=False, - share_group_id=None, share_group_snapshot_member=None): + share_group_id=None, share_group_snapshot_member=None, + availability_zones=None): """Create new share.""" policy.check_policy(context, 'share', 'create') @@ -257,7 +258,7 @@ class API(base.Base): context, share, share_network_id=share_network_id, host=host, availability_zone=availability_zone, share_group=share_group, share_group_snapshot_member=share_group_snapshot_member, - share_type_id=share_type_id) + share_type_id=share_type_id, availability_zones=availability_zones) # Retrieve the share with instance details share = self.db.share_get(context, share['id']) @@ -333,7 +334,7 @@ class API(base.Base): def create_instance(self, context, share, share_network_id=None, host=None, availability_zone=None, share_group=None, share_group_snapshot_member=None, - share_type_id=None): + share_type_id=None, availability_zones=None): policy.check_policy(context, 'share', 'create') request_spec, share_instance = ( @@ -341,7 +342,8 @@ class API(base.Base): context, share, availability_zone=availability_zone, share_group=share_group, host=host, share_network_id=share_network_id, - share_type_id=share_type_id)) + share_type_id=share_type_id, + availability_zones=availability_zones)) if share_group_snapshot_member: # Inherit properties from the share_group_snapshot_member @@ -379,7 +381,8 @@ class API(base.Base): def create_share_instance_and_get_request_spec( self, context, share, availability_zone=None, share_group=None, host=None, share_network_id=None, - share_type_id=None, cast_rules_to_readonly=False): + share_type_id=None, cast_rules_to_readonly=False, + availability_zones=None): availability_zone_id = None if availability_zone: @@ -449,6 +452,7 @@ class API(base.Base): 'share_type': share_type, 'share_group': share_group, 'availability_zone_id': availability_zone_id, + 'availability_zones': availability_zones, } return request_spec, share_instance @@ -473,6 +477,21 @@ class API(base.Base): "state.") raise exception.ReplicationException(reason=msg % share['id']) + share_type = share_types.get_share_type( + context, share.instance['share_type_id']) + type_azs = share_type['extra_specs'].get('availability_zones', '') + type_azs = [t for t in type_azs.split(',') if type_azs] + if (availability_zone and type_azs and + availability_zone not in type_azs): + msg = _("Share replica cannot be created since the share type " + "%(type)s is not supported within the availability zone " + "chosen %(az)s.") + type_name = '%s' % (share_type['name'] or '') + type_id = '(ID: %s)' % share_type['id'] + payload = {'type': '%s%s' % (type_name, type_id), + 'az': availability_zone} + raise exception.InvalidShare(message=msg % payload) + if share['replication_type'] == constants.REPLICATION_TYPE_READABLE: cast_rules_to_readonly = True else: @@ -483,7 +502,9 @@ class API(base.Base): context, share, availability_zone=availability_zone, share_network_id=share_network_id, share_type_id=share['instance']['share_type_id'], - cast_rules_to_readonly=cast_rules_to_readonly)) + cast_rules_to_readonly=cast_rules_to_readonly, + availability_zones=type_azs) + ) all_replicas = self.db.share_replicas_get_all_by_share( context, share['id']) @@ -1197,6 +1218,20 @@ class API(base.Base): service = self.db.service_get_by_args( context, dest_host_host, 'manila-share') + type_azs = share_type['extra_specs'].get('availability_zones', '') + type_azs = [t for t in type_azs.split(',') if type_azs] + if type_azs and service['availability_zone']['name'] not in type_azs: + msg = _("Share %(shr)s cannot be migrated to host %(dest)s " + "because share type %(type)s is not supported within the " + "availability zone (%(az)s) that the host is in.") + type_name = '%s' % (share_type['name'] or '') + type_id = '(ID: %s)' % share_type['id'] + payload = {'type': '%s%s' % (type_name, type_id), + 'az': service['availability_zone']['name'], + 'shr': share['id'], + 'dest': dest_host} + raise exception.InvalidShare(reason=msg % payload) + request_spec = self._get_request_spec_dict( share, share_type, diff --git a/manila/share/share_types.py b/manila/share/share_types.py index a62935a83f..4003892b8d 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -45,6 +45,9 @@ def create(context, name, extra_specs=None, is_public=True, get_valid_optional_extra_specs(extra_specs) except exception.InvalidExtraSpec as e: raise exception.InvalidShareType(reason=six.text_type(e)) + + extra_specs = sanitize_extra_specs(extra_specs) + try: type_ref = db.share_type_create(context, dict(name=name, @@ -59,6 +62,14 @@ def create(context, name, extra_specs=None, is_public=True, return type_ref +def sanitize_extra_specs(extra_specs): + """Post process extra specs here if necessary""" + az_spec = constants.ExtraSpecs.AVAILABILITY_ZONES + if az_spec in extra_specs: + extra_specs[az_spec] = sanitize_csv(extra_specs[az_spec]) + return extra_specs + + def destroy(context, id): """Marks share types as deleted.""" if id is None: @@ -92,39 +103,61 @@ def get_all_types(context, inactive=0, search_opts=None): type_args['required_extra_specs'] = required_extra_specs search_vars = {} - if 'extra_specs' in search_opts: - search_vars['extra_specs'] = search_opts.pop('extra_specs') + availability_zones = search_opts.get('extra_specs', {}).pop( + 'availability_zones', None) + extra_specs = search_opts.pop('extra_specs', {}) + + if extra_specs: + search_vars['extra_specs'] = extra_specs + + if availability_zones: + search_vars['availability_zones'] = availability_zones.split(',') if search_opts: + # No other search options are currently supported return {} - elif search_vars: - LOG.debug("Searching by: %s", search_vars) + elif not search_vars: + return share_types - def _check_extra_specs_match(share_type, searchdict): - for k, v in searchdict.items(): - if (k not in share_type['extra_specs'].keys() - or share_type['extra_specs'][k] != v): - return False + LOG.debug("Searching by: %s", search_vars) + + def _check_extra_specs_match(share_type, searchdict): + for k, v in searchdict.items(): + if (k not in share_type['extra_specs'].keys() + or share_type['extra_specs'][k] != v): + return False return True - # search_option to filter_name mapping. - filter_mapping = {'extra_specs': _check_extra_specs_match} + def _check_availability_zones_match(share_type, availability_zones): + type_azs = share_type['extra_specs'].get('availability_zones') + if type_azs: + type_azs = type_azs.split(',') + return set(availability_zones).issubset(set(type_azs)) + return True - result = {} - for type_name, type_args in share_types.items(): - # go over all filters in the list - for opt, values in search_vars.items(): - try: - filter_func = filter_mapping[opt] - except KeyError: - # no such filter - ignore it, go to next filter - continue - else: - if filter_func(type_args, values): - result[type_name] = type_args - break - share_types = result - return share_types + # search_option to filter_name mapping. + filter_mapping = { + 'extra_specs': _check_extra_specs_match, + 'availability_zones': _check_availability_zones_match, + } + + result = {} + for type_name, type_args in share_types.items(): + # go over all filters in the list (*AND* operation) + type_matches = True + for opt, value in search_vars.items(): + try: + filter_func = filter_mapping[opt] + except KeyError: + # no such filter - ignore it, go to next filter + continue + else: + if not filter_func(type_args, value): + type_matches = False + break + if type_matches: + result[type_name] = type_args + return result def get_share_type(ctxt, id, expected_fields=None): @@ -264,6 +297,18 @@ def get_valid_required_extra_specs(extra_specs): return required_extra_specs +def is_valid_csv(extra_spec_value): + if not isinstance(extra_spec_value, six.string_types): + extra_spec_value = six.text_type(extra_spec_value) + values = extra_spec_value.split(',') + return all([v.strip() for v in values]) + + +def sanitize_csv(csv_string): + return ','.join(value.strip() for value in csv_string.split(',') + if (csv_string and value)) + + def is_valid_optional_extra_spec(key, value): """Validates optional but standardized extra_spec value. @@ -285,6 +330,8 @@ def is_valid_optional_extra_spec(key, value): return value in constants.ExtraSpecs.REPLICATION_TYPES elif key == constants.ExtraSpecs.MOUNT_SNAPSHOT_SUPPORT: return parse_boolean_extra_spec(key, value) is not None + elif key == constants.ExtraSpecs.AVAILABILITY_ZONES: + return is_valid_csv(value) return False diff --git a/manila/share_group/api.py b/manila/share_group/api.py index a91b9f7a35..e502e645dc 100644 --- a/manila/share_group/api.py +++ b/manila/share_group/api.py @@ -50,7 +50,7 @@ class API(base.Base): def create(self, context, name=None, description=None, share_type_ids=None, source_share_group_snapshot_id=None, share_network_id=None, share_group_type_id=None, - availability_zone_id=None): + availability_zone_id=None, availability_zone=None): """Create new share group.""" share_group_snapshot = None @@ -132,12 +132,46 @@ class API(base.Base): supported_share_types = set( [x['share_type_id'] for x in share_group_type['share_types']]) + supported_share_type_objects = [ + share_types.get_share_type(context, share_type_id) for + share_type_id in supported_share_types + ] if not set(share_type_ids or []) <= supported_share_types: msg = _("The specified share types must be a subset of the share " "types supported by the share group type.") raise exception.InvalidInput(reason=msg) + # Grab share type AZs for scheduling + share_types_of_new_group = ( + share_type_objects or supported_share_type_objects + ) + stype_azs_of_new_group = [] + stypes_unsupported_in_az = [] + for stype in share_types_of_new_group: + stype_azs = stype.get('extra_specs', {}).get( + 'availability_zones', '') + if stype_azs: + stype_azs = stype_azs.split(',') + stype_azs_of_new_group.extend(stype_azs) + if availability_zone and availability_zone not in stype_azs: + # If an AZ is requested, it must be supported by the AZs + # configured in each of the share types requested + stypes_unsupported_in_az.append((stype['name'], + stype['id'])) + + if stypes_unsupported_in_az: + msg = _("Share group cannot be created since the following share " + "types are not supported within the availability zone " + "'%(az)s': (%(stypes)s)") + payload = {'az': availability_zone, 'stypes': ''} + for type_name, type_id in set(stypes_unsupported_in_az): + if payload['stypes']: + payload['stypes'] += ', ' + type_name = '%s ' % (type_name or '') + payload['stypes'] += type_name + '(ID: %s)' % type_id + raise exception.InvalidInput(reason=msg % payload) + try: reservations = QUOTAS.reserve(context, share_groups=1) except exception.OverQuota as e: @@ -213,6 +247,7 @@ class API(base.Base): request_spec = {'share_group_id': share_group['id']} request_spec.update(options) + request_spec['availability_zones'] = set(stype_azs_of_new_group) request_spec['share_types'] = share_type_objects request_spec['resource_type'] = share_group_type diff --git a/manila/tests/api/contrib/stubs.py b/manila/tests/api/contrib/stubs.py index 7de2782fff..d375e67498 100644 --- a/manila/tests/api/contrib/stubs.py +++ b/manila/tests/api/contrib/stubs.py @@ -122,6 +122,21 @@ def stub_snapshot(id, **kwargs): return snapshot +def stub_share_type(id, **kwargs): + share_type = { + 'id': id, + 'name': 'fakesharetype', + 'description': 'fakesharetypedescription', + 'is_public': True, + } + share_type.update(kwargs) + return share_type + + +def stub_share_type_get(context, share_type_id, **kwargs): + return stub_share_type(share_type_id, **kwargs) + + def stub_share_get(self, context, share_id, **kwargs): return stub_share(share_id, **kwargs) diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index 837de3bd3c..d471e94f1e 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -57,6 +57,8 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'delete', stubs.stub_share_delete) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + self.mock_object(share_types, 'get_share_type', + stubs.stub_share_type_get) self.maxDiff = None self.share = { "size": 100, diff --git a/manila/tests/api/v2/test_share_groups.py b/manila/tests/api/v2/test_share_groups.py index 90d351bafa..1776b91095 100644 --- a/manila/tests/api/v2/test_share_groups.py +++ b/manila/tests/api/v2/test_share_groups.py @@ -263,6 +263,7 @@ class ShareGroupAPITest(test.TestCase): self.controller.share_group_api.create.assert_called_once_with( self.context, availability_zone_id=fake_az_id, + availability_zone=fake_az_name, share_group_type_id=self.fake_share_group_type['id'], share_type_ids=[self.fake_share_type['id']]) share_groups.db.availability_zone_get.assert_called_once_with( diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index df8cbe1451..d827beb6ab 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -21,6 +21,7 @@ import ddt import mock from oslo_config import cfg from oslo_serialization import jsonutils +from oslo_utils import uuidutils import six import webob import webob.exc @@ -41,10 +42,13 @@ from manila.tests.api.contrib import stubs from manila.tests.api import fakes from manila.tests import db_utils from manila.tests import fake_share +from manila.tests import utils as test_utils from manila import utils CONF = cfg.CONF +LATEST_MICROVERSION = api_version._MAX_API_VERSION + @ddt.ddt class ShareAPITest(test.TestCase): @@ -62,6 +66,8 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'delete', stubs.stub_share_delete) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + self.mock_object(share_types, 'get_share_type', + stubs.stub_share_type_get) self.maxDiff = None self.share = { "id": "1", @@ -102,7 +108,40 @@ class ShareAPITest(test.TestCase): CONF.set_default("default_share_type", None) - def _get_expected_share_detailed_response(self, values=None, admin=False): + def _process_expected_share_detailed_response(self, shr_dict, req_version): + """Sets version based parameters on share dictionary.""" + + share_dict = copy.deepcopy(shr_dict) + changed_parameters = { + '2.2': {'snapshot_support': True}, + '2.5': {'task_state': None}, + '2.6': {'share_type_name': None}, + '2.10': {'access_rules_status': constants.ACCESS_STATE_ACTIVE}, + '2.11': {'replication_type': None, 'has_replicas': False}, + '2.16': {'user_id': 'fakeuser'}, + '2.24': {'create_share_from_snapshot_support': True}, + '2.27': {'revert_to_snapshot_support': False}, + '2.31': {'share_group_id': None, + 'source_share_group_snapshot_member_id': None}, + '2.32': {'mount_snapshot_support': False}, + } + + # Apply all the share transformations + if self.is_microversion_ge(req_version, '2.9'): + share_dict.pop('export_locations', None) + share_dict.pop('export_location', None) + + for version, parameters in changed_parameters.items(): + for param, default in parameters.items(): + if self.is_microversion_ge(req_version, version): + share_dict[param] = share_dict.get(param, default) + else: + share_dict.pop(param, None) + + return share_dict + + def _get_expected_share_detailed_response(self, values=None, + admin=False, version='2.0'): share = { 'id': '1', 'name': 'displayname', @@ -146,7 +185,10 @@ class ShareAPITest(test.TestCase): if admin: share['share_server_id'] = 'fake_share_server_id' share['host'] = 'fakehost' - return {'share': share} + return { + 'share': self._process_expected_share_detailed_response( + share, version) + } def test__revert(self): @@ -445,10 +487,8 @@ class ShareAPITest(test.TestCase): res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) - expected['share'].pop('snapshot_support') - expected['share'].pop('share_type_name') - expected['share'].pop('task_state') + expected = self._get_expected_share_detailed_response( + self.share, version=microversion) self.assertEqual(expected, res_dict) @ddt.data("2.2", "2.3") @@ -459,31 +499,20 @@ class ShareAPITest(test.TestCase): res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) - expected['share'].pop('share_type_name') - expected['share'].pop('task_state') + expected = self._get_expected_share_detailed_response( + self.share, version=microversion) self.assertEqual(expected, res_dict) - @ddt.data("2.31") - def test_share_create_with_share_group(self, microversion): + def test_share_create_with_share_group(self): self.mock_object(share_api.API, 'create', self.create_mock) body = {"share": copy.deepcopy(self.share)} - req = fakes.HTTPRequest.blank('/shares', version=microversion, + req = fakes.HTTPRequest.blank('/shares', version="2.31", experimental=True) res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) - expected['share'].pop('export_location') - expected['share'].pop('export_locations') - expected['share']['access_rules_status'] = 'active' - expected['share']['replication_type'] = None - expected['share']['has_replicas'] = False - expected['share']['user_id'] = 'fakeuser' - expected['share']['create_share_from_snapshot_support'] = True - expected['share']['revert_to_snapshot_support'] = False - expected['share']['share_group_id'] = None - expected['share']['source_share_group_snapshot_member_id'] = None + expected = self._get_expected_share_detailed_response( + self.share, version="2.31") self.assertEqual(expected, res_dict) def test_share_create_with_sg_and_availability_zone(self): @@ -586,7 +615,8 @@ class ShareAPITest(test.TestCase): req = fakes.HTTPRequest.blank('/shares', version='2.7') res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) + expected = self._get_expected_share_detailed_response(self.share, + version='2.7') share_types.get_share_type_by_name.assert_called_once_with( utils.IsAMatcher(context.RequestContext), self.vt['name']) self.assertEqual(expected, res_dict) @@ -612,15 +642,8 @@ class ShareAPITest(test.TestCase): res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) - - expected['share']['task_state'] = None - expected['share']['replication_type'] = None - expected['share']['share_type_name'] = None - expected['share']['has_replicas'] = False - expected['share']['access_rules_status'] = 'active' - expected['share'].pop('export_location') - expected['share'].pop('export_locations') + expected = self._get_expected_share_detailed_response( + self.share, version=share_replicas.MIN_SUPPORTED_API_VERSION) self.assertEqual(expected, res_dict) @@ -648,7 +671,8 @@ class ShareAPITest(test.TestCase): req = fakes.HTTPRequest.blank('/shares', version='2.7') res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(shr) + expected = self._get_expected_share_detailed_response( + shr, version='2.7') self.assertDictMatch(expected, res_dict) self.assertEqual("fakenetid", create_mock.call_args[1]['share_network_id']) @@ -661,22 +685,67 @@ class ShareAPITest(test.TestCase): res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(self.share) - if api_version.APIVersionRequest(microversion) >= ( - api_version.APIVersionRequest("2.16")): - expected['share']['user_id'] = 'fakeuser' - else: - self.assertNotIn('user_id', expected['share']) - expected['share']['task_state'] = None - expected['share']['replication_type'] = None - expected['share']['share_type_name'] = None - expected['share']['has_replicas'] = False - expected['share']['access_rules_status'] = 'active' - expected['share'].pop('export_location') - expected['share'].pop('export_locations') + expected = self._get_expected_share_detailed_response( + self.share, version=microversion) self.assertEqual(expected, res_dict) + @ddt.data(test_utils.annotated('v2.0_az_unsupported', ('2.0', False)), + test_utils.annotated('v2.0_az_supported', ('2.0', True)), + test_utils.annotated('v2.47_az_unsupported', ('2.47', False)), + test_utils.annotated('v2.47_az_supported', ('2.47', True))) + @ddt.unpack + def test_share_create_with_share_type_azs(self, version, az_supported): + """For API version<2.48, AZ validation should not be performed.""" + self.mock_object(share_api.API, 'create', self.create_mock) + create_args = copy.deepcopy(self.share) + create_args['availability_zone'] = 'az1' if az_supported else 'az2' + create_args['share_type'] = uuidutils.generate_uuid() + stype_with_azs = copy.deepcopy(self.vt) + stype_with_azs['extra_specs']['availability_zones'] = 'az1,az3' + self.mock_object(share_types, 'get_share_type', mock.Mock( + return_value=stype_with_azs)) + + req = fakes.HTTPRequest.blank('/shares', version=version) + + res_dict = self.controller.create(req, {'share': create_args}) + + expected = self._get_expected_share_detailed_response( + values=self.share, version=version) + + self.assertEqual(expected, res_dict) + + @ddt.data(*set([ + test_utils.annotated('v2.48_share_from_snap', ('2.48', True)), + test_utils.annotated('v2.48_share_not_from_snap', ('2.48', False)), + test_utils.annotated('v%s_share_from_snap' % LATEST_MICROVERSION, + (LATEST_MICROVERSION, True)), + test_utils.annotated('v%s_share_not_from_snap' % LATEST_MICROVERSION, + (LATEST_MICROVERSION, False))])) + @ddt.unpack + def test_share_create_az_not_in_share_type(self, version, snap): + """For API version>=2.48, AZ validation should be performed.""" + self.mock_object(share_api.API, 'create', self.create_mock) + create_args = copy.deepcopy(self.share) + create_args['availability_zone'] = 'az2' + create_args['share_type'] = (uuidutils.generate_uuid() if not snap + else None) + create_args['snapshot_id'] = (uuidutils.generate_uuid() if snap + else None) + stype_with_azs = copy.deepcopy(self.vt) + stype_with_azs['extra_specs']['availability_zones'] = 'az1 , az3' + self.mock_object(share_types, 'get_share_type', mock.Mock( + return_value=stype_with_azs)) + self.mock_object(share_api.API, 'get_snapshot', + stubs.stub_snapshot_get) + + req = fakes.HTTPRequest.blank('/shares', version=version) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, + req, {'share': create_args}) + share_api.API.create.assert_not_called() + def test_migration_start(self): share = db_utils.create_share() share_network = db_utils.create_share_network() @@ -1138,8 +1207,11 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'create', create_mock) body = {"share": copy.deepcopy(shr)} req = fakes.HTTPRequest.blank('/shares', version='2.7') + res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(shr) + + expected = self._get_expected_share_detailed_response( + shr, version='2.7') self.assertEqual(expected, res_dict) def test_share_create_from_snapshot_without_share_net_parent_exists(self): @@ -1175,8 +1247,11 @@ class ShareAPITest(test.TestCase): body = {"share": copy.deepcopy(shr)} req = fakes.HTTPRequest.blank('/shares', version='2.7') + res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(shr) + + expected = self._get_expected_share_detailed_response( + shr, version='2.7') self.assertEqual(expected, res_dict) self.assertEqual(parent_share_net, create_mock.call_args[1]['share_network_id']) @@ -1215,7 +1290,8 @@ class ShareAPITest(test.TestCase): body = {"share": copy.deepcopy(shr)} req = fakes.HTTPRequest.blank('/shares', version='2.7') res_dict = self.controller.create(req, body) - expected = self._get_expected_share_detailed_response(shr) + expected = self._get_expected_share_detailed_response( + shr, version='2.7') self.assertDictMatch(expected, res_dict) self.assertEqual(parent_share_net, create_mock.call_args[1]['share_network_id']) @@ -1294,9 +1370,6 @@ class ShareAPITest(test.TestCase): def test_share_show(self): req = fakes.HTTPRequest.blank('/shares/1') expected = self._get_expected_share_detailed_response() - expected['share'].pop('snapshot_support') - expected['share'].pop('share_type_name') - expected['share'].pop('task_state') res_dict = self.controller.show(req, '1') @@ -1305,17 +1378,7 @@ class ShareAPITest(test.TestCase): def test_share_show_with_share_group(self): req = fakes.HTTPRequest.blank( '/shares/1', version='2.31', experimental=True) - expected = self._get_expected_share_detailed_response() - expected['share'].pop('export_location') - expected['share'].pop('export_locations') - expected['share']['create_share_from_snapshot_support'] = True - expected['share']['revert_to_snapshot_support'] = False - expected['share']['share_group_id'] = None - expected['share']['source_share_group_snapshot_member_id'] = None - expected['share']['access_rules_status'] = 'active' - expected['share']['replication_type'] = None - expected['share']['has_replicas'] = False - expected['share']['user_id'] = 'fakeuser' + expected = self._get_expected_share_detailed_response(version='2.31') res_dict = self.controller.show(req, '1') @@ -1324,13 +1387,7 @@ class ShareAPITest(test.TestCase): def test_share_show_with_share_group_earlier_version(self): req = fakes.HTTPRequest.blank( '/shares/1', version='2.23', experimental=True) - expected = self._get_expected_share_detailed_response() - expected['share'].pop('export_location') - expected['share'].pop('export_locations') - expected['share']['access_rules_status'] = 'active' - expected['share']['replication_type'] = None - expected['share']['has_replicas'] = False - expected['share']['user_id'] = 'fakeuser' + expected = self._get_expected_share_detailed_response(version='2.23') res_dict = self.controller.show(req, '1') @@ -1338,10 +1395,10 @@ class ShareAPITest(test.TestCase): def test_share_show_with_share_type_name(self): req = fakes.HTTPRequest.blank('/shares/1', version='2.6') + res_dict = self.controller.show(req, '1') - expected = self._get_expected_share_detailed_response() - expected['share']['share_type_name'] = None - expected['share']['task_state'] = None + + expected = self._get_expected_share_detailed_response(version='2.6') self.assertEqual(expected, res_dict) @ddt.data("2.15", "2.16") @@ -1350,28 +1407,14 @@ class ShareAPITest(test.TestCase): res_dict = self.controller.show(req, '1') - expected = self._get_expected_share_detailed_response() - if api_version.APIVersionRequest(microversion) >= ( - api_version.APIVersionRequest("2.16")): - expected['share']['user_id'] = 'fakeuser' - else: - self.assertNotIn('user_id', expected['share']) - expected['share']['share_type_name'] = None - expected['share']['task_state'] = None - expected['share']['access_rules_status'] = 'active' - expected['share'].pop('export_location') - expected['share'].pop('export_locations') - expected['share']['replication_type'] = None - expected['share']['has_replicas'] = False + expected = self._get_expected_share_detailed_response( + version=microversion) self.assertEqual(expected, res_dict) def test_share_show_admin(self): req = fakes.HTTPRequest.blank('/shares/1', use_admin_context=True) expected = self._get_expected_share_detailed_response(admin=True) - expected['share'].pop('snapshot_support') - expected['share'].pop('share_type_name') - expected['share'].pop('task_state') res_dict = self.controller.show(req, '1') @@ -1390,15 +1433,8 @@ class ShareAPITest(test.TestCase): '/shares/1', version=share_replicas.MIN_SUPPORTED_API_VERSION) res_dict = self.controller.show(req, '1') - expected = self._get_expected_share_detailed_response() - - expected['share']['task_state'] = None - expected['share']['access_rules_status'] = 'active' - expected['share']['share_type_name'] = None - expected['share']['replication_type'] = None - expected['share']['has_replicas'] = False - expected['share'].pop('export_location') - expected['share'].pop('export_locations') + expected = self._get_expected_share_detailed_response( + version=share_replicas.MIN_SUPPORTED_API_VERSION) self.assertEqual(expected, res_dict) diff --git a/manila/tests/scheduler/fakes.py b/manila/tests/scheduler/fakes.py index ec861e6ce5..4a52fd77f0 100644 --- a/manila/tests/scheduler/fakes.py +++ b/manila/tests/scheduler/fakes.py @@ -22,13 +22,24 @@ from manila.scheduler.drivers import filter from manila.scheduler import host_manager from manila.scheduler.weighers import base_host as base_host_weigher + +FAKE_AZ_1 = {'name': 'zone1', 'id': '24433438-c9b5-4cb5-a472-f78462aa5f31'} +FAKE_AZ_2 = {'name': 'zone2', 'id': 'ebef050c-d20d-4c44-b272-1a0adce11cb5'} +FAKE_AZ_3 = {'name': 'zone3', 'id': '18e7e6e2-39d6-466b-a706-2717bd1086e1'} +FAKE_AZ_4 = {'name': 'zone4', 'id': '9ca40ee4-3c2a-4635-9a18-233cf6e0ad0b'} +FAKE_AZ_5 = {'name': 'zone4', 'id': 'd76d921d-d6fa-41b4-a180-fb68952784bd'} +FAKE_AZ_6 = {'name': 'zone4', 'id': 'bc09c3d6-671c-4d55-9f43-f00757aabc50'} + SHARE_SERVICES_NO_POOLS = [ dict(id=1, host='host1', topic='share', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_1['id'], + availability_zone=FAKE_AZ_1), dict(id=2, host='host2@back1', topic='share', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_2['id'], + availability_zone=FAKE_AZ_2), dict(id=3, host='host2@back2', topic='share', disabled=False, - availability_zone='zone2', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_3['id'], + availability_zone=FAKE_AZ_3), ] SERVICE_STATES_NO_POOLS = { @@ -69,18 +80,24 @@ SERVICE_STATES_NO_POOLS = { SHARE_SERVICES_WITH_POOLS = [ dict(id=1, host='host1@AAA', topic='share', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_1['id'], + availability_zone=FAKE_AZ_1), dict(id=2, host='host2@BBB', topic='share', disabled=False, - availability_zone='zone1', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_2['id'], + availability_zone=FAKE_AZ_2), dict(id=3, host='host3@CCC', topic='share', disabled=False, - availability_zone='zone2', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_3['id'], + availability_zone=FAKE_AZ_3), dict(id=4, host='host4@DDD', topic='share', disabled=False, - availability_zone='zone3', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_4['id'], + availability_zone=FAKE_AZ_4), # service on host5 is disabled dict(id=5, host='host5@EEE', topic='share', disabled=True, - availability_zone='zone4', updated_at=timeutils.utcnow()), - dict(id=5, host='host6@FFF', topic='share', disabled=True, - availability_zone='zone5', updated_at=timeutils.utcnow()), + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_5['id'], + availability_zone=FAKE_AZ_5), + dict(id=6, host='host6@FFF', topic='share', disabled=True, + updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_6['id'], + availability_zone=FAKE_AZ_6), ] SHARE_SERVICE_STATES_WITH_POOLS = { @@ -289,17 +306,23 @@ FAKE_HOST_STRING_3 = 'openstack@BackendC#PoolZ' def mock_host_manager_db_calls(mock_obj, disabled=None): services = [ dict(id=1, host='host1', topic='share', disabled=False, - availability_zone_id='zone1', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_1, availability_zone_id=FAKE_AZ_1['id'], + updated_at=timeutils.utcnow()), dict(id=2, host='host2', topic='share', disabled=False, - availability_zone_id='zone1', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_1, availability_zone_id=FAKE_AZ_1['id'], + updated_at=timeutils.utcnow()), dict(id=3, host='host3', topic='share', disabled=False, - availability_zone_id='zone2', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_2, availability_zone_id=FAKE_AZ_2['id'], + updated_at=timeutils.utcnow()), dict(id=4, host='host4', topic='share', disabled=False, - availability_zone_id='zone3', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_3, availability_zone_id=FAKE_AZ_3['id'], + updated_at=timeutils.utcnow()), dict(id=5, host='host5', topic='share', disabled=False, - availability_zone_id='zone3', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_3, availability_zone_id=FAKE_AZ_3['id'], + updated_at=timeutils.utcnow()), dict(id=6, host='host6', topic='share', disabled=False, - availability_zone_id='zone4', updated_at=timeutils.utcnow()), + availability_zone=FAKE_AZ_4, availability_zone_id=FAKE_AZ_4['id'], + updated_at=timeutils.utcnow()), ] if disabled is None: mock_obj.return_value = services diff --git a/manila/tests/scheduler/filters/test_availability_zone.py b/manila/tests/scheduler/filters/test_availability_zone.py index 56e3a6c621..b9cd0d5f29 100644 --- a/manila/tests/scheduler/filters/test_availability_zone.py +++ b/manila/tests/scheduler/filters/test_availability_zone.py @@ -16,7 +16,7 @@ """ Tests For AvailabilityZoneFilter. """ - +import ddt from oslo_context import context from manila.scheduler.filters import availability_zone @@ -24,12 +24,23 @@ from manila import test from manila.tests.scheduler import fakes +@ddt.ddt class HostFiltersTestCase(test.TestCase): """Test case for AvailabilityZoneFilter.""" def setUp(self): super(HostFiltersTestCase, self).setUp() self.filter = availability_zone.AvailabilityZoneFilter() + self.az_id = 'e3ecad6f-e984-4cd1-b149-d83c962374a8' + self.fake_service = { + 'service': { + 'availability_zone_id': self.az_id, + 'availability_zone': { + 'name': 'nova', + 'id': self.az_id + } + } + } @staticmethod def _make_zone_request(zone, is_admin=False): @@ -44,22 +55,52 @@ class HostFiltersTestCase(test.TestCase): } def test_availability_zone_filter_same(self): - service = {'availability_zone_id': 'nova'} - request = self._make_zone_request('nova') - host = fakes.FakeHostState('host1', - {'service': service}) + request = self._make_zone_request(self.az_id) + host = fakes.FakeHostState('host1', self.fake_service) self.assertTrue(self.filter.host_passes(host, request)) def test_availability_zone_filter_different(self): - service = {'availability_zone_id': 'nova'} request = self._make_zone_request('bad') - host = fakes.FakeHostState('host1', - {'service': service}) + host = fakes.FakeHostState('host1', self.fake_service) self.assertFalse(self.filter.host_passes(host, request)) def test_availability_zone_filter_empty(self): - service = {'availability_zone_id': 'nova'} request = {} - host = fakes.FakeHostState('host1', - {'service': service}) + host = fakes.FakeHostState('host1', self.fake_service) self.assertTrue(self.filter.host_passes(host, request)) + + def test_availability_zone_filter_both_request_AZ_and_type_AZs_match(self): + request = self._make_zone_request( + '9382098d-d40f-42a2-8f31-8eb78ee18c02') + request['request_spec']['availability_zones'] = [ + 'nova', 'super nova', 'hypernova'] + service = { + 'availability_zone': { + 'name': 'nova', + 'id': '9382098d-d40f-42a2-8f31-8eb78ee18c02', + }, + 'availability_zone_id': '9382098d-d40f-42a2-8f31-8eb78ee18c02', + } + host = fakes.FakeHostState('host1', {'service': service}) + + self.assertTrue(self.filter.host_passes(host, request)) + + @ddt.data((['zone1', 'zone2', 'zone 4', 'zone3'], 'zone2', True), + (['zone1zone2zone3'], 'zone2', False), + (['zone1zone2zone3'], 'nova', False), + (['zone1', 'zone2', 'zone 4', 'zone3'], 'zone 4', True)) + @ddt.unpack + def test_availability_zone_filter_only_share_type_AZs( + self, supported_azs, request_az, host_passes): + service = { + 'availability_zone': { + 'name': request_az, + 'id': '9382098d-d40f-42a2-8f31-8eb78ee18c02', + }, + 'availability_zone_id': '9382098d-d40f-42a2-8f31-8eb78ee18c02', + } + request = self._make_zone_request(None) + request['request_spec']['availability_zones'] = supported_azs + host = fakes.FakeHostState('host1', {'service': service}) + + self.assertEqual(host_passes, self.filter.host_passes(host, request)) diff --git a/manila/tests/scheduler/filters/test_capabilities.py b/manila/tests/scheduler/filters/test_capabilities.py index d383e8126e..21ebffd49e 100644 --- a/manila/tests/scheduler/filters/test_capabilities.py +++ b/manila/tests/scheduler/filters/test_capabilities.py @@ -51,6 +51,14 @@ class HostFiltersTestCase(test.TestCase): especs={'opt1': '1', 'opt2': '2'}, passes=True) + def test_capability_filter_passes_extra_specs_ignore_azs_spec(self): + self._do_test_type_filter_extra_specs( + ecaps={'opt1': '1', 'opt2': '2'}, + especs={'opt1': '1', + 'opt2': '2', + 'availability_zones': 'az1,az2'}, + passes=True) + def test_capability_filter_fails_extra_specs_simple(self): self._do_test_type_filter_extra_specs( ecaps={'opt1': '1', 'opt2': '2'}, diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 660ea86a03..cb5d844a73 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -1700,7 +1700,8 @@ class ShareAPITestCase(test.TestCase): self.context, share, share_network_id=share['share_network_id'], host=valid_host, share_type_id=share_type['id'], availability_zone=snapshot['share']['availability_zone'], - share_group=None, share_group_snapshot_member=None) + share_group=None, share_group_snapshot_member=None, + availability_zones=None) share_api.policy.check_policy.assert_has_calls([ mock.call(self.context, 'share', 'create'), mock.call(self.context, 'share_snapshot', 'get_snapshot')]) @@ -2513,7 +2514,8 @@ class ShareAPITestCase(test.TestCase): @ddt.unpack def test_migration_start(self, share_type, share_net, dhss): host = 'fake2@backend#pool' - service = {'availability_zone_id': 'fake_az_id'} + service = {'availability_zone_id': 'fake_az_id', + 'availability_zone': {'name': 'fake_az1'}} share_network = None share_network_id = None if share_net: @@ -2540,6 +2542,7 @@ class ShareAPITestCase(test.TestCase): 'revert_to_snapshot_support': False, 'mount_snapshot_support': False, 'driver_handles_share_servers': dhss, + 'availability_zones': 'fake_az1,fake_az2', }, } else: @@ -2588,6 +2591,65 @@ class ShareAPITestCase(test.TestCase): self.context, share.instance['id'], {'status': constants.STATUS_MIGRATING}) + def test_migration_start_destination_az_upsupported(self): + host = 'fake2@backend#pool' + host_without_pool = host.split('#')[0] + service = {'availability_zone_id': 'fake_az_id', + 'availability_zone': {'name': 'fake_az3'}} + share_network = db_utils.create_share_network(id='fake_net_id') + share_network_id = share_network['id'] + existing_share_type = { + 'id': '4b5b0920-a294-401b-bb7d-c55b425e1cad', + 'name': 'fake_type_1', + 'extra_specs': { + 'snapshot_support': False, + 'create_share_from_snapshot_support': False, + 'revert_to_snapshot_support': False, + 'mount_snapshot_support': False, + 'driver_handles_share_servers': 'true', + 'availability_zones': 'fake_az3' + }, + } + new_share_type = { + 'id': 'fa844ae2-494d-4da9-95e7-37ac6a26f635', + 'name': 'fake_type_2', + 'extra_specs': { + 'snapshot_support': False, + 'create_share_from_snapshot_support': False, + 'revert_to_snapshot_support': False, + 'mount_snapshot_support': False, + 'driver_handles_share_servers': 'true', + 'availability_zones': 'fake_az1,fake_az2', + }, + } + share = db_utils.create_share( + status=constants.STATUS_AVAILABLE, + host='fake@backend#pool', share_type_id=existing_share_type['id'], + share_network_id=share_network_id) + self.mock_object(self.api, '_get_request_spec_dict') + self.mock_object(self.scheduler_rpcapi, 'migrate_share_to_host') + self.mock_object(share_types, 'get_share_type') + self.mock_object(utils, 'validate_service_host') + self.mock_object(db_api, 'share_instance_update') + self.mock_object(db_api, 'share_update') + self.mock_object(db_api, 'service_get_by_args', + mock.Mock(return_value=service)) + + self.assertRaises(exception.InvalidShare, + self.api.migration_start, + self.context, share, host, False, True, True, + True, False, new_share_network=share_network, + new_share_type=new_share_type) + utils.validate_service_host.assert_called_once_with( + self.context, host_without_pool) + share_types.get_share_type.assert_not_called() + db_api.share_update.assert_not_called() + db_api.service_get_by_args.assert_called_once_with( + self.context, host_without_pool, 'manila-share') + self.api._get_request_spec_dict.assert_not_called() + db_api.share_instance_update.assert_not_called() + self.scheduler_rpcapi.migrate_share_to_host.assert_not_called() + @ddt.data({'force_host_assisted': True, 'writable': True, 'preserve_metadata': False, 'preserve_snapshots': False, 'nondisruptive': False}, @@ -2703,7 +2765,8 @@ class ShareAPITestCase(test.TestCase): }, } - service = {'availability_zone_id': 'fake_az_id'} + service = {'availability_zone_id': 'fake_az_id', + 'availability_zone': {'name': 'fake_az'}} self.mock_object(db_api, 'service_get_by_args', mock.Mock(return_value=service)) self.mock_object(utils, 'validate_service_host') @@ -2879,20 +2942,66 @@ class ShareAPITestCase(test.TestCase): self.assertFalse(mock_db_update_call.called) self.assertFalse(mock_scheduler_rpcapi_call.called) + @ddt.data(None, 'fake-share-type') + def test_create_share_replica_type_doesnt_support_AZ(self, st_name): + share_type = fakes.fake_share_type( + name=st_name, + extra_specs={'availability_zones': 'zone 1,zone 3'}) + share = fakes.fake_share( + id='FAKE_SHARE_ID', replication_type='dr', + availability_zone='zone 2') + share['instance'].update({ + 'share_type': share_type, + 'share_type_id': '359b9851-2bd5-4404-89a9-5cd22bbc5fb9', + }) + mock_request_spec_call = self.mock_object( + self.api, 'create_share_instance_and_get_request_spec') + mock_db_update_call = self.mock_object(db_api, 'share_replica_update') + mock_scheduler_rpcapi_call = self.mock_object( + self.api.scheduler_rpcapi, 'create_share_replica') + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=share_type)) + self.mock_object(db_api, 'share_replicas_get_available_active_replica', + mock.Mock(return_value=mock.Mock( + return_value={'host': 'fake_ar_host'}))) + + self.assertRaises(exception.InvalidShare, + self.api.create_share_replica, + self.context, share, availability_zone='zone 2') + share_types.get_share_type.assert_called_once_with( + self.context, '359b9851-2bd5-4404-89a9-5cd22bbc5fb9') + self.assertFalse(mock_request_spec_call.called) + self.assertFalse(mock_db_update_call.called) + self.assertFalse(mock_scheduler_rpcapi_call.called) + @ddt.data({'has_snapshots': True, - 'replication_type': constants.REPLICATION_TYPE_DR}, + 'extra_specs': { + 'replication_type': constants.REPLICATION_TYPE_DR, + }}, {'has_snapshots': False, - 'replication_type': constants.REPLICATION_TYPE_DR}, + 'extra_specs': { + 'availability_zones': 'FAKE_AZ,FAKE_AZ2', + 'replication_type': constants.REPLICATION_TYPE_DR, + }}, {'has_snapshots': True, - 'replication_type': constants.REPLICATION_TYPE_READABLE}, + 'extra_specs': { + 'availability_zones': 'FAKE_AZ,FAKE_AZ2', + 'replication_type': constants.REPLICATION_TYPE_READABLE, + }}, {'has_snapshots': False, - 'replication_type': constants.REPLICATION_TYPE_READABLE}) + 'extra_specs': { + 'replication_type': constants.REPLICATION_TYPE_READABLE, + }}) @ddt.unpack - def test_create_share_replica(self, has_snapshots, replication_type): + def test_create_share_replica(self, has_snapshots, extra_specs): request_spec = fakes.fake_replica_request_spec() + replication_type = extra_specs['replication_type'] replica = request_spec['share_instance_properties'] + share_type = db_utils.create_share_type(extra_specs=extra_specs) + share_type = db_api.share_type_get(self.context, share_type['id']) share = db_utils.create_share( - id=replica['share_id'], replication_type=replication_type) + id=replica['share_id'], replication_type=replication_type, + share_type_id=share_type['id']) snapshots = ( [fakes.fake_snapshot(), fakes.fake_snapshot()] if has_snapshots else [] @@ -2904,6 +3013,8 @@ class ShareAPITestCase(test.TestCase): fake_request_spec = fakes.fake_replica_request_spec() self.mock_object(db_api, 'share_replicas_get_available_active_replica', mock.Mock(return_value={'host': 'fake_ar_host'})) + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=share_type)) self.mock_object( share_api.API, 'create_share_instance_and_get_request_spec', mock.Mock(return_value=(fake_request_spec, fake_replica))) @@ -2922,14 +3033,19 @@ class ShareAPITestCase(test.TestCase): self.assertTrue(mock_sched_rpcapi_call.called) self.assertEqual(replica, result) + share_types.get_share_type.assert_called_once_with( + self.context, share_type['id']) mock_snapshot_get_all_call.assert_called_once_with( self.context, fake_replica['share_id']) self.assertEqual(expected_snap_instance_create_call_count, mock_snapshot_instance_create_call.call_count) + expected_azs = extra_specs.get('availability_zones', '') + expected_azs = expected_azs.split(',') if expected_azs else [] (share_api.API.create_share_instance_and_get_request_spec. assert_called_once_with( self.context, share, availability_zone='FAKE_AZ', - share_network_id=None, share_type_id=None, + share_network_id=None, share_type_id=share_type['id'], + availability_zones=expected_azs, cast_rules_to_readonly=cast_rules_to_readonly)) def test_delete_last_active_replica(self): diff --git a/manila/tests/share/test_share_types.py b/manila/tests/share/test_share_types.py index 6b65b8552a..e869a52a21 100644 --- a/manila/tests/share/test_share_types.py +++ b/manila/tests/share/test_share_types.py @@ -158,6 +158,80 @@ class ShareTypesTestCase(test.TestCase): search_opts=search_filter) self.assertItemsEqual(share_type, returned_type) + @ddt.data("nova", "supernova,nova", "supernova", + "nova,hypernova,supernova") + def test_get_all_types_search_by_availability_zone(self, search_azs): + all_share_types = { + 'gold': { + 'extra_specs': { + 'somepoolcap': 'somevalue', + 'availability_zones': 'nova,supernova,hypernova', + }, + 'required_extra_specs': { + 'driver_handles_share_servers': True, + }, + 'id': '1e8f93a8-9669-4467-88a0-7b8229a9a609', + 'name': u'gold-share-type', + 'is_public': True, + }, + 'silver': { + 'extra_specs': { + 'somepoolcap': 'somevalue', + 'availability_zones': 'nova,supernova', + }, + 'required_extra_specs': { + 'driver_handles_share_servers': False, + }, + 'id': '39a7b9a8-8c76-4b49-aed3-60b718d54325', + 'name': u'silver-share-type', + 'is_public': True, + }, + 'bronze': { + 'extra_specs': { + 'somepoolcap': 'somevalue', + 'availability_zones': 'milkyway,andromeda', + }, + 'required_extra_specs': { + 'driver_handles_share_servers': True, + }, + 'id': '5a55a54d-6688-49b4-9344-bfc2d9634f70', + 'name': u'bronze-share-type', + 'is_public': True, + }, + 'default': { + 'extra_specs': { + 'somepoolcap': 'somevalue', + }, + 'required_extra_specs': { + 'driver_handles_share_servers': True, + }, + 'id': '5a55a54d-6688-49b4-9344-bfc2d9634f70', + 'name': u'bronze-share-type', + 'is_public': True, + } + } + self.mock_object( + db, 'share_type_get_all', mock.Mock(return_value=all_share_types)) + self.mock_object(share_types, 'get_valid_required_extra_specs') + + search_opts = { + 'extra_specs': { + 'somepoolcap': 'somevalue', + 'availability_zones': search_azs + }, + 'is_public': True, + } + returned_types = share_types.get_all_types( + self.context, search_opts=search_opts) + + db.share_type_get_all.assert_called_once_with( + mock.ANY, 0, filters={'is_public': True}) + + expected_return_types = (['gold', 'silver', 'default'] + if len(search_azs.split(',')) < 3 + else ['gold', 'default']) + self.assertItemsEqual(expected_return_types, returned_types) + def test_get_share_type_extra_specs(self): share_type = self.fake_type_w_extra['test_with_extra'] self.mock_object(db, @@ -271,11 +345,14 @@ class ShareTypesTestCase(test.TestCase): constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT, constants.ExtraSpecs.REVERT_TO_SNAPSHOT_SUPPORT, constants.ExtraSpecs.MOUNT_SNAPSHOT_SUPPORT), - strutils.TRUE_STRINGS + strutils.FALSE_STRINGS))) + + strutils.TRUE_STRINGS + strutils.FALSE_STRINGS)) + list(itertools.product( (constants.ExtraSpecs.REPLICATION_TYPE_SPEC,), - constants.ExtraSpecs.REPLICATION_TYPES)) - ) + constants.ExtraSpecs.REPLICATION_TYPES)) + + [(constants.ExtraSpecs.AVAILABILITY_ZONES, 'zone a, zoneb$c'), + (constants.ExtraSpecs.AVAILABILITY_ZONES, ' zonea, zoneb'), + (constants.ExtraSpecs.AVAILABILITY_ZONES, 'zone1')] + )) @ddt.unpack def test_is_valid_optional_extra_spec_valid(self, key, value): @@ -305,14 +382,28 @@ class ShareTypesTestCase(test.TestCase): self.assertEqual({}, result) - def test_get_valid_optional_extra_specs_invalid(self): - - extra_specs = {constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'fake'} - + @ddt.data({constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'fake'}, + {constants.ExtraSpecs.AVAILABILITY_ZONES: 'ZoneA,'}) + def test_get_valid_optional_extra_specs_invalid(self, extra_specs): self.assertRaises(exception.InvalidExtraSpec, share_types.get_valid_optional_extra_specs, extra_specs) + @ddt.data(' az 1, az2 ,az 3 ', 'az 1,az2,az 3 ', None) + def test_sanitize_extra_specs(self, spec_value): + extra_specs = { + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'True', + constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'True', + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: 'False' + } + expected_specs = copy.copy(extra_specs) + if spec_value is not None: + extra_specs[constants.ExtraSpecs.AVAILABILITY_ZONES] = spec_value + expected_specs['availability_zones'] = 'az 1,az2,az 3' + + self.assertDictMatch(expected_specs, + share_types.sanitize_extra_specs(extra_specs)) + def test_add_access(self): project_id = '456' extra_specs = { diff --git a/manila/tests/share_group/test_api.py b/manila/tests/share_group/test_api.py index 66cfa1092a..6ccc642c78 100644 --- a/manila/tests/share_group/test_api.py +++ b/manila/tests/share_group/test_api.py @@ -30,6 +30,7 @@ from manila.share import share_types import manila.share_group.api as share_group_api from manila import test from manila.tests.api.contrib import stubs +from manila.tests import utils as test_utils CONF = cfg.CONF @@ -111,6 +112,8 @@ class ShareGroupsAPITestCase(test.TestCase): {'share_type_id': self.fake_share_type_2['id']}, ] } + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=self.fake_share_type)) self.mock_object( db_driver, 'share_group_type_get', mock.Mock(return_value=self.fake_share_group_type)) @@ -150,6 +153,7 @@ class ShareGroupsAPITestCase(test.TestCase): expected_values.pop(name, None) expected_request_spec = {'share_group_id': share_group['id']} expected_request_spec.update(share_group) + expected_request_spec['availability_zones'] = set([]) del expected_request_spec['id'] del expected_request_spec['created_at'] del expected_request_spec['host'] @@ -218,26 +222,59 @@ class ShareGroupsAPITestCase(test.TestCase): self.context, share_group_api.QUOTAS.reserve.return_value) share_group_api.QUOTAS.rollback.assert_not_called() - def test_create_with_multiple_share_types(self): - fake_share_types = [self.fake_share_type, self.fake_share_type_2] + @ddt.data(True, False) + def test_create_with_multiple_share_types_with_az(self, with_az): + share_type_1 = copy.deepcopy(self.fake_share_type) + share_type_2 = copy.deepcopy(self.fake_share_type_2) + share_type_1['extra_specs']['availability_zones'] = 'nova,supernova' + share_type_2['extra_specs']['availability_zones'] = 'nova' + fake_share_types = [share_type_1, share_type_2] fake_share_type_ids = [x['id'] for x in fake_share_types] - self.mock_object(share_types, 'get_share_type') + share_group_type = { + 'share_types': [ + {'share_type_id': share_type_1['id']}, + {'share_type_id': share_type_2['id']}, + {'share_type_id': self.fake_share_type['id']}, + ] + } + self.mock_object( + db_driver, 'share_group_type_get', + mock.Mock(return_value=share_group_type)) + self.mock_object(share_types, 'get_share_type', mock.Mock( + side_effect=[share_type_1, share_type_2, + share_type_1, share_type_2, self.fake_share_type])) share_group = fake_share_group( 'fakeid', user_id=self.context.user_id, project_id=self.context.project_id, - status=constants.STATUS_CREATING) + status=constants.STATUS_CREATING, + availability_zone_id=('e030620e-892c-4ff4-8764-9f3f2b560bd1' + if with_az else None) + ) expected_values = share_group.copy() for name in ('id', 'host', 'created_at'): expected_values.pop(name, None) expected_values['share_types'] = fake_share_type_ids - self.mock_object( db_driver, 'share_group_create', mock.Mock(return_value=share_group)) self.mock_object(db_driver, 'share_network_get') + az_kwargs = { + 'availability_zone': 'nova', + 'availability_zone_id': share_group['availability_zone_id'], + } - self.api.create(self.context, share_type_ids=fake_share_type_ids) + kwargs = {} if not with_az else az_kwargs + self.api.create(self.context, share_type_ids=fake_share_type_ids, + **kwargs) + scheduler_request_spec = ( + self.scheduler_rpcapi.create_share_group.call_args_list[ + 0][1]['request_spec'] + ) + az_id = az_kwargs['availability_zone_id'] if with_az else None + self.assertEqual({'nova', 'supernova'}, + scheduler_request_spec['availability_zones']) + self.assertEqual(az_id, scheduler_request_spec['availability_zone_id']) db_driver.share_group_create.assert_called_once_with( self.context, expected_values) share_group_api.QUOTAS.reserve.assert_called_once_with( @@ -246,6 +283,58 @@ class ShareGroupsAPITestCase(test.TestCase): self.context, share_group_api.QUOTAS.reserve.return_value) share_group_api.QUOTAS.rollback.assert_not_called() + @ddt.data( + test_utils.annotated('specified_stypes_one_unsupported_in_AZ', + (True, True)), + test_utils.annotated('specified_stypes_all_unsupported_in_AZ', + (True, False)), + test_utils.annotated('group_type_stypes_one_unsupported_in_AZ', + (False, True)), + test_utils.annotated('group_type_stypes_all_unsupported_in_AZ', + (False, False))) + @ddt.unpack + def test_create_unsupported_az(self, specify_stypes, all_unsupported): + share_type_1 = copy.deepcopy(self.fake_share_type) + share_type_2 = copy.deepcopy(self.fake_share_type_2) + share_type_1['extra_specs']['availability_zones'] = 'nova,supernova' + share_type_2['extra_specs']['availability_zones'] = ( + 'nova' if all_unsupported else 'nova,hypernova' + ) + share_group_type = { + 'share_types': [ + {'share_type_id': share_type_1['id'], }, + {'share_type_id': share_type_2['id']}, + ] + } + share_group = fake_share_group( + 'fakeid', user_id=self.context.user_id, + project_id=self.context.project_id, + status=constants.STATUS_CREATING, + availability_zone_id='e030620e-892c-4ff4-8764-9f3f2b560bd1') + self.mock_object( + db_driver, 'share_group_create', + mock.Mock(return_value=share_group)) + self.mock_object(db_driver, 'share_network_get') + self.mock_object( + db_driver, 'share_group_type_get', + mock.Mock(return_value=share_group_type)) + self.mock_object(share_types, 'get_share_type', + mock.Mock(side_effect=[share_type_1, share_type_1]*2)) + self.mock_object(db_driver, 'share_group_snapshot_get') + kwargs = { + 'availability_zone': 'hypernova', + 'availability_zone_id': share_group['availability_zone_id'], + } + if specify_stypes: + kwargs['share_type_ids'] = [share_type_1['id'], share_type_2['id']] + + self.assertRaises( + exception.InvalidInput, + self.api.create, + self.context, **kwargs) + db_driver.share_group_snapshot_get.assert_not_called() + db_driver.share_network_get.assert_not_called() + def test_create_with_share_type_not_found(self): self.mock_object(share_types, 'get_share_type', mock.Mock(side_effect=exception.ShareTypeNotFound( diff --git a/releasenotes/notes/bp-share-type-supported-azs-2e12ed406f181b3b.yaml b/releasenotes/notes/bp-share-type-supported-azs-2e12ed406f181b3b.yaml new file mode 100644 index 0000000000..b047c9d1d1 --- /dev/null +++ b/releasenotes/notes/bp-share-type-supported-azs-2e12ed406f181b3b.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + A new common user-visible share types extra-spec called + "availability_zones" has been introduced. When using API version 2.48, + user requests to create new shares in a specific availability zone will + be validated against the configured availability zones of the share type. + Similarly, users requests to create share groups and share replicas are + validated against the share type ``availability_zones`` extra-spec when + present. Users can also filter share types by one or more AZs that are + supported by them. \ No newline at end of file