From: John Mulligan Date: Wed, 11 Mar 2026 19:44:39 +0000 (-0400) Subject: mgr/cephadm: add test coverage for action chosen in _check_daemons X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=03f137e7c1b2b16c4ec61de8de6c4f02e50dbd56;p=ceph.git mgr/cephadm: add test coverage for action chosen in _check_daemons Add a test that verifies that the special cases for the ingress daemon when fronting nfs choice of action is handled. Fix the error in the current code block so that the test passes. I have discussed this with Shweta, the author of this code block, and we agree it should have been `last_deps`. NOTE: While this test tries to assert the correct action is chosen via a mock, I have also examined that it takes the correct code path when by viewing coverage results locally. Co-authored-by: Shweta Bhosale Signed-off-by: John Mulligan --- diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 3f1c63bcda11..26970f4368d5 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -1181,7 +1181,7 @@ class CephadmServe: backend_spec = self.mgr.spec_store[spec.backend_service].spec if backend_spec.service_type == 'nfs': svc = service_registry.get_service('ingress') - if svc.has_placement_changed(deps, spec): + if svc.has_placement_changed(last_deps, spec): self.log.debug(f'Redeploy {spec.service_name()} as placement has changed') action = 'redeploy' elif spec is not None and hasattr(spec, 'extra_container_args') and dd.extra_container_args != spec.extra_container_args: diff --git a/src/pybind/mgr/cephadm/tests/services/test_nfs.py b/src/pybind/mgr/cephadm/tests/services/test_nfs.py index a9092ebd7696..2e15cdfa4b11 100644 --- a/src/pybind/mgr/cephadm/tests/services/test_nfs.py +++ b/src/pybind/mgr/cephadm/tests/services/test_nfs.py @@ -504,3 +504,52 @@ def test_nfs_choose_next_action(cephadm_module, mock_cephadm): # NB: it appears that the code is designed to redeploy unless all # dependencies are prefixed with 'kmip' but I can't find any code # that would produce any dependencies prefixed with 'kmip'! + + +@patch("cephadm.services.nfs.NFSService.run_grace_tool", MagicMock()) +@patch("cephadm.services.nfs.NFSService.purge", MagicMock()) +@patch("cephadm.services.nfs.NFSService.create_rados_config_obj", MagicMock()) +def test_ingress_for_nfs_choose_next_action(cephadm_module, mock_cephadm): + nfs_spec = NFSServiceSpec( + service_id="foo", + placement=PlacementSpec(hosts=['test']), + ) + ingress_spec = IngressSpec( + service_id='bar', + backend_service='nfs.foo', + frontend_port=2468, + monitor_port=8642, + virtual_ip='1.2.3.0/24', + placement=PlacementSpec(hosts=['test']), + ) + with contextlib.ExitStack() as stack: + stack.enter_context(with_host(cephadm_module, "test")) + stack.enter_context(with_host(cephadm_module, "test2")) + cephadm_module.cache.update_host_networks( + 'test', + { + '1.2.3.0/24': { + 'if0': [ + '1.2.3.4', # simulate already assigned VIP + '1.2.3.1', # simulate interface IP + ] + } + }, + ) + stack.enter_context(with_service(cephadm_module, nfs_spec)) + # For whatever reason this mocked out version of the nfs deamon + # doesn't get assigned ports and ingress needs them. So we + # manually help it along here. + nfs_dds = cephadm_module.cache.get_daemons_by_service( + ingress_spec.backend_service + ) + nfs_dd = nfs_dds[0] + nfs_dd.ports = [8765] + mock_cephadm.serve(cephadm_module)._check_daemons() + stack.enter_context(with_service(cephadm_module, ingress_spec)) + ingress_spec.placement = PlacementSpec(hosts=['test', 'test2']) + cephadm_module.apply([ingress_spec]) + # manually invoke _check_daemons to trigger a call to + # _daemon_action so we can check what action was chosen + mock_cephadm.serve(cephadm_module)._check_daemons() + mock_cephadm._daemon_action.assert_called_with(ANY, action="redeploy")