mgr/cephadm: Some more typing

Make sure we don't mix daemon names, config entities and
auth entities. Cause all of them don't mix well together.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 576932c17de489f2ac1c7998314bbffa153b1b5b..8ad4a285117e847d7291603133cc90de54353a9b 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -9,7 +9,7 @@ from threading import Event
 
 import string
 from typing import List, Dict, Optional, Callable, Tuple, TypeVar, \
-    Any, Set, TYPE_CHECKING, cast, Iterator
+    Any, Set, TYPE_CHECKING, cast, Iterator, Union
 
 import datetime
 import os
@@ -45,7 +45,7 @@ from .schedule import HostAssignment, HostPlacementSpec
 from .inventory import Inventory, SpecStore, HostCache, EventStore
 from .upgrade import CEPH_UPGRADE_ORDER, CephadmUpgrade
 from .template import TemplateMgr
-from .utils import forall_hosts
+from .utils import forall_hosts, CephadmNoImage, cephadmNoImage
 
 try:
     import remoto
@@ -410,7 +410,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         self.log.debug(' checking %s' % host)
         try:
             out, err, code = self._run_cephadm(
-                host, 'client', 'check-host', [],
+                host, cephadmNoImage, 'check-host', [],
                 error_ok=True, no_fsid=True)
             self.cache.update_last_host_check(host)
             self.cache.save_host(host)
@@ -924,7 +924,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         'Check whether we can access and manage a remote host')
     def check_host(self, host, addr=None):
         try:
-            out, err, code = self._run_cephadm(host, 'client', 'check-host',
+            out, err, code = self._run_cephadm(host, cephadmNoImage, 'check-host',
                                                ['--expect-hostname', host],
                                                addr=addr,
                                                error_ok=True, no_fsid=True)
@@ -948,7 +948,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         'name=addr,type=CephString,req=false',
         'Prepare a remote host for use with cephadm')
     def _prepare_host(self, host, addr=None):
