From: Sebastian Wagner Date: Fri, 29 Jan 2021 16:18:50 +0000 (+0100) Subject: mgr/cephadm: mandate the existence of Services for Daemons X-Git-Tag: v16.2.0~178^2~16 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9e4d8b39c7ba2079bfeba64b199f4a8527d60d64;p=ceph.git mgr/cephadm: mandate the existence of Services for Daemons Signed-off-by: Sebastian Wagner (cherry picked from commit 745aa00909fe072f127ede7a5da77a489dc067ae) --- diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index b3b1e5e434e..74414396574 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1693,6 +1693,10 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, def remove_service(self, service_name: str) -> str: self.log.info('Remove service %s' % service_name) self._trigger_preview_refresh(service_name=service_name) + if service_name in self.spec_store: + if self.spec_store[service_name].spec.service_type in ('mon', 'mgr'): + return f'Unable to remove {service_name} service.\n' \ + f'Note, you might want to mark the {service_name} service as "unmanaged"' found = self.spec_store.rm(service_name) if found: self._kick_serve_loop() @@ -1894,6 +1898,11 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, Add (and place) a daemon. Require explicit host placement. Do not schedule, and do not apply the related scheduling limitations. """ + if spec.service_name() not in self.spec_store: + raise OrchestratorError('Unable to add a Daemon without Service.\n' + 'Please use `ceph orch apply ...` to create a Service.\n' + 'Note, you might want to create the service with "unmanaged=true"') + self.log.debug('_add_daemon %s spec %s' % (daemon_type, spec.placement)) if not spec.placement.hosts: raise OrchestratorError('must specify host(s) to deploy on') diff --git a/src/pybind/mgr/cephadm/tests/fixtures.py b/src/pybind/mgr/cephadm/tests/fixtures.py index a56383fd656..8ded190f311 100644 --- a/src/pybind/mgr/cephadm/tests/fixtures.py +++ b/src/pybind/mgr/cephadm/tests/fixtures.py @@ -118,11 +118,16 @@ def assert_rm_service(cephadm: CephadmOrchestrator, srv_name): @contextmanager -def with_service(cephadm_module: CephadmOrchestrator, spec: ServiceSpec, meth, host: str) -> Iterator[List[str]]: - if spec.placement.is_empty(): +def with_service(cephadm_module: CephadmOrchestrator, spec: ServiceSpec, meth=None, host: str = '') -> Iterator[List[str]]: + if spec.placement.is_empty() and host: spec.placement = PlacementSpec(hosts=[host], count=1) - c = meth(cephadm_module, spec) - assert wait(cephadm_module, c) == f'Scheduled {spec.service_name()} update...' + if meth is not None: + c = meth(cephadm_module, spec) + assert wait(cephadm_module, c) == f'Scheduled {spec.service_name()} update...' + else: + c = cephadm_module.apply([spec]) + assert wait(cephadm_module, c) == [f'Scheduled {spec.service_name()} update...'] + specs = [d.spec for d in wait(cephadm_module, cephadm_module.describe_service())] assert spec in specs @@ -130,7 +135,8 @@ def with_service(cephadm_module: CephadmOrchestrator, spec: ServiceSpec, meth, h dds = wait(cephadm_module, cephadm_module.list_daemons()) own_dds = [dd for dd in dds if dd.service_name() == spec.service_name()] - assert own_dds + if host: + assert own_dds yield [dd.name() for dd in own_dds] diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index c26e6dd7283..1a7edd5a8b4 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -24,7 +24,7 @@ from orchestrator import DaemonDescription, InventoryHost, \ HostSpec, OrchestratorError from tests import mock from .fixtures import wait, _run_cephadm, match_glob, with_host, \ - with_cephadm_module, with_service, assert_rm_service, _deploy_cephadm_binary + with_cephadm_module, with_service, _deploy_cephadm_binary from cephadm.module import CephadmOrchestrator """ @@ -106,8 +106,8 @@ class TestCephadm(object): with with_host(cephadm_module, 'test'): c = cephadm_module.list_daemons(refresh=True) assert wait(cephadm_module, c) == [] - - with with_daemon(cephadm_module, ServiceSpec('mds', 'name'), CephadmOrchestrator.add_mds, 'test'): + with with_service(cephadm_module, ServiceSpec('mds', 'name', unmanaged=True)) as _, \ + with_daemon(cephadm_module, ServiceSpec('mds', 'name'), CephadmOrchestrator.add_mds, 'test') as _: c = cephadm_module.list_daemons() @@ -132,11 +132,11 @@ class TestCephadm(object): out = [dict(o.to_json()) for o in wait(cephadm_module, c)] expected = [ { - 'placement': {'hosts': ['test']}, + 'placement': {'count': 2}, 'service_id': 'name', 'service_name': 'mds.name', 'service_type': 'mds', - 'status': {'running': 1, 'size': 0}, + 'status': {'created': mock.ANY, 'running': 1, 'size': 2}, 'unmanaged': True }, { @@ -189,7 +189,8 @@ class TestCephadm(object): def test_daemon_action(self, cephadm_module: CephadmOrchestrator): cephadm_module.service_cache_timeout = 10 with with_host(cephadm_module, 'test'): - with with_daemon(cephadm_module, RGWSpec(service_id='myrgw.foobar'), CephadmOrchestrator.add_rgw, 'test') as daemon_id: + with with_service(cephadm_module, RGWSpec(service_id='myrgw.foobar', unmanaged=True)) as _, \ + with_daemon(cephadm_module, RGWSpec(service_id='myrgw.foobar'), CephadmOrchestrator.add_rgw, 'test') as daemon_id: c = cephadm_module.daemon_action('redeploy', 'rgw.' + daemon_id) assert wait(cephadm_module, @@ -214,7 +215,8 @@ class TestCephadm(object): def test_daemon_action_fail(self, cephadm_module: CephadmOrchestrator): cephadm_module.service_cache_timeout = 10 with with_host(cephadm_module, 'test'): - with with_daemon(cephadm_module, RGWSpec(service_id='myrgw.foobar'), CephadmOrchestrator.add_rgw, 'test') as daemon_id: + with with_service(cephadm_module, RGWSpec(service_id='myrgw.foobar', unmanaged=True)) as _, \ + with_daemon(cephadm_module, RGWSpec(service_id='myrgw.foobar'), CephadmOrchestrator.add_rgw, 'test') as daemon_id: with mock.patch('ceph_module.BaseMgrModule._ceph_send_command') as _ceph_send_command: _ceph_send_command.side_effect = Exception("myerror") @@ -318,14 +320,15 @@ class TestCephadm(object): @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) def test_mon_add(self, cephadm_module): with with_host(cephadm_module, 'test'): - ps = PlacementSpec(hosts=['test:0.0.0.0=a'], count=1) - c = cephadm_module.add_mon(ServiceSpec('mon', placement=ps)) - assert wait(cephadm_module, c) == ["Deployed mon.a on host 'test'"] - - with pytest.raises(OrchestratorError, match="Must set public_network config option or specify a CIDR network,"): - ps = PlacementSpec(hosts=['test'], count=1) + with with_service(cephadm_module, ServiceSpec(service_type='mon', unmanaged=True)): + ps = PlacementSpec(hosts=['test:0.0.0.0=a'], count=1) c = cephadm_module.add_mon(ServiceSpec('mon', placement=ps)) - wait(cephadm_module, c) + assert wait(cephadm_module, c) == ["Deployed mon.a on host 'test'"] + + with pytest.raises(OrchestratorError, match="Must set public_network config option or specify a CIDR network,"): + ps = PlacementSpec(hosts=['test'], count=1) + c = cephadm_module.add_mon(ServiceSpec('mon', placement=ps)) + wait(cephadm_module, c) @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) def test_mgr_update(self, cephadm_module): @@ -527,19 +530,20 @@ class TestCephadm(object): def test_rgw_update(self, cephadm_module): with with_host(cephadm_module, 'host1'): with with_host(cephadm_module, 'host2'): - ps = PlacementSpec(hosts=['host1'], count=1) - c = cephadm_module.add_rgw( - RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) - [out] = wait(cephadm_module, c) - match_glob(out, "Deployed rgw.realm.zone1.host1.* on host 'host1'") + with with_service(cephadm_module, RGWSpec(rgw_realm='realm', rgw_zone='zone1', unmanaged=True)): + ps = PlacementSpec(hosts=['host1'], count=1) + c = cephadm_module.add_rgw( + RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) + [out] = wait(cephadm_module, c) + match_glob(out, "Deployed rgw.realm.zone1.host1.* on host 'host1'") - ps = PlacementSpec(hosts=['host1', 'host2'], count=2) - r = CephadmServe(cephadm_module)._apply_service( - RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) - assert r + ps = PlacementSpec(hosts=['host1', 'host2'], count=2) + r = CephadmServe(cephadm_module)._apply_service( + RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) + assert r - assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host1') - assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host2') + assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host1') + assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host2') @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm( json.dumps([ @@ -580,9 +584,12 @@ class TestCephadm(object): @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) @mock.patch("cephadm.services.cephadmservice.RgwService.create_realm_zonegroup_zone", lambda _, __, ___: None) def test_daemon_add(self, spec: ServiceSpec, meth, cephadm_module): + unmanaged_spec = ServiceSpec.from_json(spec.to_json()) + unmanaged_spec.unmanaged = True with with_host(cephadm_module, 'test'): - with with_daemon(cephadm_module, spec, meth, 'test'): - pass + with with_service(cephadm_module, unmanaged_spec): + with with_daemon(cephadm_module, spec, meth, 'test'): + pass @mock.patch("cephadm.serve.CephadmServe._run_cephadm") def test_daemon_add_fail(self, _run_cephadm, cephadm_module): @@ -590,15 +597,17 @@ class TestCephadm(object): with with_host(cephadm_module, 'test'): spec = ServiceSpec( service_type='mgr', - placement=PlacementSpec(hosts=[HostPlacementSpec('test', '', 'x')], count=1) + placement=PlacementSpec(hosts=[HostPlacementSpec('test', '', 'x')], count=1), + unmanaged=True ) - _run_cephadm.side_effect = OrchestratorError('fail') - with pytest.raises(OrchestratorError): - wait(cephadm_module, cephadm_module.add_mgr(spec)) - cephadm_module.assert_issued_mon_command({ - 'prefix': 'auth rm', - 'entity': 'mgr.x', - }) + with with_service(cephadm_module, spec): + _run_cephadm.side_effect = OrchestratorError('fail') + with pytest.raises(OrchestratorError): + wait(cephadm_module, cephadm_module.add_mgr(spec)) + cephadm_module.assert_issued_mon_command({ + 'prefix': 'auth rm', + 'entity': 'mgr.x', + }) @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) @mock.patch("cephadm.module.CephadmOrchestrator.rados", mock.MagicMock()) @@ -610,16 +619,14 @@ class TestCephadm(object): pool='pool', namespace='namespace', placement=ps) - c = cephadm_module.add_nfs(spec) - [out] = wait(cephadm_module, c) - match_glob(out, "Deployed nfs.name.* on host 'test'") - - assert_rm_daemon(cephadm_module, 'nfs.name.test', 'test') + unmanaged_spec = ServiceSpec.from_json(spec.to_json()) + unmanaged_spec.unmanaged = True + with with_service(cephadm_module, unmanaged_spec): + c = cephadm_module.add_nfs(spec) + [out] = wait(cephadm_module, c) + match_glob(out, "Deployed nfs.name.* on host 'test'") - # Hack. We never created the service, but we now need to remove it. - # this is in contrast to the other services, which don't create this service - # automatically. - assert_rm_service(cephadm_module, 'nfs.name') + assert_rm_daemon(cephadm_module, 'nfs.name.test', 'test') @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) @mock.patch("cephadm.module.CephadmOrchestrator.rados", mock.MagicMock()) @@ -632,16 +639,15 @@ class TestCephadm(object): api_user='user', api_password='password', placement=ps) - c = cephadm_module.add_iscsi(spec) - [out] = wait(cephadm_module, c) - match_glob(out, "Deployed iscsi.name.* on host 'test'") + unmanaged_spec = ServiceSpec.from_json(spec.to_json()) + unmanaged_spec.unmanaged = True + with with_service(cephadm_module, unmanaged_spec): - assert_rm_daemon(cephadm_module, 'iscsi.name.test', 'test') + c = cephadm_module.add_iscsi(spec) + [out] = wait(cephadm_module, c) + match_glob(out, "Deployed iscsi.name.* on host 'test'") - # Hack. We never created the service, but we now need to remove it. - # this is in contrast to the other services, which don't create this service - # automatically. - assert_rm_service(cephadm_module, 'iscsi.name') + assert_rm_daemon(cephadm_module, 'iscsi.name.test', 'test') @pytest.mark.parametrize( "on_bool",