mgr/cephadm: set service name for DaemonDescription object used during daemon removal

adk3798 · adk3798 · commit d8dae2410401 · 2025-01-13T15:09:35.000-05:00
What this is specifically fixing is that the nvmeof post_remove function needs the service spec of the daemon's service to get the pool and group tied to the nvmeof daemon. We have been using the DaemonDescription "service_name" property to get the service name in order to get the spec. This works in a regular deployment. However, it is possible to make a placement like placement: hosts: - vm-00=nvmeof.a - vm-01=nvmeof.b and one of the nvmeof CI tests was doing so, which is why we saw this. That will cause the nvmeof daemon names to be nvmeof.nvmeof.a and nvmeof.nvmeof.b and not include the service name at all. In this case, the service_name property on the DaemonDescription class will end up getting service names nvmeof.nvmeof.a and nvmeof.nvmeof.b respectively from the nvmeof daemons, which will cause us to fail to find the spec in post_remove. This change makes it so we manually set the service name for the DaemonDescription object that gets passed to post_remove based on the service name of the daemon object we get from the host cache, which will still have the correct service name even if the daemon has a custom name. Then the nvmeof post_remove function will get the correct service name and be able to find the spec. Additionally, we now take are technically taking the daemon type and id from the DaemonDescription in our HostCache as well, but this is mostly just for consistency and should have no real impact. Fixes: https://tracker.ceph.com/issues/68962 Signed-off-by: Adam King <adking@redhat.com>
diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
@@ -1551,18 +1551,21 @@ def _remove_daemon(self, name: str, host: str, no_post_remove: bool = False) ->
         """
         Remove a daemon
         """
-        (daemon_type, daemon_id) = name.split('.', 1)
+        dd = self.mgr.cache.get_daemon(name)
+        daemon_type = dd.daemon_type
+        daemon_id = dd.daemon_id
+        assert (daemon_type is not None and daemon_id is not None)
         daemon = orchestrator.DaemonDescription(
             daemon_type=daemon_type,
             daemon_id=daemon_id,
+            service_name=dd.service_name(),
             hostname=host)
 
         with set_exception_subject('service', daemon.service_id(), overwrite=True):
 
             self.mgr.cephadm_services[daemon_type_to_service(daemon_type)].pre_remove(daemon)
             # NOTE: we are passing the 'force' flag here, which means
             # we can delete a mon instances data.
-            dd = self.mgr.cache.get_daemon(daemon.daemon_name)
             if dd.ports:
                 args = ['--name', name, '--force', '--tcp-ports', ' '.join(map(str, dd.ports))]
             else:
