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:
parent
5b23ec1de6
commit
7cf7985f12
|
@ -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()
|
||||
|
||||
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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'
|
||||
|
|
Loading…
Reference in New Issue