From: Adam King Date: Thu, 4 May 2023 16:55:55 +0000 (-0400) Subject: mgr/cephadm: also don't write client files/tuned profiles to maintenance hosts X-Git-Tag: v17.2.7~101^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F53705%2Fhead;p=ceph.git mgr/cephadm: also don't write client files/tuned profiles to maintenance hosts Since they could have been taken offline for maintenance, they should be treated the same as offline hosts in this case and we should avoid trying to write files to them Fixes: https://tracker.ceph.com/issues/59650 Signed-off-by: Adam King (cherry picked from commit 4244d83f70abf4331a055dbeaa6fc2f0d7814bb6) --- diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index c4de9cf39a80..268d21963422 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1146,7 +1146,7 @@ class CephadmServe: client_files: Dict[str, Dict[str, Tuple[int, int, int, bytes, str]]], host: str) -> None: updated_files = False - if host in self.mgr.offline_hosts: + if host in [h.hostname for h in self.mgr.cache.get_unreachable_hosts()]: return old_files = self.mgr.cache.get_host_client_files(host).copy() for path, m in client_files.get(host, {}).items(): diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 468e353b33e6..eb5247e22736 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -1931,6 +1931,34 @@ class TestCephadm(object): assert f1_before_digest != f1_after_digest assert f2_before_digest != f2_after_digest + @mock.patch("cephadm.inventory.HostCache.get_host_client_files") + def test_dont_write_client_files_to_unreachable_hosts(self, _get_client_files, cephadm_module): + cephadm_module.inventory.add_host(HostSpec('host1', '1.2.3.1')) # online + cephadm_module.inventory.add_host(HostSpec('host2', '1.2.3.2')) # maintenance + cephadm_module.inventory.add_host(HostSpec('host3', '1.2.3.3')) # offline + + # mark host2 as maintenance and host3 as offline + cephadm_module.inventory._inventory['host2']['status'] = 'maintenance' + cephadm_module.offline_hosts.add('host3') + + # verify host2 and host3 are correctly marked as unreachable but host1 is not + assert 'host1' not in [h.hostname for h in cephadm_module.cache.get_unreachable_hosts()] + assert 'host2' in [h.hostname for h in cephadm_module.cache.get_unreachable_hosts()] + assert 'host3' in [h.hostname for h in cephadm_module.cache.get_unreachable_hosts()] + + _get_client_files.side_effect = Exception('Called _get_client_files') + + # with the online host, should call _get_client_files which + # we have setup to raise an Exception + with pytest.raises(Exception, match='Called _get_client_files'): + CephadmServe(cephadm_module)._write_client_files({}, 'host1') + + # for the maintenance and offline host, _get_client_files should + # not be called and it should just return immediately with nothing + # having been raised + CephadmServe(cephadm_module)._write_client_files({}, 'host2') + CephadmServe(cephadm_module)._write_client_files({}, 'host3') + def test_etc_ceph_init(self): with with_cephadm_module({'manage_etc_ceph_ceph_conf': True}) as m: assert m.manage_etc_ceph_ceph_conf is True diff --git a/src/pybind/mgr/cephadm/tests/test_tuned_profiles.py b/src/pybind/mgr/cephadm/tests/test_tuned_profiles.py index 38a3a390745e..2e0e9e5a9fe3 100644 --- a/src/pybind/mgr/cephadm/tests/test_tuned_profiles.py +++ b/src/pybind/mgr/cephadm/tests/test_tuned_profiles.py @@ -165,6 +165,32 @@ class TestTunedProfiles: _write_remote_file.assert_called_with( 'a', f'{SYSCTL_DIR}/p2-cephadm-tuned-profile.conf', tp._profile_to_str(self.tspec2).encode('utf-8')) + def test_dont_write_to_unreachable_hosts(self): + profiles = {'p1': self.tspec1, 'p2': self.tspec2, 'p3': self.tspec3} + + # list host "a" and "b" as hosts that exist, "a" will be + # a normal, schedulable host and "b" is considered unreachable + mgr = FakeMgr(['a', 'b'], + ['a'], + ['b'], + profiles) + tp = TunedProfileUtils(mgr) + + assert 'a' not in tp.mgr.cache.last_tuned_profile_update + assert 'b' not in tp.mgr.cache.last_tuned_profile_update + + # with an online host, should proceed as normal. Providing + # no actual profiles here though so the only actual action taken + # is updating the entry in the last_tuned_profile_update dict + tp._write_tuned_profiles('a', {}) + assert 'a' in tp.mgr.cache.last_tuned_profile_update + + # trying to write to an unreachable host should be a no-op + # and return immediately. No entry for 'b' should be added + # to the last_tuned_profile_update dict + tp._write_tuned_profiles('b', {}) + assert 'b' not in tp.mgr.cache.last_tuned_profile_update + def test_store(self): mgr = FakeMgr(['a', 'b', 'c'], ['a', 'b', 'c'], diff --git a/src/pybind/mgr/cephadm/tuned_profiles.py b/src/pybind/mgr/cephadm/tuned_profiles.py index 19d97f42133f..871b8b2e38cd 100644 --- a/src/pybind/mgr/cephadm/tuned_profiles.py +++ b/src/pybind/mgr/cephadm/tuned_profiles.py @@ -67,7 +67,7 @@ class TunedProfileUtils(): SHOULD be on the host. Then if we see any file names that don't match this, but DO include "-cephadm-tuned-profile.conf" (implying they're from us), remove them. """ - if host in self.mgr.offline_hosts: + if host in [h.hostname for h in self.mgr.cache.get_unreachable_hosts()]: return cmd = ['ls', SYSCTL_DIR] found_files = self.mgr.ssh.check_execute_command(host, cmd, log_command=self.mgr.log_refresh_metadata).split('\n') @@ -88,7 +88,7 @@ class TunedProfileUtils(): self.mgr.ssh.check_execute_command(host, ['sysctl', '--system']) def _write_tuned_profiles(self, host: str, profiles: List[Dict[str, str]]) -> None: - if host in self.mgr.offline_hosts: + if host in [h.hostname for h in self.mgr.cache.get_unreachable_hosts()]: return updated = False for p in profiles: