diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index a50a93e1..dda6452e 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -279,7 +279,10 @@ def config_changed(): if ch_utils.config_value_changed('region'): for rid in hookenv.relation_ids('cloud-compute'): for unit in hookenv.related_units(rid): - compute_changed(rid, unit) + # 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) ncc_utils.update_aws_compat_services() @@ -624,16 +627,71 @@ def compute_joined(rid=None, remote_restart=False): @hooks.hook('cloud-compute-relation-changed') -def compute_changed(rid=None, unit=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')) +def cloud_compute_relation_changed(): + """Performs actions associated with when the cloud compute relation changes + for a unit. + * add hosts to the cell when ready + * notifies the region to the unit, if it has 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) + update_ssh_keys_and_notify_compute_units(rid=None, unit=None) + + +def add_hosts_to_cell_when_ready(): + """Helper function to call add_hosts_to_cell() when the unit is the leader + and the cellv2 and database are ready. + + :raises: subprocess.CalledProcessError if cells command fails + """ if (hookenv.is_leader() and ncc_utils.is_cellv2_init_ready() and ncc_utils.is_db_initialised()): 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. + + :param rid: The relation to check/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')) + + +def update_ssh_keys_and_notify_compute_units(rid=None, unit=None): + """Update and notify the collected ssh keys to nova-compute units + + Update/add and notify, for the associated nova-compute unit, the ssh key to + all the other nova-compute units. + + If rid=None and unit=None, then this function is being called in the + context of a cloud-compute relation changed hook, and will relate to that + unit. If rid and unit are set, then this function is being called to + refresh and update all specific units. + + TODO: split this function in to updating/adding SSH keys and notifying + (set_relation) all of the other units. + + :param rid: The relation to check/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 'migration_auth_type' not in rel_settings: return if rel_settings['migration_auth_type'] == 'ssh': @@ -899,10 +957,13 @@ def upgrade_charm(): identity_joined(rid=r_id) for r_id in hookenv.relation_ids('cloud-compute'): for unit in hookenv.related_units(r_id): - compute_changed(r_id, unit) + 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) + add_hosts_to_cell_when_ready() + leader_init_db_if_ready_allowed_units() update_nrpe_config() diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index a39beb91..39849251 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -316,8 +316,9 @@ class NovaCCHooksTests(CharmTestCase): @patch('charmhelpers.fetch.filter_installed_packages') @patch.object(hooks, 'configure_https') @patch.object(hooks, 'compute_joined') - @patch.object(hooks, 'compute_changed') - def test_config_changed_region_change(self, mock_compute_changed, + @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, @@ -338,15 +339,27 @@ class NovaCCHooksTests(CharmTestCase): self.os_release.return_value = 'diablo' hooks.resolve_CONFIGS() hooks.config_changed() - mock_compute_changed.assert_has_calls([call('generic_rid', 'unit/0')]) + mock_notify_region_to_unit.assert_has_calls( + [call('generic_rid', 'unit/0')]) mock_compute_joined.assert_has_calls( [call(rid='generic_rid', remote_restart=False)]) self.assertTrue(mock_update_aws_compat_services.called) - @patch('hooks.nova_cc_utils.is_cellv2_init_ready') - @patch('hooks.nova_cc_utils.is_db_initialised') - def test_compute_changed_ssh_migration(self, mock_is_db_initialised, - mock_is_cellv2_init_ready): + @patch.object(hooks, 'add_hosts_to_cell_when_ready') + @patch.object(hooks, 'notify_region_to_unit') + @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_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_update_ssh_keys_and_notify_compute_units.assert_called_once_with( + rid=None, unit=None) + + def test_update_ssh_keys_and_notify_compute_units_ssh_migration(self): self.test_relation.set({ 'migration_auth_type': 'ssh', 'ssh_public_key': 'fookey', 'private-address': '10.0.0.1', 'region': 'RegionOne'}) @@ -354,9 +367,7 @@ class NovaCCHooksTests(CharmTestCase): 'k_h_0', 'k_h_1', 'k_h_2'] self.ssh_authorized_keys_lines.return_value = [ 'auth_0', 'auth_1', 'auth_2'] - mock_is_db_initialised.return_value = False - mock_is_cellv2_init_ready.return_value = False - hooks.compute_changed() + hooks.update_ssh_keys_and_notify_compute_units() self.ssh_compute_add.assert_called_with('fookey', rid=None, unit=None) expected_relations = [ call(relation_settings={'authorized_keys_0': 'auth_0'}, @@ -375,10 +386,7 @@ class NovaCCHooksTests(CharmTestCase): call(known_hosts_max_index=3, relation_id=None)] self.relation_set.assert_has_calls(expected_relations, any_order=True) - @patch('hooks.nova_cc_utils.is_cellv2_init_ready') - @patch('hooks.nova_cc_utils.is_db_initialised') - def test_compute_changed_nova_public_key(self, mock_is_db_initialised, - mock_is_cellv2_init_ready): + def test_update_ssh_keys_and_notify_compute_units_nova_public_key(self): self.test_relation.set({ 'migration_auth_type': 'sasl', 'nova_ssh_public_key': 'fookey', 'private-address': '10.0.0.1', 'region': 'RegionOne'}) @@ -386,9 +394,7 @@ class NovaCCHooksTests(CharmTestCase): 'k_h_0', 'k_h_1', 'k_h_2'] self.ssh_authorized_keys_lines.return_value = [ 'auth_0', 'auth_1', 'auth_2'] - mock_is_db_initialised.return_value = False - mock_is_cellv2_init_ready.return_value = False - hooks.compute_changed() + hooks.update_ssh_keys_and_notify_compute_units() self.ssh_compute_add.assert_called_with('fookey', user='nova', rid=None, unit=None) expected_relations = [ @@ -413,14 +419,14 @@ class NovaCCHooksTests(CharmTestCase): @patch('hooks.nova_cc_utils.is_cellv2_init_ready') @patch('hooks.nova_cc_utils.is_db_initialised') @patch('hooks.nova_cc_utils.add_hosts_to_cell') - def test_compute_changed_add_hosts_leader(self, - mock_add_hosts_to_cell, - mock_is_db_initialised, - mock_is_cellv2_init_ready): + def test_add_hosts_to_cell_when_ready_leader(self, + mock_add_hosts_to_cell, + mock_is_db_initialised, + mock_is_cellv2_init_ready): self.is_leader.return_value = True mock_is_db_initialised.return_value = True mock_is_cellv2_init_ready.return_value = True - hooks.compute_changed() + hooks.add_hosts_to_cell_when_ready() self.assertTrue(self.is_leader.called) self.assertTrue(mock_is_db_initialised.called) self.assertTrue(mock_is_cellv2_init_ready.called) @@ -429,14 +435,14 @@ class NovaCCHooksTests(CharmTestCase): @patch('hooks.nova_cc_utils.is_cellv2_init_ready') @patch('hooks.nova_cc_utils.is_db_initialised') @patch('hooks.nova_cc_utils.add_hosts_to_cell') - def test_compute_changed_add_hosts_nonleader(self, - mock_add_hosts_to_cell, - mock_is_db_initialised, - mock_is_cellv2_init_ready): + def test_add_hosts_to_cell_when_ready_nonleader(self, + mock_add_hosts_to_cell, + mock_is_db_initialised, + mock_is_cellv2_init_ready): self.is_leader.return_value = False mock_is_db_initialised.return_value = True mock_is_cellv2_init_ready.return_value = True - hooks.compute_changed() + hooks.add_hosts_to_cell_when_ready() self.assertTrue(self.is_leader.called) self.assertFalse(mock_is_db_initialised.called) self.assertFalse(mock_is_cellv2_init_ready.called)