mgr/cephadm: be aware of host's shortname and FQDN

The idea is to gether the shortname and FQDN as part
of gather-facts, and then if we ever try to check if a certain
host is in our internal inventory by hostname, we can check
these other known names. This should avoid issues where
we think a hostname specified by FQDN is not in our
inventory because we know the host by the shortname
or vice versa.

Fixes: https://tracker.ceph.com/issues/58738

Signed-off-by: Adam King <[email protected]>

src/cephadm/cephadm.py

@@ -8621,6 +8621,14 @@ def hostname(self):
         """Return the hostname"""
         return platform.node()
 
+    @property
+    def shortname(self) -> str:
+        return platform.node().split('.', 1)[0]
+
+    @property
+    def fqdn(self) -> str:
+        return get_fqdn()
+
     @property
     def subscribed(self):
         # type: () -> str

src/pybind/mgr/cephadm/inventory.py

@@ -2,6 +2,7 @@
 import enum
 from copy import copy
 import ipaddress
+import itertools
 import json
 import logging
 import math
@@ -90,20 +91,39 @@ def is_valid_ip(ip: str) -> bool:
                 self.save()
         else:
             self._inventory = dict()
+        self._all_known_names: Dict[str, List[str]] = {}
         logger.debug('Loaded inventory %s' % self._inventory)
 
     def keys(self) -> List[str]:
         return list(self._inventory.keys())
 
     def __contains__(self, host: str) -> bool:
-        return host in self._inventory
+        return host in self._inventory or host in itertools.chain.from_iterable(self._all_known_names.values())
+
+    def _get_stored_name(self, host: str) -> str:
+        self.assert_host(host)
+        if host in self._inventory:
+            return host
+        for stored_name, all_names in self._all_known_names.items():
+            if host in all_names:
+                return stored_name
+        return host
+
+    def update_known_hostnames(self, hostname: str, shortname: str, fqdn: str) -> None:
+        for hname in [hostname, shortname, fqdn]:
+            # if we know the host by any of the names, store the full set of names
+            # in order to be able to check against those names for matching a host
+            if hname in self._inventory:
+                self._all_known_names[hname] = [hostname, shortname, fqdn]
+                return
+        logger.debug(f'got hostname set from gather-facts for unknown host: {[hostname, shortname, fqdn]}')
 
     def assert_host(self, host: str) -> None:
-        if host not in self._inventory:
+        if host not in self:
             raise OrchestratorError('host %s does not exist' % host)
 
     def add_host(self, spec: HostSpec) -> None:
-        if spec.hostname in self._inventory:
+        if spec.hostname in self:
             # addr
             if self.get_addr(spec.hostname) != spec.addr:
                 self.set_addr(spec.hostname, spec.addr)
@@ -115,17 +135,18 @@ def add_host(self, spec: HostSpec) -> None:
             self.save()
 
     def rm_host(self, host: str) -> None:
-        self.assert_host(host)
+        host = self._get_stored_name(host)
         del self._inventory[host]
+        self._all_known_names.pop(host, [])
         self.save()
 
     def set_addr(self, host: str, addr: str) -> None:
-        self.assert_host(host)
+        host = self._get_stored_name(host)
         self._inventory[host]['addr'] = addr
         self.save()
 
     def add_label(self, host: str, label: str) -> None:
-        self.assert_host(host)
+        host = self._get_stored_name(host)
 
         if 'labels' not in self._inventory[host]:
             self._inventory[host]['labels'] = list()
@@ -134,7 +155,7 @@ def add_label(self, host: str, label: str) -> None:
         self.save()
 
     def rm_label(self, host: str, label: str) -> None:
-        self.assert_host(host)
+        host = self._get_stored_name(host)
 
         if 'labels' not in self._inventory[host]:
             self._inventory[host]['labels'] = list()
@@ -143,17 +164,19 @@ def rm_label(self, host: str, label: str) -> None:
         self.save()
 
     def has_label(self, host: str, label: str) -> bool:
+        host = self._get_stored_name(host)
         return (
             host in self._inventory
             and label in self._inventory[host].get('labels', [])
         )
 
     def get_addr(self, host: str) -> str:
-        self.assert_host(host)
+        host = self._get_stored_name(host)
         return self._inventory[host].get('addr', host)
 
     def spec_from_dict(self, info: dict) -> HostSpec:
         hostname = info['hostname']
