mgr/smb: only resync clusters that reference a changed resource

_find_modifications() regenerated every cluster whenever a JoinAuth,
UsersAndGroups, TLSCredential, or ExternalCephCluster changed, since
these can be shared across clusters. Use the existing lookup
helpers to resync only the clusters that actually reference it.

For rgw creds resync everything(just like old way), as narrowing
it scope means scanning every share in the store (which may not scale well),
so not worth a change.

Fixes: https://tracker.ceph.com/issues/78245
Signed-off-by: Avan Thakkar <athakkar@redhat.com>

diff --git a/src/pybind/mgr/smb/handler.py b/src/pybind/mgr/smb/handler.py
index 12ea2e06eb3381e91124e6635649492fd01a2b73..c3d1edf14bf961204ca277994bee39d431068645 100644 (file)
--- a/src/pybind/mgr/smb/handler.py
+++ b/src/pybind/mgr/smb/handler.py
@@ -654,17 +654,28 @@ class ClusterConfigHandler:
             elif isinstance(result.src, resources.ExternalCephCluster):
                 chg_extc_ids.add(result.src.external_ceph_cluster_id)
 
-        # TODO: here's a lazy bit. if any join auths or users/groups changed we
-        # will regen all clusters because these can be shared by >1 cluster.
-        # In future, make this only pick clusters using the named resources.
-        if (
-            chg_join_ids
-            or chg_ug_ids
-            or chg_tls_ids
-            or chg_extc_ids
-            or chg_rgw_cred_ids
-        ):
+        # rgw credentials are referenced by shares, not clusters, so
+        # narrowing this would mean reading every share in the store.
+        # not worth it for an infrequent change. Keeping the broad regen
+        # until we find a better approach for rgw credentials.
+        if chg_rgw_cred_ids:
             chg_cluster_ids.update(ClusterEntry.ids(self.internal_store))