-        out, err, code = self._run_cephadm(host, 'client', 'prepare-host',
+        out, err, code = self._run_cephadm(host, cephadmNoImage, 'prepare-host',
                                            ['--expect-hostname', host],
                                            addr=addr,
                                            error_ok=True, no_fsid=True)
@@ -1047,9 +1047,36 @@ you may want to run:
             self.log.exception(ex)
             raise
 
+    def _get_container_image(self, daemon_name: str) -> str:
+        daemon_type = daemon_name.split('.', 1)[0]  # type: ignore
+        if daemon_type in CEPH_TYPES or \
+                daemon_type == 'nfs' or \
+                daemon_type == 'iscsi':
+            # get container image
+            ret, image, err = self.check_mon_command({
+                'prefix': 'config get',
+                'who': utils.name_to_config_section(daemon_name),
+                'key': 'container_image',
+            })
+            image = image.strip()  # type: ignore
+        elif daemon_type == 'prometheus':
+            image = self.container_image_prometheus
+        elif daemon_type == 'grafana':
+            image = self.container_image_grafana
+        elif daemon_type == 'alertmanager':
+            image = self.container_image_alertmanager
+        elif daemon_type == 'node-exporter':
+            image = self.container_image_node_exporter
+        else:
+            assert False, daemon_type
+
+        self.log.debug('%s container image %s' % (daemon_name, image))
+
+        return image
+
     def _run_cephadm(self,
                      host: str,
-                     entity: str,
+                     entity: Union[CephadmNoImage, str],
                      command: str,
                      args: List[str],
                      addr: Optional[str] = "",
@@ -1067,28 +1094,8 @@ you may want to run:
         with self._remote_connection(host, addr) as tpl:
             conn, connr = tpl
             assert image or entity
-            if not image:
-                daemon_type = entity.split('.', 1)[0]  # type: ignore
-                if daemon_type in CEPH_TYPES or \
-                        daemon_type == 'nfs' or \
-                        daemon_type == 'iscsi':
-                    # get container image
-                    ret, image, err = self.check_mon_command({
-                        'prefix': 'config get',
-                        'who': utils.name_to_config_section(entity),
-                        'key': 'container_image',
-                    })
-                    image = image.strip()  # type: ignore
-                elif daemon_type == 'prometheus':
-                    image = self.container_image_prometheus
-                elif daemon_type == 'grafana':
-                    image = self.container_image_grafana
-                elif daemon_type == 'alertmanager':
-                    image = self.container_image_alertmanager
-                elif daemon_type == 'node-exporter':
-                    image = self.container_image_node_exporter
-
-            self.log.debug('%s container image %s' % (entity, image))
+            if not image and entity is not cephadmNoImage:
+                image = self._get_container_image(entity)
 
             final_args = []
 
@@ -1164,7 +1171,7 @@ you may want to run:
         :param host: host name
         """
         assert_valid_host(spec.hostname)
-        out, err, code = self._run_cephadm(spec.hostname, 'client', 'check-host',
+        out, err, code = self._run_cephadm(spec.hostname, cephadmNoImage, 'check-host',
                                            ['--expect-hostname', spec.hostname],
                                            addr=spec.addr,
                                            error_ok=True, no_fsid=True)

diff --git a/src/pybind/mgr/cephadm/utils.py b/src/pybind/mgr/cephadm/utils.py
index 91f36de0da8474f553688858126113a28d80a5ac..ca8bb48c04c587af60acd201ef5f984db48b3ac7 100644 (file)
--- a/src/pybind/mgr/cephadm/utils.py
+++ b/src/pybind/mgr/cephadm/utils.py
@@ -1,49 +1,65 @@
 import logging
 import re
 import json
+from enum import Enum
 from functools import wraps
-from typing import Optional, Callable, TypeVar, List
+from typing import Optional, Callable, TypeVar, List, NewType, TYPE_CHECKING
 from orchestrator import OrchestratorError
 
+if TYPE_CHECKING:
+    from cephadm import CephadmOrchestrator
 
 T = TypeVar('T')
 logger = logging.getLogger(__name__)
 
+ConfEntity = NewType('ConfEntity', str)
+AuthEntity = NewType('AuthEntity', str)
 
-def name_to_config_section(name: str) -> str:
+
+class CephadmNoImage(Enum):
+    token = 1
+
+
+# Used for _run_cephadm used for check-host etc that don't require an --image parameter
+cephadmNoImage = CephadmNoImage.token
+
+
+def name_to_config_section(name: str) -> ConfEntity:
     """
     Map from daemon names to ceph entity names (as seen in config)
     """
     daemon_type = name.split('.', 1)[0]
     if daemon_type in ['rgw', 'rbd-mirror', 'nfs', 'crash', 'iscsi']:
-        return 'client.' + name
+        return ConfEntity('client.' + name)
     elif daemon_type in ['mon', 'osd', 'mds', 'mgr', 'client']:
-        return name
+        return ConfEntity(name)
     else:
-        return 'mon'
+        return ConfEntity('mon')
 
-def name_to_auth_entity(daemon_type,  # type: str
-                        daemon_id,    # type: str
-                        host = ""     # type  Optional[str] = ""
-                        ):
+
+def name_to_auth_entity(daemon_type: str,
+                        daemon_id: str,
+                        host: str = "",
+                        ) -> AuthEntity:
     """
     Map from daemon names/host to ceph entity names (as seen in config)
     """
     if daemon_type in ['rgw', 'rbd-mirror', 'nfs', "iscsi"]:
-        return 'client.' + daemon_type + "." + daemon_id
+        return AuthEntity('client.' + daemon_type + "." + daemon_id)
     elif daemon_type == 'crash':
         if host == "":
             raise OrchestratorError("Host not provided to generate <crash> auth entity name")
-        return 'client.' + daemon_type + "." + host
+        return AuthEntity('client.' + daemon_type + "." + host)
     elif daemon_type == 'mon':
-        return 'mon.'
+        return AuthEntity('mon.')
     elif daemon_type == 'mgr':
-        return daemon_type + "." + daemon_id
+        return AuthEntity(daemon_type + "." + daemon_id)
     elif daemon_type in ['osd', 'mds', 'client']:
-        return daemon_type + "." + daemon_id
+        return AuthEntity(daemon_type + "." + daemon_id)
     else:
         raise OrchestratorError("unknown auth entity name")
 
+
 def forall_hosts(f: Callable[..., T]) -> Callable[..., List[T]]:
     @wraps(f)
     def forall_hosts_wrapper(*args) -> List[T]:
@@ -72,10 +88,10 @@ def forall_hosts(f: Callable[..., T]) -> Callable[..., List[T]]:
         assert CephadmOrchestrator.instance is not None
         return CephadmOrchestrator.instance._worker_pool.map(do_work, vals)
 
-
     return forall_hosts_wrapper
 
-def get_cluster_health(mgr):
+
+def get_cluster_health(mgr: 'CephadmOrchestrator') -> str:
     # check cluster health
     ret, out, err = mgr.check_mon_command({
         'prefix': 'health',