From 2195885e7710adfc6c4be944f22e5cc01a2361e8 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 25 Nov 2016 15:56:51 +0100 Subject: [PATCH] Fix replication freeze mechanism Freeze functionality in the replication feature doesn't work as expected, since it is not being used on the scheduler to exclude backends or used on the API or volume nodes so API-to-Vol operations like delete and create snapshot will also work. This patch fixes the freeze mechanism by excluding frozen backends in the scheduler and checking the if the service is frozen on all other modifying operations. Since extend operation now goes through the scheduler it will be frozen there. Closes-Bug: #1616974 Change-Id: I4561500746c95b96136878ddfde8ca88e96b28c6 --- cinder/consistencygroup/api.py | 6 ++ cinder/db/api.py | 8 +++ cinder/db/sqlalchemy/api.py | 59 +++++++++++----- cinder/group/api.py | 6 ++ cinder/objects/base.py | 7 ++ cinder/objects/cgsnapshot.py | 10 ++- cinder/objects/group_snapshot.py | 10 ++- cinder/objects/snapshot.py | 7 +- cinder/scheduler/host_manager.py | 7 +- .../unit/api/v2/test_snapshot_metadata.py | 5 +- cinder/tests/unit/api/v3/test_snapshots.py | 2 +- cinder/tests/unit/consistencygroup/test_cg.py | 38 ++++++++++ cinder/tests/unit/group/test_groups_api.py | 40 +++++++++++ .../test_allocated_capacity_weigher.py | 2 +- .../unit/scheduler/test_capacity_weigher.py | 2 +- .../tests/unit/scheduler/test_host_manager.py | 2 + .../scheduler/test_volume_number_weigher.py | 1 + cinder/tests/unit/test_db_api.py | 69 +++++++++++++------ cinder/tests/unit/test_volume.py | 22 ++++++ .../drivers/dell_emc/vnx/test_adapter.py | 3 - cinder/volume/api.py | 7 ++ 21 files changed, 256 insertions(+), 57 deletions(-) diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py index fbd3b698a60..96a0db7a77d 100644 --- a/cinder/consistencygroup/api.py +++ b/cinder/consistencygroup/api.py @@ -212,6 +212,8 @@ class API(base.Base): LOG.error(msg) raise exception.InvalidConsistencyGroup(reason=msg) + group.assert_not_frozen() + if cgsnapshot_id: self._create_cg_from_cgsnapshot(context, group, cgsnapshot_id) elif source_cgid: @@ -428,6 +430,8 @@ class API(base.Base): return + group.assert_not_frozen() + if force: expected = {} else: @@ -714,6 +718,7 @@ class API(base.Base): return groups def create_cgsnapshot(self, context, group, name, description): + group.assert_not_frozen() options = {'consistencygroup_id': group.id, 'user_id': context.user_id, 'project_id': context.project_id, @@ -750,6 +755,7 @@ class API(base.Base): return cgsnapshot def delete_cgsnapshot(self, context, cgsnapshot, force=False): + cgsnapshot.assert_not_frozen() values = {'status': 'deleting'} expected = {'status': ('available', 'error')} filters = [~db.cg_creating_from_src(cgsnapshot_id=cgsnapshot.id)] diff --git a/cinder/db/api.py b/cinder/db/api.py index 8de1c751846..cd31cf23dfb 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -143,6 +143,14 @@ def service_update(context, service_id, values): ############### +def is_backend_frozen(context, host, cluster_name): + """Check if a storage backend is frozen based on host and cluster_name.""" + return IMPL.is_backend_frozen(context, host, cluster_name) + + +############### + + def cluster_get(context, id=None, is_up=None, get_services=False, services_summary=False, read_deleted='no', name_match_level=None, **filters): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index f1f9dc667a4..5e4c9f77149 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -428,9 +428,29 @@ def _filter_host(field, value, match_level=None): return or_(*conditions) +def _clustered_bool_field_filter(query, field_name, filter_value): + # Now that we have clusters, a service is disabled/frozen if the service + # doesn't belong to a cluster or if it belongs to a cluster and the cluster + # itself is disabled/frozen. + if filter_value is not None: + query_filter = or_( + and_(models.Service.cluster_name.is_(None), + getattr(models.Service, field_name)), + and_(models.Service.cluster_name.isnot(None), + sql.exists().where(and_( + models.Cluster.name == models.Service.cluster_name, + models.Cluster.binary == models.Service.binary, + ~models.Cluster.deleted, + getattr(models.Cluster, field_name))))) + if not filter_value: + query_filter = ~query_filter + query = query.filter(query_filter) + return query + + def _service_query(context, session=None, read_deleted='no', host=None, cluster_name=None, is_up=None, backend_match_level=None, - disabled=None, **filters): + disabled=None, frozen=None, **filters): filters = _clean_filters(filters) if filters and not is_valid_model_filters(models.Service, filters): return None @@ -448,22 +468,9 @@ def _service_query(context, session=None, read_deleted='no', host=None, query = query.filter(_filter_host(models.Service.cluster_name, cluster_name, backend_match_level)) - # Now that we have clusters, a service is disabled if the service doesn't - # belong to a cluster or if it belongs to a cluster and the cluster itself - # is disabled. - if disabled is not None: - disabled_filter = or_( - and_(models.Service.cluster_name.is_(None), - models.Service.disabled), - and_(models.Service.cluster_name.isnot(None), - sql.exists().where(and_( - models.Cluster.name == models.Service.cluster_name, - models.Cluster.binary == models.Service.binary, - ~models.Cluster.deleted, - models.Cluster.disabled)))) - if not disabled: - disabled_filter = ~disabled_filter - query = query.filter(disabled_filter) + query = _clustered_bool_field_filter(query, 'disabled', disabled) + query = _clustered_bool_field_filter(query, 'frozen', frozen) + if filters: query = query.filter_by(**filters) @@ -553,6 +560,24 @@ def service_update(context, service_id, values): raise exception.ServiceNotFound(service_id=service_id) +################### + + +@require_admin_context +def is_backend_frozen(context, host, cluster_name): + """Check if a storage backend is frozen based on host and cluster_name.""" + if cluster_name: + model = models.Cluster + conditions = [model.name == cluster_name] + else: + model = models.Service + conditions = [model.host == host] + conditions.extend((~model.deleted, model.frozen)) + query = get_session().query(sql.exists().where(and_(*conditions))) + frozen = query.scalar() + return frozen + + ################### def _cluster_query(context, is_up=None, get_services=False, diff --git a/cinder/group/api.py b/cinder/group/api.py index 00722329dce..dcb27cb73f8 100644 --- a/cinder/group/api.py +++ b/cinder/group/api.py @@ -245,6 +245,8 @@ class API(base.Base): LOG.error(msg) raise exception.InvalidGroup(reason=msg) + group.assert_not_frozen() + if group_snapshot_id: self._create_group_from_group_snapshot(context, group, group_snapshot_id) @@ -507,6 +509,8 @@ class API(base.Base): return + group.assert_not_frozen() + if not delete_volumes and group.status not in ( [c_fields.GroupStatus.AVAILABLE, c_fields.GroupStatus.ERROR]): @@ -787,6 +791,7 @@ class API(base.Base): group.save() def create_group_snapshot(self, context, group, name, description): + group.assert_not_frozen() options = {'group_id': group.id, 'user_id': context.user_id, 'project_id': context.project_id, @@ -825,6 +830,7 @@ class API(base.Base): def delete_group_snapshot(self, context, group_snapshot, force=False): check_policy(context, 'delete_group_snapshot') + group_snapshot.assert_not_frozen() values = {'status': 'deleting'} expected = {'status': ('available', 'error')} filters = [~db.group_creating_from_src( diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 1c22469cc49..4f89e88b0a4 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -465,6 +465,13 @@ class ClusteredObject(object): def is_clustered(self): return bool(self.cluster_name) + def assert_not_frozen(self): + ctxt = self._context.elevated() + if db.is_backend_frozen(ctxt, self.host, self.cluster_name): + msg = _('Modification operations are not allowed on frozen ' + 'storage backends.') + raise exception.InvalidInput(reason=msg) + class CinderObjectSerializer(base.VersionedObjectSerializer): OBJ_BASE_CLASS = CinderObject diff --git a/cinder/objects/cgsnapshot.py b/cinder/objects/cgsnapshot.py index 7a8dc74a50a..fc48aba289d 100644 --- a/cinder/objects/cgsnapshot.py +++ b/cinder/objects/cgsnapshot.py @@ -22,7 +22,7 @@ from oslo_versionedobjects import fields @base.CinderObjectRegistry.register class CGSnapshot(base.CinderPersistentObject, base.CinderObject, - base.CinderObjectDictCompat): + base.CinderObjectDictCompat, base.ClusteredObject): # Version 1.0: Initial version # Version 1.1: Added from_group_snapshot VERSION = '1.1' @@ -43,8 +43,12 @@ class CGSnapshot(base.CinderPersistentObject, base.CinderObject, } @property - def service_topic_queue(self): - return self.consistencygroup.service_topic_queue + def host(self): + return self.consistencygroup.host + + @property + def cluster_name(self): + return self.consistencygroup.cluster_name @classmethod def _from_db_object(cls, context, cgsnapshot, db_cgsnapshots, diff --git a/cinder/objects/group_snapshot.py b/cinder/objects/group_snapshot.py index ced66bfb3d1..443e00ea9f7 100644 --- a/cinder/objects/group_snapshot.py +++ b/cinder/objects/group_snapshot.py @@ -22,7 +22,7 @@ from oslo_versionedobjects import fields @base.CinderObjectRegistry.register class GroupSnapshot(base.CinderPersistentObject, base.CinderObject, - base.CinderObjectDictCompat): + base.CinderObjectDictCompat, base.ClusteredObject): VERSION = '1.0' OPTIONAL_FIELDS = ['group', 'snapshots'] @@ -41,8 +41,12 @@ class GroupSnapshot(base.CinderPersistentObject, base.CinderObject, } @property - def service_topic_queue(self): - return self.group.service_topic_queue + def host(self): + return self.group.host + + @property + def cluster_name(self): + return self.group.cluster_name @classmethod def _from_db_object(cls, context, group_snapshot, db_group_snapshots, diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index 5c3814274df..953de2b93cb 100644 --- a/cinder/objects/snapshot.py +++ b/cinder/objects/snapshot.py @@ -30,7 +30,8 @@ CONF = cfg.CONF @base.CinderObjectRegistry.register class Snapshot(cleanable.CinderCleanableObject, base.CinderObject, - base.CinderObjectDictCompat, base.CinderComparableObject): + base.CinderObjectDictCompat, base.CinderComparableObject, + base.ClusteredObject): # Version 1.0: Initial version # Version 1.1: Changed 'status' field to use SnapshotStatusField # Version 1.2: This object is now cleanable (adds rows to workers table) @@ -71,8 +72,8 @@ class Snapshot(cleanable.CinderCleanableObject, base.CinderObject, } @property - def service_topic_queue(self): - return self.volume.service_topic_queue + def cluster_name(self): + return self.volume.cluster_name @classmethod def _get_expected_attrs(cls, context, *args, **kwargs): diff --git a/cinder/scheduler/host_manager.py b/cinder/scheduler/host_manager.py index d85aceba632..ee4dd2a8f96 100644 --- a/cinder/scheduler/host_manager.py +++ b/cinder/scheduler/host_manager.py @@ -540,9 +540,10 @@ class HostManager(object): # Get resource usage across the available volume nodes: topic = constants.VOLUME_TOPIC - volume_services = objects.ServiceList.get_all_by_topic(context, - topic, - disabled=False) + volume_services = objects.ServiceList.get_all(context, + {'topic': topic, + 'disabled': False, + 'frozen': False}) active_backends = set() active_hosts = set() no_capabilities_hosts = set() diff --git a/cinder/tests/unit/api/v2/test_snapshot_metadata.py b/cinder/tests/unit/api/v2/test_snapshot_metadata.py index f8515b0fe03..83b93b2955f 100644 --- a/cinder/tests/unit/api/v2/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v2/test_snapshot_metadata.py @@ -82,7 +82,10 @@ def return_snapshot(context, snapshot_id): 'metadata': {}} -def fake_get(context, *args, **kwargs): +# First argument needs to be self to receive the context argument in the right +# variable, as this'll be used to replace the original API.get method which +# receives self as the first argument. +def fake_get(self, context, *args, **kwargs): vol = {'id': fake.VOLUME_ID, 'size': 100, 'name': 'fake', diff --git a/cinder/tests/unit/api/v3/test_snapshots.py b/cinder/tests/unit/api/v3/test_snapshots.py index 040dd4e1633..45ea55a9986 100644 --- a/cinder/tests/unit/api/v3/test_snapshots.py +++ b/cinder/tests/unit/api/v3/test_snapshots.py @@ -33,7 +33,7 @@ UUID = '00000000-0000-0000-0000-000000000001' INVALID_UUID = '00000000-0000-0000-0000-000000000002' -def stub_get(context, *args, **kwargs): +def stub_get(self, context, *args, **kwargs): vol = {'id': fake.VOLUME_ID, 'size': 100, 'name': 'fake', diff --git a/cinder/tests/unit/consistencygroup/test_cg.py b/cinder/tests/unit/consistencygroup/test_cg.py index 0d076a697c2..cfa48e55a80 100644 --- a/cinder/tests/unit/consistencygroup/test_cg.py +++ b/cinder/tests/unit/consistencygroup/test_cg.py @@ -14,6 +14,7 @@ import ddt import mock from oslo_config import cfg +import cinder.consistencygroup from cinder import context from cinder import db from cinder import exception @@ -359,6 +360,43 @@ class ConsistencyGroupTestCase(base.BaseVolumeTestCase): self.volume.delete_consistencygroup(self.context, group) + def test_create_consistencygroup_from_src_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + cg = tests_utils.create_consistencygroup(self.context, + host=service.host) + cg_api = cinder.consistencygroup.api.API() + self.assertRaises(exception.InvalidInput, + cg_api.create_from_src, + self.context, 'cg', 'desc', cgsnapshot_id=None, + source_cgid=cg.id) + + def test_delete_consistencygroup_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + cg = tests_utils.create_consistencygroup(self.context, + host=service.host) + cg_api = cinder.consistencygroup.api.API() + self.assertRaises(exception.InvalidInput, + cg_api.delete, self.context, cg) + + def test_create_cgsnapshot_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + cg = tests_utils.create_consistencygroup(self.context, + host=service.host) + cg_api = cinder.consistencygroup.api.API() + self.assertRaises(exception.InvalidInput, + cg_api.create_cgsnapshot, + self.context, cg, 'cg', 'desc') + + def test_delete_cgsnapshot_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + cg = tests_utils.create_consistencygroup(self.context, + host=service.host) + cgsnap = tests_utils.create_cgsnapshot(self.context, cg.id) + cg_api = cinder.consistencygroup.api.API() + self.assertRaises(exception.InvalidInput, + cg_api.delete_cgsnapshot, + self.context, cgsnap) + def test_sort_snapshots(self): vol1 = {'id': fake.VOLUME_ID, 'name': 'volume 1', 'snapshot_id': fake.SNAPSHOT_ID, diff --git a/cinder/tests/unit/group/test_groups_api.py b/cinder/tests/unit/group/test_groups_api.py index a4c2ab9872f..7e05ed4793c 100644 --- a/cinder/tests/unit/group/test_groups_api.py +++ b/cinder/tests/unit/group/test_groups_api.py @@ -109,6 +109,7 @@ class GroupAPITestCase(test.TestCase): ret_group.host = "test_host@fakedrv#fakepool" ret_group.status = fields.GroupStatus.AVAILABLE + ret_group.assert_not_frozen = mock.Mock(return_value=True) self.group_api.delete(self.ctxt, ret_group, delete_volumes = True) mock_volume_get_all.assert_called_once_with(mock.ANY, ret_group.id) mock_volumes_update.assert_called_once_with(self.ctxt, []) @@ -342,6 +343,7 @@ class GroupAPITestCase(test.TestCase): description = "fake description" mock_group.id = fake.GROUP_ID mock_group.group_type_id = fake.GROUP_TYPE_ID + mock_group.assert_not_frozen = mock.Mock(return_value=True) mock_group.volumes = [] ret_group_snap = self.group_api.create_group_snapshot( self.ctxt, mock_group, name, description) @@ -363,6 +365,7 @@ class GroupAPITestCase(test.TestCase): ret_group_snap.id) mock_create_api.assert_called_once_with(self.ctxt, ret_group_snap) + ret_group_snap.assert_not_frozen = mock.Mock(return_value=True) self.group_api.delete_group_snapshot(self.ctxt, ret_group_snap) mock_delete_api.assert_called_once_with(mock.ANY, ret_group_snap) @@ -575,3 +578,40 @@ class GroupAPITestCase(test.TestCase): 'status': fields.GroupSnapshotStatus.ERROR} mock_group_snapshot.update.assert_called_once_with(update_field) mock_group_snapshot.save.assert_called_once_with() + + def test_create_group_from_src_frozen(self): + service = utils.create_service(self.ctxt, {'frozen': True}) + group = utils.create_group(self.ctxt, host=service.host, + group_type_id='gt') + group_api = cinder.group.api.API() + self.assertRaises(exception.InvalidInput, + group_api.create_from_src, + self.ctxt, 'group', 'desc', + group_snapshot_id=None, source_group_id=group.id) + + def test_delete_group_frozen(self): + service = utils.create_service(self.ctxt, {'frozen': True}) + group = utils.create_group(self.ctxt, host=service.host, + group_type_id='gt') + group_api = cinder.group.api.API() + self.assertRaises(exception.InvalidInput, + group_api.delete, self.ctxt, group) + + def test_create_group_snapshot_frozen(self): + service = utils.create_service(self.ctxt, {'frozen': True}) + group = utils.create_group(self.ctxt, host=service.host, + group_type_id='gt') + group_api = cinder.group.api.API() + self.assertRaises(exception.InvalidInput, + group_api.create_group_snapshot, + self.ctxt, group, 'group_snapshot', 'desc') + + def test_delete_group_snapshot_frozen(self): + service = utils.create_service(self.ctxt, {'frozen': True}) + group = utils.create_group(self.ctxt, host=service.host, + group_type_id='gt') + gsnap = utils.create_group_snapshot(self.ctxt, group.id) + group_api = cinder.group.api.API() + self.assertRaises(exception.InvalidInput, + group_api.delete_group_snapshot, + self.ctxt, gsnap) diff --git a/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py b/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py index a8b44081773..f212f755f50 100644 --- a/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py +++ b/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py @@ -50,7 +50,7 @@ class AllocatedCapacityWeigherTestCase(test.TestCase): _mock_service_get_all.assert_called_once_with( ctxt, None, # backend_match_level - topic=constants.VOLUME_TOPIC, disabled=disabled) + topic=constants.VOLUME_TOPIC, frozen=False, disabled=disabled) return host_states def test_default_of_spreading_first(self): diff --git a/cinder/tests/unit/scheduler/test_capacity_weigher.py b/cinder/tests/unit/scheduler/test_capacity_weigher.py index 3bdc0d7d945..b7474a2f4db 100644 --- a/cinder/tests/unit/scheduler/test_capacity_weigher.py +++ b/cinder/tests/unit/scheduler/test_capacity_weigher.py @@ -53,7 +53,7 @@ class CapacityWeigherTestCase(test.TestCase): _mock_service_get_all.assert_called_once_with( ctxt, None, # backend_match_level - topic=constants.VOLUME_TOPIC, disabled=disabled) + topic=constants.VOLUME_TOPIC, frozen=False, disabled=disabled) return backend_states # If thin and thin_provisioning_support are True, diff --git a/cinder/tests/unit/scheduler/test_host_manager.py b/cinder/tests/unit/scheduler/test_host_manager.py index f9e5f267e2a..65cdf1d552e 100644 --- a/cinder/tests/unit/scheduler/test_host_manager.py +++ b/cinder/tests/unit/scheduler/test_host_manager.py @@ -702,6 +702,7 @@ class HostManagerTestCase(test.TestCase): self.host_manager.get_all_backend_states(context) _mock_service_get_all.assert_called_with(context, disabled=False, + frozen=False, topic=topic) # verify that Service.is_up was called for each srv @@ -727,6 +728,7 @@ class HostManagerTestCase(test.TestCase): self.host_manager.get_all_backend_states(context) _mock_service_get_all.assert_called_with(context, disabled=False, + frozen=False, topic=topic) self.assertEqual(expected, _mock_service_is_up.call_args_list) diff --git a/cinder/tests/unit/scheduler/test_volume_number_weigher.py b/cinder/tests/unit/scheduler/test_volume_number_weigher.py index 64864c3ffe3..6aceaaab8a7 100644 --- a/cinder/tests/unit/scheduler/test_volume_number_weigher.py +++ b/cinder/tests/unit/scheduler/test_volume_number_weigher.py @@ -77,6 +77,7 @@ class VolumeNumberWeigherTestCase(test.TestCase): ctxt, None, # backend_match_level topic=constants.VOLUME_TOPIC, + frozen=False, disabled=disabled) return backend_states diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index ab001c87ac4..c24a0ced5a1 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -78,6 +78,7 @@ class BaseTest(test.TestCase, test.ModelsObjectComparatorMixin): self.ctxt = context.get_admin_context() +@ddt.ddt class DBAPIServiceTestCase(BaseTest): """Unit tests for cinder.db.api.service_*.""" @@ -150,38 +151,39 @@ class DBAPIServiceTestCase(BaseTest): real_service1 = db.service_get(self.ctxt, host='host1', topic='topic1') self._assertEqualObjects(service1, real_service1) - def test_service_get_all_disabled_by_cluster(self): + @ddt.data('disabled', 'frozen') + def test_service_get_all_boolean_by_cluster(self, field_name): values = [ - # Enabled services - {'host': 'host1', 'binary': 'b1', 'disabled': False}, - {'host': 'host2', 'binary': 'b1', 'disabled': False, - 'cluster_name': 'enabled_cluster'}, - {'host': 'host3', 'binary': 'b1', 'disabled': True, - 'cluster_name': 'enabled_cluster'}, + # Enabled/Unfrozen services + {'host': 'host1', 'binary': 'b1', field_name: False}, + {'host': 'host2', 'binary': 'b1', field_name: False, + 'cluster_name': 'enabled_unfrozen_cluster'}, + {'host': 'host3', 'binary': 'b1', field_name: True, + 'cluster_name': 'enabled_unfrozen_cluster'}, - # Disabled services - {'host': 'host4', 'binary': 'b1', 'disabled': True}, - {'host': 'host5', 'binary': 'b1', 'disabled': False, - 'cluster_name': 'disabled_cluster'}, - {'host': 'host6', 'binary': 'b1', 'disabled': True, - 'cluster_name': 'disabled_cluster'}, + # Disabled/Frozen services + {'host': 'host4', 'binary': 'b1', field_name: True}, + {'host': 'host5', 'binary': 'b1', field_name: False, + 'cluster_name': 'disabled_frozen_cluster'}, + {'host': 'host6', 'binary': 'b1', field_name: True, + 'cluster_name': 'disabled_frozen_cluster'}, ] - db.cluster_create(self.ctxt, {'name': 'enabled_cluster', + db.cluster_create(self.ctxt, {'name': 'enabled_unfrozen_cluster', 'binary': 'b1', - 'disabled': False}), - db.cluster_create(self.ctxt, {'name': 'disabled_cluster', + field_name: False}), + db.cluster_create(self.ctxt, {'name': 'disabled_frozen_cluster', 'binary': 'b1', - 'disabled': True}), + field_name: True}), services = [utils.create_service(self.ctxt, vals) for vals in values] - enabled = db.service_get_all(self.ctxt, disabled=False) - disabled = db.service_get_all(self.ctxt, disabled=True) + false_services = db.service_get_all(self.ctxt, **{field_name: False}) + true_services = db.service_get_all(self.ctxt, **{field_name: True}) self.assertSetEqual({s.host for s in services[:3]}, - {s.host for s in enabled}) + {s.host for s in false_services}) self.assertSetEqual({s.host for s in services[3:]}, - {s.host for s in disabled}) + {s.host for s in true_services}) def test_service_get_all(self): expired = (datetime.datetime.utcnow() @@ -3167,3 +3169,28 @@ class DBAPIGenericTestCase(BaseTest): # Admin can find it res = sqlalchemy_api.resource_exists(self.ctxt, model, snap.id) self.assertTrue(res, msg="Admin cannot find the Snapshot") + + +@ddt.ddt +class DBAPIBackendTestCase(BaseTest): + @ddt.data(True, False) + def test_is_backend_frozen_service(self, frozen): + service = utils.create_service(self.ctxt, {'frozen': frozen}) + utils.create_service(self.ctxt, {'host': service.host + '2', + 'frozen': not frozen}) + self.assertEqual(frozen, db.is_backend_frozen(self.ctxt, service.host, + service.cluster_name)) + + @ddt.data(True, False) + def test_is_backend_frozen_cluster(self, frozen): + cluster = utils.create_cluster(self.ctxt, frozen=frozen) + utils.create_service(self.ctxt, {'frozen': frozen, 'host': 'hostA', + 'cluster_name': cluster.name}) + service = utils.create_service(self.ctxt, + {'frozen': not frozen, + 'host': 'hostB', + 'cluster_name': cluster.name}) + utils.create_populated_cluster(self.ctxt, 3, 0, frozen=not frozen, + name=cluster.name + '2') + self.assertEqual(frozen, db.is_backend_frozen(self.ctxt, service.host, + service.cluster_name)) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 02a864e58a2..f0287e7312c 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -668,6 +668,28 @@ class VolumeTestCase(base.BaseVolumeTestCase): self.context, volume_id) + def test_delete_volume_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + volume = tests_utils.create_volume(self.context, host=service.host) + self.assertRaises(exception.InvalidInput, + self.volume_api.delete, self.context, volume) + + def test_delete_snapshot_frozen(self): + service = tests_utils.create_service(self.context, {'frozen': True}) + volume = tests_utils.create_volume(self.context, host=service.host) + snapshot = tests_utils.create_snapshot(self.context, volume.id) + self.assertRaises(exception.InvalidInput, + self.volume_api.delete_snapshot, self.context, + snapshot) + + @ddt.data('create_snapshot', 'create_snapshot_force') + def test_create_snapshot_frozen(self, method): + service = tests_utils.create_service(self.context, {'frozen': True}) + volume = tests_utils.create_volume(self.context, host=service.host) + method = getattr(self.volume_api, method) + self.assertRaises(exception.InvalidInput, + method, self.context, volume, 'name', 'desc') + def test_delete_volume_another_cluster_fails(self): """Test delete of volume from another cluster fails.""" self.volume.cluster = 'mycluster' diff --git a/cinder/tests/unit/volume/drivers/dell_emc/vnx/test_adapter.py b/cinder/tests/unit/volume/drivers/dell_emc/vnx/test_adapter.py index b27b7e75ced..bbffdc118f8 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/vnx/test_adapter.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/vnx/test_adapter.py @@ -174,7 +174,6 @@ class TestCommonAdapter(test.TestCase): group_snapshot = test_utils.create_group_snapshot( self.ctxt, fake_constants.CGSNAPSHOT_ID, - host='host@backend#unit_test_pool', group_type_id=fake_constants.VOLUME_TYPE_ID) vol = test_utils.create_volume(self.ctxt) snaps = [ @@ -595,7 +594,6 @@ class TestCommonAdapter(test.TestCase): group_snapshot = test_utils.create_group_snapshot( self.ctxt, fake_constants.GROUP_ID, - host='host@backend#unit_test_pool', group_type_id=fake_constants.VOLUME_TYPE_ID) vol = test_utils.create_volume(self.ctxt) snaps = [ @@ -629,7 +627,6 @@ class TestCommonAdapter(test.TestCase): group_snapshot = test_utils.create_group_snapshot( self.ctxt, fake_constants.GROUP_ID, - host='host@backend#unit_test_pool', group_type_id=fake_constants.VOLUME_TYPE_ID) vol = test_utils.create_volume(self.ctxt) snaps = [ diff --git a/cinder/volume/api.py b/cinder/volume/api.py index ab13c4bb378..c444b7b96e2 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -394,6 +394,9 @@ class API(base.Base): 'id': volume.id}) return + if not unmanage_only: + volume.assert_not_frozen() + # Build required conditions for conditional update expected = { 'attach_status': db.Not(fields.VolumeAttachStatus.ATTACHED), @@ -772,6 +775,7 @@ class API(base.Base): force=False, metadata=None, cgsnapshot_id=None, group_snapshot_id=None): + volume.assert_not_frozen() snapshot = self.create_snapshot_in_db( context, volume, name, description, force, metadata, cgsnapshot_id, @@ -993,6 +997,9 @@ class API(base.Base): @wrap_check_policy def delete_snapshot(self, context, snapshot, force=False, unmanage_only=False): + if not unmanage_only: + snapshot.assert_not_frozen() + # Build required conditions for conditional update expected = {'cgsnapshot_id': None, 'group_snapshot_id': None}