mgr/cephadm: CephadmService.create -> CephadmService.prepare_create

Refactored this to simplify the control flow. It became obvious that

* `CephadmService.make_daemon_spec`
* `CephadmService.prepare_create`
* `CephadmService.generate_config`

are basically the same thing. They're all called directly
before the deployment of a daemon. All of them should be
unified. This PR makes this refactorization simpler

By renaming `create` to `prepare_create`, we make `create`
no longer being the entrypoint to call
`create_daemon`. Thus all the functions above
return some data structures.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
(cherry picked from commit 245cf50132e1731982d19e1114116e4150b9ea97)

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 26d3b2c0d3b89309353a279b4d91951ac21cba0e..1d20a4dd3db134570b4f6342dbc6d1a2d78b0b4e 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -2153,7 +2153,9 @@ To check that the host is reachable:
             self.log.debug('Placing %s.%s on host %s' % (
                 daemon_type, daemon_id, host))
 
-            self.cephadm_services[daemon_type].create(daemon_spec)
+            daemon_spec = self.cephadm_services[daemon_type].prepare_create(daemon_spec)
+
+            self._create_daemon(daemon_spec)
 
             # add to daemon list so next name(s) will also be unique
             sd = orchestrator.DaemonDescription(
@@ -2281,7 +2283,7 @@ To check that the host is reachable:
                 self._get_cephadm_service(daemon_type).daemon_check_post(daemon_descs)
 
     def _add_daemon(self, daemon_type, spec,
-                    create_func: Callable[..., T], config_func=None) -> List[T]:
+                    create_func: Callable[..., CephadmDaemonSpec], config_func=None) -> List[str]:
         """
         Add (and place) a daemon. Require explicit host placement.  Do not
         schedule, and do not apply the related scheduling limitations.
@@ -2297,7 +2299,7 @@ To check that the host is reachable:
 
     def _create_daemons(self, daemon_type, spec, daemons,
                         hosts, count,
-                        create_func: Callable[..., T], config_func=None) -> List[T]:
+                        create_func: Callable[..., CephadmDaemonSpec], config_func=None) -> List[str]:
         if count > len(hosts):
             raise OrchestratorError('too few hosts: want %d, have %s' % (
                 count, hosts))
@@ -2333,7 +2335,8 @@ To check that the host is reachable:
 
         @forall_hosts
         def create_func_map(*args):
-            return create_func(*args)
+            daemon_spec = create_func(*args)
+            return self._create_daemon(daemon_spec)
 
         return create_func_map(args)
 
@@ -2344,12 +2347,12 @@ To check that the host is reachable:
     @trivial_completion
     def add_mon(self, spec):
         # type: (ServiceSpec) -> List[str]
-        return self._add_daemon('mon', spec, self.mon_service.create)
+        return self._add_daemon('mon', spec, self.mon_service.prepare_create)
 
     @trivial_completion
     def add_mgr(self, spec):
         # type: (ServiceSpec) -> List[str]
-        return self._add_daemon('mgr', spec, self.mgr_service.create)
+        return self._add_daemon('mgr', spec, self.mgr_service.prepare_create)
 
     def _apply(self, spec: GenericSpec) -> str:
         self.migration.verify_no_migration()
@@ -2449,7 +2452,7 @@ To check that the host is reachable:
 
     @trivial_completion
     def add_mds(self, spec: ServiceSpec) -> List[str]:
-        return self._add_daemon('mds', spec, self.mds_service.create, self.mds_service.config)
+        return self._add_daemon('mds', spec, self.mds_service.prepare_create, self.mds_service.config)
 
     @trivial_completion
     def apply_mds(self, spec: ServiceSpec) -> str:
@@ -2457,7 +2460,7 @@ To check that the host is reachable:
 
     @trivial_completion
     def add_rgw(self, spec) -> List[str]:
-        return self._add_daemon('rgw', spec, self.rgw_service.create, self.rgw_service.config)
+        return self._add_daemon('rgw', spec, self.rgw_service.prepare_create, self.rgw_service.config)
 
     @trivial_completion
     def apply_rgw(self, spec) -> str:
@@ -2466,7 +2469,7 @@ To check that the host is reachable:
     @trivial_completion
     def add_iscsi(self, spec):
         # type: (ServiceSpec) -> List[str]
-        return self._add_daemon('iscsi', spec, self.iscsi_service.create, self.iscsi_service.config)
+        return self._add_daemon('iscsi', spec, self.iscsi_service.prepare_create, self.iscsi_service.config)
 
     @trivial_completion
     def apply_iscsi(self, spec) -> str:
@@ -2474,7 +2477,7 @@ To check that the host is reachable:
 
     @trivial_completion
     def add_rbd_mirror(self, spec) -> List[str]:
-        return self._add_daemon('rbd-mirror', spec, self.rbd_mirror_service.create)
+        return self._add_daemon('rbd-mirror', spec, self.rbd_mirror_service.prepare_create)
 
     @trivial_completion
     def apply_rbd_mirror(self, spec) -> str:
@@ -2482,7 +2485,7 @@ To check that the host is reachable:
 
     @trivial_completion
     def add_nfs(self, spec) -> List[str]:
-        return self._add_daemon('nfs', spec, self.nfs_service.create, self.nfs_service.config)
+        return self._add_daemon('nfs', spec, self.nfs_service.prepare_create, self.nfs_service.config)
 
     @trivial_completion
     def apply_nfs(self, spec) -> str:
@@ -2494,7 +2497,7 @@ To check that the host is reachable:
 
     @trivial_completion
     def add_prometheus(self, spec) -> List[str]:
-        return self._add_daemon('prometheus', spec, self.prometheus_service.create)
+        return self._add_daemon('prometheus', spec, self.prometheus_service.prepare_create)
 
     @trivial_completion
     def apply_prometheus(self, spec) -> str:
@@ -2504,7 +2507,7 @@ To check that the host is reachable:
     def add_node_exporter(self, spec):
         # type: (ServiceSpec) -> List[str]
         return self._add_daemon('node-exporter', spec,
-                                self.node_exporter_service.create)
+                                self.node_exporter_service.prepare_create)
 
     @trivial_completion
     def apply_node_exporter(self, spec) -> str:
@@ -2514,7 +2517,7 @@ To check that the host is reachable:
     def add_crash(self, spec):
         # type: (ServiceSpec) -> List[str]
         return self._add_daemon('crash', spec,
-                                self.crash_service.create)
+                                self.crash_service.prepare_create)
 
     @trivial_completion
     def apply_crash(self, spec) -> str:
@@ -2523,7 +2526,7 @@ To check that the host is reachable:
     @trivial_completion
     def add_grafana(self, spec):
         # type: (ServiceSpec) -> List[str]
-        return self._add_daemon('grafana', spec, self.grafana_service.create)
+        return self._add_daemon('grafana', spec, self.grafana_service.prepare_create)
 
     @trivial_completion
     def apply_grafana(self, spec: ServiceSpec) -> str:
@@ -2532,7 +2535,7 @@ To check that the host is reachable:
     @trivial_completion
     def add_alertmanager(self, spec):
         # type: (ServiceSpec) -> List[str]
-        return self._add_daemon('alertmanager', spec, self.alertmanager_service.create)
+        return self._add_daemon('alertmanager', spec, self.alertmanager_service.prepare_create)
 
     @trivial_completion
     def apply_alertmanager(self, spec: ServiceSpec) -> str:

diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py
index b19e5068e1d3677a852cc447838588604c4ac578..70d4be20f55dc023b2bcbdd781b875df2596e1db 100644 (file)
--- a/src/pybind/mgr/cephadm/services/cephadmservice.py
+++ b/src/pybind/mgr/cephadm/services/cephadmservice.py
@@ -86,7 +86,7 @@ class CephadmService(metaclass=ABCMeta):
             network=netowrk
         )
 
-    def create(self, daemon_spec: CephadmDaemonSpec):
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         raise NotImplementedError()
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]:
@@ -310,7 +310,7 @@ class CephService(CephadmService):
 class MonService(CephService):
     TYPE = 'mon'
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         """
         Create a new monitor on the given host.
         """
@@ -356,7 +356,7 @@ class MonService(CephService):
         daemon_spec.extra_config = {'config': extra_config}
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def _check_safe_to_destroy(self, mon_id: str) -> None:
         ret, out, err = self.mgr.check_mon_command({
@@ -398,7 +398,7 @@ class MonService(CephService):
 class MgrService(CephService):
     TYPE = 'mgr'
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         """
         Create a new manager instance on a host.
         """
@@ -437,7 +437,7 @@ class MgrService(CephService):
 
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def get_active_daemon(self, daemon_descrs: List[DaemonDescription]) -> DaemonDescription:
         for daemon in daemon_descrs:
@@ -486,7 +486,7 @@ class MdsService(CephService):
             'value': spec.service_id,
         })
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         mds_id, host = daemon_spec.daemon_id, daemon_spec.host
 
@@ -500,7 +500,7 @@ class MdsService(CephService):
         })
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def get_active_daemon(self, daemon_descrs: List[DaemonDescription]) -> DaemonDescription:
         active_mds_strs = list()
@@ -581,7 +581,7 @@ class RgwService(CephService):
             spec.service_name(), spec.placement.pretty_str()))
         self.mgr.spec_store.save(spec)
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         rgw_id, host = daemon_spec.daemon_id, daemon_spec.host
 
@@ -589,7 +589,7 @@ class RgwService(CephService):
 
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def get_keyring(self, rgw_id: str):
         ret, keyring, err = self.mgr.check_mon_command({
@@ -721,7 +721,7 @@ class RgwService(CephService):
 class RbdMirrorService(CephService):
     TYPE = 'rbd-mirror'
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         daemon_id, host = daemon_spec.daemon_id, daemon_spec.host
 
@@ -734,13 +734,13 @@ class RbdMirrorService(CephService):
 
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
 
 class CrashService(CephService):
     TYPE = 'crash'
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         daemon_id, host = daemon_spec.daemon_id, daemon_spec.host
 
@@ -753,4 +753,4 @@ class CrashService(CephService):
 
         daemon_spec.keyring = keyring
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec

diff --git a/src/pybind/mgr/cephadm/services/iscsi.py b/src/pybind/mgr/cephadm/services/iscsi.py
index d4d34d4c6e1ad82685cd06a48727d3a2e41645af..d32210504c7d1f2d51a1122da0c6c6d6c8f871bb 100644 (file)
--- a/src/pybind/mgr/cephadm/services/iscsi.py
+++ b/src/pybind/mgr/cephadm/services/iscsi.py
@@ -23,7 +23,7 @@ class IscsiService(CephService):
             spec.service_name(), spec.placement.pretty_str()))
         self.mgr.spec_store.save(spec)
 
-    def create(self, daemon_spec: CephadmDaemonSpec[IscsiServiceSpec]) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec[IscsiServiceSpec]) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         assert daemon_spec.spec
 
@@ -70,7 +70,7 @@ class IscsiService(CephService):
         daemon_spec.keyring = keyring
         daemon_spec.extra_config = {'iscsi-gateway.cfg': igw_conf}
 
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def config_dashboard(self, daemon_descrs: List[DaemonDescription]):
         def get_set_cmd_dicts(out: str) -> List[dict]:

diff --git a/src/pybind/mgr/cephadm/services/monitoring.py b/src/pybind/mgr/cephadm/services/monitoring.py
index e14e17fdc230042632ecc7d1066d76a7f6530015..9bf4b5e001d80aad5e33e189a508a5105b11baab 100644 (file)
--- a/src/pybind/mgr/cephadm/services/monitoring.py
+++ b/src/pybind/mgr/cephadm/services/monitoring.py
@@ -14,9 +14,9 @@ class GrafanaService(CephadmService):
     TYPE = 'grafana'
     DEFAULT_SERVICE_PORT = 3000
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type
@@ -83,10 +83,10 @@ class AlertmanagerService(CephadmService):
     TYPE = 'alertmanager'
     DEFAULT_SERVICE_PORT = 9093
 
-    def create(self, daemon_spec: CephadmDaemonSpec[AlertManagerSpec]) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec[AlertManagerSpec]) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         assert daemon_spec.spec
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec[AlertManagerSpec]) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type
@@ -151,7 +151,8 @@ class AlertmanagerService(CephadmService):
 
     def config_dashboard(self, daemon_descrs: List[DaemonDescription]):
         dd = self.get_active_daemon(daemon_descrs)
-        service_url = 'http://{}:{}'.format(self._inventory_get_addr(dd.hostname), self.DEFAULT_SERVICE_PORT)
+        service_url = 'http://{}:{}'.format(self._inventory_get_addr(dd.hostname),
+                                            self.DEFAULT_SERVICE_PORT)
         self._set_service_url_on_dashboard(
             'AlertManager',
             'dashboard get-alertmanager-api-host',
@@ -164,9 +165,9 @@ class PrometheusService(CephadmService):
     TYPE = 'prometheus'
     DEFAULT_SERVICE_PORT = 9095
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type
@@ -257,9 +258,9 @@ class PrometheusService(CephadmService):
 class NodeExporterService(CephadmService):
     TYPE = 'node-exporter'
 
-    def create(self, daemon_spec: CephadmDaemonSpec) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type

diff --git a/src/pybind/mgr/cephadm/services/nfs.py b/src/pybind/mgr/cephadm/services/nfs.py
index 045b70445f41a32d3801b77b7e734098de589bf0..bbe6a3d65148450f7e05af99ecfdc1766c4a4fe1 100644 (file)
--- a/src/pybind/mgr/cephadm/services/nfs.py
+++ b/src/pybind/mgr/cephadm/services/nfs.py
@@ -26,7 +26,7 @@ class NFSService(CephService):
             spec.service_name(), spec.placement.pretty_str()))
         self.mgr.spec_store.save(spec)
 
-    def create(self, daemon_spec: CephadmDaemonSpec[NFSServiceSpec]) -> str:
+    def prepare_create(self, daemon_spec: CephadmDaemonSpec[NFSServiceSpec]) -> CephadmDaemonSpec:
         assert self.TYPE == daemon_spec.daemon_type
         assert daemon_spec.spec
 
@@ -36,7 +36,7 @@ class NFSService(CephService):
 
         logger.info('Create daemon %s on host %s with spec %s' % (
             daemon_id, host, spec))
-        return self.mgr._create_daemon(daemon_spec)
+        return daemon_spec
 
     def generate_config(self, daemon_spec: CephadmDaemonSpec[NFSServiceSpec]) -> Tuple[Dict[str, Any], List[str]]:
         assert self.TYPE == daemon_spec.daemon_type