cephadm: use parsed_container_mem_usage in cephadm.py

Replace the use of _parse_mem_usage and related command calls with
parsed_container_mem_usage.

This needs a lot of tests to be updated so that the call functions that
now originate in `container_engines.py` get mocked. funkypatch handles
most of the extra work for us - but it needs to be added to many tests.

Signed-off-by: John Mulligan <[email protected]>

Author: phlogistonjohn

src/cephadm/cephadm.py

@@ -88,6 +88,7 @@
     Podman,
     check_container_engine,
     find_container_engine,
+    parsed_container_mem_usage,
     pull_command,
     registry_login,
 )
@@ -98,7 +99,6 @@
     normalize_image_digest,
     try_convert_datetime,
     read_config,
-    with_units_to_int,
     _extract_host_info_from_applied_spec,
 )
 from cephadmlib.file_utils import (
@@ -1633,12 +1633,7 @@ def _daemon_ls_subset(self) -> Dict[str, Dict[str, Any]]:
         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(
@@ -3452,12 +3447,7 @@ def list_daemons(
     # 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)
+    seen_memusage_cid_len, seen_memusage = parsed_container_mem_usage(ctx)
 
     out, err, code = call(
         ctx,
@@ -3667,24 +3657,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

src/cephadm/tests/test_agent.py

@@ -3,7 +3,12 @@
 
 import pytest
 
-from tests.fixtures import with_cephadm_ctx, cephadm_fs, import_cephadm
+from tests.fixtures import (
+    cephadm_fs,
+    funkypatch,
+    import_cephadm,
+    with_cephadm_ctx,
+)
 
 from typing import Optional
 
@@ -182,7 +187,7 @@ def _ceph_volume_empty(_):
             out, _ = agent._ceph_volume(False)
 
 
-def test_agent_daemon_ls_subset(cephadm_fs):
+def test_agent_daemon_ls_subset(cephadm_fs, funkypatch):
     # Basing part of this test on some actual sample output
 
     # Some sample "podman stats --format '{{.ID}},{{.MemUsage}}' --no-stream" output
@@ -225,10 +230,11 @@ def _fake_call(ctx, cmd, desc=None, verbosity=_cephadm.CallVerbosity.VERBOSE_ON_
     cephadm_fs.create_dir(f'/var/lib/ceph/{FSID}/mgr.host1.pntmho')  # cephadm daemon
     cephadm_fs.create_dir(f'/var/lib/ceph/{FSID}/crash.host1')  # cephadm daemon
 
-    with with_cephadm_ctx([]) as ctx:
+    with with_cephadm_ctx([], mock_cephadm_call_fn=False) as ctx:
         ctx.fsid = FSID
         agent = _cephadm.CephadmAgent(ctx, FSID, AGENT_ID)
-        _cephadm.call.side_effect = _fake_call
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call')
+        _call.side_effect = _fake_call
         daemons = agent._daemon_ls_subset()
 
         assert 'agent.host1' in daemons

src/cephadm/tests/test_cephadm.py

@@ -226,7 +226,9 @@ def test_check_required_global_args(self):
 
     @mock.patch('cephadm.logger')
     def test_parse_mem_usage(self, _logger):
-        len, summary = _cephadm._parse_mem_usage(0, 'c6290e3f1489,-- / --')
+        from cephadmlib.container_engines import _parse_mem_usage
+
+        len, summary = _parse_mem_usage(0, 'c6290e3f1489,-- / --')
         assert summary == {}
 
     def test_CustomValidation(self):
@@ -1665,7 +1667,9 @@ def test_fsid(self, fsid, err, cephadm_fs, funkypatch):
 
 class TestShell(object):
 
-    def test_fsid(self, cephadm_fs):
+    def test_fsid(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
         fsid = '00000000-0000-0000-0000-0000deadbeef'
 
         cmd = ['shell', '--fsid', fsid]
@@ -1699,7 +1703,10 @@ def test_fsid(self, cephadm_fs):
                 assert retval == 1
                 assert ctx.fsid == None
 
-    def test_name(self, cephadm_fs):
+    def test_name(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell', '--name', 'foo']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
@@ -1718,7 +1725,10 @@ def test_name(self, cephadm_fs):
             retval = _cephadm.command_shell(ctx)
             assert retval == 0
 
-    def test_config(self, cephadm_fs):
+    def test_config(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
@@ -1737,7 +1747,10 @@ def test_config(self, cephadm_fs):
             assert retval == 0
             assert ctx.config == 'foo'
 
-    def test_keyring(self, cephadm_fs):
+    def test_keyring(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
@@ -1756,24 +1769,33 @@ def test_keyring(self, cephadm_fs):
             assert retval == 0
             assert ctx.keyring == 'foo'
 
-    @mock.patch('cephadm.CephContainer')
-    def test_mount_no_dst(self, _ceph_container, cephadm_fs):
+    def test_mount_no_dst(self, cephadm_fs, funkypatch):
+        _ceph_container = funkypatch.patch('cephadm.CephContainer')
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell', '--mount', '/etc/foo']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
             assert retval == 0
             assert _ceph_container.call_args.kwargs['volume_mounts']['/etc/foo'] == '/mnt/foo'
 
-    @mock.patch('cephadm.CephContainer')
-    def test_mount_with_dst_no_opt(self, _ceph_container, cephadm_fs):
+    def test_mount_with_dst_no_opt(self, cephadm_fs, funkypatch):
+        _ceph_container = funkypatch.patch('cephadm.CephContainer')
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell', '--mount', '/etc/foo:/opt/foo/bar']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
             assert retval == 0
             assert _ceph_container.call_args.kwargs['volume_mounts']['/etc/foo'] == '/opt/foo/bar'
 
-    @mock.patch('cephadm.CephContainer')
-    def test_mount_with_dst_and_opt(self, _ceph_container, cephadm_fs):
+    def test_mount_with_dst_and_opt(self, cephadm_fs, funkypatch):
+        _ceph_container = funkypatch.patch('cephadm.CephContainer')
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = ['shell', '--mount', '/etc/foo:/opt/foo/bar:Z']
         with with_cephadm_ctx(cmd) as ctx:
             retval = _cephadm.command_shell(ctx)
@@ -1790,7 +1812,10 @@ def _get_cmd(*args):
             '--', 'inventory', '--format', 'json'
         ]
 
-    def test_noop(self, cephadm_fs):
+    def test_noop(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = self._get_cmd()
         with with_cephadm_ctx(cmd) as ctx:
             _cephadm.command_ceph_volume(ctx)
@@ -1799,7 +1824,10 @@ def test_noop(self, cephadm_fs):
             assert ctx.keyring == None
             assert ctx.config_json == None
 
-    def test_fsid(self, cephadm_fs):
+    def test_fsid(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         fsid = '00000000-0000-0000-0000-0000deadbeef'
 
         cmd = self._get_cmd('--fsid', fsid)
@@ -1830,7 +1858,10 @@ def test_fsid(self, cephadm_fs):
                 _cephadm.command_ceph_volume(ctx)
                 assert ctx.fsid == None
 
-    def test_config(self, cephadm_fs):
+    def test_config(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = self._get_cmd('--config', 'foo')
         with with_cephadm_ctx(cmd) as ctx:
             err = r'No such file or directory'
@@ -1843,7 +1874,10 @@ def test_config(self, cephadm_fs):
             _cephadm.command_ceph_volume(ctx)
             assert ctx.config == 'bar'
 
-    def test_keyring(self, cephadm_fs):
+    def test_keyring(self, cephadm_fs, funkypatch):
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
+        _call.side_effect = lambda *args, **kwargs: ('', '', 0)
+
         cmd = self._get_cmd('--keyring', 'foo')
         with with_cephadm_ctx(cmd) as ctx:
             err = r'No such file or directory'
@@ -2260,7 +2294,7 @@ def test_error(self, funkypatch):
         funkypatch.patch('cephadm.logger')
         _giifi = funkypatch.patch('cephadm.get_image_info_from_inspect')
         _giifi.return_value = {}
-        _call = funkypatch.patch('cephadmlib.call_wrappers.call')
+        _call = funkypatch.patch('cephadmlib.call_wrappers.call', force=True)
         ctx = _cephadm.CephadmContext()
         ctx.container_engine = mock_podman()
         ctx.insecure = False