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 <adking@redhat.com>
This commit is contained in:
Adam King 2024-03-13 15:30:25 -04:00
parent 5288e75aab
commit 129c10b4da
4 changed files with 8 additions and 7 deletions

View 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

View File

@ -560,6 +560,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
@ -1371,7 +1372,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')

View File

@ -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',

View 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']