From 61a7dd0bbd62af3cf73635199cc1090154a5ebb9 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). Also adds requirements.txt to default tox [testenv] to get netaddr constraints applied to others that were removed as part of https://review.opendev.org/q/topic:%22batch-update%22. This is needed for -epy38 which is only run by gate. Change-Id: Ic9e4657453d8f53d1ecbee23475c7b11549ebc14 Closes-Bug: #1969971 (cherry picked from commit 05b081bf5ffa24045a1fef3b18d2e28fec52d604) (cherry picked from commit b6809e75e181a49c2209aa2c1be9e4d11442cbb3) --- hooks/nova_cc_hooks.py | 23 ++++++++++++++++++++--- hooks/nova_cc_utils.py | 7 +++++-- tox.ini | 4 +++- unit_tests/test_nova_cc_hooks.py | 25 ++++++++++++++++++++----- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index 87051e51..f656d596 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/tox.ini b/tox.ini index 2cb6ca16..ae60192b 100644 --- a/tox.ini +++ b/tox.ini @@ -32,7 +32,9 @@ passenv = CS_* OS_* TEST_* -deps = -r{toxinidir}/test-requirements.txt +deps = + -r{toxinidir}/test-requirements.txt + -r{toxinidir}/requirements.txt [testenv:build] basepython = python3 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')