diff --git a/charmhelpers/contrib/openstack/ha/utils.py b/charmhelpers/contrib/openstack/ha/utils.py index 6060ae50..add8eb9a 100644 --- a/charmhelpers/contrib/openstack/ha/utils.py +++ b/charmhelpers/contrib/openstack/ha/utils.py @@ -28,6 +28,7 @@ import json import re from charmhelpers.core.hookenv import ( + expected_related_units, log, relation_set, charm_name, @@ -110,12 +111,17 @@ def assert_charm_supports_dns_ha(): def expect_ha(): """ Determine if the unit expects to be in HA - Check for VIP or dns-ha settings which indicate the unit should expect to - be related to hacluster. + Check juju goal-state if ha relation is expected, check for VIP or dns-ha + settings which indicate the unit should expect to be related to hacluster. @returns boolean """ - return config('vip') or config('dns-ha') + ha_related_units = [] + try: + ha_related_units = list(expected_related_units(reltype='ha')) + except (NotImplementedError, KeyError): + pass + return len(ha_related_units) > 0 or config('vip') or config('dns-ha') def generate_ha_relation_data(service): diff --git a/charmhelpers/core/hookenv.py b/charmhelpers/core/hookenv.py index 68800074..2555a30e 100644 --- a/charmhelpers/core/hookenv.py +++ b/charmhelpers/core/hookenv.py @@ -509,6 +509,67 @@ def related_units(relid=None): subprocess.check_output(units_cmd_line).decode('UTF-8')) or [] +def expected_peer_units(): + """Get a generator for units we expect to join peer relation based on + goal-state. + + The local unit is excluded from the result to make it easy to gauge + completion of all peers joining the relation with existing hook tools. + + Example usage: + log('peer {} of {} joined peer relation' + .format(len(related_units()), + len(list(expected_peer_units())))) + + This function will raise NotImplementedError if used with juju versions + without goal-state support. + + :returns iterator + :rtype: types.GeneratorType + :raises: NotImplementedError + """ + if not has_juju_version("2.4.0"): + # goal-state first appeared in 2.4.0. + raise NotImplementedError("goal-state") + _goal_state = goal_state() + return (key for key in _goal_state['units'] + if '/' in key and key != local_unit()) + + +def expected_related_units(reltype=None): + """Get a generator for units we expect to join relation based on + goal-state. + + Note that you can not use this function for the peer relation, take a look + at expected_peer_units() for that. + + This function will raise KeyError if you request information for a + relation type for which juju goal-state does not have information. It will + raise NotImplementedError if used with juju versions without goal-state + support. + + Example usage: + log('consumer {} of {} joined relation {}' + .format(len(related_units()), + len(list(expected_related_units())), + relation_type())) + + :param reltype: Relation type to list data for, default is to list data for + the realtion type we are currently executing a hook for. + :type reltype: str + :returns: iterator + :rtype: types.GeneratorType + :raises: KeyError, NotImplementedError + """ + if not has_juju_version("2.4.4"): + # goal-state existed in 2.4.0, but did not list individual units to + # join a relation in 2.4.1 through 2.4.3. (LP: #1794739) + raise NotImplementedError("goal-state relation unit count") + reltype = reltype or relation_type() + _goal_state = goal_state() + return (key for key in _goal_state['relations'][reltype] if '/' in key) + + @cached def relation_for_unit(unit=None, rid=None): """Get the json represenation of a unit's relation""" @@ -997,6 +1058,7 @@ def application_version_set(version): @translate_exc(from_exc=OSError, to_exc=NotImplementedError) +@cached def goal_state(): """Juju goal state values""" cmd = ['goal-state', '--format=json'] diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py index 25f7cfbf..deddc93e 100755 --- a/hooks/keystone_hooks.py +++ b/hooks/keystone_hooks.py @@ -104,6 +104,7 @@ from keystone_utils import ( send_notifications, is_db_ready, is_db_initialised, + is_expected_scale, filter_null, is_service_present, delete_service_entry, @@ -310,6 +311,10 @@ def update_all_identity_relation_units(check_db_ready=True): log("Database not yet initialised - deferring identity-relation " "updates", level=INFO) return + if not is_expected_scale(): + log("Keystone charm and it's dependencies not yet at expected scale " + "- deferring identity-relation updates", level=INFO) + return log('Firing identity_changed hook for all related services.') for rid in relation_ids('identity-service'): diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index 0f646df0..948cc419 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -40,6 +40,10 @@ from charmhelpers.contrib.network.ip import ( get_ipv6_addr ) +from charmhelpers.contrib.openstack.ha.utils import ( + expect_ha, +) + from charmhelpers.contrib.openstack.ip import ( resolve_address, PUBLIC, @@ -73,6 +77,8 @@ from charmhelpers.core.decorators import ( from charmhelpers.core.hookenv import ( atexit, config, + expected_peer_units, + expected_related_units, is_leader, leader_get, leader_set, @@ -2183,3 +2189,41 @@ def fernet_keys_rotate_and_sync(log_func=log): key_leader_set() log_func("Rotated and started sync (via leader settings) of fernet keys", level=INFO) + + +def is_expected_scale(): + """Query juju goal-state to determine whether our peer- and dependency- + relations are at the expected scale. + + Useful for deferring per unit per relation housekeeping work until we are + ready to complete it successfully and without unnecessary repetiton. + + Always returns True if version of juju used does not support goal-state. + + :returns: True or False + :rtype: bool + """ + peer_type = 'cluster' + peer_rid = next((rid for rid in relation_ids(reltype=peer_type)), None) + if not peer_rid: + return False + deps = [ + ('shared-db', + next((rid for rid in relation_ids(reltype='shared-db')), None)), + ] + if expect_ha(): + deps.append(('ha', + next((rid for rid in relation_ids(reltype='ha')), None))) + try: + if (len(related_units(relid=peer_rid)) < + len(list(expected_peer_units()))): + return False + for dep in deps: + if not dep[1]: + return False + if (len(related_units(relid=dep[1])) < + len(list(expected_related_units(reltype=dep[0])))): + return False + except NotImplementedError: + return True + return True diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index c033c9d2..6ee9aebf 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -342,6 +342,7 @@ class KeystoneRelationTests(CharmTestCase): self.assertTrue(update.called) self.assertTrue(mock_update_domains.called) + @patch.object(hooks, 'is_expected_scale') @patch.object(hooks, 'os_release') @patch.object(hooks, 'run_in_apache') @patch.object(hooks, 'is_db_initialised') @@ -350,10 +351,12 @@ class KeystoneRelationTests(CharmTestCase): config_https, mock_db_init, mock_run_in_apache, - os_release): + os_release, + is_expected_scale): os_release.return_value = 'ocata' self.enable_memcache.return_value = False mock_run_in_apache.return_value = False + is_expected_scale.return_value = True self.openstack_upgrade_available.return_value = True self.test_config.set('action-managed-upgrade', True) @@ -754,6 +757,7 @@ class KeystoneRelationTests(CharmTestCase): self.assertFalse(self.migrate_database.called) self.assertFalse(update.called) + @patch.object(hooks, 'is_expected_scale') @patch.object(hooks, 'configure_https') @patch.object(hooks, 'admin_relation_changed') @patch.object(hooks, 'identity_credentials_changed') @@ -765,9 +769,11 @@ class KeystoneRelationTests(CharmTestCase): identity_changed, identity_credentials_changed, admin_relation_changed, - configure_https): + configure_https, + is_expected_scale): """ Verify all identity relations are updated """ is_db_initialized.return_value = True + is_expected_scale.return_value = True self.relation_ids.return_value = ['identity-relation:0'] self.related_units.return_value = ['unit/0'] log_calls = [call('Firing identity_changed hook for all related ' @@ -811,26 +817,30 @@ class KeystoneRelationTests(CharmTestCase): level='INFO') self.assertFalse(self.relation_ids.called) + @patch.object(hooks, 'is_expected_scale') @patch.object(hooks, 'configure_https') @patch.object(hooks, 'is_db_initialised') @patch.object(hooks, 'CONFIGS') def test_update_all_leader(self, configs, is_db_initialized, - configure_https): + configure_https, is_expected_scale): """ Verify update identity relations when the leader""" self.is_elected_leader.return_value = True is_db_initialized.return_value = True + is_expected_scale.return_value = True hooks.update_all_identity_relation_units(check_db_ready=False) # Still updates relations self.assertTrue(self.relation_ids.called) + @patch.object(hooks, 'is_expected_scale') @patch.object(hooks, 'configure_https') @patch.object(hooks, 'is_db_initialised') @patch.object(hooks, 'CONFIGS') def test_update_all_not_leader(self, configs, is_db_initialized, - configure_https): + configure_https, is_expected_scale): """ Verify update identity relations when not the leader""" self.is_elected_leader.return_value = False is_db_initialized.return_value = True + is_expected_scale.return_value = True hooks.update_all_identity_relation_units(check_db_ready=False) self.assertFalse(self.ensure_initial_admin.called) # Still updates relations diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index ca183853..e22c4a41 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -1283,3 +1283,75 @@ class TestKeystoneUtils(CharmTestCase): utils.fernet_keys_rotate_and_sync() mock_fernet_rotate.assert_called_once_with() mock_key_leader_set.assert_called_once_with() + + @patch.object(utils, 'expected_related_units') + @patch.object(utils, 'expected_peer_units') + @patch.object(utils, 'related_units') + @patch.object(utils, 'expect_ha') + @patch.object(utils, 'relation_ids') + def test_is_expected_scale(self, relation_ids, expect_ha, related_units, + expected_peer_units, expected_related_units): + relation_ids.return_value = ['FAKE_RID'] + expect_ha.return_value = False + related_units.return_value = ['unit/0', 'unit/1', 'unit/2'] + expected_peer_units.return_value = iter(related_units.return_value) + expected_related_units.return_value = iter(related_units.return_value) + self.assertTrue(utils.is_expected_scale()) + relation_ids.assert_has_calls([ + call(reltype='cluster'), + call(reltype='shared-db')]) + related_units.assert_called_with(relid='FAKE_RID') + + @patch.object(utils, 'expected_related_units') + @patch.object(utils, 'expected_peer_units') + @patch.object(utils, 'related_units') + @patch.object(utils, 'expect_ha') + @patch.object(utils, 'relation_ids') + def test_is_expected_scale_ha(self, relation_ids, expect_ha, related_units, + expected_peer_units, expected_related_units): + relation_ids.return_value = ['FAKE_RID'] + expect_ha.return_value = True + related_units.return_value = ['unit/0', 'unit/1', 'unit/2'] + expected_peer_units.return_value = iter(related_units.return_value) + expected_related_units.return_value = iter(related_units.return_value) + self.assertTrue(utils.is_expected_scale()) + relation_ids.assert_has_calls([ + call(reltype='cluster'), + call(reltype='shared-db'), + call(reltype='ha')]) + related_units.assert_called_with(relid='FAKE_RID') + + @patch.object(utils, 'expected_related_units') + @patch.object(utils, 'expected_peer_units') + @patch.object(utils, 'related_units') + @patch.object(utils, 'expect_ha') + @patch.object(utils, 'relation_ids') + def test_not_is_expected_scale(self, relation_ids, expect_ha, + related_units, expected_peer_units, + expected_related_units): + relation_ids.return_value = ['FAKE_RID'] + expect_ha.return_value = False + related_units.return_value = ['unit/0', 'unit/1'] + expected_peer_units.return_value = iter(['unit/0', 'unit/1', 'unit/2']) + expected_related_units.return_value = iter( + ['unit/0', 'unit/1', 'unit/2']) + self.assertFalse(utils.is_expected_scale()) + relation_ids.assert_has_calls([ + call(reltype='cluster'), + call(reltype='shared-db')]) + related_units.assert_called_with(relid='FAKE_RID') + + @patch.object(utils, 'expected_related_units') + @patch.object(utils, 'expected_peer_units') + @patch.object(utils, 'related_units') + @patch.object(utils, 'expect_ha') + @patch.object(utils, 'relation_ids') + def test_is_expected_scale_no_goal_state_support(self, relation_ids, + expect_ha, related_units, + expected_peer_units, + expected_related_units): + relation_ids.return_value = ['FAKE_RID'] + related_units.return_value = ['unit/0', 'unit/1', 'unit/2'] + expected_peer_units.side_effect = NotImplementedError + self.assertTrue(utils.is_expected_scale()) + expected_related_units.assert_not_called()