mgr/cephadm: only add deps for set NFS spec fields, skip upgrade reconfig for defaults

Only add deps when the user has set a non-default value.  In
choose_next_action(), ignore False/None-valued entries in the symmetric
diff so daemons upgrading from the old behavior are not reconfigured
solely because default deps disappeared

Fixes: https://tracker.ceph.com/issues/77901
Signed-off-by: Shweta Bhosale <Shweta.Bhosale1@ibm.com>

diff --git a/src/pybind/mgr/cephadm/services/nfs.py b/src/pybind/mgr/cephadm/services/nfs.py
index 8227e030965333eec2c3e99aa3a184a21e7b626f..128237fab4910a10d7b5089b58219c593c35fac6 100644 (file)
--- a/src/pybind/mgr/cephadm/services/nfs.py
+++ b/src/pybind/mgr/cephadm/services/nfs.py
@@ -130,12 +130,23 @@ class NFSService(CephService):
         assert spec
         deps: List[str] = []
         nfs_spec = cast(NFSServiceSpec, spec)
-        deps.append(f'enable_rdma: {nfs_spec.enable_rdma}')
-        deps.append(f'rdma_port: {nfs_spec.rdma_port}')
-        deps.append(f'tls_ktls: {nfs_spec.tls_ktls}')
-        deps.append(f'tls_debug: {nfs_spec.tls_debug}')
-        deps.append(f'tls_min_version: {nfs_spec.tls_min_version}')
-        deps.append(f'tls_ciphers: {nfs_spec.tls_ciphers}')
+        # choose_next_action() ignores False/None in the symmetric diff, so
+        # False <-> None transitions do not trigger reconfig or redeploy.
+
+        # RDMA related
+        if nfs_spec.enable_rdma:
+            deps.append(f'enable_rdma: {nfs_spec.enable_rdma}')
+            if nfs_spec.rdma_port is not None:
+                deps.append(f'rdma_port: {nfs_spec.rdma_port}')
+        # TLS related
+        if nfs_spec.tls_ktls:
+            deps.append(f'tls_ktls: {nfs_spec.tls_ktls}')
+        if nfs_spec.tls_debug:
+            deps.append(f'tls_debug: {nfs_spec.tls_debug}')
+        if nfs_spec.tls_min_version is not None:
+            deps.append(f'tls_min_version: {nfs_spec.tls_min_version}')
+        if nfs_spec.tls_ciphers is not None:
+            deps.append(f'tls_ciphers: {nfs_spec.tls_ciphers}')
         parent_deps = super().get_dependencies(mgr, spec, daemon_type)
         return sorted(deps + parent_deps)
 
@@ -585,9 +596,20 @@ class NFSService(CephService):
         current and previous dependency lists return the next action that
         this service would prefer cephadm take.
         """
+        def is_default_value_dep(dep: str) -> bool:
+            # Ignoring False and None as both represent an unset/default option
+            if ':' not in dep:
+                return False
+            return dep.split(':', 1)[1].strip() in ('False', 'None')
+
         if curr_deps == last_deps:
             return utils.NextDaemonStep(scheduled_action)
-        sym_diff = set(curr_deps).symmetric_difference(last_deps)
+        sym_diff = {
+            d for d in set(curr_deps).symmetric_difference(last_deps)
+            if not is_default_value_dep(d)
+        }
+        if not sym_diff:
+            return utils.NextDaemonStep(scheduled_action)
         logger.info(
             'Reconfigure wanted %s: deps %r -> %r (diff %r)',
             spec.service_name() if spec else daemon_type,

diff --git a/src/pybind/mgr/cephadm/tests/services/test_nfs.py b/src/pybind/mgr/cephadm/tests/services/test_nfs.py
index 0094aad24f510bcc0a1fb266259b653ef01815c4..0c55e40911f581690da4f37dc9d0200c2378b2ef 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/services/test_nfs.py
+++ b/src/pybind/mgr/cephadm/tests/services/test_nfs.py
@@ -10,6 +10,7 @@ from orchestrator import DaemonDescriptionStatus
 from cephadm.serve import CephadmServe
 from cephadm.services.service_registry import service_registry
 from cephadm.services.cephadmservice import CephadmDaemonDeploySpec
+from cephadm import utils
 from cephadm.module import CephadmOrchestrator
 from ceph.deployment.service_spec import (
     NFSServiceSpec,
@@ -846,3 +847,35 @@ def test_check_daemons_starts_keepalived_when_stopped_and_haproxy_running(
             for c in mock_cephadm._daemon_action.call_args_list
         )
         assert keepalived_started
+
+
+def test_nfs_choose_next_action_skips_legacy_default_deps():
+    nfs_svc = service_registry.get_service('nfs')
+    legacy_deps = [
+        'enable_rdma: False',
+        'rdma_port: None',
+        'tls_ktls: False',
+        'tls_debug: False',
+        'tls_min_version: None',
+        'tls_ciphers: None',
+    ]
+    step = nfs_svc.choose_next_action(
+        utils.Action.NO_ACTION,
+        'nfs',
+        None,
+        curr_deps=[],
+        last_deps=legacy_deps,
+    )
+    assert step.action is utils.Action.NO_ACTION
+
+
+def test_nfs_choose_next_action_detects_explicit_value_change():
+    nfs_svc = service_registry.get_service('nfs')
+    step = nfs_svc.choose_next_action(
+        utils.Action.NO_ACTION,
+        'nfs',
+        None,
+        curr_deps=['tls_ktls: True'],
+        last_deps=['tls_ktls: False'],
+    )
+    assert step.action is utils.Action.REDEPLOY