From 42a46833c4aab705a4b9eeca0da9a30cb5122970 Mon Sep 17 00:00:00 2001 From: Adam King Date: Thu, 4 May 2023 12:55:55 -0400 Subject: [PATCH] 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) --- src/pybind/mgr/cephadm/serve.py | 2 +- src/pybind/mgr/cephadm/tests/test_cephadm.py | 28 +++++++++++++++++++ .../mgr/cephadm/tests/test_tuned_profiles.py | 26 +++++++++++++++++ src/pybind/mgr/cephadm/tuned_profiles.py | 4 +-- 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 877a00cf714b9..c2e16f384be98 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1183,7 +1183,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 364d4db692f52..b41c296a0fa90 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -1873,6 +1873,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 38a3a390745e1..2e0e9e5a9fe3d 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 19d97f42133f0..871b8b2e38cdd 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: -- 2.39.5