From af6814f9cab7a76db8e5bf45b9d0ffdf2b7daa21 Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Wed, 26 Aug 2015 00:01:11 +0300 Subject: [PATCH] Add possibility to filter back ends by snapshot support On last midcycle meetup was decided to make snapshots optional feature. Features: - Add new boolean capability 'snapshot_support' to base share driver so every existing share driver will inherit it. Make value of it be calculated from the fact of redefinition of three main driver methods for snapshots 'create_snapshot', 'delete_snapshot' and 'create_share_from_snapshot'. - Set extra spec 'snapshot_support' with share type creation by default to 'True' - Restrict deletion of extra spec 'snapshot_support' that is expected to exist - Allow to redefine new extra spec 'snapshot_support' - Restrict API 'snapshot create' for share created with share type that has extra spec 'snapshot_support' equal to 'False'. - Add migration where new extra spec 'snapshot_support' is added to all share types that do not have it yet. Partially implements bp snapshots-optional Change-Id: I069d9e911c7d7a708fa518b38ed10572a45e5f42 --- .../test_share_types_extra_specs_negative.py | 20 +++ contrib/tempest/tempest/api/share/base.py | 5 +- contrib/tempest/tempest/config_share.py | 2 +- manila/api/contrib/types_extra_specs.py | 4 +- manila/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 5 + manila/api/v1/share_snapshots.py | 8 ++ manila/api/v1/shares.py | 10 ++ manila/common/constants.py | 17 ++- ...pshot_support_extra_spec_to_share_types.py | 123 ++++++++++++++++++ manila/db/sqlalchemy/models.py | 1 + manila/scheduler/filter_scheduler.py | 7 +- manila/scheduler/host_manager.py | 5 + manila/share/api.py | 5 + manila/share/driver.py | 31 +++++ manila/share/drivers/glusterfs.py | 13 -- manila/share/share_types.py | 11 ++ manila/tests/api/contrib/stubs.py | 1 + manila/tests/api/v1/test_share_snapshots.py | 30 ++++- manila/tests/fake_share.py | 1 + manila/tests/scheduler/fakes.py | 5 + .../tests/scheduler/test_filter_scheduler.py | 55 +++++++- manila/tests/scheduler/test_host_manager.py | 11 ++ manila/tests/share/drivers/emc/test_driver.py | 1 + manila/tests/share/drivers/hds/test_sop.py | 1 + .../share/drivers/hp/test_hp_3par_driver.py | 2 + .../share/drivers/huawei/test_huawei_nas.py | 1 + manila/tests/share/drivers/test_glusterfs.py | 1 + .../share/drivers/test_glusterfs_native.py | 1 + manila/tests/share/test_driver.py | 86 ++++++++++++ 30 files changed, 438 insertions(+), 28 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/55761e5f59c5_add_snapshot_support_extra_spec_to_share_types.py diff --git a/contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs_negative.py b/contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs_negative.py index f79928ffb7..b646df79a2 100644 --- a/contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs_negative.py +++ b/contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs_negative.py @@ -247,3 +247,23 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest): self.assertRaises(lib_exc.BadRequest, self.shares_client.update_share_type_extra_specs, st["share_type"]["id"], {"": "new_value"}) + + @test.attr(type=["gate", "smoke", ]) + def test_try_delete_spec_driver_handles_share_servers(self): + st = self._create_share_type() + + # Try delete extra spec 'driver_handles_share_servers' + self.assertRaises(lib_exc.Forbidden, + self.shares_client.delete_share_type_extra_spec, + st["share_type"]["id"], + "driver_handles_share_servers") + + @test.attr(type=["gate", "smoke", ]) + def test_try_delete_spec_snapshot_support(self): + st = self._create_share_type() + + # Try delete extra spec 'snapshot_support' + self.assertRaises(lib_exc.Forbidden, + self.shares_client.delete_share_type_extra_spec, + st["share_type"]["id"], + "snapshot_support") diff --git a/contrib/tempest/tempest/api/share/base.py b/contrib/tempest/tempest/api/share/base.py index 1400e4f1cc..9b56c4af24 100644 --- a/contrib/tempest/tempest/api/share/base.py +++ b/contrib/tempest/tempest/api/share/base.py @@ -444,7 +444,10 @@ class BaseSharesTest(test.BaseTestCase): @staticmethod def add_required_extra_specs_to_dict(extra_specs=None): value = six.text_type(CONF.share.multitenancy_enabled) - required = {"driver_handles_share_servers": value} + required = { + "driver_handles_share_servers": value, + "snapshot_support": 'True', + } if extra_specs: required.update(extra_specs) return required diff --git a/contrib/tempest/tempest/config_share.py b/contrib/tempest/tempest/config_share.py index 4d59b6bf09..4bd8933088 100644 --- a/contrib/tempest/tempest/config_share.py +++ b/contrib/tempest/tempest/config_share.py @@ -36,7 +36,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="1.2", + default="1.3", help="The maximum api microversion is configured to be the " "value of the latest microversion supported by Manila."), cfg.StrOpt("region", diff --git a/manila/api/contrib/types_extra_specs.py b/manila/api/contrib/types_extra_specs.py index 3fafc9104c..18a908f5c9 100644 --- a/manila/api/contrib/types_extra_specs.py +++ b/manila/api/contrib/types_extra_specs.py @@ -135,8 +135,8 @@ class ShareTypeExtraSpecsController(wsgi.Controller): self._check_type(context, type_id) authorize(context) - if id in share_types.get_required_extra_specs(): - msg = _("Required extra specs can't be deleted.") + if id in share_types.get_undeletable_extra_specs(): + msg = _("Extra spec '%s' can't be deleted.") % id raise webob.exc.HTTPForbidden(explanation=msg) try: diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index a70623dc33..be19108f95 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -47,6 +47,7 @@ REST_API_VERSION_HISTORY = """ * 1.0 - Initial version. Includes all V1 APIs and extensions in Kilo. * 1.1 - Versions API updated to reflect beginning of microversions epoch. * 1.2 - Share create() doesn't ignore availability_zone field of share. + * 1.3 - Snapshots become optional feature. """ @@ -54,7 +55,7 @@ REST_API_VERSION_HISTORY = """ # The default api version request is defined to be the # the minimum version of the API supported. _MIN_API_VERSION = "1.0" -_MAX_API_VERSION = "1.2" +_MAX_API_VERSION = "1.3" 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 b5e9502e2a..10a6f34a9c 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -35,3 +35,8 @@ user documentation. --- Share create() method doesn't ignore availability_zone field of provided share. + +1.3 +--- + Snapshots become optional and share payload now has + boolean attr 'snapshot_support'. diff --git a/manila/api/v1/share_snapshots.py b/manila/api/v1/share_snapshots.py index 10db81493b..52010baa28 100644 --- a/manila/api/v1/share_snapshots.py +++ b/manila/api/v1/share_snapshots.py @@ -149,6 +149,14 @@ class ShareSnapshotsController(wsgi.Controller): share_id = snapshot['share_id'] share = self.share_api.get(context, share_id) + + # Verify that share can be snapshotted + if not share['snapshot_support']: + msg = _("Snapshot cannot be created from share '%s', because " + "share back end does not support it.") % share_id + LOG.error(msg) + raise exc.HTTPUnprocessableEntity(msg) + LOG.info(_LI("Create snapshot from share %s"), share_id, context=context) diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index dffb6e7e45..6bd95b760d 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -162,7 +162,17 @@ class ShareController(wsgi.Controller): share.update(update_dict) return self._view_builder.detail(req, share) + @wsgi.Controller.api_version("1.3") def create(self, req, body): + return self._create(req, body) + + @wsgi.Controller.api_version("1.0", "1.2") # noqa + def create(self, req, body): + share = self._create(req, body) + share.pop('snapshot_support', None) + return share + + def _create(self, req, body): """Creates a new share.""" context = req.environ['manila.context'] diff --git a/manila/common/constants.py b/manila/common/constants.py index 294cdd8387..9361026c2f 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -83,5 +83,20 @@ ACCESS_LEVELS = ( class ExtraSpecs(object): + + # Extra specs key names DRIVER_HANDLES_SHARE_SERVERS = "driver_handles_share_servers" - REQUIRED = (DRIVER_HANDLES_SHARE_SERVERS, ) + SNAPSHOT_SUPPORT = "snapshot_support" + + # Extra specs containers + REQUIRED = ( + DRIVER_HANDLES_SHARE_SERVERS, + ) + UNDELETABLE = ( + DRIVER_HANDLES_SHARE_SERVERS, + SNAPSHOT_SUPPORT, + ) + BOOLEAN = ( + DRIVER_HANDLES_SHARE_SERVERS, + SNAPSHOT_SUPPORT, + ) diff --git a/manila/db/migrations/alembic/versions/55761e5f59c5_add_snapshot_support_extra_spec_to_share_types.py b/manila/db/migrations/alembic/versions/55761e5f59c5_add_snapshot_support_extra_spec_to_share_types.py new file mode 100644 index 0000000000..ce1ea0bcc4 --- /dev/null +++ b/manila/db/migrations/alembic/versions/55761e5f59c5_add_snapshot_support_extra_spec_to_share_types.py @@ -0,0 +1,123 @@ +# Copyright 2015 Mirantis Inc. +# 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 'snapshot_support' extra spec to share types + +Revision ID: 55761e5f59c5 +Revises: 1f0bd302c1a6 +Create Date: 2015-08-13 14:02:54.656864 + +""" + +# revision identifiers, used by Alembic. +revision = '55761e5f59c5' +down_revision = '1f0bd302c1a6' + +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 support feature of making snapshots optional. + + Add 'snapshot_support' extra spec to all share types and + attr 'snapshot_support' to Share model. + """ + session = sa.orm.Session(bind=op.get_bind().connect()) + + es_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))) + + st_table = table( + 'share_types', + sa.Column('deleted', sa.Integer), + sa.Column('id', sa.Integer)) + + # NOTE(vponomaryov): field 'deleted' is integer here. + existing_extra_specs = session.query(es_table).\ + filter(es_table.c.spec_key == constants.ExtraSpecs.SNAPSHOT_SUPPORT).\ + filter(es_table.c.deleted == 0).\ + all() + exclude_st_ids = [es.share_type_id for es in existing_extra_specs] + + # NOTE(vponomaryov): field 'deleted' is string here. + share_types = session.query(st_table).\ + filter(st_table.c.deleted.in_(('0', 'False', ))).\ + filter(st_table.c.id.notin_(exclude_st_ids)).\ + all() + session.close_all() + + extra_specs = [] + now = timeutils.utcnow() + for st in share_types: + extra_specs.append({ + 'spec_key': constants.ExtraSpecs.SNAPSHOT_SUPPORT, + 'spec_value': 'True', + 'deleted': 0, + 'created_at': now, + 'share_type_id': st.id, + }) + if extra_specs: + op.bulk_insert(es_table, extra_specs) + + # NOTE(vponomaryov): shares that were created before applying this + # migration can have incorrect value because they were created without + # consideration of driver capability to create snapshots. + op.add_column('shares', + sa.Column('snapshot_support', sa.Boolean, default=True)) + + connection = op.get_bind().connect() + shares = sa.Table( + 'shares', + sa.MetaData(), + autoload=True, + autoload_with=connection) + + update = shares.update().where(shares.c.deleted == 'False').values( + snapshot_support=True) + connection.execute(update) + + +def downgrade(): + """Performs DB downgrade removing support of 'optional snapshots' feature. + + Remove 'snapshot_support' extra spec from all share types and + attr 'snapshot_support' from Share model. + """ + connection = op.get_bind().connect() + 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.SNAPSHOT_SUPPORT).where( + extra_specs.c.deleted == 0).values( + deleted=extra_specs.c.id, + deleted_at=timeutils.utcnow(), + ) + connection.execute(update) + + op.drop_column('shares', 'snapshot_support') diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 4e391980c3..96ebb5b953 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -245,6 +245,7 @@ class Share(BASE, ManilaBase): display_name = Column(String(255)) display_description = Column(String(255)) snapshot_id = Column(String(36)) + snapshot_support = Column(Boolean, default=True) share_proto = Column(String(255)) share_type_id = Column(String(36), ForeignKey('share_types.id'), nullable=True) diff --git a/manila/scheduler/filter_scheduler.py b/manila/scheduler/filter_scheduler.py index 0fde31fe91..86fbc9f22f 100644 --- a/manila/scheduler/filter_scheduler.py +++ b/manila/scheduler/filter_scheduler.py @@ -126,12 +126,13 @@ class FilterScheduler(driver.Scheduler): extra_specs = share_type.get('extra_specs', {}) if extra_specs: - for extra_spec_name in share_types.get_required_extra_specs(): + for extra_spec_name in share_types.get_boolean_extra_specs(): extra_spec = extra_specs.get(extra_spec_name) if extra_spec is not None: - share_type['extra_specs'][extra_spec_name] = ( - " %s" % extra_spec) + if not extra_spec.startswith(""): + extra_spec = " %s" % extra_spec + share_type['extra_specs'][extra_spec_name] = extra_spec resource_type = request_spec.get("share_type") or {} request_spec.update({'resource_properties': resource_properties}) diff --git a/manila/scheduler/host_manager.py b/manila/scheduler/host_manager.py index 714696a8df..2e3dad7880 100644 --- a/manila/scheduler/host_manager.py +++ b/manila/scheduler/host_manager.py @@ -123,6 +123,7 @@ class HostState(object): self.thin_provisioning_support = False self.thick_provisioning_support = False self.driver_handles_share_servers = False + self.snapshot_support = True # PoolState for all pools self.pools = {} @@ -275,6 +276,9 @@ class HostState(object): pool_cap['driver_handles_share_servers'] = \ self.driver_handles_share_servers + if not pool_cap.get('snapshot_support'): + pool_cap['snapshot_support'] = True + def update_backend(self, capability): self.share_backend_name = capability.get('share_backend_name') self.vendor_name = capability.get('vendor_name') @@ -282,6 +286,7 @@ class HostState(object): self.storage_protocol = capability.get('storage_protocol') self.driver_handles_share_servers = capability.get( 'driver_handles_share_servers') + self.snapshot_support = capability.get('snapshot_support') self.updated = capability['timestamp'] def consume_from_share(self, share): diff --git a/manila/share/api.py b/manila/share/api.py index 978b5a2d78..f46ec50124 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -160,6 +160,10 @@ 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) except ValueError as e: raise exception.InvalidParameterValue(six.text_type(e)) @@ -167,6 +171,7 @@ class API(base.Base): 'user_id': context.user_id, 'project_id': context.project_id, 'snapshot_id': snapshot_id, + 'snapshot_support': snapshot_support, 'metadata': metadata, 'display_name': name, 'display_description': description, diff --git a/manila/share/driver.py b/manila/share/driver.py index 3ee2924ad2..8b1185aa05 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -445,6 +445,36 @@ class ShareDriver(object): """ raise NotImplementedError() + def _has_redefined_driver_methods(self, methods): + """Returns boolean as a result of methods presence and redefinition.""" + if not isinstance(methods, (set, list, tuple)): + methods = (methods, ) + parent = super(self.__class__, self) + for method in methods: + # NOTE(vponomaryov): criteria: + # - If parent does not have such attr, then was called method of + # this class which has no implementation. + # - If parent's method is equal to child's method then child did + # not redefine it and has no implementation. + if (not hasattr(parent, method) or + getattr(self, method) == getattr(parent, method)): + return False + return True + + @property + def snapshots_are_supported(self): + if not hasattr(self, '_snapshots_are_supported'): + methods = ( + "create_snapshot", + "delete_snapshot", + "create_share_from_snapshot") + # NOTE(vponomaryov): calculate default value for + # stat 'snapshot_support' based on implementation of + # appropriate methods of this base driver class. + self._snapshots_are_supported = self._has_redefined_driver_methods( + methods) + return self._snapshots_are_supported + def _update_share_stats(self, data=None): """Retrieve stats info from share group. @@ -469,6 +499,7 @@ class ShareDriver(object): reserved_percentage=0, QoS_support=False, pools=self.pools or None, + snapshot_support=self.snapshots_are_supported, ) if isinstance(data, dict): common.update(data) diff --git a/manila/share/drivers/glusterfs.py b/manila/share/drivers/glusterfs.py index 19ad16e4cf..0dd07a89d4 100644 --- a/manila/share/drivers/glusterfs.py +++ b/manila/share/drivers/glusterfs.py @@ -408,19 +408,6 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.GaneshaMixin, LOG.error(_LE('Unable to delete share %s'), share['name']) raise - def create_snapshot(self, context, snapshot, share_server=None): - """TBD: Is called to create snapshot.""" - raise NotImplementedError() - - def create_share_from_snapshot(self, context, share, snapshot, - share_server=None): - """Is called to create share from snapshot.""" - raise NotImplementedError() - - def delete_snapshot(self, context, snapshot, share_server=None): - """TBD: Is called to remove snapshot.""" - raise NotImplementedError() - def ensure_share(self, context, share, share_server=None): """Might not be needed?""" pass diff --git a/manila/share/share_types.py b/manila/share/share_types.py index 0977b1283f..ba757f185f 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -39,6 +39,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) except exception.InvalidExtraSpec as e: @@ -196,6 +199,14 @@ def get_required_extra_specs(): return constants.ExtraSpecs.REQUIRED +def get_undeletable_extra_specs(): + return constants.ExtraSpecs.UNDELETABLE + + +def get_boolean_extra_specs(): + return constants.ExtraSpecs.BOOLEAN + + def is_valid_required_extra_spec(key, value): """Validates required extra_spec value. diff --git a/manila/tests/api/contrib/stubs.py b/manila/tests/api/contrib/stubs.py index 054a5d051c..ea505a4a8c 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_network_id': None, 'share_server_id': 'fake_share_server_id', 'is_public': False, + 'snapshot_support': True, } share.update(kwargs) return share diff --git a/manila/tests/api/v1/test_share_snapshots.py b/manila/tests/api/v1/test_share_snapshots.py index 5ce75c3925..49fb7234af 100644 --- a/manila/tests/api/v1/test_share_snapshots.py +++ b/manila/tests/api/v1/test_share_snapshots.py @@ -15,6 +15,7 @@ import datetime +import ddt import mock import webob @@ -25,6 +26,7 @@ from manila.tests.api.contrib import stubs from manila.tests.api import fakes +@ddt.ddt class ShareSnapshotApiTest(test.TestCase): """Share Snapshot Api Test.""" @@ -48,7 +50,8 @@ class ShareSnapshotApiTest(test.TestCase): } self.maxDiff = None - def test_snapshot_create(self): + @ddt.data('true', 'True', ' True', '1') + def test_snapshot_create(self, snapshot_support): self.mock_object(share_api.API, 'create_snapshot', stubs.stub_snapshot_create) body = { @@ -60,7 +63,9 @@ class ShareSnapshotApiTest(test.TestCase): } } req = fakes.HTTPRequest.blank('/snapshots') + res_dict = self.controller.create(req, body) + expected = { 'snapshot': { 'id': 200, @@ -86,6 +91,29 @@ class ShareSnapshotApiTest(test.TestCase): } self.assertEqual(expected, res_dict) + @ddt.data(0, False) + def test_snapshot_create_no_support(self, snapshot_support): + self.mock_object(share_api.API, 'create_snapshot') + self.mock_object( + share_api.API, + 'get', + mock.Mock(return_value={'snapshot_support': snapshot_support})) + body = { + 'snapshot': { + 'share_id': 100, + 'force': False, + 'name': 'fake_share_name', + 'description': 'fake_share_description', + } + } + req = fakes.HTTPRequest.blank('/snapshots') + + self.assertRaises( + webob.exc.HTTPUnprocessableEntity, + self.controller.create, req, body) + + self.assertFalse(share_api.API.create_snapshot.called) + def test_snapshot_create_no_body(self): body = {} req = fakes.HTTPRequest.blank('/snapshots') diff --git a/manila/tests/fake_share.py b/manila/tests/fake_share.py index fcd2371890..b253019af5 100644 --- a/manila/tests/fake_share.py +++ b/manila/tests/fake_share.py @@ -27,6 +27,7 @@ def fake_share(**kwargs): 'share_server_id': 'fake share server id', 'export_location': 'fake_location:/fake_share', 'project_id': 'fake_project_uuid', + 'snapshot_support': 'True', } share.update(kwargs) return db_fakes.FakeModel(share) diff --git a/manila/tests/scheduler/fakes.py b/manila/tests/scheduler/fakes.py index 64be9d2995..150c612b15 100644 --- a/manila/tests/scheduler/fakes.py +++ b/manila/tests/scheduler/fakes.py @@ -76,6 +76,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { 'host1@AAA': dict(share_backend_name='AAA', timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, + snapshot_support=True, pools=[dict(pool_name='pool1', total_capacity_gb=51, free_capacity_gb=41, @@ -87,6 +88,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { 'host2@BBB': dict(share_backend_name='BBB', timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, + snapshot_support=True, pools=[dict(pool_name='pool2', total_capacity_gb=52, free_capacity_gb=42, @@ -98,6 +100,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { 'host3@CCC': dict(share_backend_name='CCC', timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, + snapshot_support=True, pools=[dict(pool_name='pool3', total_capacity_gb=53, free_capacity_gb=43, @@ -109,6 +112,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { 'host4@DDD': dict(share_backend_name='DDD', timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, + snapshot_support=True, pools=[dict(pool_name='pool4a', total_capacity_gb=541, free_capacity_gb=441, @@ -128,6 +132,7 @@ SHARE_SERVICE_STATES_WITH_POOLS = { 'host5@EEE': dict(share_backend_name='EEE', timestamp=None, reserved_percentage=0, driver_handles_share_servers=False, + snapshot_support=True, pools=[dict(pool_name='pool5a', total_capacity_gb=551, free_capacity_gb=451, diff --git a/manila/tests/scheduler/test_filter_scheduler.py b/manila/tests/scheduler/test_filter_scheduler.py index dc396725c2..963f602fb5 100644 --- a/manila/tests/scheduler/test_filter_scheduler.py +++ b/manila/tests/scheduler/test_filter_scheduler.py @@ -15,8 +15,12 @@ """ Tests For Filter Scheduler. """ -import mock +import ddt +import mock +from oslo_utils import strutils + +from manila.common import constants from manila import context from manila import exception from manila.scheduler import filter_scheduler @@ -24,7 +28,10 @@ from manila.scheduler import host_manager from manila.tests.scheduler import fakes from manila.tests.scheduler import test_scheduler +SNAPSHOT_SUPPORT = constants.ExtraSpecs.SNAPSHOT_SUPPORT + +@ddt.ddt class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): """Test case for Filter Scheduler.""" @@ -69,22 +76,60 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context, request_spec, {}) self.assertTrue(self.was_admin) + @ddt.data( + {'name': 'foo'}, + {'name': 'foo', 'extra_specs': {}}, + *[{'name': 'foo', 'extra_specs': {SNAPSHOT_SUPPORT: v}} + for v in ('True', ' True', 'true', '1')] + ) @mock.patch('manila.db.service_get_all_by_topic') - def test_schedule_happy_day_share(self, _mock_service_get_all_by_topic): - # Make sure there's nothing glaringly wrong with _schedule() - # by doing a happy day pass through. + def test__schedule_share_with_snapshot_support( + self, share_type, _mock_service_get_all_by_topic): sched = fakes.FakeFilterScheduler() sched.host_manager = fakes.FakeHostManager() fake_context = context.RequestContext('user', 'project', is_admin=True) fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) request_spec = { - 'share_type': {'name': 'NFS'}, + 'share_type': share_type, 'share_properties': {'project_id': 1, 'size': 1}, 'share_instance_properties': {}, } + weighed_host = sched._schedule_share(fake_context, request_spec, {}) + + self.assertIsNotNone(weighed_host) self.assertIsNotNone(weighed_host.obj) + self.assertTrue(hasattr(weighed_host.obj, SNAPSHOT_SUPPORT)) + expected_snapshot_support = strutils.bool_from_string( + share_type.get('extra_specs', {}).get( + SNAPSHOT_SUPPORT, 'True').split()[-1]) + self.assertEqual( + expected_snapshot_support, + getattr(weighed_host.obj, SNAPSHOT_SUPPORT)) + self.assertTrue(_mock_service_get_all_by_topic.called) + + @ddt.data( + *[{'name': 'foo', 'extra_specs': {SNAPSHOT_SUPPORT: v}} + for v in ('False', ' False', 'false', '0')] + ) + @mock.patch('manila.db.service_get_all_by_topic') + def test__schedule_share_without_snapshot_support( + self, share_type, _mock_service_get_all_by_topic): + sched = fakes.FakeFilterScheduler() + sched.host_manager = fakes.FakeHostManager() + fake_context = context.RequestContext('user', 'project', + is_admin=True) + fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) + request_spec = { + 'share_type': share_type, + 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {'project_id': 1, 'size': 1}, + } + + weighed_host = sched._schedule_share(fake_context, request_spec, {}) + + self.assertIsNone(weighed_host) self.assertTrue(_mock_service_get_all_by_topic.called) def test_max_attempts(self): diff --git a/manila/tests/scheduler/test_host_manager.py b/manila/tests/scheduler/test_host_manager.py index a8496199db..8b59610aea 100644 --- a/manila/tests/scheduler/test_host_manager.py +++ b/manila/tests/scheduler/test_host_manager.py @@ -195,6 +195,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host2@back1#BBB', @@ -215,6 +216,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host2@back2#CCC', @@ -235,6 +237,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, ] @@ -277,6 +280,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host2@BBB#pool2', @@ -298,6 +302,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host3@CCC#pool3', @@ -319,6 +324,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host4@DDD#pool4a', @@ -340,6 +346,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, { 'name': 'host4@DDD#pool4b', @@ -361,6 +368,7 @@ class HostManagerTestCase(test.TestCase): 'vendor_name': None, 'storage_protocol': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, }, }, ] @@ -404,6 +412,7 @@ class HostManagerTestCase(test.TestCase): 'capabilities': { 'timestamp': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, 'share_backend_name': 'AAA', 'free_capacity_gb': 200, 'driver_version': None, @@ -424,6 +433,7 @@ class HostManagerTestCase(test.TestCase): 'capabilities': { 'timestamp': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, 'share_backend_name': 'BBB', 'free_capacity_gb': 100, 'driver_version': None, @@ -470,6 +480,7 @@ class HostManagerTestCase(test.TestCase): 'pool_name': 'pool2', 'timestamp': None, 'driver_handles_share_servers': False, + 'snapshot_support': True, 'share_backend_name': 'BBB', 'free_capacity_gb': 42, 'driver_version': None, diff --git a/manila/tests/share/drivers/emc/test_driver.py b/manila/tests/share/drivers/emc/test_driver.py index 5638985cd1..9b1b6b4b65 100644 --- a/manila/tests/share/drivers/emc/test_driver.py +++ b/manila/tests/share/drivers/emc/test_driver.py @@ -138,6 +138,7 @@ class EMCShareFrameworkTestCase(test.TestCase): data['reserved_percentage'] = 0 data['QoS_support'] = False data['pools'] = None + data['snapshot_support'] = True self.assertEqual(data, self.driver._stats) def _fake_safe_get(self, value): diff --git a/manila/tests/share/drivers/hds/test_sop.py b/manila/tests/share/drivers/hds/test_sop.py index a9d7e4b4aa..8a7c195bbd 100644 --- a/manila/tests/share/drivers/hds/test_sop.py +++ b/manila/tests/share/drivers/hds/test_sop.py @@ -469,6 +469,7 @@ class SopShareDriverTestCase(test.TestCase): 'total_capacity_gb': 1234, 'free_capacity_gb': 2345, 'pools': None, + 'snapshot_support': True, } self.mock_object(self._driver, '_get_sop_filesystem_stats', mock.Mock(return_value=(1234, 2345))) diff --git a/manila/tests/share/drivers/hp/test_hp_3par_driver.py b/manila/tests/share/drivers/hp/test_hp_3par_driver.py index 298e92b855..a39bf33432 100644 --- a/manila/tests/share/drivers/hp/test_hp_3par_driver.py +++ b/manila/tests/share/drivers/hp/test_hp_3par_driver.py @@ -469,6 +469,7 @@ class HP3ParDriverTestCase(test.TestCase): 'thin_provisioning': True, 'dedupe': False, 'hpe3par_flash_cache': False, + 'snapshot_support': True, } result = self.driver.get_share_stats(refresh=True) @@ -501,6 +502,7 @@ class HP3ParDriverTestCase(test.TestCase): 'thin_provisioning': True, 'total_capacity_gb': 0, 'vendor_name': 'HP', + 'snapshot_support': True, } result = self.driver.get_share_stats(refresh=True) diff --git a/manila/tests/share/drivers/huawei/test_huawei_nas.py b/manila/tests/share/drivers/huawei/test_huawei_nas.py index 71bdddb878..a479a79afd 100644 --- a/manila/tests/share/drivers/huawei/test_huawei_nas.py +++ b/manila/tests/share/drivers/huawei/test_huawei_nas.py @@ -1103,6 +1103,7 @@ class HuaweiShareDriverTestCase(test.TestCase): expected['total_capacity_gb'] = 0.0 expected['free_capacity_gb'] = 0.0 expected['QoS_support'] = False + expected["snapshot_support"] = False expected["pools"] = [] pool_thin = dict( pool_name='OpenStack_Pool', diff --git a/manila/tests/share/drivers/test_glusterfs.py b/manila/tests/share/drivers/test_glusterfs.py index 50900aa388..151ac9975c 100644 --- a/manila/tests/share/drivers/test_glusterfs.py +++ b/manila/tests/share/drivers/test_glusterfs.py @@ -534,6 +534,7 @@ class GlusterfsShareDriverTestCase(test.TestCase): 'total_capacity_gb': 2, 'free_capacity_gb': 2, 'pools': None, + 'snapshot_support': False, } test_statvfs = mock.Mock(f_frsize=4096, f_blocks=524288, f_bavail=524288) diff --git a/manila/tests/share/drivers/test_glusterfs_native.py b/manila/tests/share/drivers/test_glusterfs_native.py index f88b7fa18d..30cbd50427 100644 --- a/manila/tests/share/drivers/test_glusterfs_native.py +++ b/manila/tests/share/drivers/test_glusterfs_native.py @@ -1104,6 +1104,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): 'total_capacity_gb': 'infinite', 'free_capacity_gb': 'infinite', 'pools': None, + 'snapshot_support': False, } self._driver._update_share_stats() diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 272c194fb8..1efe9e71b3 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -118,6 +118,7 @@ class ShareDriverTestCase(test.TestCase): 'free_capacity_gb', 'total_capacity_gb', 'driver_handles_share_servers', 'reserved_percentage', 'vendor_name', 'storage_protocol', + 'snapshot_support', ] share_driver = driver.ShareDriver(True, configuration=conf) fake_stats = {'fake_key': 'fake_value'} @@ -219,3 +220,88 @@ class ShareDriverTestCase(test.TestCase): else: self.assertRaises(exception.InvalidParameterValue, share_driver.check_for_setup_error) + + def test_snapshot_support_exists(self): + driver.CONF.set_default('driver_handles_share_servers', True) + fake_method = lambda *args, **kwargs: None + 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) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats() + + self.assertEqual( + True, child_class_instance._stats["snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data( + (), + ("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"), + ) + def test_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.assertEqual( + False, child_class_instance._stats["snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data(True, False) + def test_snapshot_support_not_exists_and_set_explicitly( + self, snapshots_are_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( + {"snapshot_support": snapshots_are_supported}) + + self.assertEqual( + snapshots_are_supported, + child_class_instance._stats["snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called) + + @ddt.data(True, False) + def test_snapshot_support_exists_and_set_explicitly( + self, snapshots_are_supported): + driver.CONF.set_default('driver_handles_share_servers', True) + fake_method = lambda *args, **kwargs: None + 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) + self.mock_object(child_class_instance, "configuration") + + child_class_instance._update_share_stats( + {"snapshot_support": snapshots_are_supported}) + + self.assertEqual( + snapshots_are_supported, + child_class_instance._stats["snapshot_support"]) + self.assertTrue(child_class_instance.configuration.safe_get.called)