From fbaef937dc33bcbbc8a4ac76948f2f67166e8e5d Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Mon, 9 Oct 2023 16:14:10 +0100 Subject: [PATCH] Ensure mgmt network hostname and fqdn in known_hosts The cloud-compute relation uses the private-address setting to reflect the hostname/address to be used for vm migrations. This can be the default management network or an alternate one. When this charm populates ssh known_hosts entries for compute hosts it needs to ensure hostname, address and fqdn for the mgmt network is included so that Nova resize operations can work if they use the hostname from the db (which will always be from the mgmt network). Change-Id: Ic9e4657453d8f53d1ecbee23475c7b11549ebc14 Closes-Bug: #1969971 --- hooks/nova_cc_hooks.py | 23 ++++++++++++++++++++--- hooks/nova_cc_utils.py | 7 +++++-- unit_tests/test_nova_cc_hooks.py | 25 ++++++++++++++++++++----- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index 34685b96..d1bf6f08 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -856,7 +856,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 @@ -870,7 +870,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( @@ -882,7 +899,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 393b5769..126b9ae6 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')