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 <adking@redhat.com>

diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index b4467947ae05280c9de2983cb5fe014039aae8c3..968f3d4871f16c3a48937c1408a2f433a2faca74 100644 (file)
--- 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 364d4db692f525e5592b3dcf53cb040d14fb0db5..b41c296a0fa901d8cd1141cb6976b38490b03201 100644 (file)
--- 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 38a3a390745e1c73d94f2d1619fdb14504d4a00f..2e0e9e5a9fe3d72c17ef34b6a65b7aadd6eef0fd 100644 (file)
--- 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 19d97f42133f0b9d8e5c723c73fdef8b7a22bff5..871b8b2e38cdd2a38337b075bc4c68c4f2a45e57 100644 (file)
--- 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: