From 16011f5cbf427b84d4de7675b608491d465867f4 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 17 Feb 2020 07:50:50 -0600 Subject: [PATCH] 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 --- src/pybind/mgr/cephadm/module.py | 28 +++++++------------- src/pybind/mgr/cephadm/tests/test_cephadm.py | 2 +- src/pybind/mgr/orchestrator/_interface.py | 14 +++++++--- src/pybind/mgr/orchestrator/module.py | 24 +++++------------ src/pybind/mgr/test_orchestrator/module.py | 5 ++-- 5 files changed, 29 insertions(+), 44 deletions(-) diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 7e1e99c77b9..35f6a8fbda1 100644 --- 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 31a87ac7e3b..fba4e485ba7 100644 --- 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 1448cdeecb0..502514e74a7 100644 --- 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 "({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 7dd39931973..0d805ef8d27 100644 --- 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 4dcd847eb91..7db7216f068 100644 --- 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") -- 2.39.5