+        elif chg_join_ids or chg_ug_ids or chg_tls_ids or chg_extc_ids:
+            # these resource types can be shared by >1 cluster. only pick
+            # clusters that actually reference a changed one.
+            for cluster_id in ClusterEntry.ids(self.internal_store):
+                try:
+                    cluster = self._cluster_entry(cluster_id).get_cluster()
+                except KeyError:
+                    continue
+                if (
+                    set(auth_refs(cluster)) & chg_join_ids
+                    or set(ug_refs(cluster)) & chg_ug_ids
+                    or set(tls_refs(cluster)) & chg_tls_ids
+                    or set(ext_cluster_refs(cluster)) & chg_extc_ids
+                ):
+                    chg_cluster_ids.add(cluster_id)
+
         return chg_cluster_ids
 
     def _save_cluster_settings(

diff --git a/src/pybind/mgr/smb/tests/test_handler.py b/src/pybind/mgr/smb/tests/test_handler.py
index 1564605c65571625a4402ce9af42b4376ca76cac..9dbf074489448c328d3a04480cf4f74aab51be5b 100644 (file)
--- a/src/pybind/mgr/smb/tests/test_handler.py
+++ b/src/pybind/mgr/smb/tests/test_handler.py
@@ -1,3 +1,5 @@
+import time
+
 import pytest
 
 import smb
@@ -856,6 +858,164 @@ def test_modify_cluster_only_touches_changed_cluster(thandler):
     assert 'join.0.json' in ekeys
 
 
+def test_modify_joinauth_only_touches_referencing_clusters(thandler):
+    # clustera and clusterb both reference the shared (unlinked) join auth
+    # "shared1". clusterc uses its own, separate join auth. A change to
+    # shared1 must regenerate clustera and clusterb but must not touch
+    # clusterc.
+    to_apply = [
+        smb.resources.JoinAuth(
+            auth_id='shared1',
+            auth=smb.resources.JoinAuthValues(
+                username='testadmin',
+                password='Passw0rd',
+            ),
+        ),
+        smb.resources.JoinAuth(
+            auth_id='join2',
+            auth=smb.resources.JoinAuthValues(
+                username='otheradmin',
+                password='Passw0rd2',
+            ),
+        ),
+    ]
+    for cluster_id, ref in (
+        ('clustera', 'shared1'),
+        ('clusterb', 'shared1'),
+        ('clusterc', 'join2'),
+    ):
+        to_apply.append(
+            _cluster(
+                cluster_id=cluster_id,
+                auth_mode=smb.enums.AuthMode.ACTIVE_DIRECTORY,
+                domain_settings=smb.resources.DomainSettings(
+                    realm='MYDOMAIN.EXAMPLE.ORG',
+                    join_sources=[
+                        smb.resources.JoinSource(
+                            source_type=smb.enums.JoinSourceType.RESOURCE,
+                            ref=ref,
+                        ),
+                    ],
+                ),
+            )
+        )
+
+    results = thandler.apply(to_apply)
+    assert results.success, results.to_simplified()
+
+    for cluster_id in ('clustera', 'clusterb', 'clusterc'):
+        assert 'cluster-info' in list(
+            thandler.public_store.contents(cluster_id)
+        )
+        thandler.public_store.remove((cluster_id, 'cluster-info'))
+
+    # only modify the join auth shared by clustera and clusterb
+    results = thandler.apply(
+        [
+            smb.resources.JoinAuth(
+                auth_id='shared1',
+                auth=smb.resources.JoinAuthValues(
+                    username='testadmin',
+                    password='NewPassw0rd!',
+                ),
+            ),
+        ]
+    )
+    assert results.success, results.to_simplified()
+
+    assert 'cluster-info' in list(thandler.public_store.contents('clustera'))
+    assert 'cluster-info' in list(thandler.public_store.contents('clusterb'))
+    # clusterc doesn't reference shared1, so it must be untouched
+    assert 'cluster-info' not in list(
+        thandler.public_store.contents('clusterc')
+    )
+
+
+def test_modify_joinauth_large_cluster_set_performance(thandler):
+    # Ensure the resync loop over 20 clusters completes quickly when a shared
+    # join auth is modified (each cluster is deserialized to check references).
+    total_clusters = 20
+    shared_auth_id = 'shared1'
+    own_auth_id = 'own1'
+
+    # first 15 clusters share an auth; last 5 use their own
+    shared_ids = [f'cluster{i:02d}' for i in range(15)]
+    own_ids = [f'cluster{i:02d}' for i in range(15, total_clusters)]
+
+    def _ad_cluster(cluster_id, ref):
+        return _cluster(
+            cluster_id=cluster_id,
+            auth_mode=smb.enums.AuthMode.ACTIVE_DIRECTORY,
+            domain_settings=smb.resources.DomainSettings(
+                realm='MYDOMAIN.EXAMPLE.ORG',
+                join_sources=[
+                    smb.resources.JoinSource(
+                        source_type=smb.enums.JoinSourceType.RESOURCE,
+                        ref=ref,
+                    ),
+                ],
+            ),
+        )
+
+    to_apply = [
+        smb.resources.JoinAuth(
+            auth_id=shared_auth_id,
+            auth=smb.resources.JoinAuthValues(
+                username='testadmin',
+                password='Passw0rd',
+            ),
+        ),
+        smb.resources.JoinAuth(
+            auth_id=own_auth_id,
+            auth=smb.resources.JoinAuthValues(
+                username='otheradmin',
+                password='Passw0rd2',
+            ),
+        ),
+    ]
+    to_apply += [_ad_cluster(cid, shared_auth_id) for cid in shared_ids]
+    to_apply += [_ad_cluster(cid, own_auth_id) for cid in own_ids]
+
+    results = thandler.apply(to_apply)
+    assert results.success, results.to_simplified()
+
+    # clear cluster-info to detect which clusters get regenerated
+    for cid in shared_ids + own_ids:
+        thandler.public_store.remove((cid, 'cluster-info'))
+
+    # modify the shared join auth and measure how long the apply takes
+    t0 = time.monotonic()
+    results = thandler.apply(
+        [
+            smb.resources.JoinAuth(
+                auth_id=shared_auth_id,
+                auth=smb.resources.JoinAuthValues(
+                    username='testadmin',
+                    password='NewPassw0rd!',
+                ),
+            ),
+        ]
+    )
+    elapsed = time.monotonic() - t0
+
+    assert results.success, results.to_simplified()
+    # must complete quickly even when all 20 clusters are to be deserialized
+    assert (
+        elapsed < 1.0
+    ), f'apply took {elapsed:.3f}s with {total_clusters} clusters'
+
+    # only clusters referencing shared_auth_id must be regenerated
+    for cid in shared_ids:
+        assert 'cluster-info' in list(
+            thandler.public_store.contents(cid)
+        ), f'{cid} should have been regenerated'
+    # clusters using own_auth_id must not be touched
+    for cid in own_ids:
+        assert 'cluster-info' not in list(
+            thandler.public_store.contents(cid)
+        ), f'{cid} should not have been touched'
+
+
 def test_apply_remove_cluster(thandler):
     test_apply_full_cluster_create(thandler)
     assert ('clusters', 'mycluster1') in thandler.internal_store.data