From f2b400ca2117f418fc5c82ade99c589e6fa93b4e Mon Sep 17 00:00:00 2001 From: Adam King Date: Wed, 13 Mar 2024 15:30:25 -0400 Subject: [PATCH] mgr/cephadm: refresh public_network for config checks before checking 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 (cherry picked from commit 129c10b4da518b8843ac0bc5397276eb695d4033) --- src/pybind/mgr/cephadm/configchecks.py | 2 +- src/pybind/mgr/cephadm/module.py | 3 ++- src/pybind/mgr/cephadm/serve.py | 5 +++-- src/pybind/mgr/cephadm/tests/test_configchecks.py | 5 ++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/pybind/mgr/cephadm/configchecks.py b/src/pybind/mgr/cephadm/configchecks.py index b9dcb18f478..9077a1d6601 100644 --- a/src/pybind/mgr/cephadm/configchecks.py +++ b/src/pybind/mgr/cephadm/configchecks.py @@ -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 diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index dd5ec0cd752..42138d71b60 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -551,6 +551,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 @@ -1361,7 +1362,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') diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index dbbdb68d3c4..7bfb3f633ee 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -67,7 +67,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") @@ -322,7 +321,9 @@ class CephadmServe: self.mgr.agent_helpers._update_agent_down_healthcheck(agents_down) self.mgr.http_server.config_update() - 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', diff --git a/src/pybind/mgr/cephadm/tests/test_configchecks.py b/src/pybind/mgr/cephadm/tests/test_configchecks.py index 3cae0a27d5b..ff1e2186109 100644 --- a/src/pybind/mgr/cephadm/tests/test_configchecks.py +++ b/src/pybind/mgr/cephadm/tests/test_configchecks.py @@ -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'] -- 2.47.3