openstack/charm-nova-cloud-controller
Code Issues Proposed changes

Use goal state to defer distributing ssh data

Browse Source 
If goal state is available, this patch uses it to defer distributing the
known_hosts and authorized_keys to the related nova-compute units until
the last nova-compute unit expected has joined and triggered a
cloud-compute-relation-changed hook.  This means that all of the hosts
have been scanned and thus the whole group (per relation id) can be set.

Note that this patch does not unify the known_hosts/authorized_keys
files across relations.  That's for a separate patch.

Change-Id: I6c26ebbad2236e66c174ef4606828db834803865
Related-Bug: #1833420
This commit is contained in:
Alex Kavanagh 2019-07-04 10:42:25 +01:00
parent 452ac31663
commit e8577fc96e
2 changed files with 134 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
hooks/nova_cc_hooks.py
View File

@ -679,7 +679,63 @@ def update_ssh_keys_and_notify_compute_units(rid=None, unit=None):
:type unit: Union[str, None]
"""
update_ssh_key(rid=rid, unit=unit)
notify_ssh_keys_to_compute_units(rid=rid, unit=unit)
# if we have goal state, then only notify the ssh authorized_keys and
# known_hosts onto the relation when the last compute unit has arrived
# (i.e. we've reached the goal state)
if _goal_state_achieved_for_relid('cloud-compute', rid):
notify_ssh_keys_to_compute_units(rid=rid, unit=unit)
def _goal_state_achieved_for_relid(reltype, rid=None):
"""Check that the goal-state has been achieved for the reltype and relid.
If goal state is not available, then the function returns True.
Otherwise, as goal state (from Juju) replies with all the units for the
relation type, without respect for the relation id. i.e. normally, Juju is
a hierarchy of relation type -> relation id -> units, but goal state is
relation type -> units (where all the units across the relation ids are
grouped.
If the relid is None, then the relation_id for the hook is used (by the
library function related_units()).
Note this function checks a particular relation id for reaching goal state,
not all relation ids. To do that (with this function) do:
all(_goal_state_achieved_for_relid(reltype, rid)
for rid in relation_ids(reltype))
:param reltype: the relation type (e.g. 'cloud-compute')
:type reltype: str
:param rid: the relation id, or None for the default
:type rid: Union[str, None]
:returns: True if goal state is achieved, or not available
:rtype: bool
"""
try:
# There should always be one unit -- if not, this block KeyErrors and
# so the goal is false.
units_so_far = hookenv.related_units(relid=rid)
# goal state returns all the units by the relation type (in this
# case 'cloud-compute'). So we need to only check the ones with
# the same prefix e.g. nova-compute/0, nova-compute/1 vs
# nova-compute-b/0, etc if there were two separate relations to two
# nova-compute applications
all_units = list(
hookenv.expected_related_units(reltype=reltype))
prefix = units_so_far[0].split('/')[0]
target_units = [u for u in all_units if u.split('/')[0] == prefix]
return units_so_far == target_units
except (KeyError, IndexError):
# expected_related_units() can raise a KeyError in the case there are
# no units - in that case assume that the goal wasn't met
# if there are no units_so_far, then Index error is raised
return False
except NotImplementedError:
# Not implemented means that goal state is not available
pass
return True
def update_ssh_key(rid=None, unit=None):

79
unit_tests/test_nova_cc_hooks.py
View File

@ -361,10 +361,77 @@ class NovaCCHooksTests(CharmTestCase):
mock_update_ssh_keys_and_notify_compute_units.assert_called_once_with(
rid=None, unit=None)
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_not_available(
self, mock_expected_related_units, mock_related_units):
mock_related_units.return_value = ['service/0']
def _side_effect(*_, **__):
raise NotImplementedError()
mock_expected_related_units.side_effect = _side_effect
self.assertTrue(hooks._goal_state_achieved_for_relid('aservice', None))
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_key_error(
self, mock_expected_related_units, mock_related_units):
mock_related_units.return_value = ['service/0']
def _side_effect(*_, **__):
raise KeyError()
mock_expected_related_units.side_effect = _side_effect
self.assertFalse(
hooks._goal_state_achieved_for_relid('aservice', None))
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_no_related_units_yet(
self, mock_expected_related_units, mock_related_units):
mock_related_units.return_value = []
mock_expected_related_units.return_value = ['service/0']
self.assertFalse(
hooks._goal_state_achieved_for_relid('aservice', None))
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_insufficient_units(
self, mock_expected_related_units, mock_related_units):
mock_related_units.return_value = ['service/0']
mock_expected_related_units.return_value = ['service/0', 'service/1']
self.assertFalse(
hooks._goal_state_achieved_for_relid('aservice', None))
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_sufficient_units(
self, mock_expected_related_units, mock_related_units):
mock_related_units.return_value = ['service/0', 'service/1']
mock_expected_related_units.return_value = ['service/0', 'service/1']
self.assertTrue(hooks._goal_state_achieved_for_relid('aservice', None))
@patch('charmhelpers.core.hookenv.related_units')
@patch('charmhelpers.core.hookenv.expected_related_units')
def test__goal_state_achieved_for_relid__goal_state_different_units(
self, mock_expected_related_units, mock_related_units):
# this shouldn't actually be able to happen, but let's check for it
# anyway
mock_related_units.return_value = ['service/0']
mock_expected_related_units.return_value = \
['service-a/0', 'service-b/0']
self.assertFalse(
hooks._goal_state_achieved_for_relid('aservice', None))
@patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid')
@patch('hooks.nova_cc_utils.remote_service_from_unit')
def test_update_ssh_keys_and_notify_compute_units_ssh_migration(
self, mock_remote_service_from_unit):
self,
mock_remote_service_from_unit,
mock__goal_state_achieved_for_relid):
mock_remote_service_from_unit.return_value = 'aservice'
mock__goal_state_achieved_for_relid.return_value = True
self.test_relation.set({
'migration_auth_type': 'ssh', 'ssh_public_key': 'fookey',
'private-address': '10.0.0.1', 'region': 'RegionOne'})
@ -392,11 +459,17 @@ class NovaCCHooksTests(CharmTestCase):
call(relation_settings={'authorized_keys_max_index': 3},
relation_id=None)]
self.relation_set.assert_has_calls(expected_relations, any_order=True)
mock__goal_state_achieved_for_relid.assert_called_once_with(
'cloud-compute', None)
@patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid')
@patch('hooks.nova_cc_utils.remote_service_from_unit')
def test_update_ssh_keys_and_notify_compute_units_nova_public_key(
self, mock_remote_service_from_unit):
self,
mock_remote_service_from_unit,
mock__goal_state_achieved_for_relid):
mock_remote_service_from_unit.return_value = 'aservice'
mock__goal_state_achieved_for_relid.return_value = True
self.test_relation.set({
'migration_auth_type': 'sasl', 'nova_ssh_public_key': 'fookey',
'private-address': '10.0.0.1', 'region': 'RegionOne'})
@ -425,6 +498,8 @@ class NovaCCHooksTests(CharmTestCase):
call(relation_settings={'nova_authorized_keys_max_index': 3},
relation_id=None)]
self.relation_set.assert_has_calls(expected_relations, any_order=True)
mock__goal_state_achieved_for_relid.assert_called_once_with(
'cloud-compute', None)
@patch('hooks.nova_cc_utils.is_cellv2_init_ready')
@patch('hooks.nova_cc_utils.is_db_initialised')