From 4249e94c6bb7f5efb1a2095c925398277046936d Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Tue, 15 Jan 2019 15:44:40 -0800 Subject: [PATCH] Allow configuring availability_zones in share types Administrators configure share types and make them available to projects within an OpenStack cloud. These share types will define capabilities to match back-end storage pools that manila provisions shares within. Administrators may want to limit share types to specific Availability zones, given they may have different classes of storage in different availability zones in the cloud. A major use case of this is edge computing, where, provisioning can be driven to specific edge locations with the help of share types. This commit will: - Introduce 'availability_zones' as a new common share type extra spec that is user visible when configured. - In and beyond microversion 2.48, validate that the AZ chosen in the POST /shares API is supported by the configured availability zones for the share type being used. - Share types can be filtered by AZs through extra-specs: $ manila type-list --extra-specs availability_zone=nova now gives you all types that explicitly (and implicitly) are supported within the AZ 'nova'. - Improve experimental APIs: - Add validation of AZ to POST /share-replicas - Add validation of AZ to POST /share-groups - Add validation of AZ to POST /shares/id {'action': 'migration_start'} - Also fix old unit tests by using a helper method to generate appropriate mock values. DocImpact Change-Id: Idf274cd73e3b1b33f49668fca768ae676ca30164 Implements: bp share-type-supported-azs --- manila/api/openstack/api_version_request.py | 4 +- .../openstack/rest_api_version_history.rst | 8 +- manila/api/v1/share_types_extra_specs.py | 6 +- manila/api/v1/shares.py | 24 +- manila/api/v2/share_groups.py | 5 +- manila/api/v2/share_types.py | 24 +- manila/api/v2/shares.py | 8 +- manila/common/constants.py | 2 + manila/scheduler/filters/availability_zone.py | 19 +- manila/scheduler/utils.py | 9 + manila/share/api.py | 47 +++- manila/share/share_types.py | 99 ++++++-- manila/share_group/api.py | 37 ++- manila/tests/api/contrib/stubs.py | 15 ++ manila/tests/api/v1/test_shares.py | 2 + manila/tests/api/v2/test_share_groups.py | 1 + manila/tests/api/v2/test_shares.py | 234 ++++++++++-------- manila/tests/scheduler/fakes.py | 55 ++-- .../filters/test_availability_zone.py | 63 ++++- .../scheduler/filters/test_capabilities.py | 8 + manila/tests/share/test_api.py | 136 +++++++++- manila/tests/share/test_share_types.py | 105 +++++++- manila/tests/share_group/test_api.py | 101 +++++++- ...e-type-supported-azs-2e12ed406f181b3b.yaml | 11 + 24 files changed, 816 insertions(+), 207 deletions(-) create mode 100644 releasenotes/notes/bp-share-type-supported-azs-2e12ed406f181b3b.yaml 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