From 2c7f899124207d0d4866e16d9eb5609777ea22e8 Mon Sep 17 00:00:00 2001 From: Adam King Date: Mon, 8 Feb 2021 08:43:59 -0500 Subject: [PATCH] mgr/cephadm: make generate_config internal makes generate_config into an internal function for prepare_create Fixes: https://tracker.ceph.com/issues/48373 Signed-off-by: Adam King --- src/cephadm/cephadm | 8 ++++++-- src/pybind/mgr/cephadm/serve.py | 7 ++----- .../mgr/cephadm/services/cephadmservice.py | 17 +++++++++++++++++ src/pybind/mgr/cephadm/services/container.py | 10 ++++++++++ src/pybind/mgr/cephadm/services/ha_rgw.py | 13 +++++++++++++ src/pybind/mgr/cephadm/services/iscsi.py | 9 +++++++++ src/pybind/mgr/cephadm/services/monitoring.py | 13 ++++++++++++- src/pybind/mgr/cephadm/services/nfs.py | 11 ++++++++++- src/pybind/mgr/cephadm/services/osd.py | 1 + src/pybind/mgr/cephadm/tests/test_cephadm.py | 3 ++- src/pybind/mgr/cephadm/tests/test_services.py | 9 +++++++-- 11 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm index 1f3f92b4ebfae..e35ee1a83adc4 100755 --- a/src/cephadm/cephadm +++ b/src/cephadm/cephadm @@ -3983,8 +3983,12 @@ def command_deploy(ctx): # Get and check ports explicitly required to be opened daemon_ports = [] # type: List[int] - if ctx.tcp_ports: - daemon_ports = list(map(int, ctx.tcp_ports.split())) + + # only check port in use if not reconfig or redeploy since service + # we are redeploying/reconfiguring will already be using the port + if not ctx.reconfig and not redeploy: + if ctx.tcp_ports: + daemon_ports = list(map(int, ctx.tcp_ports.split())) if daemon_type in Ceph.daemons: config, keyring = get_config_and_keyring(ctx) diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index cad008ab176a0..c2e95b1d594b6 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -776,9 +776,6 @@ class CephadmServe: if haspec.keepalived_container_image: image = haspec.keepalived_container_image - cephadm_config, deps = self.mgr.cephadm_services[daemon_type_to_service(daemon_spec.daemon_type)].generate_config( - daemon_spec) - # TCP port to open in the host firewall if len(ports) > 0: daemon_spec.extra_args.extend([ @@ -814,7 +811,7 @@ class CephadmServe: [ '--name', daemon_spec.name(), ] + daemon_spec.extra_args, - stdin=json.dumps(cephadm_config), + stdin=json.dumps(daemon_spec.final_config), image=image) if not code and daemon_spec.host in self.mgr.cache.daemons: # prime cached service state with what we (should have) @@ -825,7 +822,7 @@ class CephadmServe: self.mgr.requires_post_actions.add(daemon_spec.daemon_type) self.mgr.cache.invalidate_host_daemons(daemon_spec.host) self.mgr.cache.update_daemon_config_deps( - daemon_spec.host, daemon_spec.name(), deps, start_time) + daemon_spec.host, daemon_spec.name(), daemon_spec.deps, start_time) self.mgr.cache.save_host(daemon_spec.host) msg = "{} {} on host '{}'".format( 'Reconfigured' if reconfig else 'Deployed', daemon_spec.name(), daemon_spec.host) diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index 95fc14f479d87..7dad76d7478d4 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -67,6 +67,11 @@ class CephadmDaemonSpec(Generic[ServiceSpecs]): # TCP ports used by the daemon self.ports: List[int] = ports or [] + # values to be populated during generate_config calls + # and then used in _run_cephadm + self.final_config: Dict[str, Any] = {} + self.deps: List[str] = [] + def name(self) -> str: return '%s.%s' % (self.daemon_type, self.daemon_id) @@ -416,6 +421,8 @@ class MonService(CephService): daemon_spec.ceph_conf = extra_config daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec def _check_safe_to_destroy(self, mon_id: str) -> None: @@ -497,6 +504,8 @@ class MgrService(CephService): daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec def get_active_daemon(self, daemon_descrs: List[DaemonDescription]) -> DaemonDescription: @@ -577,6 +586,8 @@ class MdsService(CephService): }) daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec def get_active_daemon(self, daemon_descrs: List[DaemonDescription]) -> DaemonDescription: @@ -666,6 +677,8 @@ class RgwService(CephService): daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec def get_keyring(self, rgw_id: str) -> str: @@ -823,6 +836,8 @@ class RbdMirrorService(CephService): daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec @@ -842,6 +857,8 @@ class CrashService(CephService): daemon_spec.keyring = keyring + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec diff --git a/src/pybind/mgr/cephadm/services/container.py b/src/pybind/mgr/cephadm/services/container.py index c073d1ae9c384..bd315779e52b9 100644 --- a/src/pybind/mgr/cephadm/services/container.py +++ b/src/pybind/mgr/cephadm/services/container.py @@ -3,6 +3,8 @@ from typing import List, Any, Tuple, Dict from ceph.deployment.service_spec import CustomContainerSpec +from orchestrator import OrchestratorError + from .cephadmservice import CephadmService, CephadmDaemonSpec logger = logging.getLogger(__name__) @@ -14,6 +16,14 @@ class CustomContainerService(CephadmService): def prepare_create(self, daemon_spec: CephadmDaemonSpec[CustomContainerSpec]) \ -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + if daemon_spec.spec is None: + # Exit here immediately because the required service + # spec to create a daemon is not provided. This is only + # provided when a service is applied via 'orch apply' + # command. + msg = "Required service specification not provided" + raise OrchestratorError(msg) + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) return daemon_spec def generate_config(self, daemon_spec: CephadmDaemonSpec[CustomContainerSpec]) \ diff --git a/src/pybind/mgr/cephadm/services/ha_rgw.py b/src/pybind/mgr/cephadm/services/ha_rgw.py index 4a37e006d58b5..53826f380f0e4 100644 --- a/src/pybind/mgr/cephadm/services/ha_rgw.py +++ b/src/pybind/mgr/cephadm/services/ha_rgw.py @@ -19,6 +19,13 @@ class HA_RGWService(CephService): def prepare_create(self, daemon_spec: CephadmDaemonSpec[HA_RGWSpec]) -> CephadmDaemonSpec: assert daemon_spec.daemon_type == 'haproxy' or daemon_spec.daemon_type == 'keepalived' + # if spec is not attached to daemon_spec it is likely a redeploy or reconfig and + # spec should be in spec store + if not daemon_spec.spec: + service_name: str = "ha-rgw." + daemon_spec.daemon_id.split('.')[0] + if service_name in self.mgr.spec_store.specs: + daemon_spec.spec = cast( + HA_RGWSpec, self.mgr.spec_store.specs.get(service_name)) assert daemon_spec.spec if daemon_spec.daemon_type == 'haproxy': @@ -44,6 +51,9 @@ class HA_RGWService(CephService): logger.info('Create daemon %s on host %s with spec %s' % ( daemon_id, host, spec)) + + daemon_spec.final_config, daemon_spec.deps = self.haproxy_generate_config(daemon_spec) + return daemon_spec def keepalived_prepare_create(self, daemon_spec: CephadmDaemonSpec[HA_RGWSpec]) -> CephadmDaemonSpec: @@ -56,6 +66,9 @@ class HA_RGWService(CephService): logger.info('Create daemon %s on host %s with spec %s' % ( daemon_id, host, spec)) + + daemon_spec.final_config, daemon_spec.deps = self.keepalived_generate_config(daemon_spec) + return daemon_spec def haproxy_generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]: diff --git a/src/pybind/mgr/cephadm/services/iscsi.py b/src/pybind/mgr/cephadm/services/iscsi.py index 98e647bfc9851..dbcc55ef26cdd 100644 --- a/src/pybind/mgr/cephadm/services/iscsi.py +++ b/src/pybind/mgr/cephadm/services/iscsi.py @@ -25,6 +25,13 @@ class IscsiService(CephService): def prepare_create(self, daemon_spec: CephadmDaemonSpec[IscsiServiceSpec]) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + # if spec is not attached to daemon_spec it is likely a redeploy or reconfig and + # spec should be in spec store + if not daemon_spec.spec: + service_name: str = "iscsi." + daemon_spec.daemon_id.split('.')[0] + if service_name in self.mgr.spec_store.specs: + daemon_spec.spec = cast( + IscsiServiceSpec, self.mgr.spec_store.specs.get(service_name)) assert daemon_spec.spec spec = daemon_spec.spec @@ -71,6 +78,8 @@ class IscsiService(CephService): daemon_spec.keyring = keyring daemon_spec.extra_files = {'iscsi-gateway.cfg': igw_conf} + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + return daemon_spec def config_dashboard(self, daemon_descrs: List[DaemonDescription]) -> None: diff --git a/src/pybind/mgr/cephadm/services/monitoring.py b/src/pybind/mgr/cephadm/services/monitoring.py index 7b49fe3a40c83..8937be1b9f3a5 100644 --- a/src/pybind/mgr/cephadm/services/monitoring.py +++ b/src/pybind/mgr/cephadm/services/monitoring.py @@ -1,7 +1,7 @@ import errno import logging import os -from typing import List, Any, Tuple, Dict +from typing import List, Any, Tuple, Dict, cast from mgr_module import HandleCommandResult @@ -19,6 +19,7 @@ class GrafanaService(CephadmService): def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) return daemon_spec def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]: @@ -96,7 +97,15 @@ class AlertmanagerService(CephadmService): def prepare_create(self, daemon_spec: CephadmDaemonSpec[AlertManagerSpec]) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + # if spec is not attached to daemon_spec it is likely a redeploy or reconfig and + # spec should be in spec store + if not daemon_spec.spec: + service_name: str = "alertmanager" + if service_name in self.mgr.spec_store.specs: + daemon_spec.spec = cast( + AlertManagerSpec, self.mgr.spec_store.specs.get(service_name)) assert daemon_spec.spec + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) return daemon_spec def generate_config(self, daemon_spec: CephadmDaemonSpec[AlertManagerSpec]) -> Tuple[Dict[str, Any], List[str]]: @@ -187,6 +196,7 @@ class PrometheusService(CephadmService): def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) return daemon_spec def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]: @@ -290,6 +300,7 @@ class NodeExporterService(CephadmService): def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) return daemon_spec def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]: diff --git a/src/pybind/mgr/cephadm/services/nfs.py b/src/pybind/mgr/cephadm/services/nfs.py index 3a6015982abfa..8e35ffdcd485a 100644 --- a/src/pybind/mgr/cephadm/services/nfs.py +++ b/src/pybind/mgr/cephadm/services/nfs.py @@ -1,5 +1,5 @@ import logging -from typing import Dict, Tuple, Any, List +from typing import Dict, Tuple, Any, List, cast from ceph.deployment.service_spec import NFSServiceSpec import rados @@ -25,12 +25,21 @@ class NFSService(CephService): def prepare_create(self, daemon_spec: CephadmDaemonSpec[NFSServiceSpec]) -> CephadmDaemonSpec: assert self.TYPE == daemon_spec.daemon_type + # if spec is not attached to daemon_spec it is likely a redeploy or reconfig and + # spec should be in spec store + if not daemon_spec.spec: + service_name: str = "nfs." + daemon_spec.daemon_id.split('.')[0] + if service_name in self.mgr.spec_store.specs: + daemon_spec.spec = cast( + NFSServiceSpec, self.mgr.spec_store.specs.get(service_name)) assert daemon_spec.spec daemon_id = daemon_spec.daemon_id host = daemon_spec.host spec = daemon_spec.spec + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) + logger.info('Create daemon %s on host %s with spec %s' % ( daemon_id, host, spec)) return daemon_spec diff --git a/src/pybind/mgr/cephadm/services/osd.py b/src/pybind/mgr/cephadm/services/osd.py index 0e898b6d9db3e..dc502243981cb 100644 --- a/src/pybind/mgr/cephadm/services/osd.py +++ b/src/pybind/mgr/cephadm/services/osd.py @@ -126,6 +126,7 @@ class OSDService(CephService): host=host, daemon_type='osd', ) + daemon_spec.final_config, daemon_spec.deps = self.generate_config(daemon_spec) CephadmServe(self.mgr)._create_daemon( daemon_spec, osd_uuid_map=osd_uuid_map) diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index f494804ea3839..e08d5f78c143d 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -291,7 +291,8 @@ class TestCephadm(object): _run_cephadm.assert_called_with('test', 'mon.test', 'deploy', [ '--name', 'mon.test', '--reconfig', '--config-json', '-'], - stdin='{"config": "\\n\\n[mon]\\nk=v\\n", "keyring": ""}', + stdin='{"config": "\\n\\n[mon]\\nk=v\\n[mon.test]\\npublic network = 127.0.0.0/8\\n", ' + + '"keyring": "", "files": {"config": "[mon.test]\\npublic network = 127.0.0.0/8\\n"}}', image='') @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}')) diff --git a/src/pybind/mgr/cephadm/tests/test_services.py b/src/pybind/mgr/cephadm/tests/test_services.py index 0411b3a87b16d..06a34498c0b51 100644 --- a/src/pybind/mgr/cephadm/tests/test_services.py +++ b/src/pybind/mgr/cephadm/tests/test_services.py @@ -3,7 +3,7 @@ import pytest from unittest.mock import MagicMock, call from cephadm.services.cephadmservice import MonService, MgrService, MdsService, RgwService, \ - RbdMirrorService, CrashService, CephadmExporter + RbdMirrorService, CrashService, CephadmExporter, CephadmDaemonSpec from cephadm.services.iscsi import IscsiService from cephadm.services.nfs import NFSService from cephadm.services.osd import OSDService @@ -29,6 +29,9 @@ class FakeMgr: return 0, 'value set', '' return -1, '', 'error' + def get_minimal_ceph_conf(self) -> str: + return '' + class TestCephadmService: def test_set_service_url_on_dashboard(self): @@ -89,7 +92,9 @@ class TestCephadmService: iscsi_spec.spec.daemon_type = "iscsi" iscsi_spec.spec.ssl_cert = '' - iscsi_service.prepare_create(iscsi_spec) + iscsi_daemon_spec = CephadmDaemonSpec(host='host', daemon_id='a', spec=iscsi_spec) + + iscsi_service.prepare_create(iscsi_daemon_spec) expected_caps = ['mon', 'profile rbd, allow command "osd blocklist", allow command "config-key get" with "key" prefix "iscsi/"', -- 2.39.5