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

Refactor region notification code to not need unit

Browse Source 
The (already) refactored region notification code checked for the
'region' value in the remote unit, despite the nova-compute charm not
setting the value.  This has been removed.  Now that the function only
needs to be set for the relation, it is no longer included in 'unit'
loops.

The utility function is also renamed to
set_region_on_relation_from_config to better reflect it's actual
function.

Change-Id: I81d9924bebe4009119505b1d5dccf2e498925c7e
Related-Bug: #1833420
This commit is contained in:
Alex Kavanagh 2019-07-02 17:43:09 +01:00
parent afa3c9a58e
commit 452ac31663
2 changed files with 29 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
hooks/nova_cc_hooks.py
View File

@ -279,11 +279,7 @@ def config_changed():
# to ensure the value is propagated to the compute nodes.
if ch_utils.config_value_changed('region'):
for rid in hookenv.relation_ids('cloud-compute'):
for unit in hookenv.related_units(rid):
# Note(ajkavanagh) This used to also update all of the ssh keys
# and hosts to every unit; but that's almost certainly not
# needed on a config changed hook
notify_region_to_unit(rid, unit)
set_region_on_relation_from_config(rid)
ncc_utils.update_aws_compat_services()
@ -637,7 +633,7 @@ def cloud_compute_relation_changed():
* notifies the ssh known hosts and authorized keys to the unit
"""
add_hosts_to_cell_when_ready()
notify_region_to_unit(rid=None, unit=None)
set_region_on_relation_from_config(rid=None)
update_ssh_keys_and_notify_compute_units(rid=None, unit=None)
@ -653,21 +649,15 @@ def add_hosts_to_cell_when_ready():
ncc_utils.add_hosts_to_cell()
def notify_region_to_unit(rid=None, unit=None):
"""Helper function that if the relation data set by the unit differs from
the config on nova-cloud-controller, then nova-cc sets the new region for
that relation to trigger a change for any units that see it differently.
def set_region_on_relation_from_config(rid=None):
"""Helper function that sets the new region for that relation to trigger a
change for any units that see it differently.
:param rid: The relation to check/set, or if None, the current one related
:param rid: The relation to set, or if None, the current one related
to the hook.
:type rid: Union[str. None]
:param unit: the unit to check, of None for the current one according to
the hook.
:type unit: Union[str, None]
"""
rel_settings = hookenv.relation_get(rid=rid, unit=unit)
if not rel_settings.get('region', None) == hookenv.config('region'):
hookenv.relation_set(relation_id=rid, region=hookenv.config('region'))
hookenv.relation_set(relation_id=rid, region=hookenv.config('region'))
def update_ssh_keys_and_notify_compute_units(rid=None, unit=None):
@ -1046,8 +1036,8 @@ def upgrade_charm():
for r_id in hookenv.relation_ids('identity-service'):
identity_joined(rid=r_id)
for r_id in hookenv.relation_ids('cloud-compute'):
set_region_on_relation_from_config(r_id)
for unit in hookenv.related_units(r_id):
notify_region_to_unit(r_id, unit)
update_ssh_keys_and_notify_compute_units(r_id, unit)
for r_id in hookenv.relation_ids('shared-db'):
db_joined(relation_id=r_id)

40
unit_tests/test_nova_cc_hooks.py
View File

@ -316,46 +316,48 @@ class NovaCCHooksTests(CharmTestCase):
@patch('charmhelpers.fetch.filter_installed_packages')
@patch.object(hooks, 'configure_https')
@patch.object(hooks, 'compute_joined')
@patch.object(hooks, 'notify_region_to_unit')
def test_config_changed_region_change(self,
mock_notify_region_to_unit,
mock_compute_joined,
mock_config_https,
mock_filter_packages,
mock_is_db_initialised,
mock_update_aws_compat_services,
mock_relation_ids,
mock_resource_map,
mock_update_nrpe_config,
mock_get_shared_metadatasecret,
mock_set_shared_metadatasecret):
@patch.object(hooks, 'set_region_on_relation_from_config')
def test_config_changed_region_change(
self,
mock_set_region_on_relation_from_config,
mock_compute_joined,
mock_config_https,
mock_filter_packages,
mock_is_db_initialised,
mock_update_aws_compat_services,
mock_relation_ids,
mock_resource_map,
mock_update_nrpe_config,
mock_get_shared_metadatasecret,
mock_set_shared_metadatasecret):
mock_resource_map.return_value = {}
self.openstack_upgrade_available.return_value = False
self.config_value_changed.return_value = True
self.related_units.return_value = ['unit/0']
self.relation_ids.side_effect = \
lambda x: ['generic_rid'] if x == 'cloud-compute' else []
mock_is_db_initialised.return_value = False
mock_set_region_on_relation_from_config.return_value = False
self.os_release.return_value = 'diablo'
hooks.resolve_CONFIGS()
hooks.config_changed()
mock_notify_region_to_unit.assert_has_calls(
[call('generic_rid', 'unit/0')])
mock_set_region_on_relation_from_config.assert_has_calls(
[call('generic_rid')])
mock_compute_joined.assert_has_calls(
[call(rid='generic_rid', remote_restart=False)])
self.assertTrue(mock_update_aws_compat_services.called)
@patch.object(hooks, 'add_hosts_to_cell_when_ready')
@patch.object(hooks, 'notify_region_to_unit')
@patch.object(hooks, 'set_region_on_relation_from_config')
@patch.object(hooks, 'update_ssh_keys_and_notify_compute_units')
def test_cloud_compute_relation_changed(
self,
mock_update_ssh_keys_and_notify_compute_units,
mock_notify_region_to_unit,
mock_set_region_on_relation_from_config,
mock_add_hosts_to_cell_when_ready):
hooks.cloud_compute_relation_changed()
mock_add_hosts_to_cell_when_ready.assert_called_once_with()
mock_notify_region_to_unit.assert_called_once_with(rid=None, unit=None)
mock_set_region_on_relation_from_config.assert_called_once_with(
rid=None)
mock_update_ssh_keys_and_notify_compute_units.assert_called_once_with(
rid=None, unit=None)