Gate initial update of relations on having reached expected scale
At present the Keystone charm frequently initiates updates to its relations before it has reached a stable state. Make use information from ``juju goal-state`` to predict scale and gate initial update of relations on having reached expected scale. Depends-On: https://github.com/juju/charm-helpers/pull/226 Change-Id: I96d4aff7c4ec9fb9ea160c7e294581bab3103df8
This commit is contained in:
parent
78f2e5e049
commit
a449a53885
|
@ -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):
|
||||
|
|
|
@ -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']
|
||||
|
|
|
@ -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'):
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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()
|
||||
|
|
Loading…
Reference in New Issue