From 3587a10b3eb5a35bfaad556a65eb83c6b062526c Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 21 May 2021 12:32:49 -0400 Subject: [PATCH] mgr/cephadm: resolve IP at 'orch host add' time We prefer to always have a real IP for hosts in the cluster. This avoids a reliance on DNS for most operations. Perhaps more importantly, it means we are less sensitive to inconsistent host lookup results, for example due to (1) mismatched /etc/hosts files between machines, or (2) a lookup of the local hostname that returns 127.0.1.1. Adjust with_hosts() fixture to take an addr, and adjust tests accordingly. Signed-off-by: Sage Weil --- src/pybind/mgr/cephadm/module.py | 11 +++++++---- src/pybind/mgr/cephadm/tests/fixtures.py | 4 ++-- src/pybind/mgr/cephadm/tests/test_cephadm.py | 18 +++++++++--------- src/pybind/mgr/orchestrator/module.py | 6 +++++- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 0b686dbb809..a4a34bc0ae9 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1383,10 +1383,10 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, ) ] - def _check_valid_addr(self, host: str, addr: str) -> None: + def _check_valid_addr(self, host: str, addr: str) -> str: # make sure hostname is resolvable before trying to make a connection try: - utils.resolve_ip(addr) + ip_addr = utils.resolve_ip(addr) except OrchestratorError as e: msg = str(e) + f''' You may need to supply an address for {addr} @@ -1417,6 +1417,7 @@ Then run the following: errors = [_i.replace("ERROR: ", "") for _i in err if _i.startswith('ERROR')] raise OrchestratorError('Host %s (%s) failed check(s): %s' % ( host, addr, errors)) + return ip_addr def _add_host(self, spec): # type: (HostSpec) -> str @@ -1426,7 +1427,9 @@ Then run the following: :param host: host name """ assert_valid_host(spec.hostname) - self._check_valid_addr(spec.hostname, spec.addr) + ip_addr = self._check_valid_addr(spec.hostname, spec.addr) + if spec.addr == spec.hostname and ip_addr: + spec.addr = ip_addr # prime crush map? if spec.location: @@ -1443,7 +1446,7 @@ Then run the following: self.offline_hosts_remove(spec.hostname) self.event.set() # refresh stray health check self.log.info('Added host %s' % spec.hostname) - return "Added host '{}'".format(spec.hostname) + return "Added host '{}' with addr '{}'".format(spec.hostname, spec.addr) @handle_orch_error def add_host(self, spec: HostSpec) -> str: diff --git a/src/pybind/mgr/cephadm/tests/fixtures.py b/src/pybind/mgr/cephadm/tests/fixtures.py index 127f14f7729..11983737bca 100644 --- a/src/pybind/mgr/cephadm/tests/fixtures.py +++ b/src/pybind/mgr/cephadm/tests/fixtures.py @@ -70,9 +70,9 @@ def wait(m, c): @contextmanager -def with_host(m: CephadmOrchestrator, name, refresh_hosts=True): +def with_host(m: CephadmOrchestrator, name, addr='1.2.3.4', refresh_hosts=True): # type: (CephadmOrchestrator, str) -> None - with mock.patch("cephadm.utils.resolve_ip"): + with mock.patch("cephadm.utils.resolve_ip", return_value=addr): wait(m, m.add_host(HostSpec(hostname=name))) if refresh_hosts: CephadmServe(m)._refresh_hosts_and_daemons() diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 53d1f7283ad..84c4cb5f6a6 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -84,19 +84,19 @@ class TestCephadm(object): def test_host(self, cephadm_module): assert wait(cephadm_module, cephadm_module.get_hosts()) == [] with with_host(cephadm_module, 'test'): - assert wait(cephadm_module, cephadm_module.get_hosts()) == [HostSpec('test', 'test')] + assert wait(cephadm_module, cephadm_module.get_hosts()) == [HostSpec('test', '1.2.3.4')] # Be careful with backward compatibility when changing things here: assert json.loads(cephadm_module.get_store('inventory')) == \ - {"test": {"hostname": "test", "addr": "test", "labels": [], "status": ""}} + {"test": {"hostname": "test", "addr": "1.2.3.4", "labels": [], "status": ""}} - with with_host(cephadm_module, 'second'): + with with_host(cephadm_module, 'second', '1.2.3.5'): assert wait(cephadm_module, cephadm_module.get_hosts()) == [ - HostSpec('test', 'test'), - HostSpec('second', 'second') + HostSpec('test', '1.2.3.4'), + HostSpec('second', '1.2.3.5') ] - assert wait(cephadm_module, cephadm_module.get_hosts()) == [HostSpec('test', 'test')] + assert wait(cephadm_module, cephadm_module.get_hosts()) == [HostSpec('test', '1.2.3.4')] assert wait(cephadm_module, cephadm_module.get_hosts()) == [] @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) @@ -316,7 +316,7 @@ class TestCephadm(object): with mock.patch("cephadm.module.CephadmOrchestrator.mon_command") as _mon_cmd: CephadmServe(cephadm_module)._check_daemons() _mon_cmd.assert_any_call( - {'prefix': 'dashboard set-grafana-api-url', 'value': 'https://test:3000'}, + {'prefix': 'dashboard set-grafana-api-url', 'value': 'https://1.2.3.4:3000'}, None) @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) @@ -987,12 +987,12 @@ class TestCephadm(object): assert "Host 'test' not found" in err out = wait(cephadm_module, cephadm_module.get_hosts())[0].to_json() - assert out == HostSpec('test', 'test', status='Offline').to_json() + assert out == HostSpec('test', '1.2.3.4', status='Offline').to_json() _get_connection.side_effect = None assert CephadmServe(cephadm_module)._check_host('test') is None out = wait(cephadm_module, cephadm_module.get_hosts())[0].to_json() - assert out == HostSpec('test', 'test').to_json() + assert out == HostSpec('test', '1.2.3.4').to_json() @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) def test_dont_touch_offline_or_maintenance_host_daemons(self, cephadm_module): diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index 1a923e774bf..082df699f03 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -329,7 +329,11 @@ class OrchestratorCli(OrchestratorClientMixin, MgrModule, return cast(str, self.get_module_option("orchestrator")) @_cli_write_command('orch host add') - def _add_host(self, hostname: str, addr: Optional[str] = None, labels: Optional[List[str]] = None, maintenance: Optional[bool] = False) -> HandleCommandResult: + def _add_host(self, + hostname: str, + addr: Optional[str] = None, + labels: Optional[List[str]] = None, + maintenance: Optional[bool] = False) -> HandleCommandResult: """Add a host""" _status = 'maintenance' if maintenance else '' -- 2.39.5