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 <yazulay@redhat.com>
)
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:
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(
# 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.
# 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
+import json
from unittest import mock
from cephadm.module import CephadmOrchestrator
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(
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=({}, [])):
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
projects / ceph.git / commitdiff