From 903f4590c3dcc21c93460925ff8ac7526703166d Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sat, 14 Mar 2020 09:10:13 -0500 Subject: [PATCH] mgr/orch,cephadm: elevate --force flag for daemons to interface Apply this check in the generic orchestrator code so that it applies consistently to all implementations. Signed-off-by: Sage Weil --- qa/tasks/mgr/test_orchestrator_cli.py | 2 +- src/pybind/mgr/cephadm/module.py | 24 ++++++++------------ src/pybind/mgr/cephadm/osd.py | 2 +- src/pybind/mgr/cephadm/tests/test_cephadm.py | 4 ++-- src/pybind/mgr/orchestrator/_interface.py | 4 ++-- src/pybind/mgr/orchestrator/module.py | 5 +++- src/pybind/mgr/rook/module.py | 2 +- src/pybind/mgr/test_orchestrator/module.py | 2 +- 8 files changed, 21 insertions(+), 24 deletions(-) diff --git a/qa/tasks/mgr/test_orchestrator_cli.py b/qa/tasks/mgr/test_orchestrator_cli.py index 9753e18f67a04..5232af29905e6 100644 --- a/qa/tasks/mgr/test_orchestrator_cli.py +++ b/qa/tasks/mgr/test_orchestrator_cli.py @@ -129,7 +129,7 @@ data_devices: self._orch_cmd('daemon', 'add', "nfs", "service_name", "pool") def test_osd_rm(self): - self._orch_cmd('daemon', "rm", "osd.0") + self._orch_cmd('daemon', "rm", "osd.0", '--force') def test_mds_rm(self): self._orch_cmd("daemon", "rm", "mds.fsname") diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index e4bbcaf78aa91..378383951c024 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1858,13 +1858,13 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): ','.join(['%s.%s' % (a[0], a[1]) for a in args]))) return self._daemon_actions(args) - def remove_daemons(self, names, force): - # type: (List[str], bool) -> orchestrator.Completion + def remove_daemons(self, names): + # type: (List[str]) -> orchestrator.Completion args = [] for host, dm in self.cache.daemons.items(): for name in names: if name in dm: - args.append((name, host, force)) + args.append((name, host)) if not args: raise OrchestratorError('Unable to find daemon(s) %s' % (names)) self.log.info('Remove daemons %s' % [a[0] for a in args]) @@ -2167,10 +2167,10 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): 'Reconfigured' if reconfig else 'Deployed', name, host) @async_map_completion - def _remove_daemons(self, name, host, force=False): - return self._remove_daemon(name, host, force) + def _remove_daemons(self, name, host): + return self._remove_daemon(name, host) - def _remove_daemon(self, name, host, force=False): + def _remove_daemon(self, name, host): """ Remove a daemon """ @@ -2178,10 +2178,6 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): if daemon_type == 'mon': self._check_safe_to_destroy_mon(daemon_id) - # fail early, before we update the monmap - if not force: - raise OrchestratorError('Must pass --force to remove a monitor and DELETE potentially PRECIOUS CLUSTER DATA') - # remove mon from quorum before we destroy the daemon self.log.info('Removing monitor %s from monmap...' % name) ret, out, err = self.mon_command({ @@ -2192,9 +2188,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): raise OrchestratorError('failed to remove mon %s from monmap' % ( name)) - args = ['--name', name] - if force: - args.extend(['--force']) + args = ['--name', name, '--force'] self.log.info('Removing daemon %s from %s' % (name, host)) out, err, code = self._run_cephadm( host, name, 'rm-daemon', args) @@ -2284,7 +2278,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): if d.hostname not in target_hosts: # NOTE: we are passing the 'force' flag here, which means # we can delete a mon instances data. - self._remove_daemon(d.name(), d.hostname, True) + self._remove_daemon(d.name(), d.hostname) r = True return r @@ -2320,7 +2314,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): # (mon and mgr specs should always exist; osds aren't matched # to a service spec) self.log.info('Removing orphan daemon %s...' % dd.name()) - self._remove_daemon(dd.name(), dd.hostname, True) + self._remove_daemon(dd.name(), dd.hostname) # dependencies? if dd.daemon_type == 'grafana': diff --git a/src/pybind/mgr/cephadm/osd.py b/src/pybind/mgr/cephadm/osd.py index 4e2181d201aef..2ead0cb8077f6 100644 --- a/src/pybind/mgr/cephadm/osd.py +++ b/src/pybind/mgr/cephadm/osd.py @@ -97,7 +97,7 @@ class RemoveUtil(object): # also remove it from the remove_osd list and set a health_check warning? raise orchestrator.OrchestratorError(f"Could not purge OSD <{osd.osd_id}>") - self.mgr._remove_daemon(osd.fullname, osd.nodename, True) + self.mgr._remove_daemon(osd.fullname, osd.nodename) logger.info(f"Successfully removed OSD <{osd.osd_id}> on {osd.nodename}") logger.debug(f"Removing {osd.osd_id} from the queue.") self.to_remove_osds.remove(osd) diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 70dd171bcce9a..09eb4606f48cf 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -165,7 +165,7 @@ class TestCephadm(object): c = cephadm_module.list_daemons(refresh=True) wait(cephadm_module, c) - c = cephadm_module.remove_daemons(['osd.0'], False) + c = cephadm_module.remove_daemons(['osd.0']) out = wait(cephadm_module, c) assert out == ["Removed osd.0 from host 'test'"] @@ -226,7 +226,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_daemons(['rgw.myrgw.myhost.myid'], False) + c = cephadm_module.remove_daemons(['rgw.myrgw.myhost.myid']) out = wait(cephadm_module, c) assert out == ["Removed rgw.myrgw.myhost.myid from host 'test'"] diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py index 5a83120b065f4..558f87b0b6831 100644 --- a/src/pybind/mgr/orchestrator/_interface.py +++ b/src/pybind/mgr/orchestrator/_interface.py @@ -850,8 +850,8 @@ class Orchestrator(object): """ raise NotImplementedError() - def remove_daemons(self, names, force): - # type: (List[str], bool) -> Completion + def remove_daemons(self, names): + # type: (List[str]) -> Completion """ Remove specific daemon(s). diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index 37cbaf6d31cd7..8461d5e42a132 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -733,7 +733,10 @@ Usage: for name in names: if '.' not in name: raise OrchestratorError('%s is not a valid daemon name' % name) - completion = self.remove_daemons(names, force) + (daemon_type) = name.split('.')[0] + if not force and daemon_type in ['osd', 'mon', 'prometheus']: + raise OrchestratorError('must pass --force to REMOVE daemon with potentially PRECIOUS DATA for %s' % name) + completion = self.remove_daemons(names) self._orchestrator_wait([completion]) raise_if_exception(completion) return HandleCommandResult(stdout=completion.result_str()) diff --git a/src/pybind/mgr/rook/module.py b/src/pybind/mgr/rook/module.py index 1a2f7ad1c905e..6036d25bb68f7 100644 --- a/src/pybind/mgr/rook/module.py +++ b/src/pybind/mgr/rook/module.py @@ -434,7 +434,7 @@ class RookOrchestrator(MgrModule, orchestrator.Orchestrator): mgr=self ) - def remove_daemons(self, names, force): + def remove_daemons(self, names): return write_completion( lambda: self.rook_cluster.remove_pods(names), "Removing daemons {}".format(','.join(names)), diff --git a/src/pybind/mgr/test_orchestrator/module.py b/src/pybind/mgr/test_orchestrator/module.py index 5f22e30ce0d7a..2d8d859c65753 100644 --- a/src/pybind/mgr/test_orchestrator/module.py +++ b/src/pybind/mgr/test_orchestrator/module.py @@ -275,7 +275,7 @@ class TestOrchestrator(MgrModule, orchestrator.Orchestrator): ) @deferred_write("remove_daemons") - def remove_daemons(self, names, force): + def remove_daemons(self, names): assert isinstance(names, list) @deferred_write("remove_service") -- 2.39.5