Cleanup db initialisation notification code

This charm contains the origical implementaion of the
db initialisation code that ensures that a db init/migrate
is only performed once as and when needed and that
services are only restarted accordingly. This patch serves as
a cleanup to include fixes and improvements as landed in
the related neutron-api patch that is adopting the same
approach.

Change-Id: I20161c8676d775f4a935315796d6e6b5376ba13c
Related-Bug: 1708459
This commit is contained in:
Edward Hope-Morley 2017-08-23 13:47:14 +01:00
parent 5b23ec1de6
commit 7cf7985f12
4 changed files with 63 additions and 54 deletions

View File

@ -42,7 +42,7 @@ from cinder_utils import (
CINDER_API_CONF,
ceph_config_file,
setup_ipv6,
check_db_initialised,
check_local_db_actions_complete,
filesystem_mounted,
assess_status,
)
@ -467,14 +467,14 @@ def cluster_joined(relation_id=None):
# Only do if this is fired by cluster rel
if not relation_id:
check_db_initialised()
check_local_db_actions_complete()
@hooks.hook('cluster-relation-changed',
'cluster-relation-departed')
@restart_on_change(restart_map(), stopstart=True)
def cluster_changed():
check_db_initialised()
check_local_db_actions_complete()
CONFIGS.write_all()

View File

