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

@@ -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
 
 

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.

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):

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,

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
+                az = db.availability_zone_get(context, availability_zone)
-                kwargs['availability_zone_id'] = az_id
+                kwargs['availability_zone_id'] = az.id
+                kwargs['availability_zone'] = az.name
             except exception.AvailabilityZoneNotFound as e:
                 raise exc.HTTPNotFound(explanation=six.text_type(e))
 

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', {})
-            extra_specs = req.params.get('extra_specs')
+        extra_specs_disallowed = (req.api_version_request <
-            if extra_specs:
+                                  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)
-        else:
+        elif extra_specs:
-            extra_specs = req.params.get('extra_specs')
+            extra_specs = ast.literal_eval(extra_specs)
-            if extra_specs:
+            filters['extra_specs'] = share_types.sanitize_extra_specs(
-                filters['extra_specs'] = ast.literal_eval(extra_specs)
+                extra_specs)
 
         limited_types = share_types.get_all_types(
             context, search_opts=filters).values()

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)
 

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

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(
+        request_az_id = props.get('availability_zone_id',
-            'availability_zone_id', spec.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:
+        host_satisfied = True
-            return (availability_zone_id ==
+        if request_az_id is not None:
-                    host_state.service['availability_zone_id'])
+            host_satisfied = request_az_id == host_az_id
-        return True
+
+        if request_azs:
+            host_satisfied = host_satisfied and host_az in request_azs
+
+        return host_satisfied

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(':')
 

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,

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,12 +103,22 @@ 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:
+    availability_zones = search_opts.get('extra_specs', {}).pop(
-        search_vars['extra_specs'] = search_opts.pop('extra_specs')
+        '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:
+    elif not search_vars:
+        return share_types
+
     LOG.debug("Searching by: %s", search_vars)
 
     def _check_extra_specs_match(share_type, searchdict):
@@ -107,24 +128,36 @@ def get_all_types(context, inactive=0, search_opts=None):
                 return False
             return True
 
+    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
+
     # search_option to filter_name mapping.
-        filter_mapping = {'extra_specs': _check_extra_specs_match}
+    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
+        # go over all filters in the list (*AND* operation)
-            for opt, values in search_vars.items():
+        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 filter_func(type_args, values):
+                if not filter_func(type_args, value):
-                        result[type_name] = type_args
+                    type_matches = False
                     break
-        share_types = result
+        if type_matches:
-    return share_types
+            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
 

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
 

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)
 

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,

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(

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 = self._get_expected_share_detailed_response(
-        expected['share'].pop('snapshot_support')
+            self.share, version=microversion)
-        expected['share'].pop('share_type_name')
-        expected['share'].pop('task_state')
         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 = self._get_expected_share_detailed_response(
-        expected['share'].pop('share_type_name')
+            self.share, version=microversion)
-        expected['share'].pop('task_state')
         self.assertEqual(expected, res_dict)
 
-    @ddt.data("2.31")
+    def test_share_create_with_share_group(self):
-    def test_share_create_with_share_group(self, microversion):
         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 = self._get_expected_share_detailed_response(
-        expected['share'].pop('export_location')
+            self.share, version="2.31")
-        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
         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 = self._get_expected_share_detailed_response(
-
+            self.share, version=share_replicas.MIN_SUPPORTED_API_VERSION)
-        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')
 
         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)
+        expected = self._get_expected_share_detailed_response(
-        if api_version.APIVersionRequest(microversion) >= (
+            self.share, version=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')
 
         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 = self._get_expected_share_detailed_response(version='2.31')
-        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'
 
         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 = self._get_expected_share_detailed_response(version='2.23')
-        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'
 
         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 = self._get_expected_share_detailed_response(version='2.6')
-        expected['share']['task_state'] = None
         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()
+        expected = self._get_expected_share_detailed_response(
-        if api_version.APIVersionRequest(microversion) >= (
+            version=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
 
         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 = self._get_expected_share_detailed_response(
-
+            version=share_replicas.MIN_SUPPORTED_API_VERSION)
-        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')
 
         self.assertEqual(expected, res_dict)
 

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()),
+         updated_at=timeutils.utcnow(), availability_zone_id=FAKE_AZ_5['id'],
-    dict(id=5, host='host6@FFF', topic='share', disabled=True,
+         availability_zone=FAKE_AZ_5),
-         availability_zone='zone5', updated_at=timeutils.utcnow()),
+    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

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(self.az_id)
-        request = self._make_zone_request('nova')
+        host = fakes.FakeHostState('host1', self.fake_service)
-        host = fakes.FakeHostState('host1',
-                                   {'service': 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',
+        host = fakes.FakeHostState('host1', self.fake_service)
-                                   {'service': 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',
+        host = fakes.FakeHostState('host1', self.fake_service)
-                                   {'service': 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))

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'},

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):

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):
+    @ddt.data({constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'fake'},
-
+              {constants.ExtraSpecs.AVAILABILITY_ZONES: 'ZoneA,'})
-        extra_specs = {constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'fake'}
+    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 = {

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):
+    @ddt.data(True, False)
-        fake_share_types = [self.fake_share_type, self.fake_share_type_2]
+    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(

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.