diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index d8f429f3..1a2379e7 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -860,7 +860,7 @@ def update_ssh_key(rid=None, unit=None): hostname = rel_settings.get('hostname', '') # only resolve the hosts once, so this is the memo for it - resolved_hosts = None + resolved_hosts = [] if migration_auth_type == 'ssh': # TODO(ajkavanagh) -- the hookenv was previous behaviour, but there @@ -874,7 +874,24 @@ def update_ssh_key(rid=None, unit=None): .format(rid or hookenv.relation_id(), unit or hookenv.remote_unit())) return - resolved_hosts = ncc_utils.resolve_hosts_for(private_address, hostname) + + resolved_hosts = ncc_utils.resolve_hosts_for(private_address, + hostname) + # Ensure management address and fqdn always in known_hosts (see LP + # 1969971) + ingress_address = hookenv.ingress_address(rid=rid, unit=unit) + if ingress_address: + if ingress_address not in resolved_hosts: + resolved_hosts.append(ingress_address) + + ingress_host = ncc_utils.resolve_hosts_for(ingress_address, None) + if ingress_host: + for host in ingress_host: + if host not in resolved_hosts: + resolved_hosts.append(host) + + # sort mainly for unit tests + resolved_hosts = sorted(resolved_hosts) ncc_utils.ssh_compute_add_known_hosts( remote_service, resolved_hosts, user=None) ncc_utils.add_authorized_key_if_doesnt_exist( @@ -886,7 +903,7 @@ def update_ssh_key(rid=None, unit=None): if nova_ssh_public_key: # in the unlikely event the migration type wasn't ssh, we still have to # resolve the hosts - if resolved_hosts is None: + if not resolved_hosts: resolved_hosts = ncc_utils.resolve_hosts_for(private_address, hostname) ncc_utils.ssh_compute_add_known_hosts( diff --git a/hooks/nova_cc_utils.py b/hooks/nova_cc_utils.py index bda6e142..4dcc52ab 100644 --- a/hooks/nova_cc_utils.py +++ b/hooks/nova_cc_utils.py @@ -1398,11 +1398,14 @@ def resolve_hosts_for(private_address, hostname): # Note, the cache is maintained regardless of whether the config # 'cache-known-hosts' flag is set; the flag only affects usage and lookup. - hosts = list(hosts) + hosts = sorted(list(hosts)) db.set(db_key, hosts) db.flush() - return hosts + # NOTE: if we don't make this a fresh copy the caller can modify it which + # can break, amongst other things, unit tests that use + # assert_has_calls to test input. + return hosts[:] def clear_hostset_cache_for(private_address): diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index b3036918..fe324eea 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -451,6 +451,7 @@ class NovaCCHooksTests(CharmTestCase): self.assertFalse( hooks._goal_state_achieved_for_relid('aservice', None)) + @patch.object(utils.hookenv, 'ingress_address') @patch('charmhelpers.contrib.openstack.utils.get_hostname') @patch('charmhelpers.core.unitdata.Storage.set') @patch('hooks.nova_cc_utils.add_authorized_key_if_doesnt_exist') @@ -462,20 +463,34 @@ class NovaCCHooksTests(CharmTestCase): mock_ssh_compute_add_known_hosts, mock_add_authorized_key_if_doesnt_exist, mock_db_set, - mock_get_hostname): - mock_get_hostname.return_value = None - mock_remote_service_from_unit.return_value = 'aservice' + mock_get_hostname, + mock_ingress_address): + def fake_get_hostname(addr): + if addr == '10.0.0.1': + return "bond0.bighost.ace" + elif addr == '10.1.2.3': + return "bighost.ace" + + raise Exception("unexpected resolve for address {}".format(addr)) + + mock_get_hostname.side_effect = fake_get_hostname + mock_remote_service_from_unit.return_value = 'aservice' + mock_ingress_address.return_value = '10.1.2.3' self.relation_get.side_effect = [{ 'migration_auth_type': 'ssh', 'ssh_public_key': 'fookey', 'private-address': '10.0.0.1', 'region': 'RegionOne'}] hooks.update_ssh_key(rid=None, unit=None) mock_ssh_compute_add_known_hosts.assert_called_once_with( - 'aservice', ['10.0.0.1'], user=None) + 'aservice', sorted(['10.0.0.1', 'bond0.bighost.ace', '10.1.2.3', + 'bighost.ace']), user=None) mock_add_authorized_key_if_doesnt_exist.assert_called_once_with( 'fookey', 'aservice', '10.0.0.1', user=None) - mock_db_set.assert_called_once_with('hostset-10.0.0.1', ['10.0.0.1']) + mock_db_set.assert_has_calls([call('hostset-10.0.0.1', + ['10.0.0.1', 'bond0.bighost.ace']), + call('hostset-10.1.2.3', + ['10.1.2.3', 'bighost.ace'])]) @patch('charmhelpers.contrib.openstack.utils.get_hostname') @patch('charmhelpers.core.unitdata.Storage.set')