]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/cephadm: refresh public_network for config checks before checking 56492/head
authorAdam King <adking@redhat.com>
Wed, 13 Mar 2024 19:30:25 +0000 (15:30 -0400)
committerAdam King <adking@redhat.com>
Tue, 26 Mar 2024 19:52:22 +0000 (15:52 -0400)
The place it was being run before meant it would only grab the
public_network setting once at startup of the module. This meant
if a user changed the setting, which they are likely to do if they
get the warning, cephadm would ignore the change and continue
reporting that the hosts don't match up with the old setting
for the public_network. This moves the call to refresh the
setting to right before we actually run the checks. It does
mean we'll do the `ceph config dump --format json` call
each serve loop iteration, but I've found that only tends
to take a few milliseconds, which is nothing compared to
the time to refresh other things we check during the serve
loop.

I additionally modified the use of this option to use
the attribute on the mgr, rather than calling
`get_module_option`. This was just to get it more in
line with how we tend to handle other config options

Fixes: https://tracker.ceph.com/issues/64902
Signed-off-by: Adam King <adking@redhat.com>
(cherry picked from commit 129c10b4da518b8843ac0bc5397276eb695d4033)

src/pybind/mgr/cephadm/configchecks.py
src/pybind/mgr/cephadm/module.py
src/pybind/mgr/cephadm/serve.py
src/pybind/mgr/cephadm/tests/test_configchecks.py

index dc7a098277f77bb35ae1048531360b4f0a705c23..bf6b5a34b6a85a2b9d7ff23f3c22058487df369f 100644 (file)
@@ -674,7 +674,7 @@ class CephadmConfigChecks:
             self.host_to_role[hostname] = list(self.mgr.cache.get_daemon_types(hostname))
 
     def run_checks(self) -> None:
-        checks_enabled = self.mgr.get_module_option('config_checks_enabled')
+        checks_enabled = self.mgr.config_checks_enabled
         if checks_enabled is not True:
             return
 
index b10e15290c4fa3f3e62bfdaed7dc79849cdfaa05..3f3653b053b257fedfe87007750592c5d285a328 100644 (file)
@@ -483,6 +483,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
             self.registry_password: Optional[str] = None
             self.registry_insecure: bool = False
             self.use_repo_digest = True
+            self.config_checks_enabled = False
             self.default_registry = ''
             self.autotune_memory_target_ratio = 0.0
             self.autotune_interval = 0
@@ -1270,7 +1271,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
     @orchestrator._cli_read_command('cephadm config-check status')
     def _config_check_status(self) -> HandleCommandResult:
         """Show whether the configuration checker feature is enabled/disabled"""
-        status = self.get_module_option('config_checks_enabled')
+        status = self.config_checks_enabled
         return HandleCommandResult(stdout="Enabled" if status else "Disabled")
 
     @orchestrator._cli_write_command('cephadm config-check enable')
index 0014e5421239a7a4bb69262f0c6cb6727fd7de43..63b8c570069ed0e60d461b1d9ad50211ceec159f 100644 (file)
@@ -64,7 +64,6 @@ class CephadmServe:
         of cephadm. This loop will then attempt to apply this new state.
         """
         self.log.debug("serve starting")
-        self.mgr.config_checker.load_network_config()
 
         while self.mgr.run:
             self.log.debug("serve loop start")
@@ -277,7 +276,9 @@ class CephadmServe:
 
         self.mgr.agent_helpers._update_agent_down_healthcheck(agents_down)
 
-        self.mgr.config_checker.run_checks()
+        if self.mgr.config_checks_enabled:
+            self.mgr.config_checker.load_network_config()
+            self.mgr.config_checker.run_checks()
 
         for k in [
                 'CEPHADM_HOST_CHECK_FAILED',
index 3cae0a27d5b8b8057d529aa360b03edfe437a401..ff1e2186109fae16d189bff2b058ce802cfb34d2 100644 (file)
@@ -238,6 +238,7 @@ class FakeMgr:
         self.default_version = 'quincy'
         self.version_overrides = {}
         self.daemon_to_host = {}
+        self.config_checks_enabled = True
 
         self.cache = HostCache(self)
         self.upgrade = CephadmUpgrade(self)
@@ -623,9 +624,7 @@ class TestConfigCheck:
         assert 'ceph_release' in checker.skipped_checks
 
     def test_skip_when_disabled(self, mgr):
-        mgr.module_option.update({
-            "config_checks_enabled": "false"
-        })
+        mgr.config_checks_enabled = False
         checker = CephadmConfigChecks(mgr)
         checker.cluster_network_list = []
         checker.public_network_list = ['10.9.64.0/24']