From 4d9b4a2600faff4875b145d15b374c3e1a81fe4c Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Mon, 15 Jul 2019 21:39:16 +0100 Subject: [PATCH] Refactor compute hostname resolving functionality The main driver here is to separate the concerns of resolving host names and adding them to service/user related files. This is to enable the (eventual) resolution of the feature to allow migrations across relation ids (i.e. between nova-compute applications) and to enable caching of hostname look ups. Change-Id: I406d1daacbcc74eb6f3e090f9a46e01dd3e19cc8 --- hooks/nova_cc_hooks.py | 15 +++- hooks/nova_cc_utils.py | 139 +++++++++++++++++++++---------- unit_tests/test_nova_cc_hooks.py | 25 ++++-- 3 files changed, 127 insertions(+), 52 deletions(-) diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index a5517542..2e537d85 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -770,6 +770,10 @@ def update_ssh_key(rid=None, unit=None): if migration_auth_type is None: return + remote_service = ncc_utils.remote_service_from_unit(unit) + private_address = rel_settings.get('private-address', None) + hostname = rel_settings.get('hostname', '') + if migration_auth_type == 'ssh': # TODO(ajkavanagh) -- the hookenv was previous behaviour, but there # isn't a good place to put this yet; it will be moved or removed at @@ -782,14 +786,19 @@ def update_ssh_key(rid=None, unit=None): .format(rid or hookenv.relation_id(), unit or hookenv.remote_unit())) return - ncc_utils.ssh_compute_add(key, rid=rid, unit=unit) + ncc_utils.ssh_resolve_compute_hosts( + remote_service, private_address, hostname, user=None) + ncc_utils.add_authorized_key_if_doesnt_exist( + key, remote_service, private_address, user=None) nova_ssh_public_key = rel_settings.get('nova_ssh_public_key', None) # Always try to fetch the user 'nova' key on the remote compute unit if nova_ssh_public_key: - ncc_utils.ssh_compute_add(nova_ssh_public_key, - rid=rid, unit=unit, user='nova') + ncc_utils.ssh_resolve_compute_hosts( + remote_service, private_address, hostname, user='nova') + ncc_utils.add_authorized_key_if_doesnt_exist( + nova_ssh_public_key, remote_service, private_address, user='nova') def notify_ssh_keys_to_compute_units(rid=None, unit=None): diff --git a/hooks/nova_cc_utils.py b/hooks/nova_cc_utils.py index e09df0eb..1a808b62 100644 --- a/hooks/nova_cc_utils.py +++ b/hooks/nova_cc_utils.py @@ -1184,63 +1184,116 @@ def ssh_authorized_key_exists(public_key, remote_service, user=None): return ' {} '.format(public_key) in keys.read() -def add_authorized_key(public_key, remote_service, user=None): - """Add a public key to a specified authorized_keys file +def add_authorized_key_if_doesnt_exist(public_key, + remote_service, + private_address, + user=None): + """Add the public key to the authorized_keys file if it doesn't already + exist. The authorized_keys file is determined by the remote_service param passed and (optionally) the user, if it is not None. + If the private_address is None, then the function bails until a further + hook makes it available. + :param public_key: The public_key to add to specified authorized_keys file :type public_key: str :param remote_service: the remote service used to determine the known_hosts file. :type remote_service: str + :param private_address: The private address of the unit + :type private_address: Union[str, None] :param user: optional user used to determine the known_hosts file. :type user: Union[str, None] """ - with open(authorized_keys(remote_service, user), 'a') as keys: - keys.write(public_key + '\n') - - -def ssh_compute_add(public_key, rid=None, unit=None, user=None): - # If remote compute node hands us a hostname, ensure we have a - # known hosts entry for its IP, hostname and FQDN. - private_address = hookenv.relation_get(rid=rid, unit=unit, - attribute='private-address') - hosts = [] - - if not ch_ip.is_ipv6(private_address): - hostname = hookenv.relation_get(rid=rid, unit=unit, - attribute='hostname') - if hostname: - hosts.append(hostname.lower()) - - if not ch_utils.is_ip(private_address): - hosts.append(private_address.lower()) - hosts.append(ch_utils.get_host_ip(private_address)) - short = private_address.split('.')[0] - if ch_ip.ns_query(short): - hosts.append(short.lower()) - else: - hosts.append(private_address) - hn = ch_utils.get_hostname(private_address) - if hn: - hosts.append(hn.lower()) - short = hn.split('.')[0] - if ch_ip.ns_query(short): - hosts.append(short.lower()) - else: - hosts.append(private_address) - - remote_service = remote_service_from_unit(unit) - - for host in set(hosts): - add_known_host(host, remote_service, user) - + if private_address is None: + return if not ssh_authorized_key_exists(public_key, remote_service, user): hookenv.log('Saving SSH authorized key for compute host at %s.' % private_address) - add_authorized_key(public_key, remote_service, user) + with open(authorized_keys(remote_service, user), 'a') as keys: + keys.write(public_key + '\n') + + +def ssh_resolve_compute_hosts(remote_service, + private_address, + hostname, + user=None): + """Resolve all the host names for the private address, and store it against + the remote service (effectively the relation) and an optional user. + + Note(ajkavanagh) a further patch will remove the remote_service aspect so + that the hosts are just stored per user at the target. However, how to + upgrade an existing system still needs to be considered. + + :param remote_service: The remote service against which to store the hosts + file. + :type remote_service: str + :param private_address: The private address to resolve hostnames against. + :type private_address: Union[str, None] + :param hostname: An optional hostname (extracted from the relation data of + the unit), to also use to resolve hostnames for the compute unit. + :type hostname: str + :param user: an optional user against which to store the resolved + hostnames. + :type user: Union[str, None] + """ + for host in _resolve_hosts(private_address, hostname): + # TODO(ajkavanagh) expensive + add_known_host(host, remote_service, user) + + +def _resolve_hosts(private_address, hostname): + """Return all of the resolved hosts for a unit + + Using private-address and (if availble) hostname attributes on the + relation, create a definite list of hostnames for that unit according to + the DNS set up for the system. + + If remote compute node hands us a hostname, ensure we have a known hosts + entry for its IP, hostname and FQDN. + + :param private_address: the private address of the unit from its relation + data. + :type private_address: Union[str, None] + :param hostname: the 'hostname' from the relation data for the unit. + :type hostname: str + :returns: list of hostname strings + :rtype: List[str] + """ + if private_address is None: + return [] + + # Use a set to enforce uniqueness; order doesn't matter + hosts = set() + + if not ch_ip.is_ipv6(private_address): + if hostname: + hosts.add(hostname.lower()) + + if not ch_utils.is_ip(private_address): + hosts.append(private_address.lower()) + # TODO(ajkavanagh) expensive + hosts.add(ch_utils.get_host_ip(private_address)) + short = private_address.split('.')[0] + # TODO(ajkavanagh) expensive + if ch_ip.ns_query(short): + hosts.add(short.lower()) + else: + hosts.add(private_address) + # TODO(ajkavanagh) expensive + hn = ch_utils.get_hostname(private_address) + if hn: + hosts.add(hn.lower()) + short = hn.split('.')[0] + # TODO(ajkananagh) expensive + if ch_ip.ns_query(short): + hosts.add(short.lower()) + else: + hosts.add(private_address) + + return list(hosts) def ssh_known_hosts_lines(remote_service, user=None): @@ -1552,7 +1605,7 @@ def check_optional_relations(configs): if hookenv.relation_ids('ha'): try: ch_cluster.get_hacluster_config() - except: + except Exception: return ('blocked', 'hacluster missing configuration: ' 'vip, vip_iface, vip_cidr') diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index fc557d28..07ab958f 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -68,7 +68,7 @@ TO_PATCH = [ 'hooks.nova_cc_utils.serial_console_settings', 'hooks.nova_cc_utils.services', 'hooks.nova_cc_utils.ssh_authorized_keys_lines', - 'hooks.nova_cc_utils.ssh_compute_add', + 'hooks.nova_cc_utils.ssh_resolve_compute_hosts', 'hooks.nova_cc_utils.ssh_known_hosts_lines', 'uuid', ] @@ -424,12 +424,16 @@ class NovaCCHooksTests(CharmTestCase): self.assertFalse( hooks._goal_state_achieved_for_relid('aservice', None)) + @patch('hooks.nova_cc_utils.add_authorized_key_if_doesnt_exist') + @patch('hooks.nova_cc_utils.ssh_resolve_compute_hosts') @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') @patch('hooks.nova_cc_utils.remote_service_from_unit') def test_update_ssh_keys_and_notify_compute_units_ssh_migration( self, mock_remote_service_from_unit, - mock__goal_state_achieved_for_relid): + mock__goal_state_achieved_for_relid, + mock_ssh_resolve_compute_hosts, + mock_add_authorized_key_if_doesnt_exist): mock_remote_service_from_unit.return_value = 'aservice' mock__goal_state_achieved_for_relid.return_value = True self.test_relation.set({ @@ -440,7 +444,10 @@ class NovaCCHooksTests(CharmTestCase): self.ssh_authorized_keys_lines.return_value = [ 'auth_0', 'auth_1', 'auth_2'] hooks.update_ssh_keys_and_notify_compute_units() - self.ssh_compute_add.assert_called_with('fookey', rid=None, unit=None) + mock_ssh_resolve_compute_hosts.assert_called_once_with( + 'aservice', '10.0.0.1', '', user=None) + mock_add_authorized_key_if_doesnt_exist.assert_called_once_with( + 'fookey', 'aservice', '10.0.0.1', user=None) expected_relations = [ call(relation_settings={'authorized_keys_0': 'auth_0'}, relation_id=None), @@ -462,12 +469,16 @@ class NovaCCHooksTests(CharmTestCase): mock__goal_state_achieved_for_relid.assert_called_once_with( 'cloud-compute', None) + @patch('hooks.nova_cc_utils.add_authorized_key_if_doesnt_exist') + @patch('hooks.nova_cc_utils.ssh_resolve_compute_hosts') @patch('hooks.nova_cc_hooks._goal_state_achieved_for_relid') @patch('hooks.nova_cc_utils.remote_service_from_unit') def test_update_ssh_keys_and_notify_compute_units_nova_public_key( self, mock_remote_service_from_unit, - mock__goal_state_achieved_for_relid): + mock__goal_state_achieved_for_relid, + mock_ssh_resolve_compute_hosts, + mock_add_authorized_key_if_doesnt_exist): mock_remote_service_from_unit.return_value = 'aservice' mock__goal_state_achieved_for_relid.return_value = True self.test_relation.set({ @@ -478,8 +489,10 @@ class NovaCCHooksTests(CharmTestCase): self.ssh_authorized_keys_lines.return_value = [ 'auth_0', 'auth_1', 'auth_2'] hooks.update_ssh_keys_and_notify_compute_units() - self.ssh_compute_add.assert_called_with('fookey', user='nova', - rid=None, unit=None) + mock_ssh_resolve_compute_hosts.assert_called_once_with( + 'aservice', '10.0.0.1', '', user='nova') + mock_add_authorized_key_if_doesnt_exist.assert_called_once_with( + 'fookey', 'aservice', '10.0.0.1', user='nova') expected_relations = [ call(relation_settings={'nova_authorized_keys_0': 'auth_0'}, relation_id=None),