diff --git a/contrib/ci/post_test_hook.sh b/contrib/ci/post_test_hook.sh index 0a85c91738..bb2c81d1b2 100755 --- a/contrib/ci/post_test_hook.sh +++ b/contrib/ci/post_test_hook.sh @@ -77,6 +77,10 @@ MANILA_CONF=${MANILA_CONF:-/etc/manila/manila.conf} RUN_MANILA_REPLICATION_TESTS=${RUN_MANILA_REPLICATION_TESTS:-False} iniset $TEMPEST_CONFIG share run_replication_tests $RUN_MANILA_REPLICATION_TESTS +# Capability "create_share_from_snapshot_support" +CAPABILITY_CREATE_SHARE_FROM_SNAPSHOT_SUPPORT=${CAPABILITY_CREATE_SHARE_FROM_SNAPSHOT_SUPPORT:-True} +iniset $TEMPEST_CONFIG share capability_create_share_from_snapshot_support $CAPABILITY_CREATE_SHARE_FROM_SNAPSHOT_SUPPORT + if [[ -z "$MULTITENANCY_ENABLED" ]]; then # Define whether share drivers handle share servers or not. # Requires defined config option 'driver_handles_share_servers'. @@ -163,6 +167,7 @@ if [[ "$DRIVER" == "lvm" ]]; then iniset $TEMPEST_CONFIG share image_with_share_tools 'manila-service-image-master' iniset $TEMPEST_CONFIG auth use_dynamic_credentials True iniset $TEMPEST_CONFIG share capability_snapshot_support True + iniset $TEMPEST_CONFIG share capability_create_share_from_snapshot_support True if ! grep $USERNAME_FOR_USER_RULES "/etc/passwd"; then sudo useradd $USERNAME_FOR_USER_RULES fi @@ -190,6 +195,7 @@ elif [[ "$DRIVER" == "zfsonlinux" ]]; then iniset $TEMPEST_CONFIG share enable_ro_access_level_for_protocols 'nfs' iniset $TEMPEST_CONFIG share build_timeout 180 iniset $TEMPEST_CONFIG share share_creation_retry_number 0 + iniset $TEMPEST_CONFIG share capability_create_share_from_snapshot_support True iniset $TEMPEST_CONFIG share capability_storage_protocol 'NFS' iniset $TEMPEST_CONFIG share enable_protocols 'nfs' iniset $TEMPEST_CONFIG share suppress_errors_in_cleanup False @@ -214,6 +220,7 @@ elif [[ "$DRIVER" == "dummy" ]]; then iniset $TEMPEST_CONFIG share enable_ro_access_level_for_protocols 'nfs,cifs' iniset $TEMPEST_CONFIG share build_timeout 180 iniset $TEMPEST_CONFIG share share_creation_retry_number 0 + iniset $TEMPEST_CONFIG share capability_create_share_from_snapshot_support True iniset $TEMPEST_CONFIG share capability_storage_protocol 'NFS_CIFS' iniset $TEMPEST_CONFIG share enable_protocols 'nfs,cifs' iniset $TEMPEST_CONFIG share suppress_errors_in_cleanup False @@ -230,6 +237,7 @@ elif [[ "$DRIVER" == "container" ]]; then iniset $TEMPEST_CONFIG share run_replication_tests False iniset $TEMPEST_CONFIG share run_shrink_tests False iniset $TEMPEST_CONFIG share run_snapshot_tests False + iniset $TEMPEST_CONFIG share capability_create_share_from_snapshot_support False iniset $TEMPEST_CONFIG share capability_storage_protocol 'CIFS' iniset $TEMPEST_CONFIG share enable_protocols 'cifs' iniset $TEMPEST_CONFIG share enable_user_rules_for_protocols 'cifs' diff --git a/contrib/ci/pre_test_hook.sh b/contrib/ci/pre_test_hook.sh index d3a9331673..1f9afe2abf 100755 --- a/contrib/ci/pre_test_hook.sh +++ b/contrib/ci/pre_test_hook.sh @@ -86,6 +86,8 @@ save_configuration "MANILA_DATA_COPY_CHECK_HASH" "${MANILA_DATA_COPY_CHECK_HASH: save_configuration "MANILA_SHARE_MIGRATION_PERIOD_TASK_INTERVAL" "${MANILA_SHARE_MIGRATION_PERIOD_TASK_INTERVAL:=5}" MANILA_SERVICE_IMAGE_ENABLED=${MANILA_SERVICE_IMAGE_ENABLED:-False} +DEFAULT_EXTRA_SPECS=${DEFAULT_EXTRA_SPECS:-"'snapshot_support=True create_share_from_snapshot_support=True'"} + if [[ "$DRIVER" == "generic" ]]; then MANILA_SERVICE_IMAGE_ENABLED=True save_configuration "SHARE_DRIVER" "manila.share.drivers.generic.GenericShareDriver" @@ -161,12 +163,13 @@ elif [[ "$DRIVER" == "zfsonlinux" ]]; then save_configuration "MANILA_ZFSONLINUX_SHARE_EXPORT_IP" "$HOST" save_configuration "MANILA_ZFSONLINUX_SERVICE_IP" "127.0.0.1" elif [[ "$DRIVER" == "container" ]]; then + DEFAULT_EXTRA_SPECS="'snapshot_support=false'" save_configuration "SHARE_DRIVER" "manila.share.drivers.container.driver.ContainerShareDriver" save_configuration "SHARE_BACKING_FILE_SIZE" "64000M" - save_configuration "MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS" "'snapshot_support=false'" fi save_configuration "MANILA_SERVICE_IMAGE_ENABLED" "$MANILA_SERVICE_IMAGE_ENABLED" +save_configuration "MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS" "$DEFAULT_EXTRA_SPECS" # Enabling isolated metadata in Neutron is required because # Tempest creates isolated networks and created vm's in scenario tests don't diff --git a/doc/source/devref/capabilities_and_extra_specs.rst b/doc/source/devref/capabilities_and_extra_specs.rst index b9da0c0915..1729313fd8 100644 --- a/doc/source/devref/capabilities_and_extra_specs.rst +++ b/doc/source/devref/capabilities_and_extra_specs.rst @@ -1,3 +1,5 @@ +.. _capabilities_and_extra_specs: + Capabilities and Extra-Specs ============================ Manila Administrators create share types with extra-specs to allow users @@ -86,7 +88,8 @@ indicator of vendor capabilities vs. common capabilities. Common Capabilities ------------------- For capabilities that apply to multiple backends a common capability can -be created. +be created. Like all other backend reported capabilities, these capabilities +can be used verbatim as extra_specs in share types used to create shares. * `driver_handles_share_servers` is a special, required, user-visible common capability. Added in Kilo. @@ -156,6 +159,19 @@ be created. the `active` replica can be mounted, read from and written into. Added in Mitaka. +* `snapshot_support` - indicates whether snapshots are supported for shares + created on the pool/backend. When administrators do not set this capability + as an extra-spec in a share type, the scheduler can place new shares of that + type in pools without regard for whether snapshots are supported, and those + shares will not support snapshots. + +* `create_share_from_snapshot_support` - indicates whether a backend can create + a new share from a snapshot. When administrators do not set this capability + as an extra-spec in a share type, the scheduler can place new shares of that + type in pools without regard for whether creating shares from snapshots is + supported, and those shares will not support creating shares from snapshots. + + Reporting Capabilities ---------------------- Drivers report capabilities as part of the updated stats (e.g. capacity) @@ -174,47 +190,54 @@ example vendor prefix: :: { - 'driver_handles_share_servers': 'False', #\ - 'share_backend_name': 'My Backend', # backend level - 'vendor_name': 'MY', # mandatory/fixed - 'driver_version': '1.0', # stats & capabilities - 'storage_protocol': 'NFS_CIFS', #/ - #\ - 'my_capability_1': 'custom_val', # "my" optional vendor - 'my_capability_2': True, # stats & capabilities - #/ + 'driver_handles_share_servers': 'False', #\ + 'share_backend_name': 'My Backend', # backend level + 'vendor_name': 'MY', # mandatory/fixed + 'driver_version': '1.0', # stats & capabilities + 'storage_protocol': 'NFS_CIFS', #/ + #\ + 'my_capability_1': 'custom_val', # "my" optional vendor + 'my_capability_2': True, # stats & capabilities + #/ 'pools': [ {'pool_name': - 'thin-dedupe-compression pool', #\ - 'total_capacity_gb': 500, # mandatory stats for - 'free_capacity_gb': 230, # pools - 'reserved_percentage': 0, #/ - #\ - 'dedupe': True, # common capabilities - 'compression': True, # - 'qos': True, # this backend supports QoS - 'thin_provisioning': True, # - 'max_over_subscription_ratio': 10, # (mandatory for thin) - 'provisioned_capacity_gb': 270, # (mandatory for thin) - # - # - 'replication_type': 'dr', # this backend supports - # replication_type 'dr' - #/ - 'my_dying_disks': 100, #\ - 'my_super_hero_1': 'Hulk', # "my" optional vendor - 'my_super_hero_2': 'Spider-Man', # stats & capabilities - #/ - #\ - # can replicate to other - 'replication_domain': 'asgard', # backends in - # replication_domain 'asgard' - #/ + 'thin-dedupe-compression pool', #\ + 'total_capacity_gb': 500, # mandatory stats for + 'free_capacity_gb': 230, # pools + 'reserved_percentage': 0, #/ + #\ + 'dedupe': True, # common capabilities + 'compression': True, # + 'snapshot_support': True, # + 'create_share_from_snapshot_support': True, + 'qos': True, # this backend supports QoS + 'thin_provisioning': True, # + 'max_over_subscription_ratio': 10, # (mandatory for thin) + 'provisioned_capacity_gb': 270, # (mandatory for thin) + # + # + 'replication_type': 'dr', # this backend supports + # replication_type 'dr' + #/ + 'my_dying_disks': 100, #\ + 'my_super_hero_1': 'Hulk', # "my" optional vendor + 'my_super_hero_2': 'Spider-Man', # stats & capabilities + #/ + #\ + # can replicate to other + 'replication_domain': 'asgard', # backends in + # replication_domain 'asgard' + #/ }, {'pool_name': 'thick pool', 'total_capacity_gb': 1024, 'free_capacity_gb': 1024, 'qos': False, + 'snapshot_support': True, + 'create_share_from_snapshot_support': False, # this pool does not + # allow creating + # shares from + # snapshots 'reserved_percentage': 0, 'dedupe': False, 'compression': False, diff --git a/doc/source/devref/share_back_ends_feature_support_mapping.rst b/doc/source/devref/share_back_ends_feature_support_mapping.rst index 4805466008..3ab154b144 100644 --- a/doc/source/devref/share_back_ends_feature_support_mapping.rst +++ b/doc/source/devref/share_back_ends_feature_support_mapping.rst @@ -191,56 +191,61 @@ Mapping of share drivers and security services support Mapping of share drivers and common capabilities ------------------------------------------------ -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Driver name | DHSS=True | DHSS=False | dedupe | compression | thin_provisioning | thick_provisioning | qos | -+========================================+===========+============+========+=============+===================+====================+=====+ -| ZFSonLinux | \- | M | M | M | M | \- | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Container | N | \- | \- | \- | \- | N | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Generic (Cinder as back-end) | J | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| NetApp Clustered Data ONTAP | J | K | M | M | M | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| EMC VNX | J | \- | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| EMC Unity | N | \- | \- | \- | N | \- | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| EMC Isilon | \- | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Red Hat GlusterFS | \- | J | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Red Hat GlusterFS-Native | \- | J | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| HDFS | \- | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Hitachi HNAS | \- | L | N | \- | L | \- | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Hitachi HSP | \- | N | \- | \- | N | \- | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| HPE 3PAR | L | K | L | \- | L | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Huawei | M | K | L | L | L | L | M | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| LVM | \- | M | \- | \- | \- | M | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Quobyte | \- | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Windows SMB | L | L | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| IBM GPFS | \- | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Oracle ZFSSA | \- | K | \- | \- | \- | L | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| CephFS Native | \- | M | \- | \- | \- | M | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| Tegile | \- | M | M | M | M | \- | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| NexentaStor4 | \- | N | N | N | N | N | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ -| NexentaStor5 | \- | N | N | N | N | N | \- | -+----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+ +More information: :ref:`capabilities_and_extra_specs` + ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Driver name | DHSS=True | DHSS=False | dedupe | compression | thin_provisioning | thick_provisioning | qos | create share from snapshot | ++========================================+===========+============+========+=============+===================+====================+=====+============================+ +| ZFSonLinux | \- | M | M | M | M | \- | \- | M | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Container | N | \- | \- | \- | \- | N | \- | \- | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Generic (Cinder as back-end) | J | K | \- | \- | \- | L | \- | J | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| NetApp Clustered Data ONTAP | J | K | M | M | M | L | \- | J | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| EMC VNX | J | \- | \- | \- | \- | L | \- | J | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| EMC Unity | N | \- | \- | \- | N | \- | \- | N | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| EMC Isilon | \- | K | \- | \- | \- | L | \- | K | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Red Hat GlusterFS | \- | J | \- | \- | \- | L | \- | volume layout (L) | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Red Hat GlusterFS-Native | \- | J | \- | \- | \- | L | \- | L | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| HDFS | \- | K | \- | \- | \- | L | \- | K | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Hitachi HNAS | \- | L | N | \- | L | \- | \- | L | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Hitachi HSP | \- | N | \- | \- | N | \- | \- | \- | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| HPE 3PAR | L | K | L | \- | L | L | \- | K | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Huawei | M | K | L | L | L | L | M | M | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| LVM | \- | M | \- | \- | \- | M | \- | K | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Quobyte | \- | K | \- | \- | \- | L | \- | M | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Windows SMB | L | L | \- | \- | \- | L | \- | \- | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| IBM GPFS | \- | K | \- | \- | \- | L | \- | L | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Oracle ZFSSA | \- | K | \- | \- | \- | L | \- | K | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| CephFS Native | \- | M | \- | \- | \- | M | \- | \- | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| Tegile | \- | M | M | M | M | \- | \- | M | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| NexentaStor4 | \- | N | N | N | N | N | \- | N | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ +| NexentaStor5 | \- | N | N | N | N | N | \- | N | ++----------------------------------------+-----------+------------+--------+-------------+-------------------+--------------------+-----+----------------------------+ .. note:: - See details for :term:`DHSS` + The common capability reported by back ends differs from some names seen in the above table: + + * `DHSS` is reported as ``driver_handles_share_servers`` (See details for :term:`DHSS`) + * `create share from snapshot` is reported as ``create_share_from_snapshot_support`` diff --git a/doc/source/devref/tempest_tests.rst b/doc/source/devref/tempest_tests.rst index e66d6bf420..298cdd4dc4 100644 --- a/doc/source/devref/tempest_tests.rst +++ b/doc/source/devref/tempest_tests.rst @@ -29,6 +29,7 @@ Here is a configuration example: # Capabilities capability_storage_protocol = NFS capability_snapshot_support = True + capability_create_share_from_snapshot_support = True backend_names = Backendname1,BackendName2 backend_replication_type = readable diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 69f9174288..3556653377 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -79,13 +79,17 @@ REST_API_VERSION_HISTORY = """ 'notify' parameter and removed previous migrate_share API support. Updated reset_task_state API to accept 'None' value. * 2.23 - Added share_type to filter results of scheduler-stats/pools API. + * 2.24 - Added optional create_share_from_snapshot_support extra spec, + which was previously inferred from the 'snapshot_support' extra + spec. Also made the 'snapshot_support' extra spec optional. + """ # 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.23" +_MAX_API_VERSION = "2.24" 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 5fb5cafc5f..4de289c2e7 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -142,3 +142,8 @@ user documentation. 2.23 ---- Added share_type to filter results of scheduler-stats/pools API. + +2.24 +---- + Added optional create_share_from_snapshot_support extra spec. Made + snapshot_support extra spec optional. diff --git a/manila/api/v1/share_types_extra_specs.py b/manila/api/v1/share_types_extra_specs.py index acda6fe267..9024759323 100644 --- a/manila/api/v1/share_types_extra_specs.py +++ b/manila/api/v1/share_types_extra_specs.py @@ -18,6 +18,7 @@ import webob from manila.api import common from manila.api.openstack import wsgi +from manila.common import constants from manila import db from manila import exception from manila.i18n import _ @@ -58,13 +59,17 @@ class ShareTypeExtraSpecsController(wsgi.Controller): valid_type = is_valid_string(v) or isinstance(v, bool) valid_required_extra_spec = ( share_types.is_valid_required_extra_spec(k, v) in (None, True)) + valid_optional_extra_spec = ( + share_types.is_valid_optional_extra_spec(k, v) in (None, True)) return (valid_extra_spec_key and valid_type - and valid_required_extra_spec) + and valid_required_extra_spec + and valid_optional_extra_spec) for k, v in extra_specs.items(): if is_valid_string(k) and isinstance(v, dict): - self._verify_extra_specs(v) + self._verify_extra_specs( + v, verify_all_required=verify_all_required) elif not is_valid_extra_spec(k, v): expl = _('Invalid extra_spec: %(key)s: %(value)s') % { 'key': k, 'value': v @@ -87,7 +92,11 @@ class ShareTypeExtraSpecsController(wsgi.Controller): self._check_type(context, type_id) specs = body['extra_specs'] - self._verify_extra_specs(specs, False) + try: + self._verify_extra_specs(specs, False) + except exception.InvalidExtraSpec as e: + raise webob.exc.HTTPBadRequest(e.message) + self._check_key_names(specs.keys()) db.share_type_extra_specs_update_or_create(context, type_id, specs) notifier_info = dict(type_id=type_id, specs=specs) @@ -126,13 +135,33 @@ class ShareTypeExtraSpecsController(wsgi.Controller): else: raise webob.exc.HTTPNotFound() + @wsgi.Controller.api_version('1.0', '2.23') @wsgi.Controller.authorize def delete(self, req, type_id, id): """Deletes an existing extra spec.""" context = req.environ['manila.context'] self._check_type(context, type_id) - if id in share_types.get_undeletable_extra_specs(): + if id == constants.ExtraSpecs.SNAPSHOT_SUPPORT: + msg = _("Extra spec '%s' can't be deleted.") % id + raise webob.exc.HTTPForbidden(explanation=msg) + + return self._delete(req, type_id, id) + + @wsgi.Controller.api_version('2.24') # noqa + @wsgi.Controller.authorize + def delete(self, req, type_id, id): # pylint: disable=E0102 + """Deletes an existing extra spec.""" + context = req.environ['manila.context'] + self._check_type(context, type_id) + + return self._delete(req, type_id, id) + + def _delete(self, req, type_id, id): + """Deletes an existing extra spec.""" + context = req.environ['manila.context'] + + if id in share_types.get_required_extra_specs(): msg = _("Extra spec '%s' can't be deleted.") % id raise webob.exc.HTTPForbidden(explanation=msg) diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index f022c79058..02ac1dab52 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -196,7 +196,8 @@ class ShareMixin(object): share = self._create(req, body) return share - def _create(self, req, body): + def _create(self, req, body, + check_create_share_from_snapshot_support=False): """Creates a new share.""" context = req.environ['manila.context'] @@ -263,6 +264,14 @@ class ShareMixin(object): elif parent_share_net_id: share_network_id = parent_share_net_id + # Verify that share can be created from a snapshot + if (check_create_share_from_snapshot_support and + not parent_share['create_share_from_snapshot_support']): + msg = _("Share cannot be created from snapshot '%s', because " + "share back end does not support it.") % snapshot_id + LOG.error(msg) + raise exc.HTTPBadRequest(explanation=msg) + if share_network_id: try: self.share_api.get_share_network( diff --git a/manila/api/v2/share_types.py b/manila/api/v2/share_types.py index 74128b32be..e28a81be03 100644 --- a/manila/api/v2/share_types.py +++ b/manila/api/v2/share_types.py @@ -23,6 +23,7 @@ from webob import exc from manila.api.openstack import wsgi from manila.api.views import types as views_types +from manila.common import constants from manila import exception from manila.i18n import _ from manila import rpc @@ -127,9 +128,18 @@ class ShareTypesController(wsgi.Controller): msg = _('Invalid is_public filter [%s]') % is_public raise exc.HTTPBadRequest(explanation=msg) + @wsgi.Controller.api_version("1.0", "2.23") @wsgi.action("create") + def create(self, req, body): + return self._create(req, body, set_defaults=True) + + @wsgi.Controller.api_version("2.24") # noqa + @wsgi.action("create") + def create(self, req, body): # pylint: disable=E0102 + return self._create(req, body, set_defaults=False) + @wsgi.Controller.authorize('create') - def _create(self, req, body): + def _create(self, req, body, set_defaults=False): """Creates a new share type.""" context = req.environ['manila.context'] @@ -152,14 +162,16 @@ class ShareTypesController(wsgi.Controller): msg = _("Type name is not valid.") raise webob.exc.HTTPBadRequest(explanation=msg) + # Note(cknight): Set the default extra spec value for snapshot_support + # for API versions before it was required. + if set_defaults: + if constants.ExtraSpecs.SNAPSHOT_SUPPORT not in specs: + specs[constants.ExtraSpecs.SNAPSHOT_SUPPORT] = True + try: required_extra_specs = ( share_types.get_valid_required_extra_specs(specs) ) - except exception.InvalidExtraSpec as e: - raise webob.exc.HTTPBadRequest(explanation=six.text_type(e)) - - try: share_types.create(context, name, specs, is_public) share_type = share_types.get_share_type_by_name(context, name) share_type['required_extra_specs'] = required_extra_specs @@ -168,6 +180,8 @@ class ShareTypesController(wsgi.Controller): rpc.get_notifier('shareType').info( context, 'share_type.create', notifier_info) + except exception.InvalidExtraSpec as e: + raise webob.exc.HTTPBadRequest(explanation=six.text_type(e)) except exception.ShareTypeExists as err: notifier_err = dict(share_types=share_type, error_message=six.text_type(err)) diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index db7cd2ebe1..9cf0f9bec7 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -47,17 +47,22 @@ class ShareController(shares.ShareMixin, self._access_view_builder = share_access_views.ViewBuilder() self._migration_view_builder = share_migration_views.ViewBuilder() - @wsgi.Controller.api_version("2.4") + @wsgi.Controller.api_version("2.0", "2.3") def create(self, req, body): - return self._create(req, body) - - @wsgi.Controller.api_version("2.0", "2.3") # noqa - def create(self, req, body): # pylint: disable=E0102 # Remove consistency group attributes body.get('share', {}).pop('consistency_group_id', None) share = self._create(req, body) return share + @wsgi.Controller.api_version("2.4", "2.23") # noqa + def create(self, req, body): # pylint: disable=E0102 + return self._create(req, body) + + @wsgi.Controller.api_version("2.24") # noqa + def create(self, req, body): # pylint: disable=E0102 + return self._create(req, body, + check_create_share_from_snapshot_support=True) + @wsgi.Controller.api_version('2.0', '2.6') @wsgi.action('os-reset_status') def share_reset_status_legacy(self, req, id, body): diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index 34c237bc89..f70d623932 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -29,6 +29,7 @@ class ViewBuilder(common.ViewBuilder): "add_access_rules_status_field", "add_replication_fields", "add_user_id", + "add_create_share_from_snapshot_support_field", ] def summary_list(self, request, shares): @@ -141,6 +142,12 @@ class ViewBuilder(common.ViewBuilder): def add_user_id(self, context, share_dict, share): share_dict['user_id'] = share.get('user_id') + @common.ViewBuilder.versioned_method("2.24") + def add_create_share_from_snapshot_support_field(self, context, + share_dict, share): + share_dict['create_share_from_snapshot_support'] = share.get( + 'create_share_from_snapshot_support') + def _list_view(self, func, request, shares): """Provide a view for a list of shares.""" shares_list = [func(request, share)['share'] for share in shares] diff --git a/manila/api/views/types.py b/manila/api/views/types.py index 39b3c5ea56..823fbcee79 100644 --- a/manila/api/views/types.py +++ b/manila/api/views/types.py @@ -14,6 +14,7 @@ # under the License. from manila.api import common +from manila.common import constants from manila.share import share_types @@ -24,6 +25,7 @@ class ViewBuilder(common.ViewBuilder): _detail_version_modifiers = [ "add_is_public_attr_core_api_like", "add_is_public_attr_extension_like", + "add_inferred_optional_extra_specs", ] def show(self, request, share_type, brief=False): @@ -64,6 +66,20 @@ class ViewBuilder(common.ViewBuilder): share_type_dict['os-share-type-access:is_public'] = share_type.get( 'is_public', True) + @common.ViewBuilder.versioned_method("2.24") + def add_inferred_optional_extra_specs(self, context, share_type_dict, + share_type): + + # NOTE(cknight): The admin sees exactly which extra specs have been set + # on the type, but in order to know how shares of a type will behave, + # the user must also see the default values of any public extra specs + # that aren't explicitly set on the type. + if not context.is_admin: + for extra_spec in constants.ExtraSpecs.INFERRED_OPTIONAL_MAP: + if extra_spec not in share_type_dict['extra_specs']: + share_type_dict['extra_specs'][extra_spec] = ( + constants.ExtraSpecs.INFERRED_OPTIONAL_MAP[extra_spec]) + def index(self, request, share_types): """Index over trimmed share types.""" share_types_list = [self.show(request, share_type, True) diff --git a/manila/common/constants.py b/manila/common/constants.py index 6d8946393a..cebe0a249b 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -160,19 +160,36 @@ class ExtraSpecs(object): DRIVER_HANDLES_SHARE_SERVERS = "driver_handles_share_servers" SNAPSHOT_SUPPORT = "snapshot_support" REPLICATION_TYPE_SPEC = "replication_type" + CREATE_SHARE_FROM_SNAPSHOT_SUPPORT = "create_share_from_snapshot_support" # Extra specs containers REQUIRED = ( DRIVER_HANDLES_SHARE_SERVERS, ) - UNDELETABLE = ( - DRIVER_HANDLES_SHARE_SERVERS, + + OPTIONAL = ( SNAPSHOT_SUPPORT, + CREATE_SHARE_FROM_SNAPSHOT_SUPPORT, + REPLICATION_TYPE_SPEC, ) + # NOTE(cknight): Some extra specs are necessary parts of the Manila API and - # should be visible to non-admin users. UNDELETABLE specs are user-visible. - TENANT_VISIBLE = UNDELETABLE + (REPLICATION_TYPE_SPEC, ) + # should be visible to non-admin users. REQUIRED specs are user-visible, as + # are a handful of community-agreed standardized OPTIONAL ones. + TENANT_VISIBLE = REQUIRED + OPTIONAL + BOOLEAN = ( DRIVER_HANDLES_SHARE_SERVERS, SNAPSHOT_SUPPORT, + CREATE_SHARE_FROM_SNAPSHOT_SUPPORT, ) + + # NOTE(cknight): Some extra specs are optional, but a nominal (typically + # False, but may be non-boolean) default value for each is still needed + # when creating shares. + INFERRED_OPTIONAL_MAP = { + SNAPSHOT_SUPPORT: False, + CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: False, + } + + REPLICATION_TYPES = ('writable', 'readable', 'dr') diff --git a/manila/db/migrations/alembic/versions/3e7d62517afa_add_create_share_from_snapshot_support.py b/manila/db/migrations/alembic/versions/3e7d62517afa_add_create_share_from_snapshot_support.py new file mode 100644 index 0000000000..0006181ca5 --- /dev/null +++ b/manila/db/migrations/alembic/versions/3e7d62517afa_add_create_share_from_snapshot_support.py @@ -0,0 +1,149 @@ +# Copyright (c) 2016 Clinton Knight. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Add 'create_share_from_snapshot_support' extra spec to share types + +Revision ID: 3e7d62517afa +Revises: 48a7beae3117 +Create Date: 2016-08-16 10:48:11.497499 + +""" + +# revision identifiers, used by Alembic. +revision = '3e7d62517afa' +down_revision = '48a7beae3117' + +from alembic import op +from oslo_utils import timeutils +import sqlalchemy as sa +from sqlalchemy.sql import table + +from manila.common import constants + + +def upgrade(): + """Performs DB upgrade to add create_share_from_snapshot_support. + + Prior to this migration, the 'snapshot_support' extra spec meant two + different things: a snapshot may be created, and a new share may be created + from a snapshot. With the planned addition of new snapshot semantics + (revert to snapshot, mountable snapshots), it is likely a driver may be + able to support one or both of the new semantics but *not* be able to + create a share from a snapshot. So this migration separates the existing + snapshot_support extra spec and share attribute into two values to enable + logical separability of the features. + + Add 'create_share_from_snapshot_support' extra spec to all share types and + attribute 'create_share_from_snapshot_support' to Share model. + """ + session = sa.orm.Session(bind=op.get_bind().connect()) + + extra_specs_table = table( + 'share_type_extra_specs', + sa.Column('created_at', sa.DateTime), + sa.Column('deleted', sa.Integer), + sa.Column('share_type_id', sa.String(length=36)), + sa.Column('spec_key', sa.String(length=255)), + sa.Column('spec_value', sa.String(length=255))) + + share_type_table = table( + 'share_types', + sa.Column('deleted', sa.Integer), + sa.Column('id', sa.Integer)) + + # Get list of share type IDs that don't already have the new required + # create_share_from_snapshot_support extra spec defined. + existing_extra_specs = session.query( + extra_specs_table).filter( + extra_specs_table.c.spec_key == + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT).filter( + extra_specs_table.c.deleted == 0).all() + excluded_st_ids = [es.share_type_id for es in existing_extra_specs] + + # Get share types for the IDs we got in the previous query + share_types = session.query(share_type_table).filter( + share_type_table.c.deleted.in_(('0', 'False', ))).filter( + share_type_table.c.id.notin_(excluded_st_ids)).all() + + extra_specs = [] + now = timeutils.utcnow() + for share_type in share_types: + + # Get the value of snapshot_support for each extant share type + snapshot_support_extra_spec = session.query( + extra_specs_table).filter( + extra_specs_table.c.spec_key == + constants.ExtraSpecs.SNAPSHOT_SUPPORT).filter( + extra_specs_table.c.share_type_id == share_type.id).first() + + spec_value = (snapshot_support_extra_spec.spec_value if + snapshot_support_extra_spec else 'False') + + # Copy the snapshot_support value to create_share_from_snapshot_support + extra_specs.append({ + 'spec_key': + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT, + 'spec_value': spec_value, + 'deleted': 0, + 'created_at': now, + 'share_type_id': share_type.id, + }) + if extra_specs: + op.bulk_insert(extra_specs_table, extra_specs) + + # Add create_share_from_snapshot_support attribute to shares table + op.add_column('shares', + sa.Column('create_share_from_snapshot_support', + sa.Boolean, default=True)) + + # Copy snapshot_support to create_share_from_snapshot_support on each share + shares_table = sa.Table( + 'shares', + sa.MetaData(), + sa.Column('id', sa.String(length=36)), + sa.Column('deleted', sa.String(length=36)), + sa.Column('snapshot_support', sa.Boolean), + sa.Column('create_share_from_snapshot_support', sa.Boolean), + ) + update = shares_table.update().where( + shares_table.c.deleted == 'False').values( + create_share_from_snapshot_support=shares_table.c.snapshot_support) + session.execute(update) + session.commit() + + session.close_all() + + +def downgrade(): + """Performs DB downgrade removing create_share_from_snapshot_support. + + Remove 'create_share_from_snapshot_support' extra spec from all share types + and attribute 'create_share_from_snapshot_support' from Share model. + """ + connection = op.get_bind().connect() + deleted_at = timeutils.utcnow() + extra_specs = sa.Table( + 'share_type_extra_specs', + sa.MetaData(), + autoload=True, + autoload_with=connection) + + update = extra_specs.update().where( + extra_specs.c.spec_key == + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT).where( + extra_specs.c.deleted == 0).values(deleted=extra_specs.c.id, + deleted_at=deleted_at) + connection.execute(update) + + op.drop_column('shares', 'create_share_from_snapshot_support') diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index e4ae41c09e..4868b38a9c 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -302,6 +302,7 @@ class Share(BASE, ManilaBase): display_description = Column(String(255)) snapshot_id = Column(String(36)) snapshot_support = Column(Boolean, default=True) + create_share_from_snapshot_support = Column(Boolean, default=True) replication_type = Column(String(255), nullable=True) share_proto = Column(String(255)) is_public = Column(Boolean, default=False) diff --git a/manila/scheduler/drivers/filter.py b/manila/scheduler/drivers/filter.py index b8e8e3503f..eea7acc679 100644 --- a/manila/scheduler/drivers/filter.py +++ b/manila/scheduler/drivers/filter.py @@ -357,7 +357,7 @@ class FilterScheduler(base.Scheduler): share_type['extra_specs'] = share_type.get('extra_specs', {}) if share_type['extra_specs']: - for spec_name in share_types.get_undeletable_extra_specs(): + for spec_name in share_types.get_required_extra_specs(): extra_spec = share_type['extra_specs'].get(spec_name) if extra_spec is not None: diff --git a/manila/scheduler/host_manager.py b/manila/scheduler/host_manager.py index 2b6a681211..df6ca3b338 100644 --- a/manila/scheduler/host_manager.py +++ b/manila/scheduler/host_manager.py @@ -130,6 +130,7 @@ class HostState(object): self.thin_provisioning = False self.driver_handles_share_servers = False self.snapshot_support = True + self.create_share_from_snapshot_support = True self.consistency_group_support = False self.dedupe = False self.compression = False @@ -294,6 +295,10 @@ class HostState(object): if 'snapshot_support' not in pool_cap: pool_cap['snapshot_support'] = self.snapshot_support + if 'create_share_from_snapshot_support' not in pool_cap: + pool_cap['create_share_from_snapshot_support'] = ( + self.create_share_from_snapshot_support) + if not pool_cap.get('consistency_group_support'): pool_cap['consistency_group_support'] = \ self.consistency_group_support @@ -318,6 +323,8 @@ class HostState(object): self.driver_handles_share_servers = capability.get( 'driver_handles_share_servers') self.snapshot_support = capability.get('snapshot_support') + self.create_share_from_snapshot_support = capability.get( + 'create_share_from_snapshot_support') self.consistency_group_support = capability.get( 'consistency_group_support', False) self.updated = capability['timestamp'] diff --git a/manila/scheduler/utils.py b/manila/scheduler/utils.py index 30e97c86f3..7569b2ee2b 100644 --- a/manila/scheduler/utils.py +++ b/manila/scheduler/utils.py @@ -44,6 +44,8 @@ def generate_stats(host_state, properties): 'dedupe': host_state.dedupe, 'compression': host_state.compression, 'snapshot_support': host_state.snapshot_support, + 'create_share_from_snapshot_support': + host_state.create_share_from_snapshot_support, 'replication_domain': host_state.replication_domain, 'replication_type': host_state.replication_type, 'provisioned_capacity_gb': host_state.provisioned_capacity_gb, diff --git a/manila/share/api.py b/manila/share/api.py index 3c574f8177..29b8fb5c50 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -166,12 +166,6 @@ class API(base.Base): try: is_public = strutils.bool_from_string(is_public, strict=True) - snapshot_support = strutils.bool_from_string( - share_type.get('extra_specs', {}).get( - 'snapshot_support', True) if share_type else True, - strict=True) - replication_type = share_type.get('extra_specs', {}).get( - 'replication_type') if share_type else None except ValueError as e: raise exception.InvalidParameterValue(six.text_type(e)) @@ -218,19 +212,20 @@ class API(base.Base): "(%(cg)s).") % params raise exception.InvalidParameterValue(msg) - options = {'size': size, - 'user_id': context.user_id, - 'project_id': context.project_id, - 'snapshot_id': snapshot_id, - 'snapshot_support': snapshot_support, - 'replication_type': replication_type, - 'metadata': metadata, - 'display_name': name, - 'display_description': description, - 'share_proto': share_proto, - 'is_public': is_public, - 'consistency_group_id': consistency_group_id, - } + options = { + 'size': size, + 'user_id': context.user_id, + 'project_id': context.project_id, + 'snapshot_id': snapshot_id, + 'metadata': metadata, + 'display_name': name, + 'display_description': description, + 'share_proto': share_proto, + 'is_public': is_public, + 'consistency_group_id': consistency_group_id, + } + options.update(self._get_share_attributes_from_share_type(share_type)) + if cgsnapshot_member: options['source_cgsnapshot_member_id'] = cgsnapshot_member['id'] @@ -262,6 +257,52 @@ class API(base.Base): return share + def _get_share_attributes_from_share_type(self, share_type): + """Determine share attributes from the share type. + + The share type can change any time after shares of that type are + created, so we copy some share type attributes to the share to + consistently govern the behavior of that share over its lifespan. + """ + + inferred_map = constants.ExtraSpecs.INFERRED_OPTIONAL_MAP + snapshot_support_default = inferred_map.get( + constants.ExtraSpecs.SNAPSHOT_SUPPORT) + create_share_from_snapshot_support_default = inferred_map.get( + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT) + create_share_from_snapshot_key = ( + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT) + + try: + if share_type: + snapshot_support = share_types.parse_boolean_extra_spec( + constants.ExtraSpecs.SNAPSHOT_SUPPORT, + share_type.get('extra_specs', {}).get( + constants.ExtraSpecs.SNAPSHOT_SUPPORT, + snapshot_support_default)) + create_share_from_snapshot_support = ( + share_types.parse_boolean_extra_spec( + create_share_from_snapshot_key, share_type.get( + 'extra_specs', {}).get( + create_share_from_snapshot_key, + create_share_from_snapshot_support_default))) + replication_type = share_type.get('extra_specs', {}).get( + 'replication_type') + else: + snapshot_support = snapshot_support_default + create_share_from_snapshot_support = ( + create_share_from_snapshot_support_default) + replication_type = None + except Exception as e: + raise exception.InvalidParameterValue(six.text_type(e)) + + return { + 'snapshot_support': snapshot_support, + 'create_share_from_snapshot_support': + create_share_from_snapshot_support, + 'replication_type': replication_type, + } + def create_instance(self, context, share, share_network_id=None, host=None, availability_zone=None, consistency_group=None, cgsnapshot_member=None, @@ -339,6 +380,8 @@ class API(base.Base): 'metadata': self.db.share_metadata_get(context, share['id']), 'share_server_id': share_instance['share_server_id'], 'snapshot_support': share['snapshot_support'], + 'create_share_from_snapshot_support': + share['create_share_from_snapshot_support'], 'share_proto': share['share_proto'], 'share_type_id': share_type_id, 'is_public': share['is_public'], @@ -517,21 +560,14 @@ class API(base.Base): share_type_id = share_data['share_type_id'] share_type = share_types.get_share_type(context, share_type_id) - snapshot_support = strutils.bool_from_string( - share_type.get('extra_specs', {}).get( - 'snapshot_support', True) if share_type else True, - strict=True) - replication_type = share_type.get('extra_specs', {}).get( - 'replication_type') - share_data.update({ 'user_id': context.user_id, 'project_id': context.project_id, 'status': constants.STATUS_MANAGING, 'scheduled_at': timeutils.utcnow(), - 'snapshot_support': snapshot_support, - 'replication_type': replication_type, }) + share_data.update( + self._get_share_attributes_from_share_type(share_type)) LOG.debug("Manage: Found shares %s.", len(shares)) @@ -571,7 +607,13 @@ class API(base.Base): 'project_id': kwargs.get('project_id', share.get('project_id')), 'snapshot_support': kwargs.get( 'snapshot_support', - share_type['extra_specs']['snapshot_support']), + share_type.get('extra_specs', {}).get('snapshot_support') + ), + 'create_share_from_snapshot_support': kwargs.get( + 'create_share_from_snapshot_support', + share_type.get('extra_specs', {}).get( + 'create_share_from_snapshot_support') + ), 'share_proto': kwargs.get('share_proto', share.get('share_proto')), 'share_type_id': share_type['id'], 'is_public': kwargs.get('is_public', share.get('is_public')), diff --git a/manila/share/driver.py b/manila/share/driver.py index 5f81a0f2f4..5ef9f260a1 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -908,10 +908,7 @@ class ShareDriver(object): @property def snapshots_are_supported(self): if not hasattr(self, '_snapshots_are_supported'): - methods = ( - "create_snapshot", - "delete_snapshot", - "create_share_from_snapshot") + methods = ('create_snapshot', 'delete_snapshot') # NOTE(vponomaryov): calculate default value for # stat 'snapshot_support' based on implementation of # appropriate methods of this base driver class. @@ -919,6 +916,20 @@ class ShareDriver(object): methods) return self._snapshots_are_supported + @property + def creating_shares_from_snapshots_is_supported(self): + """Calculate default value for create_share_from_snapshot_support.""" + + if not hasattr(self, '_creating_shares_from_snapshots_is_supported'): + methods = ('create_share_from_snapshot', ) + self._creating_shares_from_snapshots_is_supported = ( + self._has_redefined_driver_methods(methods)) + + return ( + self._creating_shares_from_snapshots_is_supported and + self.snapshots_are_supported + ) + def _update_share_stats(self, data=None): """Retrieve stats info from share group. @@ -944,6 +955,8 @@ class ShareDriver(object): qos=False, pools=self.pools or None, snapshot_support=self.snapshots_are_supported, + create_share_from_snapshot_support=( + self.creating_shares_from_snapshots_is_supported), replication_domain=self.replication_domain, filter_function=self.get_filter_function(), goodness_function=self.get_goodness_function(), diff --git a/manila/share/drivers/container/driver.py b/manila/share/drivers/container/driver.py index e5adac20dd..067bc4733c 100644 --- a/manila/share/drivers/container/driver.py +++ b/manila/share/drivers/container/driver.py @@ -110,6 +110,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): self.configuration.reserved_share_percentage, 'consistency_group_support': None, 'snapshot_support': False, + 'create_share_from_snapshot_support': False, 'driver_name': 'ContainerShareDriver', 'pools': self.storage.get_share_server_pools() } diff --git a/manila/share/drivers/huawei/huawei_nas.py b/manila/share/drivers/huawei/huawei_nas.py index 0c69a034bf..2398b3345e 100644 --- a/manila/share/drivers/huawei/huawei_nas.py +++ b/manila/share/drivers/huawei/huawei_nas.py @@ -206,6 +206,7 @@ class HuaweiNasDriver(driver.ShareDriver): total_capacity_gb=0.0, free_capacity_gb=0.0, snapshot_support=self.plugin.snapshot_support, + create_share_from_snapshot_support=self.plugin.snapshot_support, ) # huawei array doesn't support snapshot replication, so driver can't diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index cac19c99b6..80570d9947 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -187,6 +187,7 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): self.configuration.reserved_share_percentage, 'consistency_group_support': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'driver_name': 'LVMShareDriver', 'pools': self.get_share_server_pools() } diff --git a/manila/share/drivers/tegile/tegile.py b/manila/share/drivers/tegile/tegile.py index f632c93148..f75b18fb1d 100644 --- a/manila/share/drivers/tegile/tegile.py +++ b/manila/share/drivers/tegile/tegile.py @@ -458,6 +458,7 @@ class TegileShareDriver(driver.ShareDriver): data['driver_version'] = VERSION data['storage_protocol'] = 'NFS_CIFS' data['snapshot_support'] = True + data['create_share_from_snapshot_support'] = True data['qos'] = False super(TegileShareDriver, self)._update_share_stats(data) diff --git a/manila/share/drivers/zfsonlinux/driver.py b/manila/share/drivers/zfsonlinux/driver.py index c1f3861655..71a325e8c9 100644 --- a/manila/share/drivers/zfsonlinux/driver.py +++ b/manila/share/drivers/zfsonlinux/driver.py @@ -377,6 +377,7 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver): self.configuration.reserved_share_percentage, 'consistency_group_support': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'driver_name': 'ZFS', 'pools': self._get_pools_info(), } diff --git a/manila/share/share_types.py b/manila/share/share_types.py index 0a2bd7e5fc..1b4ad698cf 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -40,11 +40,9 @@ def create(context, name, extra_specs=None, is_public=True, projects=None): extra_specs = extra_specs or {} projects = projects or [] - if constants.ExtraSpecs.SNAPSHOT_SUPPORT not in list(extra_specs): - extra_specs[constants.ExtraSpecs.SNAPSHOT_SUPPORT] = 'True' - try: get_valid_required_extra_specs(extra_specs) + get_valid_optional_extra_specs(extra_specs) except exception.InvalidExtraSpec as e: raise exception.InvalidShareType(reason=six.text_type(e)) @@ -198,8 +196,8 @@ def get_required_extra_specs(): return constants.ExtraSpecs.REQUIRED -def get_undeletable_extra_specs(): - return constants.ExtraSpecs.UNDELETABLE +def get_optional_extra_specs(): + return constants.ExtraSpecs.OPTIONAL def get_tenant_visible_extra_specs(): @@ -228,10 +226,10 @@ def is_valid_required_extra_spec(key, value): def get_valid_required_extra_specs(extra_specs): - """Returns required extra specs from dict. + """Validates and returns required extra specs from dict. - Returns None if extra specs are not valid, or if - some required extras specs is missed. + Raises InvalidExtraSpec if extra specs are not valid, or if any required + extra specs are missing. """ extra_specs = extra_specs or {} @@ -255,6 +253,50 @@ def get_valid_required_extra_specs(extra_specs): return required_extra_specs +def is_valid_optional_extra_spec(key, value): + """Validates optional but standardized extra_spec value. + + :param key: extra_spec name + :param value: extra_spec value + :return: None if provided extra_spec is not required + True/False if extra_spec is required and valid or not. + """ + if key not in get_optional_extra_specs(): + return + + if key == constants.ExtraSpecs.SNAPSHOT_SUPPORT: + return parse_boolean_extra_spec(key, value) is not None + elif key == constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: + return parse_boolean_extra_spec(key, value) is not None + elif key == constants.ExtraSpecs.REPLICATION_TYPE_SPEC: + return value in constants.ExtraSpecs.REPLICATION_TYPES + + return False + + +def get_valid_optional_extra_specs(extra_specs): + """Validates and returns optional/standard extra specs from dict. + + Raises InvalidExtraSpec if extra specs are not valid. + """ + + extra_specs = extra_specs or {} + present_optional_extra_spec_keys = set(extra_specs).intersection( + set(get_optional_extra_specs())) + + optional_extra_specs = {} + + for key in present_optional_extra_spec_keys: + value = extra_specs.get(key, '') + if not is_valid_optional_extra_spec(key, value): + msg = _("Value of optional extra_spec %s is not valid.") % key + raise exception.InvalidExtraSpec(reason=msg) + + optional_extra_specs[key] = value + + return optional_extra_specs + + def add_share_type_access(context, share_type_id, project_id): """Add access to share type for project_id.""" if share_type_id is None: @@ -271,49 +313,6 @@ def remove_share_type_access(context, share_type_id, project_id): return db.share_type_access_remove(context, share_type_id, project_id) -def share_types_diff(context, share_type_id1, share_type_id2): - """Returns a 'diff' of two share types and whether they are equal. - - Returns a tuple of (diff, equal), where 'equal' is a boolean indicating - whether there is any difference, and 'diff' is a dictionary with the - following format: - {'extra_specs': { - 'key1': (value_in_1st_share_type, value_in_2nd_share_type), - 'key2': (value_in_1st_share_type, value_in_2nd_share_type), - ...} - """ - - def _dict_diff(dict1, dict2): - res = {} - equal = True - if dict1 is None: - dict1 = {} - if dict2 is None: - dict2 = {} - for k, v in dict1.items(): - res[k] = (v, dict2.get(k)) - if k not in dict2 or res[k][0] != res[k][1]: - equal = False - for k, v in dict2.items(): - res[k] = (dict1.get(k), v) - if k not in dict1 or res[k][0] != res[k][1]: - equal = False - return (res, equal) - - all_equal = True - diff = {} - share_type1 = get_share_type(context, share_type_id1) - share_type2 = get_share_type(context, share_type_id2) - - extra_specs1 = share_type1.get('extra_specs') - extra_specs2 = share_type2.get('extra_specs') - diff['extra_specs'], equal = _dict_diff(extra_specs1, extra_specs2) - if not equal: - all_equal = False - - return (diff, all_equal) - - def get_extra_specs_from_share(share): type_id = share.get('share_type_id', None) return get_share_type_extra_specs(type_id) @@ -326,18 +325,16 @@ def parse_boolean_extra_spec(extra_spec_key, extra_spec_value): the value does not conform to the standard boolean pattern, it raises an InvalidExtraSpec exception. """ + if not isinstance(extra_spec_value, six.string_types): + extra_spec_value = six.text_type(extra_spec_value) + match = re.match(r'^\s*(?PTrue|False)$', + extra_spec_value.strip(), + re.IGNORECASE) + if match: + extra_spec_value = match.group('value') try: - if not isinstance(extra_spec_value, six.string_types): - raise ValueError - - match = re.match(r'^\s*(?PTrue|False)$', - extra_spec_value.strip(), - re.IGNORECASE) - if not match: - raise ValueError - else: - return strutils.bool_from_string(match.group('value'), strict=True) + return strutils.bool_from_string(extra_spec_value, strict=True) except ValueError: msg = (_('Invalid boolean extra spec %(key)s : %(value)s') % {'key': extra_spec_key, 'value': extra_spec_value}) diff --git a/manila/test.py b/manila/test.py index ace36479e1..1176b3e684 100644 --- a/manila/test.py +++ b/manila/test.py @@ -34,6 +34,7 @@ from oslo_messaging import conffixture as messaging_conffixture from oslo_utils import uuidutils import oslotest.base as base_test +from manila.api.openstack import api_version_request as api_version from manila.db import migration from manila.db.sqlalchemy import api as db_api from manila.db.sqlalchemy import models as db_models @@ -351,3 +352,7 @@ class TestCase(base_test.BaseTestCase): conv_and_sort = lambda obj: sorted(map(obj_to_dict, obj), key=sort_key) self.assertEqual(conv_and_sort(objs1), conv_and_sort(objs2)) + + def is_microversion_ge(self, left, right): + return (api_version.APIVersionRequest(left) >= + api_version.APIVersionRequest(right)) diff --git a/manila/tests/api/contrib/stubs.py b/manila/tests/api/contrib/stubs.py index 98a6dc6994..ed339c3232 100644 --- a/manila/tests/api/contrib/stubs.py +++ b/manila/tests/api/contrib/stubs.py @@ -41,6 +41,7 @@ def stub_share(id, **kwargs): 'share_type_id': '1', 'is_public': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_type': None, 'has_replicas': False, } diff --git a/manila/tests/api/v1/test_share_types_extra_specs.py b/manila/tests/api/v1/test_share_types_extra_specs.py index 40e89c075e..e84cf5400b 100644 --- a/manila/tests/api/v1/test_share_types_extra_specs.py +++ b/manila/tests/api/v1/test_share_types_extra_specs.py @@ -32,6 +32,7 @@ import manila.wsgi DRIVER_HANDLES_SHARE_SERVERS = ( constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS) +SNAPSHOT_SUPPORT = constants.ExtraSpecs.SNAPSHOT_SUPPORT def return_create_share_type_extra_specs(context, share_type_id, extra_specs): @@ -149,15 +150,23 @@ class ShareTypesExtraSpecsTest(test.TestCase): self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'show') - def test_delete(self): + @ddt.data( + ('1.0', 'key5'), + ('2.23', 'key5'), + ('2.24', 'key5'), + ('2.24', SNAPSHOT_SUPPORT), + ) + @ddt.unpack + def test_delete(self, version, key): self.mock_object(manila.db, 'share_type_extra_specs_delete', delete_share_type_extra_specs) self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - req = fakes.HTTPRequest.blank(self.api_path + '/key5') + req = fakes.HTTPRequest.blank(self.api_path + '/' + key, + version=version) req_context = req.environ['manila.context'] - self.controller.delete(req, 1, 'key5') + self.controller.delete(req, 1, key) self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'delete') @@ -174,13 +183,22 @@ class ShareTypesExtraSpecsTest(test.TestCase): self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'delete') - def test_delete_forbidden(self): + @ddt.data( + ('1.0', DRIVER_HANDLES_SHARE_SERVERS), + ('1.0', SNAPSHOT_SUPPORT), + ('2.23', DRIVER_HANDLES_SHARE_SERVERS), + ('2.23', SNAPSHOT_SUPPORT), + ('2.24', DRIVER_HANDLES_SHARE_SERVERS), + ) + @ddt.unpack + def test_delete_forbidden(self, version, key): req = fakes.HTTPRequest.blank( - self.api_path + '/' + DRIVER_HANDLES_SHARE_SERVERS) + self.api_path + '/' + key, version=version) req_context = req.environ['manila.context'] - self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete, - req, 1, DRIVER_HANDLES_SHARE_SERVERS) + self.assertRaises(webob.exc.HTTPForbidden, + self.controller.delete, + req, 1, key) self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'delete') @@ -213,12 +231,18 @@ class ShareTypesExtraSpecsTest(test.TestCase): self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'create') - def test_create_with_too_small_key(self): + @ddt.data( + {"": "value"}, + {"k" * 256: "value"}, + {"key": ""}, + {"key": "v" * 256}, + {constants.ExtraSpecs.SNAPSHOT_SUPPORT: "non_boolean"}, + ) + def test_create_with_invalid_extra_specs(self, extra_specs): self.mock_object( manila.db, 'share_type_extra_specs_update_or_create', mock.Mock(return_value=return_create_share_type_extra_specs)) - too_small_key = "" - body = {"extra_specs": {too_small_key: "value"}} + body = {"extra_specs": extra_specs} self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) req = fakes.HTTPRequest.blank(self.api_path) req_context = req.environ['manila.context'] @@ -231,58 +255,6 @@ class ShareTypesExtraSpecsTest(test.TestCase): self.mock_policy_check.assert_called_once_with( req_context, self.resource_name, 'create') - def test_create_with_too_big_key(self): - self.mock_object( - manila.db, 'share_type_extra_specs_update_or_create', - mock.Mock(return_value=return_create_share_type_extra_specs)) - too_big_key = "k" * 256 - body = {"extra_specs": {too_big_key: "value"}} - self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - req = fakes.HTTPRequest.blank(self.api_path) - req_context = req.environ['manila.context'] - - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, 1, body) - - self.assertFalse( - manila.db.share_type_extra_specs_update_or_create.called) - self.mock_policy_check.assert_called_once_with( - req_context, self.resource_name, 'create') - - def test_create_with_too_small_value(self): - self.mock_object( - manila.db, 'share_type_extra_specs_update_or_create', - mock.Mock(return_value=return_create_share_type_extra_specs)) - too_small_value = "" - body = {"extra_specs": {"key": too_small_value}} - self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - req = fakes.HTTPRequest.blank(self.api_path) - req_context = req.environ['manila.context'] - - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, 1, body) - self.mock_policy_check.assert_called_once_with( - req_context, self.resource_name, 'create') - self.assertFalse( - manila.db.share_type_extra_specs_update_or_create.called) - - def test_create_with_too_big_value(self): - self.mock_object( - manila.db, 'share_type_extra_specs_update_or_create', - mock.Mock(return_value=return_create_share_type_extra_specs)) - too_big_value = "v" * 256 - body = {"extra_specs": {"key": too_big_value}} - self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - req = fakes.HTTPRequest.blank(self.api_path) - req_context = req.environ['manila.context'] - - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, req, 1, body) - self.mock_policy_check.assert_called_once_with( - req_context, self.resource_name, 'create') - self.assertFalse( - manila.db.share_type_extra_specs_update_or_create.called) - def test_create_key_allowed_chars(self): mock_return_value = {"key1": "value1", "key2": "value2", diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index 7e66d70437..f46d73f6ac 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -289,9 +289,11 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'create', create_mock) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=True) self.mock_object(share_api.API, 'get', mock.Mock( - return_value=mock.Mock( - instance={'share_network_id': parent_share_net}))) + return_value=parent_share)) self.mock_object(share_api.API, 'get_share_network', mock.Mock( return_value={'id': parent_share_net})) @@ -327,9 +329,11 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'create', create_mock) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=True) self.mock_object(share_api.API, 'get', mock.Mock( - return_value=mock.Mock( - instance={'share_network_id': parent_share_net}))) + return_value=parent_share)) self.mock_object(share_api.API, 'get_share_network', mock.Mock( return_value={'id': parent_share_net})) @@ -360,6 +364,52 @@ class ShareAPITest(test.TestCase): req, body) + @ddt.data("1.0", "2.0") + def test_share_create_from_snapshot_not_supported(self, microversion): + # This create operation should work, because the 1.0 API doesn't check + # create_share_from_snapshot_support. + + parent_share_net = 444 + shr = { + "size": 100, + "name": "Share Test Name", + "description": "Share Test Desc", + "share_proto": "fakeproto", + "availability_zone": "zone1:host1", + "snapshot_id": 333, + "share_network_id": parent_share_net + } + create_mock = mock.Mock(return_value=stubs.stub_share('1', + display_name=shr['name'], + display_description=shr['description'], + size=shr['size'], + share_proto=shr['share_proto'].upper(), + snapshot_id=shr['snapshot_id'], + instance=dict( + availability_zone=shr['availability_zone'], + share_network_id=shr['share_network_id']))) + self.mock_object(share_api.API, 'create', create_mock) + self.mock_object(share_api.API, 'get_snapshot', + stubs.stub_snapshot_get) + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=False) + self.mock_object(share_api.API, 'get', mock.Mock( + return_value=parent_share)) + self.mock_object(share_api.API, 'get_share_network', mock.Mock( + return_value={'id': parent_share_net})) + + body = {"share": copy.deepcopy(shr)} + req = fakes.HTTPRequest.blank('/shares', version=microversion) + + res_dict = self.controller.create(req, body) + + expected = self._get_expected_share_detailed_response(shr) + expected['share'].pop('snapshot_support') + self.assertDictEqual(expected, res_dict) + self.assertEqual(parent_share_net, + create_mock.call_args[1]['share_network_id']) + def test_share_creation_fails_with_bad_size(self): shr = {"size": '', "name": "Share Test Name", diff --git a/manila/tests/api/v2/test_share_types.py b/manila/tests/api/v2/test_share_types.py index ef0750344e..a683d00d24 100644 --- a/manila/tests/api/v2/test_share_types.py +++ b/manila/tests/api/v2/test_share_types.py @@ -123,14 +123,18 @@ class ShareTypesAPITest(test.TestCase): mock.Mock(return_value=True)) fake_notifier.reset() self.addCleanup(fake_notifier.reset) - self.mock_object(share_types, 'create', - return_share_types_create) - self.mock_object(share_types, 'get_share_type_by_name', - return_share_types_get_by_name) - self.mock_object(share_types, 'get_share_type', - return_share_types_get_share_type) - self.mock_object(share_types, 'destroy', - return_share_types_destroy) + self.mock_object( + share_types, 'create', + mock.Mock(side_effect=return_share_types_create)) + self.mock_object( + share_types, 'get_share_type_by_name', + mock.Mock(side_effect=return_share_types_get_by_name)) + self.mock_object( + share_types, 'get_share_type', + mock.Mock(side_effect=return_share_types_get_share_type)) + self.mock_object( + share_types, 'destroy', + mock.Mock(side_effect=return_share_types_destroy)) @ddt.data(True, False) def test_share_types_index(self, admin): @@ -217,13 +221,21 @@ class ShareTypesAPITest(test.TestCase): req.environ['manila.context'], self.resource_name, 'default') @ddt.data( - ('1.0', 'os-share-type-access'), - ('2.0', 'os-share-type-access'), - ('2.6', 'os-share-type-access'), - ('2.7', 'share_type_access'), + ('1.0', 'os-share-type-access', True), + ('1.0', 'os-share-type-access', False), + ('2.0', 'os-share-type-access', True), + ('2.0', 'os-share-type-access', False), + ('2.6', 'os-share-type-access', True), + ('2.6', 'os-share-type-access', False), + ('2.7', 'share_type_access', True), + ('2.7', 'share_type_access', False), + ('2.23', 'share_type_access', True), + ('2.23', 'share_type_access', False), + ('2.24', 'share_type_access', True), + ('2.24', 'share_type_access', False), ) @ddt.unpack - def test_view_builder_show(self, version, prefix): + def test_view_builder_show(self, version, prefix, admin): view_builder = views_types.ViewBuilder() now = timeutils.utcnow().isoformat() @@ -238,7 +250,8 @@ class ShareTypesAPITest(test.TestCase): id=42, ) - request = fakes.HTTPRequest.blank("/v%s" % version[0], version=version) + request = fakes.HTTPRequest.blank("/v%s" % version[0], version=version, + use_admin_context=admin) request.headers['X-Openstack-Manila-Api-Version'] = version output = view_builder.show(request, raw_share_type) @@ -251,11 +264,36 @@ class ShareTypesAPITest(test.TestCase): 'required_extra_specs': {}, 'id': 42, } + if self.is_microversion_ge(version, '2.24') and not admin: + for extra_spec in constants.ExtraSpecs.INFERRED_OPTIONAL_MAP: + expected_share_type['extra_specs'][extra_spec] = ( + constants.ExtraSpecs.INFERRED_OPTIONAL_MAP[extra_spec]) + self.assertDictMatch(output['share_type'], expected_share_type) - def test_view_builder_list(self): + @ddt.data( + ('1.0', 'os-share-type-access', True), + ('1.0', 'os-share-type-access', False), + ('2.0', 'os-share-type-access', True), + ('2.0', 'os-share-type-access', False), + ('2.6', 'os-share-type-access', True), + ('2.6', 'os-share-type-access', False), + ('2.7', 'share_type_access', True), + ('2.7', 'share_type_access', False), + ('2.23', 'share_type_access', True), + ('2.23', 'share_type_access', False), + ('2.24', 'share_type_access', True), + ('2.24', 'share_type_access', False), + ) + @ddt.unpack + def test_view_builder_list(self, version, prefix, admin): view_builder = views_types.ViewBuilder() + extra_specs = { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: False, + } + now = timeutils.utcnow().isoformat() raw_share_types = [] for i in range(0, 10): @@ -265,22 +303,23 @@ class ShareTypesAPITest(test.TestCase): deleted=False, created_at=now, updated_at=now, - extra_specs={}, + extra_specs=extra_specs, required_extra_specs={}, deleted_at=None, id=42 + i ) ) - request = fakes.HTTPRequest.blank("/v2") + request = fakes.HTTPRequest.blank("/v%s" % version[0], version=version, + use_admin_context=admin) output = view_builder.index(request, raw_share_types) self.assertIn('share_types', output) for i in range(0, 10): expected_share_type = { 'name': 'new_type', - 'extra_specs': {}, - 'os-share-type-access:is_public': True, + 'extra_specs': extra_specs, + '%s:is_public' % prefix: True, 'required_extra_specs': {}, 'id': 42 + i, } @@ -310,6 +349,18 @@ class ShareTypesAPITest(test.TestCase): req, '777') self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + def test_share_types_delete_in_use(self): + + req = fakes.HTTPRequest.blank('/v2/fake/types/1') + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + side_effect = exception.ShareTypeInUse(share_type_id='fake_id') + self.mock_object(share_types, 'destroy', + mock.Mock(side_effect=side_effect)) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._delete, + req, 1) + def test_share_types_with_volumes_destroy(self): req = fakes.HTTPRequest.blank('/v2/fake/types/1') self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) @@ -317,21 +368,53 @@ class ShareTypesAPITest(test.TestCase): self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) @ddt.data(make_create_body("share_type_1"), - make_create_body(spec_driver_handles_share_servers="false"), - make_create_body(spec_driver_handles_share_servers="true"), - make_create_body(spec_driver_handles_share_servers="1"), - make_create_body(spec_driver_handles_share_servers="0"), - make_create_body(spec_driver_handles_share_servers="True"), - make_create_body(spec_driver_handles_share_servers="False"), - make_create_body(spec_driver_handles_share_servers="FalsE")) - def test_create(self, body): - req = fakes.HTTPRequest.blank('/v2/fake/types') + make_create_body(spec_driver_handles_share_servers=True), + make_create_body(spec_driver_handles_share_servers=False)) + def test_create_2_23(self, body): + + req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.23") self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - res_dict = self.controller._create(req, body) + + res_dict = self.controller.create(req, body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) self.assertEqual(2, len(res_dict)) self.assertEqual('share_type_1', res_dict['share_type']['name']) self.assertEqual('share_type_1', res_dict['volume_type']['name']) + for extra_spec in constants.ExtraSpecs.REQUIRED: + self.assertIn(extra_spec, + res_dict['share_type']['required_extra_specs']) + expected_extra_specs = { + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: True, + constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, + } + expected_extra_specs.update(body['share_type']['extra_specs']) + share_types.create.assert_called_once_with( + mock.ANY, body['share_type']['name'], expected_extra_specs, True) + + @ddt.data(make_create_body("share_type_1"), + make_create_body(spec_driver_handles_share_servers=True), + make_create_body(spec_driver_handles_share_servers=False)) + def test_create_2_24(self, body): + + req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.24") + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + + res_dict = self.controller.create(req, body) + + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + self.assertEqual(2, len(res_dict)) + self.assertEqual('share_type_1', res_dict['share_type']['name']) + self.assertEqual('share_type_1', res_dict['volume_type']['name']) + for extra_spec in constants.ExtraSpecs.REQUIRED: + self.assertIn(extra_spec, + res_dict['share_type']['required_extra_specs']) + expected_extra_specs = { + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: True, + } + expected_extra_specs.update(body['share_type']['extra_specs']) + share_types.create.assert_called_once_with( + mock.ANY, body['share_type']['name'], expected_extra_specs, True) @ddt.data(None, make_create_body(""), @@ -342,13 +425,52 @@ class ShareTypesAPITest(test.TestCase): make_create_body(spec_driver_handles_share_servers=""), make_create_body(spec_driver_handles_share_servers=[]), ) - def test_create_invalid_request(self, body): - req = fakes.HTTPRequest.blank('/v2/fake/types') + def test_create_invalid_request_1_0(self, body): + req = fakes.HTTPRequest.blank('/v2/fake/types', version="1.0") self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) self.assertRaises(webob.exc.HTTPBadRequest, - self.controller._create, req, body) + self.controller.create, req, body) self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + @ddt.data(*constants.ExtraSpecs.REQUIRED) + def test_create_invalid_request_2_23(self, required_extra_spec): + + req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.24") + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + body = make_create_body("share_type_1") + del body['share_type']['extra_specs'][required_extra_spec] + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, req, body) + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + + def test_create_already_exists(self): + + side_effect = exception.ShareTypeExists(id='fake_id') + self.mock_object(share_types, 'create', + mock.Mock(side_effect=side_effect)) + + req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.24") + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + body = make_create_body('share_type_1') + + self.assertRaises(webob.exc.HTTPConflict, + self.controller.create, req, body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + + def test_create_not_found(self): + + self.mock_object(share_types, 'create', + mock.Mock(side_effect=exception.NotFound)) + + req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.24") + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + body = make_create_body('share_type_1') + + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.create, req, body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + def assert_share_type_list_equal(self, expected, observed): self.assertEqual(len(expected), len(observed)) expected = sorted(expected, key=lambda item: item['id']) @@ -429,6 +551,7 @@ class FakeRequest(object): return SHARE_TYPES[resource_id] +@ddt.ddt class ShareTypeAccessTest(test.TestCase): def setUp(self): @@ -466,15 +589,23 @@ class ShareTypeAccessTest(test.TestCase): self.assertEqual(expected, result) def test_list_with_no_context(self): - req = fakes.HTTPRequest.blank('/v1/types/fake/types') - def fake_authorize(context, target=None, action=None): - raise exception.PolicyNotAuthorized(action='index') + req = fakes.HTTPRequest.blank('/v1/types/fake/types') self.assertRaises(webob.exc.HTTPForbidden, self.controller.share_type_access, req, 'fake') + def test_list_not_found(self): + + side_effect = exception.ShareTypeNotFound(share_type_id='fake_id') + self.mock_object(share_types, 'get_share_type', + mock.Mock(side_effect=side_effect)) + + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.share_type_access, + self.req, 'fake') + def test_list_type_with_admin_default_proj1(self): expected = {'share_types': [{'id': '0'}, {'id': '1'}]} req = fakes.HTTPRequest.blank('/v1/fake/types', use_admin_context=True) @@ -592,6 +723,17 @@ class ShareTypeAccessTest(test.TestCase): self.assertEqual(202, result.status_code) + @ddt.data({'addProjectAccess': {'project': 'fake_project'}}, + {'invalid': {'project': PROJ2_UUID}}) + def test_add_project_access_bad_request(self, body): + + req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', + use_admin_context=True) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._add_project_access, + req, '2', body) + def test_add_project_access_with_no_admin_user(self): req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', use_admin_context=False) @@ -630,6 +772,32 @@ class ShareTypeAccessTest(test.TestCase): share_types.get_share_type.assert_called_once_with( mock.ANY, share_type_id) + def test_remove_project_access(self): + + share_type = stub_share_type(2) + share_type['is_public'] = False + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=share_type)) + self.mock_object(share_types, 'remove_share_type_access') + body = {'removeProjectAccess': {'project': PROJ2_UUID}} + req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', + use_admin_context=True) + + result = self.controller._remove_project_access(req, '2', body) + + self.assertEqual(202, result.status_code) + + @ddt.data({'removeProjectAccess': {'project': 'fake_project'}}, + {'invalid': {'project': PROJ2_UUID}}) + def test_remove_project_access_bad_request(self, body): + + req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', + use_admin_context=True) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._remove_project_access, + req, '2', body) + def test_remove_project_access_with_bad_access(self): def stub_remove_share_type_access(context, type_id, project_id): raise exception.ShareTypeAccessNotFound(share_type_id=type_id, diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index 712b893d70..89fca6c3ba 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -691,9 +691,11 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'create', create_mock) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=True) self.mock_object(share_api.API, 'get', mock.Mock( - return_value=mock.Mock( - instance={'share_network_id': parent_share_net}))) + return_value=parent_share)) self.mock_object(share_api.API, 'get_share_network', mock.Mock( return_value={'id': parent_share_net})) @@ -714,7 +716,7 @@ class ShareAPITest(test.TestCase): "share_proto": "fakeproto", "availability_zone": "zone1:host1", "snapshot_id": 333, - "share_network_id": parent_share_net + "share_network_id": parent_share_net, } create_mock = mock.Mock(return_value=stubs.stub_share('1', display_name=shr['name'], @@ -728,9 +730,11 @@ class ShareAPITest(test.TestCase): self.mock_object(share_api.API, 'create', create_mock) self.mock_object(share_api.API, 'get_snapshot', stubs.stub_snapshot_get) + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=True) self.mock_object(share_api.API, 'get', mock.Mock( - return_value=mock.Mock( - instance={'share_network_id': parent_share_net}))) + return_value=parent_share)) self.mock_object(share_api.API, 'get_share_network', mock.Mock( return_value={'id': parent_share_net})) @@ -751,13 +755,38 @@ class ShareAPITest(test.TestCase): "share_proto": "fakeproto", "availability_zone": "zone1:host1", "snapshot_id": 333, - "share_network_id": 1234 + "share_network_id": 1234, } body = {"share": shr} req = fakes.HTTPRequest.blank('/shares', version='2.7') self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, body) + def test_share_create_from_snapshot_not_supported(self): + parent_share_net = 444 + self.mock_object(share_api.API, 'create') + shr = { + "size": 100, + "name": "Share Test Name", + "description": "Share Test Desc", + "share_proto": "fakeproto", + "availability_zone": "zone1:host1", + "snapshot_id": 333, + "share_network_id": parent_share_net, + } + parent_share = stubs.stub_share( + '1', instance={'share_network_id': parent_share_net}, + create_share_from_snapshot_support=False) + self.mock_object(share_api.API, 'get', mock.Mock( + return_value=parent_share)) + self.mock_object(share_api.API, 'get_share_network', mock.Mock( + return_value={'id': parent_share_net})) + + body = {"share": shr} + req = fakes.HTTPRequest.blank('/shares', version='2.24') + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, req, body) + def test_share_creation_fails_with_bad_size(self): shr = {"size": '', "name": "Share Test Name", diff --git a/manila/tests/api/views/test_shares.py b/manila/tests/api/views/test_shares.py index aeae3ea096..d55b1a1090 100644 --- a/manila/tests/api/views/test_shares.py +++ b/manila/tests/api/views/test_shares.py @@ -13,36 +13,69 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt + from manila.api.views import shares from manila import test +from manila.tests.api.contrib import stubs from manila.tests.api import fakes +@ddt.ddt class ViewBuilderTestCase(test.TestCase): def setUp(self): super(ViewBuilderTestCase, self).setUp() self.builder = shares.ViewBuilder() - self.req = fakes.HTTPRequest.blank('/shares', version="2.6") + self.fake_share = self._get_fake_share() + + def _get_fake_share(self): + + fake_share = { + 'share_type_id': 'fake_share_type_id', + 'share_type': { + 'name': 'fake_share_type_name', + }, + 'export_location': 'fake_export_location', + 'export_locations': ['fake_export_location'], + 'access_rules_status': 'fake_rule_status', + 'instance': {}, + 'replication_type': 'fake_replication_type', + 'has_replicas': False, + 'user_id': 'fake_userid', + 'snapshot_support': True, + 'create_share_from_snapshot_support': True, + } + return stubs.stub_share('fake_id', **fake_share) def test__collection_name(self): self.assertEqual('shares', self.builder._collection_name) - def test_detail_v_2_6(self): - fake_share = { - 'id': 'fake_id', - 'share_type_id': 'fake_share_type_id', - 'share_type': {'name': 'fake_share_type_name'}, - 'instance': {}, + @ddt.data('2.6', '2.9', '2.10', '2.11', '2.16', '2.24') + def test_detail(self, microversion): + req = fakes.HTTPRequest.blank('/shares', version=microversion) + + result = self.builder.detail(req, self.fake_share) + + expected = { + 'id': self.fake_share['id'], + 'share_type': self.fake_share['share_type_id'], + 'share_type_name': self.fake_share['share_type']['name'], + 'export_location': 'fake_export_location', + 'export_locations': ['fake_export_location'], + 'snapshot_support': True, } + if self.is_microversion_ge(microversion, '2.9'): + expected.pop('export_location') + expected.pop('export_locations') + if self.is_microversion_ge(microversion, '2.10'): + expected['access_rules_status'] = 'fake_rule_status' + if self.is_microversion_ge(microversion, '2.11'): + expected['replication_type'] = 'fake_replication_type' + expected['has_replicas'] = False + if self.is_microversion_ge(microversion, '2.16'): + expected['user_id'] = 'fake_userid' + if self.is_microversion_ge(microversion, '2.24'): + expected['create_share_from_snapshot_support'] = True - actual_result = self.builder.detail(self.req, fake_share) - - self.assertSubDictMatch( - { - 'id': fake_share['id'], - 'share_type': fake_share['share_type_id'], - 'share_type_name': fake_share['share_type']['name'], - }, - actual_result['share'] - ) + self.assertSubDictMatch(expected, result['share']) diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py index 5a0d9bfcd2..7ad256d8b1 100644 --- a/manila/tests/db/migrations/alembic/migrations_data_checks.py +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -40,6 +40,7 @@ from oslo_utils import uuidutils import six from sqlalchemy import exc as sa_exc +from manila.common import constants from manila.db.migrations import utils @@ -121,6 +122,42 @@ class BaseMigrationChecks(object): """ +def fake_share(**kwargs): + share = { + 'id': uuidutils.generate_uuid(), + 'display_name': 'fake_share', + 'display_description': 'my fake share', + 'snapshot_id': uuidutils.generate_uuid(), + 'share_proto': 'nfs', + 'is_public': '1', + 'size': '1', + 'deleted': 'False', + 'share_proto': 'fake_proto', + 'user_id': uuidutils.generate_uuid(), + 'project_id': uuidutils.generate_uuid(), + 'snapshot_support': '1', + 'task_state': None, + } + share.update(kwargs) + return share + + +def fake_instance(share_id=None, **kwargs): + instance = { + 'id': uuidutils.generate_uuid(), + 'share_id': share_id or uuidutils.generate_uuid(), + 'deleted': 'False', + 'host': 'openstack@BackendZ#PoolA', + 'status': 'available', + 'scheduled_at': datetime.datetime(2015, 8, 10, 0, 5, 58), + 'launched_at': datetime.datetime(2015, 8, 10, 0, 5, 58), + 'terminated_at': None, + 'access_rules_status': 'active', + } + instance.update(kwargs) + return instance + + @map_to_migration('38e632621e5a') class ShareTypeMigrationChecks(BaseMigrationChecks): def _get_fake_data(self): @@ -467,36 +504,6 @@ class ShareReplicationMigrationChecks(BaseMigrationChecks): return shares, share_instances - def _new_share(self, **kwargs): - share = { - 'id': uuidutils.generate_uuid(), - 'display_name': 'fake_share', - 'size': '1', - 'deleted': 'False', - 'share_proto': 'fake_proto', - 'user_id': 'fake_user_id', - 'project_id': 'fake_project_uuid', - 'snapshot_support': '1', - 'task_state': None, - } - share.update(kwargs) - return share - - def _new_instance(self, share_id=None, **kwargs): - instance = { - 'id': uuidutils.generate_uuid(), - 'share_id': share_id or uuidutils.generate_uuid(), - 'deleted': 'False', - 'host': 'openstack@BackendZ#PoolA', - 'status': 'available', - 'scheduled_at': datetime.datetime(2015, 8, 10, 0, 5, 58), - 'launched_at': datetime.datetime(2015, 8, 10, 0, 5, 58), - 'terminated_at': None, - 'access_rules_status': 'active', - } - instance.update(kwargs) - return instance - def setup_upgrade_data(self, engine): shares_data = [] @@ -504,9 +511,9 @@ class ShareReplicationMigrationChecks(BaseMigrationChecks): self.valid_share_ids = [] for share_display_name in self.valid_share_display_names: - share_ref = self._new_share(display_name=share_display_name) + share_ref = fake_share(display_name=share_display_name) shares_data.append(share_ref) - instances_data.append(self._new_instance(share_id=share_ref['id'])) + instances_data.append(fake_instance(share_id=share_ref['id'])) shares_table = utils.load_table('shares', engine) @@ -1172,3 +1179,155 @@ class MoveShareTypeIdToInstancesCheck(BaseMigrationChecks): next((x for x in self.some_shares if share['id'] == x['id']), None)['share_type_id'], share['share_type_id']) + + @map_to_migration('3e7d62517afa') + class CreateFromSnapshotExtraSpecAndShareColumn(BaseMigrationChecks): + + expected_attr = constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT + snap_support_attr = constants.ExtraSpecs.SNAPSHOT_SUPPORT + + def _get_fake_data(self): + extra_specs = [] + shares = [] + share_instances = [] + share_types = [ + { + 'id': uuidutils.generate_uuid(), + 'deleted': 'False', + 'name': 'share-type-1', + 'is_public': False, + }, + { + 'id': uuidutils.generate_uuid(), + 'deleted': 'False', + 'name': 'share-type-2', + 'is_public': True, + + }, + ] + snapshot_support = ('0', '1') + dhss = ('True', 'False') + for idx, share_type in enumerate(share_types): + extra_specs.append({ + 'share_type_id': share_type['id'], + 'spec_key': 'snapshot_support', + 'spec_value': snapshot_support[idx], + 'deleted': 0, + }) + extra_specs.append({ + 'share_type_id': share_type['id'], + 'spec_key': 'driver_handles_share_servers', + 'spec_value': dhss[idx], + 'deleted': 0, + }) + share = fake_share(snapshot_support=snapshot_support[idx]) + shares.append(share) + share_instances.append( + fake_instance(share_id=share['id'], + share_type_id=share_type['id']) + ) + + return share_types, extra_specs, shares, share_instances + + def setup_upgrade_data(self, engine): + + (self.share_types, self.extra_specs, self.shares, + self.share_instances) = self._get_fake_data() + + share_types_table = utils.load_table('share_types', engine) + engine.execute(share_types_table.insert(self.share_types)) + extra_specs_table = utils.load_table('share_type_extra_specs', + engine) + engine.execute(extra_specs_table.insert(self.extra_specs)) + shares_table = utils.load_table('shares', engine) + engine.execute(shares_table.insert(self.shares)) + share_instances_table = utils.load_table('share_instances', engine) + engine.execute(share_instances_table.insert(self.share_instances)) + + def check_upgrade(self, engine, data): + share_type_ids = [st['id'] for st in self.share_types] + share_ids = [s['id'] for s in self.shares] + shares_table = utils.load_table('shares', engine) + share_types_table = utils.load_table('share_types', engine) + extra_specs_table = utils.load_table('share_type_extra_specs', + engine) + + # Pre-existing Shares must be present + shares_in_db = engine.execute(shares_table.select()).fetchall() + share_ids_in_db = [s['id'] for s in shares_in_db] + for share_id in share_ids: + self.test_case.assertIn(share_id, share_ids_in_db) + + # new shares attr must match snapshot support + for share in shares_in_db: + self.test_case.assertTrue(hasattr(share, self.expected_attr)) + self.test_case.assertEqual(share[self.snap_support_attr], + share[self.expected_attr]) + + # Pre-existing Share types must be present + share_types_in_db = ( + engine.execute(share_types_table.select()).fetchall()) + share_type_ids_in_db = [s['id'] for s in share_types_in_db] + for share_type_id in share_type_ids: + self.test_case.assertIn(share_type_id, share_type_ids_in_db) + + # Pre-existing extra specs must be present + extra_specs_in_db = ( + engine.execute(extra_specs_table.select().where( + extra_specs_table.c.deleted == 0)).fetchall()) + self.test_case.assertGreaterEqual(len(extra_specs_in_db), + len(self.extra_specs)) + + # New Extra spec for share types must match snapshot support + for share_type_id in share_type_ids: + new_extra_spec = [x for x in extra_specs_in_db + if x['spec_key'] == self.expected_attr + and x['share_type_id'] == share_type_id] + snapshot_support_spec = [ + x for x in extra_specs_in_db + if x['spec_key'] == self.snap_support_attr + and x['share_type_id'] == share_type_id] + self.test_case.assertEqual(1, len(new_extra_spec)) + self.test_case.assertEqual(1, len(snapshot_support_spec)) + self.test_case.assertEqual( + snapshot_support_spec[0]['spec_value'], + new_extra_spec[0]['spec_value']) + + def check_downgrade(self, engine): + share_type_ids = [st['id'] for st in self.share_types] + share_ids = [s['id'] for s in self.shares] + shares_table = utils.load_table('shares', engine) + share_types_table = utils.load_table('share_types', engine) + extra_specs_table = utils.load_table('share_type_extra_specs', + engine) + + # Pre-existing Shares must be present + shares_in_db = engine.execute(shares_table.select()).fetchall() + share_ids_in_db = [s['id'] for s in shares_in_db] + for share_id in share_ids: + self.test_case.assertIn(share_id, share_ids_in_db) + + # Shares should have no attr to create share from snapshot + for share in shares_in_db: + self.test_case.assertFalse(hasattr(share, self.expected_attr)) + + # Pre-existing Share types must be present + share_types_in_db = ( + engine.execute(share_types_table.select()).fetchall()) + share_type_ids_in_db = [s['id'] for s in share_types_in_db] + for share_type_id in share_type_ids: + self.test_case.assertIn(share_type_id, share_type_ids_in_db) + + # Pre-existing extra specs must be present + extra_specs_in_db = ( + engine.execute(extra_specs_table.select().where( + extra_specs_table.c.deleted == 0)).fetchall()) + self.test_case.assertGreaterEqual(len(extra_specs_in_db), + len(self.extra_specs)) + + # Share types must not have create share from snapshot extra spec + for share_type_id in share_type_ids: + new_extra_spec = [x for x in extra_specs_in_db + if x['spec_key'] == self.expected_attr + and x['share_type_id'] == share_type_id] + self.test_case.assertEqual(0, len(new_extra_spec)) diff --git a/manila/tests/fake_share.py b/manila/tests/fake_share.py index c667a93fb4..0f0f19878f 100644 --- a/manila/tests/fake_share.py +++ b/manila/tests/fake_share.py @@ -76,7 +76,6 @@ def fake_share_type(**kwargs): 'is_public': False, 'extra_specs': { 'driver_handles_share_servers': 'False', - 'snapshot_support': 'True', } } diff --git a/manila/tests/scheduler/drivers/test_filter.py b/manila/tests/scheduler/drivers/test_filter.py index d365bfe2ac..61eaafb58d 100644 --- a/manila/tests/scheduler/drivers/test_filter.py +++ b/manila/tests/scheduler/drivers/test_filter.py @@ -443,7 +443,7 @@ class FilterSchedulerTestCase(test_base.SchedulerTestCase): fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) request_spec = {'share_types': [{'name': 'NFS', 'extra_specs': { - SNAPSHOT_SUPPORT: True, + SNAPSHOT_SUPPORT: 'True', }}]} hosts = sched._get_weighted_candidates_cg(fake_context, @@ -460,7 +460,7 @@ class FilterSchedulerTestCase(test_base.SchedulerTestCase): fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) request_spec = {'share_types': [{'name': 'NFS', 'extra_specs': { - SNAPSHOT_SUPPORT: False + SNAPSHOT_SUPPORT: 'False', }}]} hosts = sched._get_weighted_candidates_cg(fake_context, @@ -477,7 +477,7 @@ class FilterSchedulerTestCase(test_base.SchedulerTestCase): fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) request_spec = {'share_types': [{'name': 'NFS', 'extra_specs': { - SNAPSHOT_SUPPORT: True + SNAPSHOT_SUPPORT: 'True', }}]} hosts = sched._get_weighted_candidates_cg(fake_context, diff --git a/manila/tests/scheduler/fakes.py b/manila/tests/scheduler/fakes.py index f4431c44f9..0d704f9816 100644 --- a/manila/tests/scheduler/fakes.py +++ b/manila/tests/scheduler/fakes.py @@ -39,6 +39,7 @@ SERVICE_STATES_NO_POOLS = { max_over_subscription_ratio=1.0, thin_provisioning=False, snapshot_support=False, + create_share_from_snapshot_support=False, driver_handles_share_servers=False), 'host2@back1': dict(share_backend_name='BBB', total_capacity_gb=256, free_capacity_gb=100, @@ -47,6 +48,7 @@ SERVICE_STATES_NO_POOLS = { max_over_subscription_ratio=2.0, thin_provisioning=True, snapshot_support=True, + create_share_from_snapshot_support=True, driver_handles_share_servers=False), 'host2@back2': dict(share_backend_name='CCC', total_capacity_gb=10000, free_capacity_gb=700, @@ -55,6 +57,7 @@ SERVICE_STATES_NO_POOLS = { max_over_subscription_ratio=20.0, thin_provisioning=True, snapshot_support=True, + create_share_from_snapshot_support=True, driver_handles_share_servers=False), } @@ -79,6 +82,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool1', total_capacity_gb=51, @@ -91,6 +95,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool2', total_capacity_gb=52, @@ -103,6 +108,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool3', total_capacity_gb=53, @@ -116,6 +122,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool4a', total_capacity_gb=541, @@ -137,6 +144,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool5a', total_capacity_gb=551, @@ -156,6 +164,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, snapshot_support=True, + create_share_from_snapshot_support=True, replication_type=None, pools=[dict(pool_name='pool6a', total_capacity_gb='unknown', @@ -192,6 +201,7 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 10, 'timestamp': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_type': 'writable', 'replication_domain': 'endor', }, @@ -204,6 +214,7 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 10, 'timestamp': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_type': 'readable', 'replication_domain': 'kashyyyk', }, @@ -216,6 +227,7 @@ class FakeHostManager(host_manager.HostManager): 'consistency_group_support': 'host', 'reserved_percentage': 0, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'timestamp': None, }, 'host4': {'total_capacity_gb': 2048, @@ -227,6 +239,7 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 5, 'timestamp': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_type': 'dr', 'replication_domain': 'naboo', }, @@ -239,6 +252,7 @@ class FakeHostManager(host_manager.HostManager): 'reserved_percentage': 5, 'timestamp': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': 'pool', 'replication_type': None, }, @@ -248,6 +262,7 @@ class FakeHostManager(host_manager.HostManager): 'thin_provisioning': False, 'reserved_percentage': 5, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'timestamp': None, }, } diff --git a/manila/tests/scheduler/test_host_manager.py b/manila/tests/scheduler/test_host_manager.py index 5b575240dd..a926a11e89 100644 --- a/manila/tests/scheduler/test_host_manager.py +++ b/manila/tests/scheduler/test_host_manager.py @@ -210,6 +210,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': False, + 'create_share_from_snapshot_support': False, 'consistency_group_support': False, 'dedupe': False, 'compression': False, @@ -235,6 +236,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': False, 'dedupe': False, 'compression': False, @@ -260,6 +262,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': False, 'dedupe': False, 'compression': False, @@ -307,6 +310,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': False, 'dedupe': False, 'compression': False, @@ -333,6 +337,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': False, 'dedupe': False, 'compression': False, @@ -359,6 +364,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': 'pool', 'dedupe': False, 'compression': False, @@ -385,6 +391,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': 'host', 'dedupe': False, 'compression': False, @@ -411,6 +418,7 @@ class HostManagerTestCase(test.TestCase): 'storage_protocol': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'consistency_group_support': 'host', 'dedupe': False, 'compression': False, @@ -460,6 +468,7 @@ class HostManagerTestCase(test.TestCase): 'timestamp': None, 'driver_handles_share_servers': False, 'snapshot_support': False, + 'create_share_from_snapshot_support': False, 'share_backend_name': 'AAA', 'free_capacity_gb': 200, 'driver_version': None, @@ -485,6 +494,7 @@ class HostManagerTestCase(test.TestCase): 'timestamp': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'share_backend_name': 'BBB', 'free_capacity_gb': 100, 'driver_version': None, @@ -538,6 +548,7 @@ class HostManagerTestCase(test.TestCase): 'timestamp': None, 'driver_handles_share_servers': False, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'share_backend_name': 'BBB', 'free_capacity_gb': 42, 'driver_version': None, diff --git a/manila/tests/share/drivers/dell_emc/test_driver.py b/manila/tests/share/drivers/dell_emc/test_driver.py index 00e4383bdc..360f241d04 100644 --- a/manila/tests/share/drivers/dell_emc/test_driver.py +++ b/manila/tests/share/drivers/dell_emc/test_driver.py @@ -124,6 +124,7 @@ class EMCShareFrameworkTestCase(test.TestCase): data['qos'] = False data['pools'] = None data['snapshot_support'] = True + data['create_share_from_snapshot_support'] = True data['replication_domain'] = None data['filter_function'] = None data['goodness_function'] = None diff --git a/manila/tests/share/drivers/dummy.py b/manila/tests/share/drivers/dummy.py index ab8e53aa2d..a8fa569799 100644 --- a/manila/tests/share/drivers/dummy.py +++ b/manila/tests/share/drivers/dummy.py @@ -337,6 +337,7 @@ class DummyDriver(driver.ShareDriver): self.configuration.reserved_share_percentage, "consistency_group_support": "pool", "snapshot_support": True, + "create_share_from_snapshot_support": True, "driver_name": "Dummy", "pools": self._get_pools_info(), } diff --git a/manila/tests/share/drivers/glusterfs/test_glusterfs_native.py b/manila/tests/share/drivers/glusterfs/test_glusterfs_native.py index 55de3b13ba..005f76b01c 100644 --- a/manila/tests/share/drivers/glusterfs/test_glusterfs_native.py +++ b/manila/tests/share/drivers/glusterfs/test_glusterfs_native.py @@ -256,6 +256,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): 'free_capacity_gb': 'unknown', 'pools': None, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_domain': None, 'filter_function': None, 'goodness_function': None, diff --git a/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py b/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py index 78bb10c287..28c5dd7c0f 100644 --- a/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py +++ b/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py @@ -733,6 +733,7 @@ class HPE3ParDriverTestCase(test.TestCase): 'provisioned_capacity_gb': 0, 'share_backend_name': 'HPE_3PAR', 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'storage_protocol': 'NFS_CIFS', 'thin_provisioning': True, 'total_capacity_gb': 0, @@ -807,6 +808,7 @@ class HPE3ParDriverTestCase(test.TestCase): 'reserved_percentage': 0, 'provisioned_capacity_gb': expected_capacity}], 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_domain': None, 'filter_function': None, 'goodness_function': None, @@ -843,6 +845,7 @@ class HPE3ParDriverTestCase(test.TestCase): 'total_capacity_gb': 0, 'vendor_name': 'HPE', 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'replication_domain': None, 'filter_function': None, 'goodness_function': None, diff --git a/manila/tests/share/drivers/huawei/test_huawei_nas.py b/manila/tests/share/drivers/huawei/test_huawei_nas.py index f9134a5016..73a5118a71 100644 --- a/manila/tests/share/drivers/huawei/test_huawei_nas.py +++ b/manila/tests/share/drivers/huawei/test_huawei_nas.py @@ -2424,6 +2424,7 @@ class HuaweiShareDriverTestCase(test.TestCase): "free_capacity_gb": 0.0, "qos": True, "snapshot_support": snapshot_support, + "create_share_from_snapshot_support": snapshot_support, "replication_domain": None, "filter_function": None, "goodness_function": None, diff --git a/manila/tests/share/drivers/zfsonlinux/test_driver.py b/manila/tests/share/drivers/zfsonlinux/test_driver.py index 8d3a31975c..8bff2b2b5b 100644 --- a/manila/tests/share/drivers/zfsonlinux/test_driver.py +++ b/manila/tests/share/drivers/zfsonlinux/test_driver.py @@ -354,6 +354,7 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase): 'reserved_percentage': 0, 'share_backend_name': self.driver.backend_name, 'snapshot_support': True, + 'create_share_from_snapshot_support': True, 'storage_protocol': 'NFS', 'total_capacity_gb': 'unknown', 'vendor_name': 'Open Source', diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 7bb4074e0e..a8cf570519 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -596,6 +596,51 @@ class ShareAPITestCase(test.TestCase): self.assertSubDictMatch(share_data, db_api.share_create.call_args[0][1]) + @ddt.data( + {}, + { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, + }, + { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: False, + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: False, + }, + { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: False, + }, + { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: True, + } + ) + def test_create_default_snapshot_semantics(self, extra_specs): + share, share_data = self._setup_create_mocks(is_public=False) + az = share_data.pop('availability_zone') + share_type = fakes.fake_share_type(extra_specs=extra_specs) + + self.api.create( + self.context, + share_data['share_proto'], + share_data['size'], + share_data['display_name'], + share_data['display_description'], + availability_zone=az, + share_type=share_type + ) + + share['status'] = constants.STATUS_CREATING + share['host'] = None + + share_data.update(extra_specs) + if extra_specs.get('snapshot_support') is None: + share_data['snapshot_support'] = False + if extra_specs.get('create_share_from_snapshot_support') is None: + share_data['create_share_from_snapshot_support'] = False + + self.assertSubDictMatch(share_data, + db_api.share_create.call_args[0][1]) + @ddt.data('', 'fake', 'Truebar', 'Bartrue') def test_create_share_with_invalid_is_public_value(self, is_public): self.assertRaises(exception.InvalidParameterValue, @@ -767,6 +812,40 @@ class ShareAPITestCase(test.TestCase): self.assertFalse(mock_scheduler_rpcapi_call.called) self.assertFalse(mock_share_rpcapi_call.called) + def test_get_share_attributes_from_share_type(self): + + share_type = { + 'extra_specs': { + 'snapshot_support': True, + 'create_share_from_snapshot_support': False, + 'replication_type': 'dr', + } + } + + result = self.api._get_share_attributes_from_share_type(share_type) + + self.assertEqual(share_type['extra_specs'], result) + + @ddt.data({}, {'extra_specs': {}}, None) + def test_get_share_attributes_from_share_type_defaults(self, share_type): + + result = self.api._get_share_attributes_from_share_type(share_type) + + expected = { + 'snapshot_support': False, + 'create_share_from_snapshot_support': False, + 'replication_type': None, + } + self.assertEqual(expected, result) + + @ddt.data({'extra_specs': {'snapshot_support': 'fake'}}, + {'extra_specs': {'create_share_from_snapshot_support': 'fake'}}) + def test_get_share_attributes_from_share_type_invalid(self, share_type): + + self.assertRaises(exception.InvalidParameterValue, + self.api._get_share_attributes_from_share_type, + share_type) + @ddt.data('dr', 'readable', None) def test_manage_new(self, replication_type): share_data = { @@ -787,6 +866,7 @@ class ShareAPITestCase(test.TestCase): 'extra_specs': { 'snapshot_support': False, 'replication_type': replication_type, + 'create_share_from_snapshot_support': False, }, } @@ -811,6 +891,8 @@ class ShareAPITestCase(test.TestCase): 'status': constants.STATUS_MANAGING, 'scheduled_at': date, 'snapshot_support': fake_type['extra_specs']['snapshot_support'], + 'create_share_from_snapshot_support': + fake_type['extra_specs']['create_share_from_snapshot_support'], 'replication_type': replication_type, }) @@ -841,6 +923,7 @@ class ShareAPITestCase(test.TestCase): 'id': 'fake_type_id', 'extra_specs': { 'snapshot_support': False, + 'create_share_from_snapshot_support': False, }, } shares = [{'id': 'fake', 'status': status}] @@ -865,6 +948,10 @@ class ShareAPITestCase(test.TestCase): 'snapshot_support': kwargs.get( 'snapshot_support', share_type['extra_specs']['snapshot_support']), + 'create_share_from_snapshot_support': kwargs.get( + 'create_share_from_snapshot_support', + share_type['extra_specs'] + ['create_share_from_snapshot_support']), 'share_proto': kwargs.get('share_proto', share.get('share_proto')), 'share_type_id': share_type['id'], 'is_public': kwargs.get('is_public', share.get('is_public')), @@ -2032,6 +2119,7 @@ class ShareAPITestCase(test.TestCase): 'id': 'fake_type_id', 'extra_specs': { 'snapshot_support': False, + 'create_share_from_snapshot_support': False, 'driver_handles_share_servers': dhss, }, } @@ -2041,6 +2129,7 @@ class ShareAPITestCase(test.TestCase): 'id': 'fake_type_2_id', 'extra_specs': { 'snapshot_support': False, + 'create_share_from_snapshot_support': False, 'driver_handles_share_servers': dhss, }, } diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 71ef98a793..769be5b409 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -43,8 +43,7 @@ class ShareDriverWithExecuteMixin(driver.ShareDriver, driver.ExecuteMixin): @ddt.ddt class ShareDriverTestCase(test.TestCase): - _SNAPSHOT_METHOD_NAMES = ["create_snapshot", "delete_snapshot", - "create_share_from_snapshot"] + _SNAPSHOT_METHOD_NAMES = ["create_snapshot", "delete_snapshot"] def setUp(self): super(ShareDriverTestCase, self).setUp() @@ -257,7 +256,6 @@ class ShareDriverTestCase(test.TestCase): child_methods = { "create_snapshot": fake_method, "delete_snapshot": fake_method, - "create_share_from_snapshot": fake_method, } child_class_instance = type( "NotRedefined", (driver.ShareDriver, ), child_methods)(True) @@ -272,11 +270,8 @@ class ShareDriverTestCase(test.TestCase): ([], [], False), (_SNAPSHOT_METHOD_NAMES, [], True), (_SNAPSHOT_METHOD_NAMES, _SNAPSHOT_METHOD_NAMES, True), - (_SNAPSHOT_METHOD_NAMES[0:1], _SNAPSHOT_METHOD_NAMES[1:], - True), + (_SNAPSHOT_METHOD_NAMES[0:1], _SNAPSHOT_METHOD_NAMES[1:], True), ([], _SNAPSHOT_METHOD_NAMES, True), - (_SNAPSHOT_METHOD_NAMES[0:1], _SNAPSHOT_METHOD_NAMES[1:2], - False), ) @ddt.unpack def test_check_redefined_driver_methods(self, common_drv_meth_names, @@ -308,14 +303,7 @@ class ShareDriverTestCase(test.TestCase): (), ("create_snapshot"), ("delete_snapshot"), - ("create_share_from_snapshot"), - ("create_snapshot", "delete_snapshot"), - ("create_snapshot", "create_share_from_snapshot"), - ("delete_snapshot", "create_share_from_snapshot"), - ("create_snapshot", "delete_snapshot", - "create_share_from_snapshotFOO"), - ("create_snapshot", "delete_snapshot", - "FOOcreate_share_from_snapshot"), + ("create_snapshot", "delete_snapshotFOO"), ) def test_snapshot_support_absent(self, methods): driver.CONF.set_default('driver_handles_share_servers', True) @@ -329,8 +317,7 @@ class ShareDriverTestCase(test.TestCase): child_class_instance._update_share_stats() - self.assertEqual( - False, child_class_instance._stats["snapshot_support"]) + self.assertFalse(child_class_instance._stats["snapshot_support"]) self.assertTrue(child_class_instance.configuration.safe_get.called) @ddt.data(True, False) @@ -357,7 +344,6 @@ class ShareDriverTestCase(test.TestCase): child_methods = { "create_snapshot": fake_method, "delete_snapshot": fake_method, - "create_share_from_snapshot": fake_method, } child_class_instance = type( "NotRedefined", (driver.ShareDriver, ), child_methods)(True) @@ -371,6 +357,83 @@ class ShareDriverTestCase(test.TestCase): child_class_instance._stats["snapshot_support"]) self.assertTrue(child_class_instance.configuration.safe_get.called) + def test_create_share_from_snapshot_support_exists(self): + driver.CONF.set_default('driver_handles_share_servers', True) + fake_method = lambda *args, **kwargs: None + child_methods = { + "create_share_from_snapshot": fake_method, + "create_snapshot": fake_method, + "delete_snapshot": fake_method, + } + child_class_instance = type( + "NotRedefined", (driver.ShareDriver, ), child_methods)(True) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats() + + self.assertTrue( + child_class_instance._stats["create_share_from_snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data( + (), + ("create_snapshot"), + ("create_share_from_snapshotFOO"), + ) + def test_create_share_from_snapshot_support_absent(self, methods): + driver.CONF.set_default('driver_handles_share_servers', True) + fake_method = lambda *args, **kwargs: None + child_methods = {} + for method in methods: + child_methods[method] = fake_method + child_class_instance = type( + "NotRedefined", (driver.ShareDriver, ), child_methods)(True) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats() + + self.assertFalse( + child_class_instance._stats["create_share_from_snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data(True, False) + def test_create_share_from_snapshot_not_exists_and_set_explicitly( + self, creating_shares_from_snapshot_is_supported): + driver.CONF.set_default('driver_handles_share_servers', True) + child_class_instance = type( + "NotRedefined", (driver.ShareDriver, ), {})(True) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats({ + "create_share_from_snapshot_support": + creating_shares_from_snapshot_is_supported, + }) + + self.assertEqual( + creating_shares_from_snapshot_is_supported, + child_class_instance._stats["create_share_from_snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data(True, False) + def test_create_share_from_snapshot_exists_and_set_explicitly( + self, create_share_from_snapshot_supported): + driver.CONF.set_default('driver_handles_share_servers', True) + fake_method = lambda *args, **kwargs: None + child_methods = {"create_share_from_snapshot": fake_method} + child_class_instance = type( + "NotRedefined", (driver.ShareDriver, ), child_methods)(True) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats({ + "create_share_from_snapshot_support": + create_share_from_snapshot_supported, + }) + + self.assertEqual( + create_share_from_snapshot_supported, + child_class_instance._stats["create_share_from_snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + def test_get_periodic_hook_data(self): share_driver = self._instantiate_share_driver(None, False) share_instances = ["list", "of", "share", "instances"] diff --git a/manila/tests/share/test_share_types.py b/manila/tests/share/test_share_types.py index 3bdb203350..e2a8391ddb 100644 --- a/manila/tests/share/test_share_types.py +++ b/manila/tests/share/test_share_types.py @@ -18,9 +18,11 @@ """Test of Share Type methods for Manila.""" import copy import datetime +import itertools import ddt import mock +from oslo_utils import strutils from manila.common import constants from manila import context @@ -69,17 +71,22 @@ class ShareTypesTestCase(test.TestCase): } } + fake_required_extra_specs = { + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true', + } + + fake_optional_extra_specs = { + constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'true', + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: 'false', + } + fake_type_w_valid_extra = { 'test_with_extra': { 'created_at': datetime.datetime(2015, 1, 22, 11, 45, 31), 'deleted': '0', 'deleted_at': None, - 'extra_specs': { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true' - }, - 'required_extra_specs': { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true' - }, + 'extra_specs': fake_required_extra_specs, + 'required_extra_specs': fake_required_extra_specs, 'id': u'fooid-2', 'name': u'test_with_extra', 'updated_at': None @@ -131,29 +138,6 @@ class ShareTypesTestCase(test.TestCase): extra_spec = share_types.get_share_type_extra_specs(id) self.assertEqual(share_type['extra_specs'], extra_spec) - def test_share_types_diff(self): - share_type1 = self.fake_type['test'] - share_type2 = self.fake_type_w_extra['test_with_extra'] - expeted_diff = {'extra_specs': {u'gold': (None, u'True')}} - self.mock_object(db, - 'share_type_get', - mock.Mock(side_effect=[share_type1, share_type2])) - (diff, equal) = share_types.share_types_diff(self.context, - share_type1['id'], - share_type2['id']) - self.assertFalse(equal) - self.assertEqual(expeted_diff, diff) - - def test_share_types_diff_equal(self): - share_type = self.fake_type['test'] - self.mock_object(db, - 'share_type_get', - mock.Mock(return_value=share_type)) - (diff, equal) = share_types.share_types_diff(self.context, - share_type['id'], - share_type['id']) - self.assertTrue(equal) - def test_get_extra_specs_from_share(self): expected = self.fake_extra_specs self.mock_object(share_types, 'get_share_type_extra_specs', @@ -165,63 +149,143 @@ class ShareTypesTestCase(test.TestCase): share_types.get_share_type_extra_specs.assert_called_once_with( self.fake_share_type_id) - @ddt.data({}, - {"fake": "fake"}, - {constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: None}) - def test_create_without_required_extra_spec(self, extra_specs): - name = "fake_share_type" + @ddt.data({}, {"fake": "fake"}) + def test_create_without_required_extra_spec(self, optional_specs): + + specs = copy.copy(self.fake_required_extra_specs) + del specs['driver_handles_share_servers'] + specs.update(optional_specs) self.assertRaises(exception.InvalidShareType, share_types.create, - self.context, name, extra_specs) + self.context, "fake_share_type", specs) - def test_get_share_type_required_extra_specs(self): - valid_required_extra_specs = ( - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS,) + @ddt.data({"snapshot_support": "fake"}) + def test_create_with_invalid_optional_extra_spec(self, optional_specs): - actual_result = share_types.get_required_extra_specs() + specs = copy.copy(self.fake_required_extra_specs) + specs.update(optional_specs) - self.assertEqual(valid_required_extra_specs, actual_result) + self.assertRaises(exception.InvalidShareType, share_types.create, + self.context, "fake_share_type", specs) - def test_validate_required_extra_spec_other(self): + def test_get_required_extra_specs(self): + + result = share_types.get_required_extra_specs() + + self.assertEqual(constants.ExtraSpecs.REQUIRED, result) + + def test_get_optional_extra_specs(self): + + result = share_types.get_optional_extra_specs() + + self.assertEqual(constants.ExtraSpecs.OPTIONAL, result) + + def test_get_tenant_visible_extra_specs(self): + + result = share_types.get_tenant_visible_extra_specs() + + self.assertEqual(constants.ExtraSpecs.TENANT_VISIBLE, result) + + def test_get_boolean_extra_specs(self): + + result = share_types.get_boolean_extra_specs() + + self.assertEqual(constants.ExtraSpecs.BOOLEAN, result) + + def test_is_valid_required_extra_spec_other(self): actual_result = share_types.is_valid_required_extra_spec( 'fake', 'fake') self.assertIsNone(actual_result) - @ddt.data('1', 'True', 'false', '0', True, False) - def test_validate_required_extra_spec_valid(self, value): - key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS + @ddt.data(*itertools.product( + constants.ExtraSpecs.REQUIRED, + strutils.TRUE_STRINGS + strutils.FALSE_STRINGS)) + @ddt.unpack + def test_is_valid_required_extra_spec_valid(self, key, value): actual_result = share_types.is_valid_required_extra_spec(key, value) self.assertTrue(actual_result) @ddt.data('invalid', {}, '0000000000') - def test_validate_required_extra_spec_invalid(self, value): + def test_is_valid_required_extra_spec_invalid(self, value): key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS actual_result = share_types.is_valid_required_extra_spec(key, value) self.assertFalse(actual_result) - @ddt.data({constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true'}, - {constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true', - 'another_key': True}) - def test_get_valid_required_extra_specs_valid(self, specs): + @ddt.data({}, + {'another_key': True}) + def test_get_valid_required_extra_specs_valid(self, optional_specs): + + specs = copy.copy(self.fake_required_extra_specs) + specs.update(optional_specs) + actual_result = share_types.get_valid_required_extra_specs(specs) - valid_result = { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true' - } - self.assertEqual(valid_result, actual_result) + self.assertEqual(self.fake_required_extra_specs, actual_result) + + @ddt.data(None, + {}, + {constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'fake'}) + def test_get_valid_required_extra_specs_invalid(self, extra_specs): - @ddt.data(None, {}) - def test_get_valid_required_extra_specs_invalid(self, specs): self.assertRaises(exception.InvalidExtraSpec, - share_types.get_valid_required_extra_specs, specs) + share_types.get_valid_required_extra_specs, + extra_specs) + + @ddt.data(*( + list(itertools.product( + (constants.ExtraSpecs.SNAPSHOT_SUPPORT, + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT), + strutils.TRUE_STRINGS + strutils.FALSE_STRINGS))) + + list(itertools.product( + (constants.ExtraSpecs.REPLICATION_TYPE_SPEC,), + constants.ExtraSpecs.REPLICATION_TYPES)) + ) + @ddt.unpack + def test_is_valid_optional_extra_spec_valid(self, key, value): + + result = share_types.is_valid_optional_extra_spec(key, value) + + self.assertTrue(result) + + def test_is_valid_optional_extra_spec_valid_unknown_key(self): + + result = share_types.is_valid_optional_extra_spec('fake', 'fake') + + self.assertIsNone(result) + + def test_get_valid_optional_extra_specs(self): + + extra_specs = copy.copy(self.fake_required_extra_specs) + extra_specs.update(self.fake_optional_extra_specs) + extra_specs.update({'fake': 'fake'}) + + result = share_types.get_valid_optional_extra_specs(extra_specs) + + self.assertEqual(self.fake_optional_extra_specs, result) + + def test_get_valid_optional_extra_specs_empty(self): + + result = share_types.get_valid_optional_extra_specs({}) + + self.assertEqual({}, result) + + def test_get_valid_optional_extra_specs_invalid(self): + + extra_specs = {constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'fake'} + + self.assertRaises(exception.InvalidExtraSpec, + share_types.get_valid_optional_extra_specs, + extra_specs) def test_add_access(self): project_id = '456' extra_specs = { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true' + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true', + constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'true', + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: 'false', } share_type = share_types.create(self.context, 'type1', extra_specs) share_type_id = share_type.get('id') @@ -240,7 +304,9 @@ class ShareTypesTestCase(test.TestCase): def test_remove_access(self): project_id = '456' extra_specs = { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true' + constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true', + constants.ExtraSpecs.SNAPSHOT_SUPPORT: 'true', + constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT: 'false', } share_type = share_types.create( self.context, 'type1', projects=['456'], extra_specs=extra_specs) @@ -269,7 +335,7 @@ class ShareTypesTestCase(test.TestCase): self.assertEqual(expected, result) - @ddt.data('True', 'False', ' True', ' Wrong', None, 5) + @ddt.data(' True', ' Wrong', None, 5) def test_parse_boolean_extra_spec_invalid(self, spec_value): self.assertRaises(exception.InvalidExtraSpec, diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py index 6d0a466cdf..7d7f3fbb45 100644 --- a/manila_tempest_tests/config.py +++ b/manila_tempest_tests/config.py @@ -30,7 +30,7 @@ ShareGroup = [ help="The minimum api microversion is configured to be the " "value of the minimum microversion supported by Manila."), cfg.StrOpt("max_api_microversion", - default="2.23", + default="2.24", help="The maximum api microversion is configured to be the " "value of the latest microversion supported by Manila."), cfg.StrOpt("region", @@ -96,6 +96,13 @@ ShareGroup = [ "capability called 'snapshot_support' and will be used " "for setting up custom share type. Defaults to value of " "other config option 'run_snapshot_tests'."), + cfg.BoolOpt("capability_create_share_from_snapshot_support", + help="Defines extra spec that satisfies specific back end " + "capability called 'create_share_from_snapshot_support' " + "and will be used for setting up a custom share type. " + "Defaults to the value of run_snapshot_tests. Set it to " + "False if the driver being tested does not support " + "creating shares from snapshots."), cfg.StrOpt("share_network_id", default="", diff --git a/manila_tempest_tests/plugin.py b/manila_tempest_tests/plugin.py index 0a97747e4c..dfec0b182e 100644 --- a/manila_tempest_tests/plugin.py +++ b/manila_tempest_tests/plugin.py @@ -35,14 +35,21 @@ class ManilaTempestPlugin(plugins.TempestPlugin): conf.register_group(config_share.share_group) conf.register_opts(config_share.ShareGroup, group='share') - # NOTE(vponomaryov): set opt 'capability_snapshot_support' by - # default equal to opt 'run_snapshot_tests'. + # NOTE(vponomaryov): Set options 'capability_snapshot_support' and + # 'capability_create_share_from_snapshot_support' to opt + # 'run_snapshot_tests' if not configured. if conf.share.capability_snapshot_support is None: conf.set_default( "capability_snapshot_support", conf.share.run_snapshot_tests, group="share", ) + if conf.share.capability_create_share_from_snapshot_support is None: + conf.set_default( + "capability_create_share_from_snapshot_support", + conf.share.run_snapshot_tests, + group="share", + ) def get_opt_lists(self): return [(config_share.share_group.name, config_share.ShareGroup), diff --git a/manila_tempest_tests/services/share/v2/json/shares_client.py b/manila_tempest_tests/services/share/v2/json/shares_client.py index 0ccc18b712..de8c62ed82 100644 --- a/manila_tempest_tests/services/share/v2/json/shares_client.py +++ b/manila_tempest_tests/services/share/v2/json/shares_client.py @@ -726,6 +726,13 @@ class SharesV2Client(shares_client.SharesClient): self.expected_success(200, resp.status) return self._parse_resp(body) + def delete_share_type_extra_spec(self, share_type_id, extra_spec_name, + version=LATEST_MICROVERSION): + uri = "types/%s/extra_specs/%s" % (share_type_id, extra_spec_name) + resp, body = self.delete(uri, version=version) + self.expected_success(202, resp.status) + return body + def list_access_to_share_type(self, share_type_id, version=LATEST_MICROVERSION, action_name=None): diff --git a/manila_tempest_tests/tests/api/admin/test_consistency_group_actions.py b/manila_tempest_tests/tests/api/admin/test_consistency_group_actions.py index b9d349ca53..8b2a21c560 100644 --- a/manila_tempest_tests/tests/api/admin/test_consistency_group_actions.py +++ b/manila_tempest_tests/tests/api/admin/test_consistency_group_actions.py @@ -32,7 +32,7 @@ class ConsistencyGroupActionsTest(base.BaseSharesAdminTest): super(ConsistencyGroupActionsTest, cls).resource_setup() # Create 2 share_types name = data_utils.rand_name("tempest-manila") - extra_specs = cls.add_required_extra_specs_to_dict() + extra_specs = cls.add_extra_specs_to_dict() share_type = cls.create_share_type(name, extra_specs=extra_specs) cls.share_type = share_type['share_type'] diff --git a/manila_tempest_tests/tests/api/admin/test_consistency_groups.py b/manila_tempest_tests/tests/api/admin/test_consistency_groups.py index efa2471555..22678e5358 100644 --- a/manila_tempest_tests/tests/api/admin/test_consistency_groups.py +++ b/manila_tempest_tests/tests/api/admin/test_consistency_groups.py @@ -34,7 +34,7 @@ class ConsistencyGroupsTest(base.BaseSharesAdminTest): super(ConsistencyGroupsTest, cls).resource_setup() # Create 2 share_types name = data_utils.rand_name("tempest-manila") - extra_specs = cls.add_required_extra_specs_to_dict() + extra_specs = cls.add_extra_specs_to_dict() share_type = cls.create_share_type(name, extra_specs=extra_specs) cls.share_type = share_type['share_type'] diff --git a/manila_tempest_tests/tests/api/admin/test_consistency_groups_negative.py b/manila_tempest_tests/tests/api/admin/test_consistency_groups_negative.py index 0a0d0f4dcd..a85037976a 100644 --- a/manila_tempest_tests/tests/api/admin/test_consistency_groups_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_consistency_groups_negative.py @@ -33,7 +33,7 @@ class ConsistencyGroupsNegativeTest(base.BaseSharesAdminTest): super(ConsistencyGroupsNegativeTest, cls).resource_setup() # Create share_type name = data_utils.rand_name("tempest-manila") - extra_specs = cls.add_required_extra_specs_to_dict() + extra_specs = cls.add_extra_specs_to_dict() share_type = cls.create_share_type(name, extra_specs=extra_specs) cls.share_type = share_type['share_type'] diff --git a/manila_tempest_tests/tests/api/admin/test_multi_backend.py b/manila_tempest_tests/tests/api/admin/test_multi_backend.py index 8b7e078318..8aeabc3ece 100644 --- a/manila_tempest_tests/tests/api/admin/test_multi_backend.py +++ b/manila_tempest_tests/tests/api/admin/test_multi_backend.py @@ -48,7 +48,7 @@ class ShareMultiBackendTest(base.BaseSharesAdminTest): } st = cls.create_share_type( name=st_name, - extra_specs=cls.add_required_extra_specs_to_dict(extra_specs)) + extra_specs=cls.add_extra_specs_to_dict(extra_specs)) cls.sts.append(st["share_type"]) st_id = st["share_type"]["id"] share_data_list.append({"kwargs": {"share_type_id": st_id}}) diff --git a/manila_tempest_tests/tests/api/admin/test_replication.py b/manila_tempest_tests/tests/api/admin/test_replication.py index 38546e3f93..e41c92b340 100644 --- a/manila_tempest_tests/tests/api/admin/test_replication.py +++ b/manila_tempest_tests/tests/api/admin/test_replication.py @@ -48,7 +48,7 @@ class ReplicationAdminTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, diff --git a/manila_tempest_tests/tests/api/admin/test_replication_actions.py b/manila_tempest_tests/tests/api/admin/test_replication_actions.py index 377bb4139f..5b5653341f 100644 --- a/manila_tempest_tests/tests/api/admin/test_replication_actions.py +++ b/manila_tempest_tests/tests/api/admin/test_replication_actions.py @@ -49,7 +49,7 @@ class ReplicationAdminTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, diff --git a/manila_tempest_tests/tests/api/admin/test_scheduler_stats.py b/manila_tempest_tests/tests/api/admin/test_scheduler_stats.py index fe4cc43a37..aa298288ad 100644 --- a/manila_tempest_tests/tests/api/admin/test_scheduler_stats.py +++ b/manila_tempest_tests/tests/api/admin/test_scheduler_stats.py @@ -35,8 +35,7 @@ class SchedulerStatsAdminTest(base.BaseSharesAdminTest): 'share_backend_name': data_utils.rand_name("fake_name"), } - extra_specs = cls.add_required_extra_specs_to_dict( - extra_specs=extra_specs) + extra_specs = cls.add_extra_specs_to_dict(extra_specs=extra_specs) return cls.create_share_type( name, extra_specs=extra_specs, client=cls.admin_client) diff --git a/manila_tempest_tests/tests/api/admin/test_share_manage.py b/manila_tempest_tests/tests/api/admin/test_share_manage.py index 810004039a..f943284b63 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_manage.py +++ b/manila_tempest_tests/tests/api/admin/test_share_manage.py @@ -54,12 +54,16 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): 'driver_handles_share_servers': False, 'snapshot_support': six.text_type( CONF.share.capability_snapshot_support), + 'create_share_from_snapshot_support': six.text_type( + CONF.share.capability_create_share_from_snapshot_support) } cls.extra_specs_invalid = { 'storage_protocol': CONF.share.capability_storage_protocol, 'driver_handles_share_servers': True, 'snapshot_support': six.text_type( CONF.share.capability_snapshot_support), + 'create_share_from_snapshot_support': six.text_type( + CONF.share.capability_create_share_from_snapshot_support), } cls.st = cls.create_share_type( diff --git a/manila_tempest_tests/tests/api/admin/test_share_types.py b/manila_tempest_tests/tests/api/admin/test_share_types.py index af354bbc55..076e4bc150 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types.py @@ -31,7 +31,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): @tc.attr(base.TAG_POSITIVE, base.TAG_API) def test_share_type_create_delete(self): name = data_utils.rand_name("tempest-manila") - extra_specs = self.add_required_extra_specs_to_dict() + extra_specs = self.add_extra_specs_to_dict() # Create share type st_create = self.shares_v2_client.create_share_type( @@ -64,7 +64,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): self.skip_if_microversion_not_supported(version) name = data_utils.rand_name("tempest-manila") - extra_specs = self.add_required_extra_specs_to_dict({"key": "value", }) + extra_specs = self.add_extra_specs_to_dict({"key": "value", }) # Create share type st_create = self.create_share_type( @@ -89,7 +89,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): self.skip_if_microversion_not_supported(version) name = data_utils.rand_name("tempest-manila") - extra_specs = self.add_required_extra_specs_to_dict() + extra_specs = self.add_extra_specs_to_dict() # Create share type st_create = self.create_share_type( @@ -117,7 +117,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): # Data share_name = data_utils.rand_name("share") shr_type_name = data_utils.rand_name("share-type") - extra_specs = self.add_required_extra_specs_to_dict({ + extra_specs = self.add_extra_specs_to_dict({ "storage_protocol": CONF.share.capability_storage_protocol, }) @@ -144,7 +144,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): @tc.attr(base.TAG_POSITIVE, base.TAG_API) def test_private_share_type_access(self): name = data_utils.rand_name("tempest-manila") - extra_specs = self.add_required_extra_specs_to_dict({"key": "value", }) + extra_specs = self.add_extra_specs_to_dict({"key": "value", }) project_id = self.shares_client.tenant_id # Create private share type diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs.py b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs.py index 81d130f37b..f4c21b8a02 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs.py @@ -15,19 +15,25 @@ import copy +import ddt +from tempest import config from tempest.lib.common.utils import data_utils from testtools import testcase as tc from manila_tempest_tests.tests.api import base +CONF = config.CONF +LATEST_MICROVERSION = CONF.share.max_api_microversion + + class ExtraSpecsReadAdminTest(base.BaseSharesAdminTest): @classmethod def resource_setup(cls): super(ExtraSpecsReadAdminTest, cls).resource_setup() cls.share_type_name = data_utils.rand_name("share-type") - cls.required_extra_specs = cls.add_required_extra_specs_to_dict() + cls.required_extra_specs = cls.add_extra_specs_to_dict() cls.share_type = cls.create_share_type( cls.share_type_name, extra_specs=cls.required_extra_specs) @@ -54,11 +60,12 @@ class ExtraSpecsReadAdminTest(base.BaseSharesAdminTest): self.assertEqual(self.expected_extra_specs, es_get_all) +@ddt.ddt class ExtraSpecsWriteAdminTest(base.BaseSharesAdminTest): def setUp(self): super(ExtraSpecsWriteAdminTest, self).setUp() - self.required_extra_specs = self.add_required_extra_specs_to_dict() + self.required_extra_specs = self.add_extra_specs_to_dict() self.custom_extra_specs = {"key1": "value1", "key2": "value2"} self.share_type_name = data_utils.rand_name("share-type") @@ -109,3 +116,17 @@ class ExtraSpecsWriteAdminTest(base.BaseSharesAdminTest): get = self.shares_client.get_share_type_extra_specs(self.st_id) self.assertNotIn('key1', get) + + @tc.attr(base.TAG_POSITIVE, base.TAG_API) + @ddt.data(*set(['2.24', LATEST_MICROVERSION])) + def test_delete_snapshot_support_extra_spec(self, version): + self.skip_if_microversion_not_supported(version) + # Delete one extra spec for share type + self.shares_v2_client.delete_share_type_extra_spec( + self.st_id, 'snapshot_support', version=version) + + # Get metadata + share_type_extra_specs = self.shares_client.get_share_type_extra_specs( + self.st_id) + + self.assertNotIn('snapshot_support', share_type_extra_specs) diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py index 34e5009c60..85cbc5df95 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt from tempest.lib.common.utils import data_utils from tempest.lib import exceptions as lib_exc from testtools import testcase as tc @@ -20,11 +21,12 @@ from testtools import testcase as tc from manila_tempest_tests.tests.api import base +@ddt.ddt class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): def _create_share_type(self): name = data_utils.rand_name("unique_st_name") - extra_specs = self.add_required_extra_specs_to_dict({"key": "value"}) + extra_specs = self.add_extra_specs_to_dict({"key": "value"}) return self.create_share_type( name, extra_specs=extra_specs, client=self.admin_shares_v2_client) @@ -35,7 +37,7 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): lib_exc.Forbidden, self.shares_v2_client.create_share_type_extra_specs, st["share_type"]["id"], - self.add_required_extra_specs_to_dict({"key": "new_value"})) + self.add_extra_specs_to_dict({"key": "new_value"})) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_try_list_extra_specs_with_user(self): @@ -67,7 +69,8 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): share_type = self.shares_v2_client.get_share_type( st['share_type']['id']) # Verify a non-admin can only read the required extra-specs - expected_keys = ['driver_handles_share_servers', 'snapshot_support'] + expected_keys = ['driver_handles_share_servers', 'snapshot_support', + 'create_share_from_snapshot_support'] actual_keys = share_type['share_type']['extra_specs'].keys() self.assertEqual(sorted(expected_keys), sorted(actual_keys), 'Incorrect extra specs visible to non-admin user; ' @@ -105,7 +108,7 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): lib_exc.BadRequest, self.admin_shares_v2_client.create_share_type_extra_specs, st["share_type"]["id"], - self.add_required_extra_specs_to_dict({too_big_key: "value"})) + self.add_extra_specs_to_dict({too_big_key: "value"})) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_try_set_too_long_value_with_creation(self): @@ -115,7 +118,7 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): lib_exc.BadRequest, self.admin_shares_v2_client.create_share_type_extra_specs, st["share_type"]["id"], - self.add_required_extra_specs_to_dict({"key": too_big_value})) + self.add_extra_specs_to_dict({"key": too_big_value})) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_try_set_too_long_value_with_update(self): @@ -123,12 +126,12 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): st = self._create_share_type() self.admin_shares_v2_client.create_share_type_extra_specs( st["share_type"]["id"], - self.add_required_extra_specs_to_dict({"key": "value"})) + self.add_extra_specs_to_dict({"key": "value"})) self.assertRaises( lib_exc.BadRequest, self.admin_shares_v2_client.update_share_type_extra_specs, st["share_type"]["id"], - self.add_required_extra_specs_to_dict({"key": too_big_value})) + self.add_extra_specs_to_dict({"key": too_big_value})) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_try_set_too_long_value_with_update_of_one_key(self): @@ -136,7 +139,7 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): st = self._create_share_type() self.admin_shares_v2_client.create_share_type_extra_specs( st["share_type"]["id"], - self.add_required_extra_specs_to_dict({"key": "value"})) + self.add_extra_specs_to_dict({"key": "value"})) self.assertRaises( lib_exc.BadRequest, self.admin_shares_v2_client.update_share_type_extra_spec, @@ -286,12 +289,12 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesMixedTest): "driver_handles_share_servers") @tc.attr(base.TAG_NEGATIVE, base.TAG_API) - def test_try_delete_spec_snapshot_support(self): + @ddt.data('2.0', '2.23') + def test_try_delete_required_spec_snapshot_support_version(self, version): + self.skip_if_microversion_not_supported(version) st = self._create_share_type() - # Try delete extra spec 'snapshot_support' self.assertRaises( lib_exc.Forbidden, self.admin_shares_v2_client.delete_share_type_extra_spec, - st["share_type"]["id"], - "snapshot_support") + st["share_type"]["id"], "snapshot_support", version=version) diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py index b0fb9db652..b3bf855e49 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py @@ -24,7 +24,7 @@ class ShareTypesAdminNegativeTest(base.BaseSharesMixedTest): def _create_share_type(self): name = data_utils.rand_name("unique_st_name") - extra_specs = self.add_required_extra_specs_to_dict({"key": "value"}) + extra_specs = self.add_extra_specs_to_dict({"key": "value"}) return self.create_share_type( name, extra_specs=extra_specs, client=self.admin_shares_v2_client) @@ -66,7 +66,7 @@ class ShareTypesAdminNegativeTest(base.BaseSharesMixedTest): self.assertRaises(lib_exc.Conflict, self.create_share_type, st["share_type"]["name"], - extra_specs=self.add_required_extra_specs_to_dict(), + extra_specs=self.add_extra_specs_to_dict(), client=self.admin_shares_v2_client) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) diff --git a/manila_tempest_tests/tests/api/admin/test_shares_actions.py b/manila_tempest_tests/tests/api/admin/test_shares_actions.py index eb049c61ad..779cd18449 100644 --- a/manila_tempest_tests/tests/api/admin/test_shares_actions.py +++ b/manila_tempest_tests/tests/api/admin/test_shares_actions.py @@ -34,7 +34,7 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest): # create share type for share filtering purposes cls.st_name = data_utils.rand_name("tempest-st-name") - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {'storage_protocol': CONF.share.capability_storage_protocol}) cls.st = cls.create_share_type( name=cls.st_name, @@ -64,20 +64,23 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest): cls.snap = cls.create_snapshot_wait_for_active( cls.shares[0]["id"], cls.snap_name, cls.snap_desc) - # create second share from snapshot for purposes of sorting and - # snapshot filtering - cls.share_name2 = data_utils.rand_name("tempest-share-name") - cls.share_desc2 = data_utils.rand_name("tempest-share-description") - cls.metadata2 = { - 'foo_key_share_2': 'foo_value_share_2', - 'bar_key_share_2': 'foo_value_share_2', - } - cls.shares.append(cls.create_share( - name=cls.share_name2, - description=cls.share_desc2, - metadata=cls.metadata2, - snapshot_id=cls.snap['id'], - )) + if CONF.share.capability_create_share_from_snapshot_support: + + # create second share from snapshot for purposes of sorting and + # snapshot filtering + cls.share_name2 = data_utils.rand_name("tempest-share-name") + cls.share_desc2 = data_utils.rand_name( + "tempest-share-description") + cls.metadata2 = { + 'foo_key_share_2': 'foo_value_share_2', + 'bar_key_share_2': 'foo_value_share_2', + } + cls.shares.append(cls.create_share( + name=cls.share_name2, + description=cls.share_desc2, + metadata=cls.metadata2, + snapshot_id=cls.snap['id'], + )) @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) def test_get_share(self): @@ -154,7 +157,7 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest): for share in shares: self.assertDictContainsSubset( filters['metadata'], share['metadata']) - if CONF.share.run_snapshot_tests: + if CONF.share.capability_create_share_from_snapshot_support: self.assertFalse(self.shares[1]['id'] in [s['id'] for s in shares]) @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) @@ -250,8 +253,9 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest): filters['share_network_id'], share['share_network_id']) @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) - @testtools.skipUnless(CONF.share.run_snapshot_tests, - "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_list_shares_with_detail_filter_by_snapshot_id(self): filters = {'snapshot_id': self.snap['id']} diff --git a/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py b/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py index 3cde3e6f78..6436e5b2c3 100644 --- a/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py +++ b/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py @@ -55,6 +55,8 @@ class ManageNFSSnapshotTest(base.BaseSharesAdminTest): 'driver_handles_share_servers': False, 'snapshot_support': six.text_type( CONF.share.capability_snapshot_support), + 'create_share_from_snapshot_support': six.text_type( + CONF.share.capability_create_share_from_snapshot_support) } cls.st = cls.create_share_type( diff --git a/manila_tempest_tests/tests/api/admin/test_snapshot_manage_negative.py b/manila_tempest_tests/tests/api/admin/test_snapshot_manage_negative.py index dfd892ca69..3825be122b 100644 --- a/manila_tempest_tests/tests/api/admin/test_snapshot_manage_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_snapshot_manage_negative.py @@ -49,6 +49,8 @@ class ManageNFSSnapshotNegativeTest(base.BaseSharesAdminTest): 'driver_handles_share_servers': False, 'snapshot_support': six.text_type( CONF.share.capability_snapshot_support), + 'create_share_from_snapshot_support': six.text_type( + CONF.share.capability_create_share_from_snapshot_support), } cls.st = cls.create_share_type( diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py index 7bb727da77..dcf0bed396 100644 --- a/manila_tempest_tests/tests/api/base.py +++ b/manila_tempest_tests/tests/api/base.py @@ -727,17 +727,30 @@ class BaseSharesTest(test.BaseTestCase): return share_type @staticmethod - def add_required_extra_specs_to_dict(extra_specs=None): + def add_extra_specs_to_dict(extra_specs=None): + """Add any required extra-specs to share type dictionary""" dhss = six.text_type(CONF.share.multitenancy_enabled) snapshot_support = six.text_type( CONF.share.capability_snapshot_support) - required = { + create_from_snapshot_support = six.text_type( + CONF.share.capability_create_share_from_snapshot_support) + + extra_specs_dict = { "driver_handles_share_servers": dhss, - "snapshot_support": snapshot_support, } + + optional = { + "snapshot_support": snapshot_support, + "create_share_from_snapshot_support": create_from_snapshot_support, + } + # NOTE(gouthamr): In micro-versions < 2.24, snapshot_support is a + # required extra-spec + extra_specs_dict.update(optional) + if extra_specs: - required.update(extra_specs) - return required + extra_specs_dict.update(extra_specs) + + return extra_specs_dict @classmethod def clear_isolated_creds(cls, creds=None): diff --git a/manila_tempest_tests/tests/api/test_replication.py b/manila_tempest_tests/tests/api/test_replication.py index 0bdb7adf75..1738e874eb 100644 --- a/manila_tempest_tests/tests/api/test_replication.py +++ b/manila_tempest_tests/tests/api/test_replication.py @@ -51,7 +51,7 @@ class ReplicationTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, @@ -307,7 +307,7 @@ class ReplicationActionsTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, diff --git a/manila_tempest_tests/tests/api/test_replication_negative.py b/manila_tempest_tests/tests/api/test_replication_negative.py index cfbf210662..48b8d5c1a7 100644 --- a/manila_tempest_tests/tests/api/test_replication_negative.py +++ b/manila_tempest_tests/tests/api/test_replication_negative.py @@ -48,7 +48,7 @@ class ReplicationNegativeTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, @@ -80,7 +80,7 @@ class ReplicationNegativeTest(base.BaseSharesMixedTest): # Create share without replication type share_type = self.create_share_type( data_utils.rand_name(constants.TEMPEST_MANILA_PREFIX), - extra_specs=self.add_required_extra_specs_to_dict(), + extra_specs=self.add_extra_specs_to_dict(), client=self.admin_client)["share_type"] share = self.create_share(share_type_id=share_type["id"]) self.assertRaises(lib_exc.BadRequest, diff --git a/manila_tempest_tests/tests/api/test_replication_snapshots.py b/manila_tempest_tests/tests/api/test_replication_snapshots.py index e3081ccb80..331437cadb 100644 --- a/manila_tempest_tests/tests/api/test_replication_snapshots.py +++ b/manila_tempest_tests/tests/api/test_replication_snapshots.py @@ -49,7 +49,7 @@ class ReplicationSnapshotTest(base.BaseSharesMixedTest): cls.share_zone = cls.zones[0] cls.replica_zone = cls.zones[-1] - cls.extra_specs = cls.add_required_extra_specs_to_dict( + cls.extra_specs = cls.add_extra_specs_to_dict( {"replication_type": cls.replication_type}) share_type = cls.create_share_type( name, @@ -86,7 +86,12 @@ class ReplicationSnapshotTest(base.BaseSharesMixedTest): snapshot = self.create_snapshot_wait_for_active(share["id"]) self.promote_share_replica(share_replica['id']) self.delete_share_replica(original_replica['id']) - self.create_share(snapshot_id=snapshot['id']) + + snapshot = self.shares_v2_client.get_snapshot(snapshot['id']) + self.assertEqual(constants.STATUS_AVAILABLE, snapshot['status']) + + if CONF.share.capability_create_share_from_snapshot_support: + self.create_share(snapshot_id=snapshot['id']) @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) def test_snapshot_before_share_replica(self): @@ -116,7 +121,12 @@ class ReplicationSnapshotTest(base.BaseSharesMixedTest): self.promote_share_replica(share_replica['id']) self.delete_share_replica(original_replica['id']) - self.create_share(snapshot_id=snapshot['id']) + + snapshot = self.shares_v2_client.get_snapshot(snapshot['id']) + self.assertEqual(constants.STATUS_AVAILABLE, snapshot['status']) + + if CONF.share.capability_create_share_from_snapshot_support: + self.create_share(snapshot_id=snapshot['id']) @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) def test_snapshot_before_and_after_share_replica(self): @@ -152,8 +162,15 @@ class ReplicationSnapshotTest(base.BaseSharesMixedTest): # still being created successfully. self.delete_share_replica(original_replica['id']) - self.create_share(snapshot_id=snapshot1['id']) - self.create_share(snapshot_id=snapshot2['id']) + snapshot1 = self.shares_v2_client.get_snapshot(snapshot1['id']) + self.assertEqual(constants.STATUS_AVAILABLE, snapshot1['status']) + + snapshot2 = self.shares_v2_client.get_snapshot(snapshot2['id']) + self.assertEqual(constants.STATUS_AVAILABLE, snapshot2['status']) + + if CONF.share.capability_create_share_from_snapshot_support: + self.create_share(snapshot_id=snapshot1['id']) + self.create_share(snapshot_id=snapshot2['id']) @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) def test_delete_snapshot_after_adding_replica(self): @@ -176,6 +193,9 @@ class ReplicationSnapshotTest(base.BaseSharesMixedTest): snapshot_id=snapshot["id"]) @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_create_replica_from_snapshot_share(self): """Test replica for a share that was created from snapshot.""" diff --git a/manila_tempest_tests/tests/api/test_share_types_negative.py b/manila_tempest_tests/tests/api/test_share_types_negative.py index 9c7883b754..8a243256e0 100644 --- a/manila_tempest_tests/tests/api/test_share_types_negative.py +++ b/manila_tempest_tests/tests/api/test_share_types_negative.py @@ -25,7 +25,7 @@ class ShareTypesNegativeTest(base.BaseSharesMixedTest): @classmethod def _create_share_type(cls): name = data_utils.rand_name("unique_st_name") - extra_specs = cls.add_required_extra_specs_to_dict() + extra_specs = cls.add_extra_specs_to_dict() return cls.create_share_type( name, extra_specs=extra_specs, client=cls.admin_client) diff --git a/manila_tempest_tests/tests/api/test_shares.py b/manila_tempest_tests/tests/api/test_shares.py index af4a54a98f..d78c9137d9 100644 --- a/manila_tempest_tests/tests/api/test_shares.py +++ b/manila_tempest_tests/tests/api/test_shares.py @@ -92,6 +92,12 @@ class SharesNFSTest(base.BaseSharesTest): detailed_elements.add('user_id') self.assertTrue(detailed_elements.issubset(share.keys()), msg) + # In v 2.24 and beyond, we add create_share_from_snapshot_support in + # show/create/manage share echo. + if utils.is_microversion_supported('2.24'): + detailed_elements.add('create_share_from_snapshot_support') + self.assertTrue(detailed_elements.issubset(share.keys()), msg) + # Delete share self.shares_v2_client.delete_share(share['id']) self.shares_v2_client.wait_for_resource_deletion(share_id=share['id']) @@ -136,6 +142,9 @@ class SharesNFSTest(base.BaseSharesTest): @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) @testtools.skipUnless(CONF.share.run_snapshot_tests, "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_create_share_from_snapshot(self): # If multitenant driver used, share_network will be provided by default @@ -164,9 +173,13 @@ class SharesNFSTest(base.BaseSharesTest): "Only for multitenancy.") @testtools.skipUnless(CONF.share.run_snapshot_tests, "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_create_share_from_snapshot_share_network_not_provided(self): # We expect usage of share network from parent's share - # when creating share from snapshot using multitenant driver. + # when creating share from snapshot using a driver that supports + # multi-tenancy. # get parent share parent = self.shares_client.get_share(self.share["id"]) diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py index da44c7caa1..d03df33db3 100644 --- a/manila_tempest_tests/tests/api/test_shares_actions.py +++ b/manila_tempest_tests/tests/api/test_shares_actions.py @@ -58,20 +58,23 @@ class SharesActionsTest(base.BaseSharesTest): cls.snap = cls.create_snapshot_wait_for_active( cls.shares[0]["id"], cls.snap_name, cls.snap_desc) - # create second share from snapshot for purposes of sorting and - # snapshot filtering - cls.share_name2 = data_utils.rand_name("tempest-share-name") - cls.share_desc2 = data_utils.rand_name("tempest-share-description") - cls.metadata2 = { - 'foo_key_share_2': 'foo_value_share_2', - 'bar_key_share_2': 'foo_value_share_2', - } - cls.shares.append(cls.create_share( - name=cls.share_name2, - description=cls.share_desc2, - metadata=cls.metadata2, - snapshot_id=cls.snap['id'], - )) + if CONF.share.capability_create_share_from_snapshot_support: + + # create second share from snapshot for purposes of sorting and + # snapshot filtering + cls.share_name2 = data_utils.rand_name("tempest-share-name") + cls.share_desc2 = data_utils.rand_name( + "tempest-share-description") + cls.metadata2 = { + 'foo_key_share_2': 'foo_value_share_2', + 'bar_key_share_2': 'foo_value_share_2', + } + cls.shares.append(cls.create_share( + name=cls.share_name2, + description=cls.share_desc2, + metadata=cls.metadata2, + snapshot_id=cls.snap['id'], + )) def _get_share(self, version): @@ -101,6 +104,8 @@ class SharesActionsTest(base.BaseSharesTest): expected_keys.append("replication_type") if utils.is_microversion_ge(version, '2.16'): expected_keys.append("user_id") + if utils.is_microversion_ge(version, '2.24'): + expected_keys.append("create_share_from_snapshot_support") actual_keys = list(share.keys()) [self.assertIn(key, actual_keys) for key in expected_keys] @@ -157,6 +162,11 @@ class SharesActionsTest(base.BaseSharesTest): def test_get_share_with_user_id(self): self._get_share('2.16') + @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) + @utils.skip_if_microversion_not_supported('2.24') + def test_get_share_with_create_share_from_snapshot_support(self): + self._get_share('2.24') + @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) def test_list_shares(self): @@ -201,6 +211,8 @@ class SharesActionsTest(base.BaseSharesTest): keys.append("replication_type") if utils.is_microversion_ge(version, '2.16'): keys.append("user_id") + if utils.is_microversion_ge(version, '2.24'): + keys.append("create_share_from_snapshot_support") [self.assertIn(key, sh.keys()) for sh in shares for key in keys] # our shares in list and have no duplicates @@ -247,6 +259,11 @@ class SharesActionsTest(base.BaseSharesTest): def test_list_shares_with_user_id(self): self._list_shares_with_detail('2.16') + @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) + def test_list_shares_with_detail_and_create_share_from_snapshot_support( + self): + self._list_shares_with_detail('2.24') + @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) def test_list_shares_with_detail_filter_by_metadata(self): filters = {'metadata': self.metadata} @@ -259,7 +276,7 @@ class SharesActionsTest(base.BaseSharesTest): for share in shares: self.assertDictContainsSubset( filters['metadata'], share['metadata']) - if CONF.share.run_snapshot_tests: + if CONF.share.capability_create_share_from_snapshot_support: self.assertFalse(self.shares[1]['id'] in [s['id'] for s in shares]) @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) @@ -294,6 +311,9 @@ class SharesActionsTest(base.BaseSharesTest): @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) @testtools.skipUnless(CONF.share.run_snapshot_tests, "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_list_shares_with_detail_filter_by_snapshot_id(self): filters = {'snapshot_id': self.snap['id']} diff --git a/manila_tempest_tests/tests/api/test_shares_negative.py b/manila_tempest_tests/tests/api/test_shares_negative.py index 7c5619ef82..42204e1edd 100644 --- a/manila_tempest_tests/tests/api/test_shares_negative.py +++ b/manila_tempest_tests/tests/api/test_shares_negative.py @@ -60,6 +60,9 @@ class SharesNegativeTest(base.BaseSharesTest): @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND) @testtools.skipUnless(CONF.share.run_snapshot_tests, "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_create_share_from_snap_with_less_size(self): # requires minimum 5Gb available space @@ -96,7 +99,13 @@ class SharesNegativeTest(base.BaseSharesTest): "Only for multitenancy.") @testtools.skipUnless(CONF.share.run_snapshot_tests, "Snapshot tests are disabled.") + @testtools.skipUnless( + CONF.share.capability_create_share_from_snapshot_support, + "Create share from snapshot tests are disabled.") def test_create_share_from_snap_with_different_share_network(self): + # We can't create a share from a snapshot whose base share does not + # have 'create_share_from_snapshot_support'. + # create share share = self.create_share(cleanup_in_class=False) diff --git a/releasenotes/notes/add-create_share_from_snapshot_support-extra-spec-9b1c3ad6796dd07d.yaml b/releasenotes/notes/add-create_share_from_snapshot_support-extra-spec-9b1c3ad6796dd07d.yaml new file mode 100644 index 0000000000..60021afc5e --- /dev/null +++ b/releasenotes/notes/add-create_share_from_snapshot_support-extra-spec-9b1c3ad6796dd07d.yaml @@ -0,0 +1,7 @@ +--- +features: + - Added optional create_share_from_snapshot_support extra spec, which was + previously implied by the overloaded snapshot_support extra spec. +upgrade: + - The snapshot_support extra spec is now optional and has no default value + set when creating share types.