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 <Shweta.Bhosale1@ibm.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>

diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index 3f1c63bcda114821d6aecda836a2ebfbd1dcb017..26970f4368d5c86fca42b7d4bbca711586f43a6c 100644 (file)
--- 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 a9092ebd7696f47ca2da8ea6e3b0790865efb6fb..2e15cdfa4b1143f8fb7e5f028200e42152e90660 100644 (file)
--- 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")