@ -622,12 +622,11 @@ def _parse_block_device(block_device):
return ('/dev/{}'.format(block_device), 0)
def is_db_intialised(cluster_rid=None):
def is_db_initialised(cluster_rid=None):
"""
Check whether a db intialisation has been performed by any unit in this
cluster.
Check whether a db intialisation has been performed by any peer unit.
We make decision based in whether we or any of our peers has previously
We base our decision on whether we or any of our peers has previously
sent or echoed an initialisation notification.
@param cluster_rid: current relation id. If none provided, all cluster
@ -639,30 +638,37 @@ def is_db_intialised(cluster_rid=None):
else:
rids = relation_ids('cluster')
settings = {}
for c_rid in rids:
units = related_units(relid=c_rid) + [local_unit()]
for unit in units:
_settings = relation_get(unit=unit, rid=c_rid) or {}
for key in [CINDER_DB_INIT_RKEY, CINDER_DB_INIT_ECHO_RKEY]:
if _settings.get(key):
settings[key] = _settings.get(key)
settings = relation_get(unit=unit, rid=c_rid) or {}
for key in [CINDER_DB_INIT_RKEY, CINDER_DB_INIT_ECHO_RKEY]:
if settings.get(key):
return True
if not settings:
return False
if (settings.get(CINDER_DB_INIT_RKEY) or
settings.get(CINDER_DB_INIT_ECHO_RKEY)):
return True
return False
def check_db_initialised():
"""Check if we have received db init'd notify and restart services if we
have not already.
def is_new_dbinit_notification(init_id, echoed_init_id):
"""Returns True if we have a received a new db initialisation notification
from a peer unit and we have not previously echoed it to indicate that we
have already performed the necessary actions as result.
@param init_db: received initialisation notification.
@param echoed_init_db: value currently set for the echo key.
@return: True if new notification and False if not.
"""
return (init_id and (local_unit() not in init_id) and
(echoed_init_id != init_id))
def check_local_db_actions_complete():
"""Check if we have received db init'd notification and restart services
if we have not already.
NOTE: this must only be called from peer relation context.
"""
if not is_db_intialised():
if not is_db_initialised():
return
settings = relation_get() or {}
@ -671,18 +677,18 @@ def check_db_initialised():
echoed_init_id = relation_get(unit=local_unit(),
attribute=CINDER_DB_INIT_ECHO_RKEY)
# If this unit has previously received an init notification (and echoed
# it) then we can ignore further notifications.
if not echoed_init_id:
if init_id and local_unit() not in init_id:
log("Restarting cinder services following db initialisation",
level=DEBUG)
if not is_unit_paused_set():
for svc in enabled_services():
service_restart(svc)
# If we have received an init notification from a peer unit
# (assumed to be the leader) then restart cinder-* and echo the
# notification and don't restart again unless we receive a new
# (different) notification.
if is_new_dbinit_notification(init_id, echoed_init_id):
if not is_unit_paused_set():
log("Restarting cinder services following db "
"initialisation", level=DEBUG)
for svc in enabled_services():
service_restart(svc)
# Set echo
if init_id and local_unit() not in init_id:
# Echo notification
relation_set(**{CINDER_DB_INIT_ECHO_RKEY: init_id})
@ -697,7 +703,7 @@ def migrate_database(upgrade=False):
(leader) unit to perform this action should have broadcast this information
to its peers so first we check whether this has already occurred.
"""
if not upgrade and is_db_intialised():
if not upgrade and is_db_initialised():
log("Database is already initialised.", level=DEBUG)
return

View File

@ -644,7 +644,7 @@ class TestCinderUtils(CharmTestCase):
mock_relation_get.side_effect = mock_rel_get
mock_related_units.return_value = ['1']
mock_relation_ids.return_value = ['cluster:1']
self.assertFalse(cinder_utils.is_db_intialised())
self.assertFalse(cinder_utils.is_db_initialised())
@patch.object(cinder_utils, 'relation_get')
@patch.object(cinder_utils, 'relation_ids')
@ -668,16 +668,16 @@ class TestCinderUtils(CharmTestCase):
mock_relation_get.side_effect = mock_rel_get
mock_related_units.return_value = ['1']
mock_relation_ids.return_value = ['cluster:1']
self.assertTrue(cinder_utils.is_db_intialised())
self.assertTrue(cinder_utils.is_db_initialised())
@patch.object(cinder_utils, 'is_db_intialised')
@patch.object(cinder_utils, 'is_db_initialised')
@patch.object(cinder_utils, 'enabled_services')
@patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
@patch.object(cinder_utils, 'uuid')
def test_migrate_database(self, mock_uuid, mock_enabled_services,
mock_is_db_intialised):
mock_is_db_initialised):
'It migrates database with cinder-manage'
mock_is_db_intialised.return_value = False
mock_is_db_initialised.return_value = False
uuid = 'a-great-uuid'
mock_uuid.uuid4.return_value = uuid
mock_enabled_services.return_value = ['svc1']
@ -690,10 +690,10 @@ class TestCinderUtils(CharmTestCase):
self.relation_set.assert_called_with(relation_id=rid, **args)
self.service_restart.assert_called_with('svc1')
@patch.object(cinder_utils, 'is_db_intialised')
@patch.object(cinder_utils, 'is_db_initialised')
def test_migrate_database_already_initisalised(self,
mock_is_db_intialised):
mock_is_db_intialised.return_value = True
mock_is_db_initialised):
mock_is_db_initialised.return_value = True
with patch('subprocess.check_call') as check_call:
cinder_utils.migrate_database()
self.assertFalse(check_call.called)
@ -951,22 +951,22 @@ class TestCinderUtils(CharmTestCase):
self.assertEquals(self.render.call_args_list, expected)
@patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
def test_check_db_initialised_by_self(self):
def test_check_local_db_actions_complete_by_self(self):
self.relation_get.return_value = {}
cinder_utils.check_db_initialised()
cinder_utils.check_local_db_actions_complete()
self.assertFalse(self.relation_set.called)
self.relation_get.return_value = {'cinder-db-initialised':
'unit/0-1234'}
cinder_utils.check_db_initialised()
cinder_utils.check_local_db_actions_complete()
self.assertFalse(self.relation_set.called)
@patch.object(cinder_utils, 'is_db_intialised')
@patch.object(cinder_utils, 'is_db_initialised')
@patch.object(cinder_utils, 'enabled_services')
@patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
def test_check_db_initialised(self, enabled_services,
mock_is_db_intialised):
mock_is_db_intialised.return_value = True
def test_check_local_db_actions_complete(self, enabled_services,
mock_is_db_initialised):
mock_is_db_initialised.return_value = True
enabled_services.return_value = ['svc1']
r_settings = {}
@ -977,10 +977,10 @@ class TestCinderUtils(CharmTestCase):
return r_settings
self.relation_get.side_effect = mock_relation_get
cinder_utils.check_db_initialised()
cinder_utils.check_local_db_actions_complete()
self.assertFalse(self.relation_set.called)
r_settings = {'cinder-db-initialised': 'unit/1-1234'}
cinder_utils.check_db_initialised()
cinder_utils.check_local_db_actions_complete()
calls = [call(**{'cinder-db-initialised-echo': 'unit/1-1234'})]
self.relation_set.assert_has_calls(calls)
self.service_restart.assert_called_with('svc1')

View File

@ -77,7 +77,8 @@ class TestClusterHooks(CharmTestCase):
super(TestClusterHooks, self).setUp(hooks, TO_PATCH)
self.config.side_effect = self.test_config.get
@patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
@patch.object(hooks, 'check_local_db_actions_complete',
lambda *args, **kwargs: None)
@patch('charmhelpers.core.host.service')
@patch('charmhelpers.core.host.path_hash')
def test_cluster_hook(self, path_hash, service):
@ -104,13 +105,15 @@ class TestClusterHooks(CharmTestCase):
]
self.assertEquals(ex, service.call_args_list)
@patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
@patch.object(hooks, 'check_local_db_actions_complete',
lambda *args, **kwargs: None)
def test_cluster_joined_hook(self):
self.config.side_effect = self.test_config.get
hooks.hooks.execute(['hooks/cluster-relation-joined'])
self.assertTrue(self.relation_set.called)
@patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
@patch.object(hooks, 'check_local_db_actions_complete',
lambda *args, **kwargs: None)
def test_cluster_joined_hook_multinet(self):
self.config.side_effect = self.test_config.get
self.get_relation_ip.return_value = '10.1.1.1'