From daa0a2f018e1681044911d322b7fd1b88e988d08 Mon Sep 17 00:00:00 2001 From: Kobi Ginon Date: Fri, 15 May 2026 19:22:30 +0300 Subject: [PATCH] cephadm: fix mgr ports list growth; add unit tests (#76564) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Problem ------- MgrService.prepare_create built module ports from ``mgr services`` but only assigned them when non-empty, then always appended service_discovery_port. After rehydration from mgr/cephadm/host.*, an empty ``mgr services`` response left stale ports in place and appended another 8765 each redeploy (https://tracker.ceph.com/issues/76564). Fix --- Always set ``daemon_spec.ports = ports + [service_discovery_port]`` so each prepare uses a fresh list plus exactly one discovery port. Tests ----- Add src/pybind/mgr/cephadm/tests/services/test_mgr.py: empty vs non-empty ``mgr services`` with carried-over duplicate ports. Cases consolidated from parallel PR https://github.com/ceph/ceph/pull/68879 . Authors ------- Kobi Ginon (@kginonredhat) — fix + test integration for this PR. Raimund Sacherer (@rsacherer) — original fix + tests in #68879; coordinated to land a single PR (#68915). Fixes: https://tracker.ceph.com/issues/76564 Signed-off-by: Kobi Ginon Signed-off-by: Raimund Sacherer --- .../mgr/cephadm/tests/services/test_mgr.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/pybind/mgr/cephadm/tests/services/test_mgr.py diff --git a/src/pybind/mgr/cephadm/tests/services/test_mgr.py b/src/pybind/mgr/cephadm/tests/services/test_mgr.py new file mode 100644 index 00000000000..591a79ecc72 --- /dev/null +++ b/src/pybind/mgr/cephadm/tests/services/test_mgr.py @@ -0,0 +1,67 @@ +from unittest import mock + +from cephadm.module import CephadmOrchestrator +from cephadm.services.cephadmservice import ( + CephadmDaemonDeploySpec, + MgrService, +) + + +class TestMgrService: + def _prepare(self, cephadm_module: CephadmOrchestrator, + mgr_services_payload: str, + carried_over_ports: list) -> CephadmDaemonDeploySpec: + """Build a daemon_spec that mimics rehydration from the persisted + DaemonDescription (which copies `ports=dd.ports`), then call + MgrService.prepare_create with `mgr services` returning the supplied + payload. + """ + svc = MgrService(cephadm_module) + daemon_spec = CephadmDaemonDeploySpec( + host='ceph-1', + daemon_id='ceph-1.xyz', + service_name='mgr', + daemon_type='mgr', + ports=list(carried_over_ports), + ) + with mock.patch.object(cephadm_module, 'check_mon_command', + return_value=(0, mgr_services_payload, '')), \ + mock.patch.object(svc, 'get_keyring_with_caps', + return_value='[mgr.ceph-1.xyz]\n\tkey = X\n'), \ + mock.patch.object(svc, 'generate_config', + return_value=({}, [])): + return svc.prepare_create(daemon_spec) + + def test_prepare_create_empty_mgr_services_discards_carryover( + self, cephadm_module: CephadmOrchestrator): + # Regression for https://tracker.ceph.com/issues/76564 + # When `mgr services` returns no endpoints (e.g. dashboard module not + # yet listening during an upgrade), the carried-over daemon_spec.ports + # list from the persisted DaemonDescription must be discarded; the + # final list must contain exactly one service_discovery_port entry. + carried = [ + cephadm_module.service_discovery_port, + cephadm_module.service_discovery_port, + cephadm_module.service_discovery_port, + cephadm_module.service_discovery_port, + cephadm_module.service_discovery_port, + ] + out = self._prepare(cephadm_module, '', carried) + assert out.ports == [cephadm_module.service_discovery_port] + + def test_prepare_create_with_mgr_services_includes_module_port( + self, cephadm_module: CephadmOrchestrator): + # When `mgr services` reports a module endpoint (e.g. prometheus on + # 9283), the result must include that port plus exactly one + # service_discovery_port — with no duplicates from the carried-over + # daemon_spec.ports. + carried = [ + cephadm_module.service_discovery_port, + cephadm_module.service_discovery_port, + ] + out = self._prepare( + cephadm_module, + '{"prometheus": "http://192.0.2.10:9283/"}', + carried, + ) + assert out.ports == [9283, cephadm_module.service_discovery_port] -- 2.47.3