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:
Frode Nordahl 2018-10-02 16:36:04 +02:00
parent 78f2e5e049
commit a449a53885
No known key found for this signature in database
GPG Key ID: 6A5D59A3BA48373F
6 changed files with 206 additions and 7 deletions

View File

@ -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):

View File

@ -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']

View File

@ -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'):

View File

@ -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

View File

@ -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

View File

@ -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()