Merge pull request ceph#61579 from phlogistonjohn/jjm-cephadm-small-moves

cephadm: move three functions out of cephadm.py

Reviewed-by: Adam King <[email protected]>

Author: adk3798

src/cephadm/cephadm.py

@@ -88,17 +88,18 @@
     Podman,
     check_container_engine,
     find_container_engine,
+    parsed_container_mem_usage,
+    parsed_container_cpu_perc,
     pull_command,
     registry_login,
 )
 from cephadmlib.data_utils import (
     dict_get_join,
-    get_legacy_config_fsid,
+    get_legacy_daemon_fsid,
     is_fsid,
     normalize_image_digest,
     try_convert_datetime,
     read_config,
-    with_units_to_int,
     _extract_host_info_from_applied_spec,
 )
 from cephadmlib.file_utils import (
@@ -738,27 +739,6 @@ def lookup_unit_name_by_daemon_name(ctx: CephadmContext, fsid: str, name: str) -
         raise Error('Failed to get unit name for {}'.format(daemon))
 
 
-def get_legacy_daemon_fsid(ctx, cluster,
-                           daemon_type, daemon_id, legacy_dir=None):
-    # type: (CephadmContext, str, str, Union[int, str], Optional[str]) -> Optional[str]
-    fsid = None
-    if daemon_type == 'osd':
-        try:
-            fsid_file = os.path.join(ctx.data_dir,
-                                     daemon_type,
-                                     'ceph-%s' % daemon_id,
-                                     'ceph_fsid')
-            if legacy_dir is not None:
-                fsid_file = os.path.abspath(legacy_dir + fsid_file)
-            with open(fsid_file, 'r') as f:
-                fsid = f.read().strip()
-        except IOError:
-            pass
-    if not fsid:
-        fsid = get_legacy_config_fsid(cluster, legacy_dir=legacy_dir)
-    return fsid
-
-
 def create_daemon_dirs(
     ctx: CephadmContext,
     ident: 'DaemonIdentity',
@@ -1653,13 +1633,7 @@ def _daemon_ls_subset(self) -> Dict[str, Dict[str, Any]]:
         # not changed for any daemon, we assume our cached info is good.
         daemons: Dict[str, Dict[str, Any]] = {}
         data_dir = self.ctx.data_dir
-        seen_memusage = {}  # type: Dict[str, int]
-        out, err, code = call(
-            self.ctx,
-            [self.ctx.container_engine.path, 'stats', '--format', '{{.ID}},{{.MemUsage}}', '--no-stream'],
-            verbosity=CallVerbosity.DEBUG
-        )
-        seen_memusage_cid_len, seen_memusage = _parse_mem_usage(code, out)
+        seen_memusage_cid_len, seen_memusage = parsed_container_mem_usage(self.ctx)
         # we need a mapping from container names to ids. Later we will convert daemon
         # names to container names to get daemons container id to see if it has changed
         out, err, code = call(
@@ -3471,21 +3445,8 @@ def list_daemons(
     seen_digests = {}   # type: Dict[str, List[str]]
 
     # keep track of memory and cpu usage we've seen
-    seen_memusage = {}  # type: Dict[str, int]
-    seen_cpuperc = {}  # type: Dict[str, str]
-    out, err, code = call(
-        ctx,
-        [container_path, 'stats', '--format', '{{.ID}},{{.MemUsage}}', '--no-stream'],
-        verbosity=CallVerbosity.QUIET
-    )
-    seen_memusage_cid_len, seen_memusage = _parse_mem_usage(code, out)
-
-    out, err, code = call(
-        ctx,
-        [container_path, 'stats', '--format', '{{.ID}},{{.CPUPerc}}', '--no-stream'],
-        verbosity=CallVerbosity.QUIET
-    )
-    seen_cpuperc_cid_len, seen_cpuperc = _parse_cpu_perc(code, out)
+    seen_memusage_cid_len, seen_memusage = parsed_container_mem_usage(ctx)
+    seen_cpuperc_cid_len, seen_cpuperc = parsed_container_cpu_perc(ctx)
 
     # /var/lib/ceph
     if os.path.exists(data_dir):
@@ -3688,40 +3649,6 @@ def list_daemons(
     return ls
 
 
-def _parse_mem_usage(code: int, out: str) -> Tuple[int, Dict[str, int]]:
-    # keep track of memory usage we've seen
-    seen_memusage = {}  # type: Dict[str, int]
-    seen_memusage_cid_len = 0
-    if not code:
-        for line in out.splitlines():
-            (cid, usage) = line.split(',')
-            (used, limit) = usage.split(' / ')
-            try:
-                seen_memusage[cid] = with_units_to_int(used)
-                if not seen_memusage_cid_len:
-                    seen_memusage_cid_len = len(cid)
-            except ValueError:
-                logger.info('unable to parse memory usage line\n>{}'.format(line))
-                pass
-    return seen_memusage_cid_len, seen_memusage
-
-
-def _parse_cpu_perc(code: int, out: str) -> Tuple[int, Dict[str, str]]:
-    seen_cpuperc = {}
-    seen_cpuperc_cid_len = 0
-    if not code:
-        for line in out.splitlines():
-            (cid, cpuperc) = line.split(',')
-            try:
-                seen_cpuperc[cid] = cpuperc
-                if not seen_cpuperc_cid_len:
-                    seen_cpuperc_cid_len = len(cid)
-            except ValueError:
-                logger.info('unable to parse cpu percentage line\n>{}'.format(line))
-                pass
-    return seen_cpuperc_cid_len, seen_cpuperc
-
-
 def get_daemon_description(ctx, fsid, name, detail=False, legacy_dir=None):
     # type: (CephadmContext, str, str, bool, Optional[str]) -> Dict[str, str]
 

src/cephadm/cephadmlib/container_engines.py

@@ -1,10 +1,11 @@
 # container_engines.py - container engine types and selection funcs
 
 import os
+import logging
 
 from typing import Tuple, List, Optional, Dict
 
-from .call_wrappers import call_throws, CallVerbosity
+from .call_wrappers import call_throws, call, CallVerbosity
 from .context import CephadmContext
 from .container_engine_base import ContainerEngine
 from .constants import (
@@ -13,9 +14,13 @@
     MIN_PODMAN_VERSION,
     PIDS_LIMIT_UNLIMITED_PODMAN_VERSION,
 )
+from .data_utils import with_units_to_int
 from .exceptions import Error
 
 
+logger = logging.getLogger()
+
+
 class Podman(ContainerEngine):
     EXE = 'podman'
 
@@ -189,3 +194,111 @@ def pull_command(
         if os.path.exists('/etc/ceph/podman-auth.json'):
             cmd.append('--authfile=/etc/ceph/podman-auth.json')
     return cmd
+
+
+def _container_mem_usage(
+    ctx: CephadmContext,
+    *,
+    container_path: str = '',
+    verbosity: CallVerbosity = CallVerbosity.QUIET,
+) -> Tuple[str, str, int]:
+    container_path = container_path or ctx.container_engine.path
+    out, err, code = call(
+        ctx,
+        [
+            container_path,
+            'stats',
+            '--format',
+            '{{.ID}},{{.MemUsage}}',
+            '--no-stream',
+        ],
+        verbosity=verbosity,
+    )
+    return out, err, code
+
+
+def _parse_mem_usage(code: int, out: str) -> Tuple[int, Dict[str, int]]:
+    # keep track of memory usage we've seen
+    seen_memusage = {}  # type: Dict[str, int]
+    seen_memusage_cid_len = 0
+    if not code:
+        for line in out.splitlines():
+            (cid, usage) = line.split(',')
+            (used, limit) = usage.split(' / ')
+            try:
+                seen_memusage[cid] = with_units_to_int(used)
+                if not seen_memusage_cid_len:
+                    seen_memusage_cid_len = len(cid)
+            except ValueError:
+                logger.info(
+                    'unable to parse memory usage line\n>{}'.format(line)
+                )
+                pass
+    return seen_memusage_cid_len, seen_memusage
+
+
+def parsed_container_mem_usage(
+    ctx: CephadmContext,
+    *,
+    container_path: str = '',
+    verbosity: CallVerbosity = CallVerbosity.QUIET,
+) -> Tuple[int, Dict[str, int]]:
+    """Return memory useage values parsed from the container engine's container status."""
+    out, _, code = _container_mem_usage(
+        ctx, container_path=container_path, verbosity=verbosity
+    )
+    return _parse_mem_usage(code, out)
+
+
+def _container_cpu_perc(
+    ctx: CephadmContext,
+    *,
+    container_path: str = '',
+    verbosity: CallVerbosity = CallVerbosity.QUIET,
+) -> Tuple[str, str, int]:
+    container_path = container_path or ctx.container_engine.path
+    out, err, code = call(
+        ctx,
+        [
+            container_path,
+            'stats',
+            '--format',
+            '{{.ID}},{{.CPUPerc}}',
+            '--no-stream',
+        ],
+        verbosity=CallVerbosity.QUIET,
+    )
+    return out, err, code
+
+
+def _parse_cpu_perc(code: int, out: str) -> Tuple[int, Dict[str, str]]:
+    seen_cpuperc = {}
+    seen_cpuperc_cid_len = 0
+    if not code:
+        for line in out.splitlines():
+            (cid, cpuperc) = line.split(',')
+            try:
+                seen_cpuperc[cid] = cpuperc
+                if not seen_cpuperc_cid_len:
+                    seen_cpuperc_cid_len = len(cid)
+            except ValueError:
+                logger.info(
+                    'unable to parse cpu percentage line\n>{}'.format(line)
+                )
+                pass
+    return seen_cpuperc_cid_len, seen_cpuperc
+
+
+def parsed_container_cpu_perc(
+    ctx: CephadmContext,
+    *,
+    container_path: str = '',
+    verbosity: CallVerbosity = CallVerbosity.QUIET,
+) -> Tuple[int, Dict[str, str]]:
+    """Return cpu percentage used values parsed from the container engine's
+    container status.
+    """
+    out, _, code = _container_cpu_perc(
+        ctx, container_path=container_path, verbosity=verbosity
+    )
+    return _parse_cpu_perc(code, out)

src/cephadm/cephadmlib/data_utils.py

@@ -9,9 +9,10 @@
 
 from configparser import ConfigParser
 
-from typing import Dict, Any, Optional, Iterable, List
+from typing import Dict, Any, Optional, Iterable, List, Union
 
 from .constants import DATEFMT, DEFAULT_REGISTRY
+from .context import CephadmContext
 from .exceptions import Error
 
 
@@ -205,6 +206,30 @@ def get_legacy_config_fsid(
     return None
 
 
+def get_legacy_daemon_fsid(
+    ctx: CephadmContext,
+    cluster: str,
+    daemon_type: str,
+    daemon_id: Union[int, str],
+    legacy_dir: Optional[str] = None,
+) -> Optional[str]:
+    fsid = None
+    if daemon_type == 'osd':
+        try:
+            fsid_file = os.path.join(
+                ctx.data_dir, daemon_type, 'ceph-%s' % daemon_id, 'ceph_fsid'
+            )
+            if legacy_dir is not None:
+                fsid_file = os.path.abspath(legacy_dir + fsid_file)
+            with open(fsid_file, 'r') as f:
+                fsid = f.read().strip()
+        except IOError:
+            pass
+    if not fsid:
+        fsid = get_legacy_config_fsid(cluster, legacy_dir=legacy_dir)
+    return fsid
+
+
 def _extract_host_info_from_applied_spec(
     f: Iterable[str],
 ) -> List[Dict[str, str]]:

src/cephadm/tests/fixtures.py

@@ -1,3 +1,4 @@
+import contextlib
 import mock
 import os
 import pytest
@@ -153,6 +154,8 @@ def with_cephadm_ctx(
     cmd: List[str],
     list_networks: Optional[Dict[str, Dict[str, List[str]]]] = None,
     hostname: Optional[str] = None,
+    *,
+    mock_cephadm_call_fn: bool = True,
 ):
     """
     :param cmd: cephadm command argv
@@ -163,25 +166,25 @@ def with_cephadm_ctx(
         hostname = 'host1'
 
     _cephadm = import_cephadm()
-    with mock.patch('cephadmlib.net_utils.attempt_bind'), \
-         mock.patch('cephadmlib.call_wrappers.call', return_value=('', '', 0)), \
-         mock.patch('cephadmlib.call_wrappers.call_timeout', return_value=0), \
-         mock.patch('cephadm.call', return_value=('', '', 0)), \
-         mock.patch('cephadm.call_timeout', return_value=0), \
-         mock.patch('cephadmlib.exe_utils.find_executable', return_value='foo'), \
-         mock.patch('cephadm.get_container_info', return_value=None), \
-         mock.patch('cephadm.is_available', return_value=True), \
-         mock.patch('cephadm.json_loads_retry', return_value={'epoch' : 1}), \
-         mock.patch('cephadm.logger'), \
-         mock.patch('cephadm.FileLock'), \
-         mock.patch('socket.gethostname', return_value=hostname):
+    with contextlib.ExitStack() as stack:
+        stack.enter_context(mock.patch('cephadmlib.net_utils.attempt_bind'))
+        stack.enter_context(mock.patch('cephadmlib.exe_utils.find_executable', return_value='foo'))
+        stack.enter_context(mock.patch('cephadm.get_container_info', return_value=None))
+        stack.enter_context(mock.patch('cephadm.is_available', return_value=True))
+        stack.enter_context(mock.patch('cephadm.json_loads_retry', return_value={'epoch' : 1}))
+        stack.enter_context(mock.patch('cephadm.logger'))
+        stack.enter_context(mock.patch('cephadm.FileLock'))
+        stack.enter_context(mock.patch('socket.gethostname', return_value=hostname))
+        if mock_cephadm_call_fn:
+            stack.enter_context(mock.patch('cephadmlib.call_wrappers.call', return_value=('', '', 0)))
+            stack.enter_context(mock.patch('cephadmlib.call_wrappers.call_timeout', return_value=0))
+            stack.enter_context(mock.patch('cephadm.call', return_value=('', '', 0)))
+            stack.enter_context(mock.patch('cephadm.call_timeout', return_value=0))
+        if list_networks is not None:
+            stack.enter_context(mock.patch('cephadm.list_networks', return_value=list_networks))
         ctx: _cephadm.CephadmContext = _cephadm.cephadm_init_ctx(cmd)
         ctx.container_engine = mock_podman()
-        if list_networks is not None:
-            with mock.patch('cephadm.list_networks', return_value=list_networks):
-                yield ctx
-        else:
-            yield ctx
+        yield ctx
 
 
 @pytest.fixture()