From 49405967620c9dbb69a1ea4660af46c240fcd298 Mon Sep 17 00:00:00 2001 From: Adam King Date: Tue, 8 Apr 2025 11:36:57 -0400 Subject: [PATCH] cephadm: still set keep_container_info key in CoreStatusUpdater when cinfo is None When testing an unrelated thing with rm-cluster cleaning up OSD devices, I was hitting ``` Traceback (most recent call last): File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main return _run_code(code, main_globals, None, File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code exec(code, run_globals) File "/usr/sbin/cephadm/__main__.py", line 5400, in File "/usr/sbin/cephadm/__main__.py", line 5388, in main File "/usr/sbin/cephadm/__main__.py", line 3996, in command_rm_cluster File "/usr/sbin/cephadm/__main__.py", line 4034, in _rm_cluster File "/usr/sbin/cephadm/__main__.py", line 428, in _infer_image File "/usr/sbin/cephadm/cephadmlib/container_lookup.py", line 127, in infer_local_ceph_image File "/usr/sbin/cephadm/cephadmlib/container_lookup.py", line 128, in KeyError: '_container_info' ``` which appears to have been caused by the CoreStatusUpdater only setting the "keep_container_info" key entry when the container info it gets back from get_container_stats is not None. This patch has it set that key entry even when the container info is None, and updates infer_local_ceph_image to be able to handle the container info at that entry being None as well. Signed-off-by: Adam King --- src/cephadm/cephadmlib/container_lookup.py | 4 +++- src/cephadm/cephadmlib/listing_updaters.py | 4 ++-- src/cephadm/tests/test_cephadm.py | 24 ++++++++++++++++++++++ src/cephadm/tests/test_listing.py | 22 ++++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/cephadm/cephadmlib/container_lookup.py b/src/cephadm/cephadmlib/container_lookup.py index 153c71c5765..3d44b57d8d4 100644 --- a/src/cephadm/cephadmlib/container_lookup.py +++ b/src/cephadm/cephadmlib/container_lookup.py @@ -134,7 +134,9 @@ def infer_local_ceph_image( images_in_use_by_daemon = set( d.image_id for d, n in matching_daemons if n == daemon_name ) - images_in_use = set(d.image_id for d, _ in matching_daemons) + images_in_use = set( + d.image_id for d, _ in matching_daemons if d is not None + ) # prioritize images def _keyfunc(image: ImageInfo) -> Tuple[bool, bool, str]: diff --git a/src/cephadm/cephadmlib/listing_updaters.py b/src/cephadm/cephadmlib/listing_updaters.py index 801dd064a5d..8344fb9b03d 100644 --- a/src/cephadm/cephadmlib/listing_updaters.py +++ b/src/cephadm/cephadmlib/listing_updaters.py @@ -59,9 +59,9 @@ class CoreStatusUpdater(DaemonStatusUpdater): data_dir, identity.fsid, identity.daemon_name ) cinfo = get_container_stats(ctx, identity) + if self.keep_container_info: + val[self.keep_container_info] = cinfo if cinfo: - if self.keep_container_info: - val[self.keep_container_info] = cinfo container_id = cinfo.container_id image_name = cinfo.image_name image_id = cinfo.image_id diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py index 3790da60dfb..4e10e32cd8a 100644 --- a/src/cephadm/tests/test_cephadm.py +++ b/src/cephadm/tests/test_cephadm.py @@ -804,6 +804,30 @@ quay.ceph.io/ceph-ci/ceph@sha256:8eb43767c40d3e2d8cdb7577904f5f0b94373afe2dde296 image = _cephadm.infer_local_ceph_image(ctx, ctx.container_engine) assert image == expected + def test_infer_local_ceph_image_no_cinfo(self, funkypatch): + ctx = _cephadm.CephadmContext() + ctx.fsid = '00000000-0000-0000-0000-0000deadbeez' + ctx.container_engine = mock_podman() + + out = """quay.ceph.io/ceph-ci/ceph@sha256:d6d1f4ab7148145467d9b632efc89d75710196434cba00aec5571b01e15b8a99|1b58ca4f6df|test|2025-01-21 16:54:41 +0000 UTC +quay.ceph.io/ceph-ci/ceph@sha256:8eb43767c40d3e2d8cdb7577904f5f0b94373afe2dde29672c2b0001bd098789|c17226ffd482|test|2025-01-22 16:54:41 +0000 UTC""" + funkypatch.patch('cephadmlib.call_wrappers.call').return_value = ( + out, + '', + 0, + ) + funkypatch.patch( + 'cephadmlib.listing_updaters.CoreStatusUpdater' + )().expand.side_effect = lambda ctx, v: v + funkypatch.patch( + 'cephadmlib.listing.daemons_matching' + ).return_value = [ + {'_container_info': None, 'name': 'mon.vm-00'}, + {'_container_info': None, 'name': 'mgr.vm-00.cdjeee'} + ] + image = _cephadm.infer_local_ceph_image(ctx, ctx.container_engine) + assert image == 'quay.ceph.io/ceph-ci/ceph@sha256:8eb43767c40d3e2d8cdb7577904f5f0b94373afe2dde29672c2b0001bd098789' + @pytest.mark.parametrize('daemon_filter, by_name, daemon_list, container_stats, output', [ # get container info by type ('mon') diff --git a/src/cephadm/tests/test_listing.py b/src/cephadm/tests/test_listing.py index c4ea8891a1d..14d0f153429 100644 --- a/src/cephadm/tests/test_listing.py +++ b/src/cephadm/tests/test_listing.py @@ -229,6 +229,28 @@ def test_list_daemons_detail_minimal(cephadm_fs, funkypatch): edl.assert_checked_all() +def test_core_status_update_no_cinfo(cephadm_fs, funkypatch): + _cephadm = import_cephadm() + + _get_container_stats = funkypatch.patch('cephadmlib.container_types.get_container_stats') + _get_container_stats.return_value = None + + fsid = 'dc93cfee-ddc5-11ef-a056-525400220000' + _cinfo_key = '_keep_container_info' + + updater = _cephadm.CoreStatusUpdater(keep_container_info=_cinfo_key) + d_id = _cephadm.DaemonIdentity( + fsid = fsid, + daemon_type = 'mon', + daemon_id = 'host1', + ) + val = {} + with with_cephadm_ctx([], mock_cephadm_call_fn=False) as ctx: + updater.update(val, ctx, d_id, f'/var/lib/ceph/{fsid}') + assert _cinfo_key in val + assert val[_cinfo_key] is None + + def test_list_daemons_detail_mgrnotrunning(cephadm_fs, funkypatch): _cephadm = import_cephadm() _call = funkypatch.patch('cephadmlib.call_wrappers.call') -- 2.39.5