Skip to content

Commit 926011a

Browse files
cephadm: use get_container_image_stats in cephadm.py
Replace the existing get_container_stats_by_image_name with a version from container_engines.py that returns the parsed results of the container status command (as a ContainerInfo, None on error). Fix up a few tests. Part of the test is somewhat pointless now as the input and the output are both the same ContainerInfo, but this was never a great test to test parsing anyway. Signed-off-by: John Mulligan <[email protected]>
1 parent e965813 commit 926011a

File tree

2 files changed

+10
-27
lines changed

2 files changed

+10
-27
lines changed

src/cephadm/cephadm.py

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@
8989
Podman,
9090
check_container_engine,
9191
find_container_engine,
92-
parsed_container_mem_usage,
9392
parsed_container_cpu_perc,
93+
parsed_container_image_stats,
94+
parsed_container_mem_usage,
9495
pull_command,
9596
registry_login,
9697
)
@@ -500,16 +501,9 @@ def daemon_name_or_type(daemon: Dict[str, str]) -> str:
500501
# container will not help us. If we have the image name from the list_daemons output
501502
# we can try that.
502503
image_name = matching_daemons[0]['container_image_name']
503-
out, _, code = get_container_stats_by_image_name(ctx, ctx.container_engine.path, image_name)
504-
if not code:
505-
# keep in mind, the daemon container is not running, so no container id here
506-
(image_id, start, version) = out.strip().split(',')
507-
return ContainerInfo(
508-
container_id='',
509-
image_name=image_name,
510-
image_id=image_id,
511-
start=start,
512-
version=version)
504+
cinfo = parsed_container_image_stats(ctx, image_name)
505+
if cinfo:
506+
return cinfo
513507
else:
514508
d_type, d_id = matching_daemons[0]['name'].split('.', 1)
515509
cinfo = get_container_stats(
@@ -3646,18 +3640,6 @@ def get_daemon_description(ctx, fsid, name, detail=False, legacy_dir=None):
36463640
return d
36473641
raise Error('Daemon not found: {}. See `cephadm ls`'.format(name))
36483642

3649-
3650-
def get_container_stats_by_image_name(ctx: CephadmContext, container_path: str, image_name: str) -> Tuple[str, str, int]:
3651-
"""returns image id, created time, and ceph version if available"""
3652-
out, err, code = '', '', -1
3653-
cmd = [
3654-
container_path, 'image', 'inspect',
3655-
'--format', '{{.Id}},{{.Created}},{{index .Config.Labels "io.ceph.version"}}',
3656-
image_name
3657-
]
3658-
out, err, code = call(ctx, cmd, verbosity=CallVerbosity.QUIET)
3659-
return out, err, code
3660-
36613643
##################################
36623644

36633645

src/cephadm/tests/test_cephadm.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ def test_get_container_info(
803803
)
804804

805805
def test_get_container_info_daemon_down(self, funkypatch):
806-
_get_stats_by_name = funkypatch.patch('cephadm.get_container_stats_by_image_name')
806+
_get_stats_by_name = funkypatch.patch('cephadmlib.container_engines.parsed_container_image_stats')
807807
_get_stats = funkypatch.patch('cephadmlib.container_types.get_container_stats')
808808
_list_daemons = funkypatch.patch('cephadm.list_daemons')
809809

@@ -845,16 +845,17 @@ def test_get_container_info_daemon_down(self, funkypatch):
845845
"configured": "2024-03-11T17:37:28.494075Z"
846846
}
847847
_list_daemons.return_value = [down_osd_json]
848-
_get_stats_by_name.return_value = (('a03c201ff4080204949932f367545cd381c4acee0d48dbc15f2eac1e35f22318,'
849-
'2023-11-28 21:34:38.045413692 +0000 UTC,'),
850-
'', 0)
851848

852849
expected_container_info = _cephadm.ContainerInfo(
853850
container_id='',
854851
image_name='quay.io/adk3798/ceph@sha256:7da0af22ce45aac97dff00125af590506d8e36ab97d78e5175149643562bfb0b',
855852
image_id='a03c201ff4080204949932f367545cd381c4acee0d48dbc15f2eac1e35f22318',
856853
start='2023-11-28 21:34:38.045413692 +0000 UTC',
857854
version='')
855+
# refactoring get_container_stats_by_image_name into
856+
# parsed_container_image_stats has made this part of the test somewhat
857+
# redundant
858+
_get_stats_by_name.return_value = expected_container_info
858859

859860
assert _cephadm.get_container_info(ctx, 'osd.2', by_name=True) == expected_container_info
860861
assert not _get_stats.called, 'only get_container_stats_by_image_name should have been called'

0 commit comments

Comments
 (0)