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>
(cherry picked from commit 4244d83f70abf4331a055dbeaa6fc2f0d7814bb6)

diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index c4de9cf39a80c40265c0851d1218fb934317e4a2..268d21963422cb07431c9b01c2c8002be3be1307 100644 (file)
--- 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 468e353b33e67d19401c275d0be279c3671434c4..eb5247e22736849152b723d9967f06928a35c7d2 100644 (file)
--- 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 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: