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
This commit is contained in:
Gorka Eguileor 2016-11-25 15:56:51 +01:00
parent 73603d5248
commit 2195885e77
21 changed files with 256 additions and 57 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -77,6 +77,7 @@ class VolumeNumberWeigherTestCase(test.TestCase):
ctxt,
None, # backend_match_level
topic=constants.VOLUME_TOPIC,
frozen=False,
disabled=disabled)
return backend_states

View File

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

View File

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

View File

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

View File

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