+        hostname = self._get_stored_name(hostname)
         return HostSpec(
             hostname,
             addr=info.get('addr', hostname),
@@ -641,6 +664,11 @@ def update_host_daemons(self, host, dm):
     def update_host_facts(self, host, facts):
         # type: (str, Dict[str, Dict[str, Any]]) -> None
         self.facts[host] = facts
+        hostnames: List[str] = []
+        for k in ['hostname', 'shortname', 'fqdn']:
+            v = facts.get(k, '')
+            hostnames.append(v if isinstance(v, str) else '')
+        self.mgr.inventory.update_known_hostnames(hostnames[0], hostnames[1], hostnames[2])
         self.last_facts_update[host] = datetime_now()
 
     def update_autotune(self, host: str) -> None:

src/pybind/mgr/cephadm/tests/test_cephadm.py

@@ -2147,3 +2147,48 @@ def test_set_unmanaged(self, cephadm_module):
         assert cephadm_module.spec_store._specs['crash'].unmanaged
         cephadm_module.spec_store.set_unmanaged('crash', False)
         assert not cephadm_module.spec_store._specs['crash'].unmanaged
+
+    def test_inventory_known_hostnames(self, cephadm_module):
+        cephadm_module.inventory.add_host(HostSpec('host1', '1.2.3.1'))
+        cephadm_module.inventory.add_host(HostSpec('host2', '1.2.3.2'))
+        cephadm_module.inventory.add_host(HostSpec('host3.domain', '1.2.3.3'))
+        cephadm_module.inventory.add_host(HostSpec('host4.domain', '1.2.3.4'))
+        cephadm_module.inventory.add_host(HostSpec('host5', '1.2.3.5'))
+
+        # update_known_hostname expects args to be <hostname, shortname, fqdn>
+        # as are gathered from cephadm gather-facts. Although, passing the
+        # names in the wrong order should actually have no effect on functionality
+        cephadm_module.inventory.update_known_hostnames('host1', 'host1', 'host1.domain')
+        cephadm_module.inventory.update_known_hostnames('host2.domain', 'host2', 'host2.domain')
+        cephadm_module.inventory.update_known_hostnames('host3', 'host3', 'host3.domain')
+        cephadm_module.inventory.update_known_hostnames('host4.domain', 'host4', 'host4.domain')
+        cephadm_module.inventory.update_known_hostnames('host5', 'host5', 'host5')
+
+        assert 'host1' in cephadm_module.inventory
+        assert 'host1.domain' in cephadm_module.inventory
+        assert cephadm_module.inventory.get_addr('host1') == '1.2.3.1'
+        assert cephadm_module.inventory.get_addr('host1.domain') == '1.2.3.1'
+
+        assert 'host2' in cephadm_module.inventory
+        assert 'host2.domain' in cephadm_module.inventory
+        assert cephadm_module.inventory.get_addr('host2') == '1.2.3.2'
+        assert cephadm_module.inventory.get_addr('host2.domain') == '1.2.3.2'
+
+        assert 'host3' in cephadm_module.inventory
+        assert 'host3.domain' in cephadm_module.inventory
+        assert cephadm_module.inventory.get_addr('host3') == '1.2.3.3'
+        assert cephadm_module.inventory.get_addr('host3.domain') == '1.2.3.3'
+
+        assert 'host4' in cephadm_module.inventory
+        assert 'host4.domain' in cephadm_module.inventory
+        assert cephadm_module.inventory.get_addr('host4') == '1.2.3.4'
+        assert cephadm_module.inventory.get_addr('host4.domain') == '1.2.3.4'
+
+        assert 'host4.otherdomain' not in cephadm_module.inventory
+        with pytest.raises(OrchestratorError):
+            cephadm_module.inventory.get_addr('host4.otherdomain')
+
+        assert 'host5' in cephadm_module.inventory
+        assert cephadm_module.inventory.get_addr('host5') == '1.2.3.5'
+        with pytest.raises(OrchestratorError):
+            cephadm_module.inventory.get_addr('host5.domain')

src/pybind/mgr/cephadm/tests/test_facts.py

@@ -2,9 +2,30 @@
 
 from .fixtures import wait
 
+from tests import mock
+
 
 def test_facts(cephadm_module: CephadmOrchestrator):
     facts = {'node-1.ceph.com': {'bios_version': 'F2', 'cpu_cores': 16}}
     cephadm_module.cache.facts = facts
     ret_facts = cephadm_module.get_facts('node-1.ceph.com')
     assert wait(cephadm_module, ret_facts) == [{'bios_version': 'F2', 'cpu_cores': 16}]
+
+
+@mock.patch("cephadm.inventory.Inventory.update_known_hostnames")
+def test_known_hostnames(_update_known_hostnames, cephadm_module: CephadmOrchestrator):
+    host_facts = {'hostname': 'host1.domain',
+                  'shortname': 'host1',
+                  'fqdn': 'host1.domain',
+                  'memory_free_kb': 37383384,
+                  'memory_total_kb': 40980612,
+                  'nic_count': 2}
+    cephadm_module.cache.update_host_facts('host1', host_facts)
+    _update_known_hostnames.assert_called_with('host1.domain', 'host1', 'host1.domain')
+
+    host_facts = {'hostname': 'host1.domain',
+                  'memory_free_kb': 37383384,
+                  'memory_total_kb': 40980612,
+                  'nic_count': 2}
+    cephadm_module.cache.update_host_facts('host1', host_facts)
+    _update_known_hostnames.assert_called_with('host1.domain', '', '')