From a6c5b52d7d5e0064d44ebe1cadfa262ee4c34775 Mon Sep 17 00:00:00 2001 From: Adam King Date: Wed, 15 Feb 2023 17:07:09 -0500 Subject: [PATCH] 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 (cherry picked from commit 6443cf15d54a5e50c245dd08c3db005bb8521b6a) Conflicts: src/pybind/mgr/cephadm/tests/test_cephadm.py --- src/cephadm/cephadm | 8 ++++ src/pybind/mgr/cephadm/inventory.py | 44 +++++++++++++++---- src/pybind/mgr/cephadm/tests/test_cephadm.py | 45 ++++++++++++++++++++ src/pybind/mgr/cephadm/tests/test_facts.py | 21 +++++++++ 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index a3a7ca6eb0039..3283ef6581ddf 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -7308,6 +7308,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 9d439657fd1b2..8299383bda447 100644 --- a/src/pybind/mgr/cephadm/inventory.py +++ b/src/pybind/mgr/cephadm/inventory.py @@ -1,6 +1,7 @@ import datetime from copy import copy import ipaddress +import itertools import json import logging import socket @@ -80,20 +81,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) @@ -105,17 +125,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() @@ -124,7 +145,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() @@ -133,17 +154,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), @@ -517,6 +540,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 bbd355b25336d..1905979247ed8 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -1803,3 +1803,48 @@ Traceback (most recent call last): with with_host(cephadm_module, 'test1', refresh_hosts=False, rm_with_force=True): with with_host(cephadm_module, 'test2', refresh_hosts=False, rm_with_force=False): cephadm_module.inventory.add_label('test2', '_admin') + + 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 + # 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 6c33f5368aba8..7838ee5d422ed 100644 --- 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', '', '') -- 2.39.5