From 0f8f085058dc3319c073fb50491d1862e45ba9a2 Mon Sep 17 00:00:00 2001 From: Melissa Li Date: Thu, 5 Aug 2021 14:28:35 -0400 Subject: [PATCH] mgr/cephadm: set health check warning in `_apply_service_config` for invalid config options and failures to set options Fixes: https://tracker.ceph.com/issues/44414 Signed-off-by: Melissa Li --- src/pybind/mgr/cephadm/serve.py | 21 ++++++++++++++------ src/pybind/mgr/cephadm/tests/test_cephadm.py | 13 ++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 282bcaf2c7f12..8b65b96588bbc 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -583,16 +583,20 @@ class CephadmServe: def _apply_service_config(self, spec: ServiceSpec) -> None: if spec.config: section = utils.name_to_config_section(spec.service_name()) + for name in ['CEPHADM_INVALID_CONFIG_OPTION', 'CEPHADM_FAILED_SET_OPTION']: + self.mgr.remove_health_warning(name) + invalid_config_options = [] + options_failed_to_set = [] for k, v in spec.config.items(): try: current = self.mgr.get_foreign_ceph_option(section, k) except KeyError: - self.log.warning( - f'Ignoring invalid {spec.service_name()} config option {k}' - ) + msg = f'Ignoring invalid {spec.service_name()} config option {k}' + self.log.warning(msg) self.mgr.events.for_service( spec, OrchestratorEvent.ERROR, f'Invalid config option {k}' ) + invalid_config_options.append(msg) continue if current != v: self.log.debug(f'setting [{section}] {k} = {v}') @@ -604,9 +608,14 @@ class CephadmServe: 'who': section, }) except MonCommandFailed as e: - self.log.warning( - f'Failed to set {spec.service_name()} option {k}: {e}' - ) + msg = f'Failed to set {spec.service_name()} option {k}: {e}' + self.log.warning(msg) + options_failed_to_set.append(msg) + + if invalid_config_options: + self.mgr.set_health_warning('CEPHADM_INVALID_CONFIG_OPTION', f'Ignoring {len(invalid_config_options)} invalid config option(s)', len(invalid_config_options), invalid_config_options) + if options_failed_to_set: + self.mgr.set_health_warning('CEPHADM_FAILED_SET_OPTION', f'Failed to set {len(options_failed_to_set)} option(s)', len(options_failed_to_set), options_failed_to_set) def _apply_service(self, spec: ServiceSpec) -> bool: """ diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 32c98598e49b3..1cbf009230e21 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -888,6 +888,19 @@ spec: 'entity': entity, }) + @mock.patch("cephadm.module.CephadmOrchestrator.get_foreign_ceph_option") + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") + def test_invalid_config_option_health_warning(self, _run_cephadm, get_foreign_ceph_option, cephadm_module: CephadmOrchestrator): + _run_cephadm.return_value = ('{}', '', 0) + with with_host(cephadm_module, 'test'): + ps = PlacementSpec(hosts=['test:0.0.0.0=a'], count=1) + get_foreign_ceph_option.side_effect = KeyError + CephadmServe(cephadm_module)._apply_service_config(ServiceSpec('mgr', placement=ps, config={'test': 'foo'})) + assert cephadm_module.health_checks.get('CEPHADM_INVALID_CONFIG_OPTION') is not None + assert cephadm_module.health_checks['CEPHADM_INVALID_CONFIG_OPTION']['count'] == 1 + assert 'Ignoring 1 invalid config option(s)' in cephadm_module.health_checks['CEPHADM_INVALID_CONFIG_OPTION']['summary'] + assert 'Ignoring invalid mgr config option test' in cephadm_module.health_checks['CEPHADM_INVALID_CONFIG_OPTION']['detail'] + @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) @mock.patch("cephadm.services.nfs.NFSService.run_grace_tool", mock.MagicMock()) @mock.patch("cephadm.services.nfs.NFSService.purge", mock.MagicMock()) -- 2.39.5