mgr/cephadm: remove spec.config options when a service is removed

For each key in spec.config, validate the option name and run
config rm on the service's config section. Call this from
remove_service() before spec_store.rm() schedules daemon removal.

Fixes: https://tracker.ceph.com/issues/77891
Signed-off-by: Shweta Bhosale <Shweta.Bhosale1@ibm.com>

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 24c6483007ad478af96cb334e0aad0237093ec94..bb4c0f7a46fc816d1a042b5da04943d2b4f6ecd5 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -3246,6 +3246,8 @@ Then run the following:
                     f'will remain, --force to proceed anyway\n{msg}'
                 )
 
+        spec = self.spec_store[service_name].spec
+        CephadmServe(self)._remove_service_config(spec)
         found = self.spec_store.rm(service_name, force_delete_data)
         if found and service_name.startswith('osd.'):
             self.spec_store.finally_rm(service_name)

diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index 6849c586c33a411c0b2ba5bcdf23dac631ecc589..af6784a82673f9c596cdc4719bae8ec7e32ad04b 100644 (file)
--- a/src/pybind/mgr/cephadm/serve.py
+++ b/src/pybind/mgr/cephadm/serve.py
@@ -709,6 +709,30 @@ class CephadmServe:
                 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 _remove_service_config(self, spec: ServiceSpec) -> None:
+        if not spec.config:
+            return
+        section = utils.name_to_config_section(spec.service_name())
+        for k in spec.config.keys():
+            try:
+                self.mgr.get_foreign_ceph_option(section, k)
+            except KeyError:
+                self.log.debug(
+                    'Ignoring invalid %s config option %s on removal', spec.service_name(), k
+                )
+                continue
+            self.log.debug(
+                'Removing config keys for %s, section: %s, key: %s', spec.service_name(), section, k
+            )
+            try:
+                self.mgr.check_mon_command({
+                    'prefix': 'config rm',
+                    'name': k,
+                    'who': section,
+                })
+            except MonCommandFailed as e:
+                self.log.warning('Failed to remove %s option %s: %s', spec.service_name(), k, e)
+
     def _update_rgw_endpoints(self, rgw_spec: RGWSpec) -> None:
 
         if not rgw_spec.update_endpoints or rgw_spec.rgw_realm_token is None:

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 79fed21c6c4b084a6c3b76ed1ad4eb572404c025..a83e72804c993bfcc366128efd74e75ee9185fba 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -1815,6 +1815,46 @@ class TestCephadm(object):
             assert 'Ignoring invalid mgr config option test' in cephadm_module.health_checks[
                 'CEPHADM_INVALID_CONFIG_OPTION']['detail']
 
+    @mock.patch("cephadm.module.CephadmOrchestrator.check_mon_command")
+    @mock.patch("cephadm.module.CephadmOrchestrator.get_foreign_ceph_option")
+    def test_remove_service_config(self, get_foreign_ceph_option, check_mon_command, cephadm_module: CephadmOrchestrator):
+        get_foreign_ceph_option.return_value = 'foo'
+        ps = PlacementSpec(hosts=['test'], count=1)
+        spec = ServiceSpec('nfs', service_id='a', placement=ps, config={'rados_replica_read_policy': 'localize'})
+        CephadmServe(cephadm_module)._remove_service_config(spec)
+        check_mon_command.assert_called_once_with({
+            'prefix': 'config rm',
+            'name': 'rados_replica_read_policy',
+            'who': 'client.nfs.a',
+        })
+
+    @mock.patch("cephadm.module.CephadmOrchestrator.check_mon_command")
+    @mock.patch("cephadm.module.CephadmOrchestrator.get_foreign_ceph_option")
+    def test_remove_service_config_skips_invalid(self, get_foreign_ceph_option, check_mon_command, cephadm_module: CephadmOrchestrator):
+        get_foreign_ceph_option.side_effect = KeyError
+        ps = PlacementSpec(hosts=['test'], count=1)
+        spec = ServiceSpec('nfs', placement=ps, config={'invalid_key': 'val'})
+        CephadmServe(cephadm_module)._remove_service_config(spec)
+        check_mon_command.assert_not_called()
+
+    @mock.patch("cephadm.module.CephadmOrchestrator.check_mon_command")
+    @mock.patch("cephadm.module.CephadmOrchestrator.get_foreign_ceph_option")
+    @mock.patch("cephadm.module.CephadmOrchestrator._kick_serve_loop")
+    def test_remove_service_cleans_spec_config(
+        self, _kick_serve_loop, get_foreign_ceph_option, check_mon_command, cephadm_module: CephadmOrchestrator
+    ):
+        get_foreign_ceph_option.return_value = 'foo'
+        ps = PlacementSpec(hosts=['test'], count=1)
+        spec = ServiceSpec('rgw', service_id='foo', placement=ps, config={'rgw_frontends': 'beast port=8080'})
+        cephadm_module.spec_store.save(spec)
+        cephadm_module.remove_service('rgw.foo')
+        check_mon_command.assert_called_once_with({
+            'prefix': 'config rm',
+            'name': 'rgw_frontends',
+            'who': 'client.rgw.foo',
+        })
+        assert 'rgw.foo' in cephadm_module.spec_store.spec_deleted
+
     @mock.patch("cephadm.module.CephadmOrchestrator.get_foreign_ceph_option")
     @mock.patch("cephadm.serve.CephadmServe._run_cephadm")
     @mock.patch("cephadm.module.CephadmOrchestrator.set_store")