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

@@ -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(

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):

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,
-        mock_get_hostname.return_value = None
+            mock_ingress_address):
-        mock_remote_service_from_unit.return_value = 'aservice'
 
+        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')