From: Yael Azulay Date: Thu, 21 May 2026 11:38:28 +0000 (+0300) Subject: cephadm: keep mgr ports in sync when modules are enabled/disabled X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1966b0c91755ab018dd9b43bfbdf72f9405f4761;p=ceph.git cephadm: keep mgr ports in sync when modules are enabled/disabled ceph orch ps - was showing stale or incorrect port information for `mgr` daemons after enabling or disabling mgr modules (e.g., dashboard, prometheus). This fix ensures the displayed ports always reflect the actual active module endpoints. Code Fixes: ----------- Changes in src/pybind/mgr/cephadm/services/cephadmservice.py: - Extract and refactor _get_mgr_service_ports: The logic for reading active mgr module ports was inline inside prepare_create, using check_mon_command({'prefix': 'mgr services'}). It was extracted into a static helper method and refactored to read directly from mgr.get('mgr_map')['services'] instead -- which is faster and consistent with how get_dependencies reads the same data. The method is now shared by both prepare_create (existing use) and get_dependencies (new use). - Extract `_get_mgr_service_ports`: The logic for reading active mgr module ports from `mgr_map['services']` was inline inside `prepare_create`. It was extracted into a static helper method so it can be reused by both `prepare_create` (existing use) and `get_dependencies` (new use). Returns the port numbers currently registered by active mgr modules (e.g., `[8443, 9283]`). - Override `get_dependencies` in `MgrService`: The base class returns `[]` for all Ceph daemons. The serve loop compares `get_dependencies()` output against `last_deps` (saved after the previous reconfig). If they differ, a reconfig is triggered. By overriding this method in `MgrService`, we make the serve loop aware of mgr port changes. Returns a sorted list such as `['port:8443', 'port:9283', 'sd_port:8765']`. - Override `generate_config` in `MgrService`: After a reconfig completes, the result of `generate_config` is saved as `last_deps`. The inherited implementation returns `[]` as deps. If `[]` is saved but `get_dependencies` returns `['port:8443', 'sd_port:8765']`, they would always differ, causing an infinite reconfig loop. This override ensures that what is saved as `last_deps` matches what `get_dependencies` will return in the next iteration. Changes in `src/pybind/mgr/cephadm/serve.py`: - Update mgr cache on reconfig: The existing logic skipped the cache update for all Ceph daemon reconfigs. A new `elif` branch for mgr reconfigs fetches the existing cached `DaemonDescription`, updates only its `ports` field, and saves it back. This makes `ceph orch ps` reflect the correct ports immediately after a reconfig, without showing a misleading "starting" status. Changes in `src/cephadm/cephadm.py`: - Update `unit.meta` on reconfig: `unit.meta` is a per-daemon file on the host that stores metadata including ports. It is read periodically by `cephadm ls` and fed back into the orchestrator cache. Previously it was only written on initial deployment, so every periodic cache refresh would overwrite the corrected ports with stale data from `unit.meta`. This change updates the `ports` field in `unit.meta` during reconfig, ensuring the cache refresh confirms the correct state rather than reverting it. Changes to src/pybind/mgr/cephadm/tests/services/test_mgr.py: - Updated _prepare helper: The original mocked check_mon_command to feed services data to prepare_create. Since our _get_mgr_service_ports now reads from mgr.get('mgr_map') instead of check_mon_command, the mock was replaced with mock_store_set('_ceph_get', 'mgr_map', ...) which sets the data in the right place. - Added test_get_dependencies_changes_when_module_enabled: A new test that verifies get_dependencies returns different dep strings before and after a module is enabled. This directly tests the core sync mechanism -- the serve loop compares get_dependencies output against last_deps to detect port changes and trigger a reconfig. to run new tests: cd src/pybind/mgr tox -e py3 -- cephadm/tests/services/test_mgr.py -v Fixes: https://tracker.ceph.com/issues/76565 Signed-off-by: Yael Azulay --- diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py index c822efa3f6c..f85dc929711 100755 --- a/src/cephadm/cephadm.py +++ b/src/cephadm/cephadm.py @@ -1189,6 +1189,19 @@ def deploy_daemon( ) else: raise RuntimeError('attempting to deploy a daemon without a container image') + else: + # On reconfig, update unit.meta so that port metadata + # stays current without requiring a full redeploy. + meta_path = os.path.join(data_dir, 'unit.meta') + ports = [e.port for e in endpoints] if endpoints else [] + try: + update_meta_file(meta_path, {'ports': ports}) + except FileNotFoundError: + logger.warning(f'unit.meta not found at {meta_path}, skipping port update') + except Exception as e: + logger.warning( + f'failed to update unit.meta at {meta_path}, skipping port update: {e}' + ) if not os.path.exists(data_dir + '/unit.created'): with write_new(data_dir + '/unit.created', owner=(uid, gid)) as f: diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 8f1af9b18b0..4b4b60a09b8 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1543,6 +1543,14 @@ class CephadmServe: sd.update_pending_daemon_config(True) self.mgr.cache.add_daemon(daemon_spec.host, sd) self.mgr.cache.invalidate_host_daemons(daemon_spec.host) + elif reconfig and daemon_spec.daemon_type == 'mgr': + cache_host = daemon_spec.host + if not code and self.mgr.cache.has_daemon(daemon_spec.name()): + existing_dd = self.mgr.cache.get_daemon(daemon_spec.name()) + existing_dd.ports = daemon_spec.ports + cache_host = existing_dd.hostname or daemon_spec.host + self.mgr.cache.add_daemon(cache_host, existing_dd) + self.mgr.cache.invalidate_host_daemons(cache_host) if daemon_spec.daemon_type != 'agent': self.mgr.cache.update_daemon_config_deps( diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index 649474c3be5..2a3591f9834 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -1231,6 +1231,30 @@ class MgrService(CephService): # are not a concern. return True + @staticmethod + def _get_mgr_service_ports(mgr: "CephadmOrchestrator") -> List[int]: + """Return the list of ports from the mgr map services.""" + ports: List[int] = [] + mgr_map = mgr.get('mgr_map') + for end_point in mgr_map.get('services', {}).values(): + port = re.search(r'\:(\d+)\/', end_point) + if port: + ports.append(int(port.group(1))) + return ports + + @classmethod + def get_dependencies(cls, mgr: "CephadmOrchestrator", + spec: Optional[ServiceSpec] = None, + daemon_type: Optional[str] = None) -> List[str]: + return sorted( + [f'port:{p}' for p in cls._get_mgr_service_ports(mgr)] + + [f'sd_port:{mgr.service_discovery_port}'] + ) + + def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[str, Any], List[str]]: + config, _ = super().generate_config(daemon_spec) + return config, self.get_dependencies(self.mgr) + def prepare_create(self, daemon_spec: CephadmDaemonDeploySpec) -> CephadmDaemonDeploySpec: """ Create a new manager instance on a host. @@ -1250,17 +1274,7 @@ class MgrService(CephService): # user has decided to use different dashboard ports in each server # If this is the case then the dashboard port opened will be only the used # as default. - ports = [] - ret, mgr_services, err = self.mgr.check_mon_command({ - 'prefix': 'mgr services', - }) - if mgr_services: - mgr_endpoints = json.loads(mgr_services) - for end_point in mgr_endpoints.values(): - port = re.search(r'\:\d+\/', end_point) - if port: - ports.append(int(port[0][1:-1])) - + ports = self._get_mgr_service_ports(self.mgr) # Always replace ports (do not append onto a list rehydrated from the # persisted host cache). When ``mgr services`` is empty, ``ports`` is # empty and we must not retain old entries + append service discovery diff --git a/src/pybind/mgr/cephadm/tests/services/test_mgr.py b/src/pybind/mgr/cephadm/tests/services/test_mgr.py index 591a79ecc72..6a01a9aee68 100644 --- a/src/pybind/mgr/cephadm/tests/services/test_mgr.py +++ b/src/pybind/mgr/cephadm/tests/services/test_mgr.py @@ -1,3 +1,4 @@ +import json from unittest import mock from cephadm.module import CephadmOrchestrator @@ -13,8 +14,8 @@ class TestMgrService: 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. + MgrService.prepare_create with mgr_map returning the supplied + services payload. """ svc = MgrService(cephadm_module) daemon_spec = CephadmDaemonDeploySpec( @@ -24,9 +25,9 @@ class TestMgrService: 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', + services = json.loads(mgr_services_payload) if mgr_services_payload else {} + cephadm_module.mock_store_set('_ceph_get', 'mgr_map', {'services': services}) + with 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=({}, [])): @@ -65,3 +66,25 @@ class TestMgrService: carried, ) assert out.ports == [9283, cephadm_module.service_discovery_port] + + def test_get_dependencies_changes_when_module_enabled( + self, cephadm_module: CephadmOrchestrator): + # Verify that get_dependencies reflects the current active modules. + # This is the mechanism that causes the serve loop to detect port + # changes and trigger a reconfig when modules are enabled/disabled. + + # Before: no modules enabled + cephadm_module.mock_store_set('_ceph_get', 'mgr_map', {'services': {}}) + deps_before = MgrService.get_dependencies(cephadm_module) + assert deps_before == [f'sd_port:{cephadm_module.service_discovery_port}'] + + # After: prometheus enabled + cephadm_module.mock_store_set('_ceph_get', 'mgr_map', { + 'services': {'prometheus': 'http://192.0.2.10:9283/'} + }) + deps_after = MgrService.get_dependencies(cephadm_module) + assert deps_after == ['port:9283', f'sd_port:{cephadm_module.service_discovery_port}'] + + # The difference between deps_before and deps_after is what the serve + # loop detects -- this is what triggers the reconfig + assert deps_before != deps_after