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 <adking@redhat.com>
(cherry picked from commit 6443cf15d54a5e50c245dd08c3db005bb8521b6a)

diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm
index dbe2e639b661455d16e4dcd0a1bb5695d64d2876..7f05d74bd83cf5183bc9e874c4c1934bbf876d77 100755 (executable)
--- a/src/cephadm/cephadm
+++ b/src/cephadm/cephadm
@@ -8361,6 +8361,14 @@ class HostFacts():
         """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

diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py
index bb7be85f67ff5ebe71c87a6569baa017402f3eda..9ffc9b1806b041b39a1909b35281334063924bc2 100644 (file)
--- a/src/pybind/mgr/cephadm/inventory.py
+++ b/src/pybind/mgr/cephadm/inventory.py
@@ -2,6 +2,7 @@ import datetime
 import enum
 from copy import copy
 import ipaddress
+import itertools
 import json
 import logging
 import math
@@ -90,20 +91,39 @@ class Inventory:
                 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 @@ class Inventory:
             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 @@ class Inventory:
         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 @@ class Inventory:
         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),
@@ -632,6 +655,11 @@ class HostCache():
     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:

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 33185cad6346dba46b158c2dfda7618510cdd94a..908926c83a9dc325e58036769f4e569fc9fd9224 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -2197,3 +2197,48 @@ Traceback (most recent call last):
                 cephadm_module.inventory.all_specs = mock.Mock(
                     return_value=[mock.Mock().hostname, mock.Mock().hostname])
                 cephadm_module._validate_tuned_profile_spec(spec)
+
+    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')

diff --git a/src/pybind/mgr/cephadm/tests/test_facts.py b/src/pybind/mgr/cephadm/tests/test_facts.py
index 6c33f5368aba86f298f43ad2579d92d7f33b998c..7838ee5d422edc81eaf64a031c841b38a09f474c 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_facts.py
+++ b/src/pybind/mgr/cephadm/tests/test_facts.py
@@ -2,9 +2,30 @@ from ..import CephadmOrchestrator
 
 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', '', '')