diff --git a/hooks/cinder_hooks.py b/hooks/cinder_hooks.py index d29d38b4..13f7d437 100755 --- a/hooks/cinder_hooks.py +++ b/hooks/cinder_hooks.py @@ -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() diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index 5b57442b..918d50d5 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -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 diff --git a/unit_tests/test_cinder_utils.py b/unit_tests/test_cinder_utils.py index 83958f79..5b3de021 100644 --- a/unit_tests/test_cinder_utils.py +++ b/unit_tests/test_cinder_utils.py @@ -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') diff --git a/unit_tests/test_cluster_hooks.py b/unit_tests/test_cluster_hooks.py index 07f55042..7ae68cc1 100644 --- a/unit_tests/test_cluster_hooks.py +++ b/unit_tests/test_cluster_hooks.py @@ -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'