mgr/orch: clean up service_action, remove_service

Standardize on service_name argument that looks like 'mgr', 'mds.fsname',
'mds', or some other daemon name prefix.  Avoid the ambiguously-named
service_type+service_name combinations.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 7e1e99c77b97c29e0ba058a51a621867eb7dc252..35f6a8fbda1848293cc8e9c73087f277a2f025da 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -1450,23 +1450,18 @@ class CephadmOrchestrator(MgrModule, orchestrator.OrchestratorClientMixin):
         self.log.debug('list_daemons result %s' % result)
         return trivial_result(result)
 
-    def service_action(self, action, service_type, service_name):
-        self.log.debug('service_action action %s type %s name %s' % (
-            action, service_type, service_name))
-        if service_name:
-            prefix = service_name + '.'
-        else:
-            prefix = ''
+    def service_action(self, action, service_name):
+        self.log.debug('service_action action %s name %s' % (
+            action, service_name))
         args = []
         for host, dm in self.daemon_cache.data.items():
             for name, d in dm.items():
-                if d.daemon_type == service_type and d.daemon_id.startswith(prefix):
+                if d.matches_service(service_name):
                     args.append((d.daemon_type, d.daemon_id,
                                  d.nodename, action))
         if not args:
             raise orchestrator.OrchestratorError(
-                'Unable to find %s.%s.* daemon(s)' % (
-                    service_type, service_name))
+                'Unable to find %s.%s.* daemon(s)' % (service_name))
         return self._daemon_action(args)
 
     @async_map_completion
@@ -1520,22 +1515,17 @@ class CephadmOrchestrator(MgrModule, orchestrator.OrchestratorClientMixin):
             raise OrchestratorError('Unable to find daemon(s) %s' % (names))
         return self._remove_daemon(args)
 
-    def remove_service(self, service_type, service_name):
-        if service_name:
-            prefix = service_name + '.'
-        else:
-            prefix = ''
+    def remove_service(self, service_name):
         args = []
         for host, dm in self.daemon_cache.data.items():
             for name, d in dm.items():
-                if d.daemon_type == service_type and \
-                   d.daemon_id.startswith(prefix):
+                if d.matches_service(service_name):
                     args.append(
                         ('%s.%s' % (d.daemon_type, d.daemon_id), d.nodename)
                     )
         if not args:
-            raise OrchestratorError('Unable to find daemons in %s.%s* service' % (
-                service_type, prefix))
+            raise OrchestratorError('Unable to find daemons in %s service' % (
+                service_name))
         return self._remove_daemon(args)
 
     def get_inventory(self, node_filter=None, refresh=False):

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 31a87ac7e3b3c798081fbe078c46e896c1324f5f..fba4e485ba7878aff2c253b7e8b78ffc61c56d1e 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -289,7 +289,7 @@ class TestCephadm(object):
         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')
+            c = cephadm_module.remove_service('rgw.myrgw')
             out = wait(cephadm_module, c)
             assert out == ["Removed rgw.myrgw.foobar from host 'test'"]
 

diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py
index 1448cdeecb0fafcfa174d094fd71747e3a76afda..502514e74a71ae19a086b345f7ed81091de8968e 100644 (file)
--- a/src/pybind/mgr/orchestrator/_interface.py
+++ b/src/pybind/mgr/orchestrator/_interface.py
@@ -920,8 +920,8 @@ class Orchestrator(object):
         """
         raise NotImplementedError()
 
-    def remove_service(self, service_type, service_name=None):
-        # type: (str, Optional[str]) -> Completion
+    def remove_service(self, service_name):
+        # type: (str) -> Completion
         """
         Remove a service (a collection of daemons).
 
@@ -929,8 +929,8 @@ class Orchestrator(object):
         """
         raise NotImplementedError()
 
-    def service_action(self, action, service_type, service_name):
-        # type: (str, str, str) -> Completion
+    def service_action(self, action, service_name):
+        # type: (str, str) -> Completion
         """
         Perform an action (start/stop/reload) on a service (i.e., all daemons
         providing the logical service).
@@ -1215,6 +1215,12 @@ class DaemonDescription(object):
     def name(self):
         return '%s.%s' % (self.daemon_type, self.daemon_id)
 
+    def matches_service(self, service_name):
+        # type: (Optional[str]) -> bool
+        if service_name:
+            return self.name().startswith(service_name + '.')
+        return False
+
     def __repr__(self):
         return "<DaemonDescription>({type}.{id})".format(type=self.daemon_type,
                                                          id=self.daemon_id)

diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index 7dd39931973caf5f1e005ef3475f02070afa8e75..0d805ef8d275cce8944817a177a7219cd13e6e20 100644 (file)
--- a/src/pybind/mgr/orchestrator/module.py
+++ b/src/pybind/mgr/orchestrator/module.py
@@ -523,15 +523,10 @@ Usage:
     @_cli_write_command(
         'orch',
         "name=action,type=CephChoices,strings=start|stop|restart|redeploy|reconfig "
-        "name=svc_name,type=CephString",
+        "name=service_name,type=CephString",
         'Start, stop, restart, redeploy, or reconfig an entire service (i.e. all daemons)')
-    def _service_action(self, action, svc_name):
-        if '.' in svc_name:
-            (service_type, service_id) = svc_name.split('.', 1)
-        else:
-            service_type = svc_name;
-            service_id = None
-        completion = self.service_action(action, service_type, service_id)
+    def _service_action(self, action, service_name):
+        completion = self.service_action(action, service_name)
         self._orchestrator_wait([completion])
         raise_if_exception(completion)
         return HandleCommandResult(stdout=completion.result_str())
@@ -566,17 +561,12 @@ Usage:
 
     @_cli_write_command(
         'orch rm',
-        "name=name,type=CephString",
+        "name=service_name,type=CephString",
         'Remove a service')
-    def _service_rm(self, name):
-        if '.' in name:
-            (service_type, service_name) = name.split('.')
-        else:
-            service_type = name;
-            service_name = None
-        if name in ['mon', 'mgr']:
+    def _service_rm(self, service_name):
+        if service_name in ['mon', 'mgr']:
             raise OrchestratorError('The mon and mgr services cannot be removed')
-        completion = self.remove_service(service_type, service_name)
+        completion = self.remove_service(service_name)
         self._orchestrator_wait([completion])
         raise_if_exception(completion)
         return HandleCommandResult(stdout=completion.result_str())

diff --git a/src/pybind/mgr/test_orchestrator/module.py b/src/pybind/mgr/test_orchestrator/module.py
index 4dcd847eb912502dade4c98575184c4780715afc..7db7216f06858bc4cdb012031b265fc2d9b205db 100644 (file)
--- a/src/pybind/mgr/test_orchestrator/module.py
+++ b/src/pybind/mgr/test_orchestrator/module.py
@@ -217,8 +217,7 @@ class TestOrchestrator(MgrModule, orchestrator.Orchestrator):
         assert isinstance(names, list)
 
     @deferred_write("remove_service")
-    def remove_service(self, service_type, service_name):
-        assert isinstance(service_type, str)
+    def remove_service(self, service_name):
         assert isinstance(service_name, str)
 
     @deferred_write("blink_device_light")
@@ -228,7 +227,7 @@ class TestOrchestrator(MgrModule, orchestrator.Orchestrator):
         return ''
 
     @deferred_write("service_action")
-    def service_action(self, action, service_type, service_name=None, service_id=None):
+    def service_action(self, action, service_name):
         pass
 
     @deferred_write("Adding NFS service")