From 0d6db3588c212a7d121bb5d95ea0a82b034490a1 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Tue, 16 Aug 2016 22:36:13 -0400 Subject: [PATCH] Add create_share_from_snapshot_support extra spec The snapshot_support extra spec has always meant two things: a driver can take snapshots and create shares from snapshots. As we add alternate snapshot semantics, it is likely that some drivers will want to support snapshots and some of the new semantics while being unable to create new shares from snapshots. This work adds a new extra spec, create_share_from_snapshot_support, that removes the overloading on snapshot_support. It also makes the existing snapshot_support extra spec optional, allowing admins to create types without setting snapshot_support; shares created with such types will not support snapshots. APIImpact DocImpact Co-Authored-By: Goutham Pacha Ravi Implements: blueprint add-create-share-from-snapshot-extra-spec Change-Id: Ib0ad5fbfdf6297665c208149b08c8d21b3c232be --- contrib/ci/post_test_hook.sh | 8 + contrib/ci/pre_test_hook.sh | 5 +- .../devref/capabilities_and_extra_specs.rst | 93 ++++--- ...hare_back_ends_feature_support_mapping.rst | 105 ++++---- doc/source/devref/tempest_tests.rst | 1 + manila/api/openstack/api_version_request.py | 6 +- .../openstack/rest_api_version_history.rst | 5 + manila/api/v1/share_types_extra_specs.py | 37 ++- manila/api/v1/shares.py | 11 +- manila/api/v2/share_types.py | 24 +- manila/api/v2/shares.py | 15 +- manila/api/views/shares.py | 7 + manila/api/views/types.py | 16 ++ manila/common/constants.py | 25 +- ..._add_create_share_from_snapshot_support.py | 149 +++++++++++ manila/db/sqlalchemy/models.py | 1 + manila/scheduler/drivers/filter.py | 2 +- manila/scheduler/host_manager.py | 7 + manila/scheduler/utils.py | 2 + manila/share/api.py | 100 +++++--- manila/share/driver.py | 21 +- manila/share/drivers/container/driver.py | 1 + manila/share/drivers/huawei/huawei_nas.py | 1 + manila/share/drivers/lvm.py | 1 + manila/share/drivers/tegile/tegile.py | 1 + manila/share/drivers/zfsonlinux/driver.py | 1 + manila/share/share_types.py | 119 +++++---- manila/test.py | 5 + manila/tests/api/contrib/stubs.py | 1 + .../api/v1/test_share_types_extra_specs.py | 96 +++---- manila/tests/api/v1/test_shares.py | 58 ++++- manila/tests/api/v2/test_share_types.py | 238 +++++++++++++++--- manila/tests/api/v2/test_shares.py | 41 ++- manila/tests/api/views/test_shares.py | 67 +++-- .../alembic/migrations_data_checks.py | 223 +++++++++++++--- manila/tests/fake_share.py | 1 - manila/tests/scheduler/drivers/test_filter.py | 6 +- manila/tests/scheduler/fakes.py | 15 ++ manila/tests/scheduler/test_host_manager.py | 11 + .../share/drivers/dell_emc/test_driver.py | 1 + manila/tests/share/drivers/dummy.py | 1 + .../glusterfs/test_glusterfs_native.py | 1 + .../share/drivers/hpe/test_hpe_3par_driver.py | 3 + .../share/drivers/huawei/test_huawei_nas.py | 1 + .../share/drivers/zfsonlinux/test_driver.py | 1 + manila/tests/share/test_api.py | 89 +++++++ manila/tests/share/test_driver.py | 99 ++++++-- manila/tests/share/test_share_types.py | 184 +++++++++----- manila_tempest_tests/config.py | 9 +- manila_tempest_tests/plugin.py | 11 +- .../services/share/v2/json/shares_client.py | 7 + .../admin/test_consistency_group_actions.py | 2 +- .../api/admin/test_consistency_groups.py | 2 +- .../admin/test_consistency_groups_negative.py | 2 +- .../tests/api/admin/test_multi_backend.py | 2 +- .../tests/api/admin/test_replication.py | 2 +- .../api/admin/test_replication_actions.py | 2 +- .../tests/api/admin/test_scheduler_stats.py | 3 +- .../tests/api/admin/test_share_manage.py | 4 + .../tests/api/admin/test_share_types.py | 10 +- .../api/admin/test_share_types_extra_specs.py | 25 +- .../test_share_types_extra_specs_negative.py | 27 +- .../api/admin/test_share_types_negative.py | 4 +- .../tests/api/admin/test_shares_actions.py | 40 +-- .../tests/api/admin/test_snapshot_manage.py | 2 + .../admin/test_snapshot_manage_negative.py | 2 + manila_tempest_tests/tests/api/base.py | 23 +- .../tests/api/test_replication.py | 4 +- .../tests/api/test_replication_negative.py | 4 +- .../tests/api/test_replication_snapshots.py | 30 ++- .../tests/api/test_share_types_negative.py | 2 +- manila_tempest_tests/tests/api/test_shares.py | 15 +- .../tests/api/test_shares_actions.py | 50 ++-- .../tests/api/test_shares_negative.py | 9 + ...t_support-extra-spec-9b1c3ad6796dd07d.yaml | 7 + 75 files changed, 1687 insertions(+), 519 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/3e7d62517afa_add_create_share_from_snapshot_support.py create mode 100644 releasenotes/notes/add-create_share_from_snapshot_support-extra-spec-9b1c3ad6796dd07d.yaml 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.