From: Sebastian Wagner <sebastian.wagner@suse.com>
Date: Fri, 17 Apr 2020 10:47:46 +0000 (+0200)
Subject: mgr/cephadm: remove_service: Feedback, if spec not found
X-Git-Tag: v16.1.0~2514^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=33b6c501f12078f4fca3f7efd27684dc9b236258;p=ceph.git

mgr/cephadm: remove_service: Feedback, if spec not found

`ceph orch rm my-unknown-service` still returned
`Removed service my-unknown-service`.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
---

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 130377ecdc57..3afc4e239ca9 100644
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -138,11 +138,13 @@ class SpecStore():
         )
 
     def rm(self, service_name):
-        # type: (str) -> None
-        if service_name in self.specs:
+        # type: (str) -> bool
+        found = service_name in self.specs
+        if found:
             del self.specs[service_name]
             del self.spec_created[service_name]
             self.mgr.set_store(SPEC_STORE_PREFIX + service_name, None)
+        return found
 
     def find(self, service_name: Optional[str] = None) -> List[ServiceSpec]:
         specs = []
@@ -2016,9 +2018,13 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule):
     @trivial_completion
     def remove_service(self, service_name):
         self.log.info('Remove service %s' % service_name)
-        self.spec_store.rm(service_name)
-        self._kick_serve_loop()
-        return ['Removed service %s' % service_name]
+        found = self.spec_store.rm(service_name)
+        if found:
+            self._kick_serve_loop()
+            return ['Removed service %s' % service_name]
+        else:
+            # must be idempotent: still a success.
+            return [f'Failed to remove service. <{service_name}> was not found.']
 
     @trivial_completion
     def get_inventory(self, host_filter=None, refresh=False):
@@ -3682,4 +3688,3 @@ class HostAssignment(object):
         logger.debug('Combine hosts with existing daemons %s + new hosts %s' % (
             existing, chosen))
         return existing + chosen
-
diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 21cf5afe4f53..cc619b2c4c3f 100644
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -390,26 +390,6 @@ class TestCephadm(object):
             out = wait(cephadm_module, c)
             assert out == ["Removed rgw.myrgw.myhost.myid from host 'test'"]
 
-    @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm", _run_cephadm(
-        json.dumps([
-            dict(
-                name='rgw.myrgw.foobar',
-                style='cephadm',
-                fsid='fsid',
-                container_id='container_id',
-                version='version',
-                state='running',
-            )
-        ])
-    ))
-    def test_remove_service(self, cephadm_module):
-        with self._with_host(cephadm_module, 'test'):
-            c = cephadm_module.list_daemons(refresh=True)
-            wait(cephadm_module, c)
-            c = cephadm_module.remove_service('rgw.myrgw')
-            out = wait(cephadm_module, c)
-            assert out == ["Removed service rgw.myrgw"]
-
     @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm", _run_cephadm('{}'))
     def test_rbd_mirror(self, cephadm_module):
         with self._with_host(cephadm_module, 'test'):