From 1ce2042e141a6971573de452fc207f2cc882ca21 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 02:07:02 -0400 Subject: [PATCH] fix(mesh): also scope the reconciler's observed membership to the own cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 3 review: scoping only the rows/enabled DB queries by ClusterId left the observed set (from Cluster.State.Members) filtered by the driver role alone. In the mixed-mesh window — DB scoping live via a cluster role, but the physical Akka mesh not yet split so central still sees foreign members via gossip — a foreign-cluster driver member stays in observed while its row is filtered out, hitting the RunningNodeHasNoRow branch, which logs at ERROR. That traded the Warning-level false positive for a worse Error-level one. Filter observed by the node's own cluster ROLE too. Extracted the projection into a static FilterOwnClusterDriverMembers(members, ownClusterRole) so the membership filter is unit-testable without seeding a live cluster (the actor harness joins as admin only, so observed is always empty). Sourced at registration from IClusterRoleInfo.ClusterRole alongside ClusterId. Null role (legacy, no cluster role) keeps every driver member — reconcile-all, unchanged. Also clarified the ownClusterId doc wording (non-null-and-non-empty). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../Fleet/ClusterNodeAddressReconciler.cs | 21 +++-- .../ClusterNodeAddressReconcilerActor.cs | 62 ++++++++++--- .../ServiceCollectionExtensions.cs | 14 ++- .../ClusterNodeAddressReconcilerTests.cs | 91 +++++++++++++++++++ 4 files changed, 165 insertions(+), 23 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs index de4972dc..6ae0d5dd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs @@ -61,13 +61,20 @@ public sealed record ClusterNodeAddress(string NodeId, string Host, int AkkaPort /// Scoped to the admin node's own cluster (Phase 6). Once the fleet splits into one /// mesh per cluster, an admin node no longer sees site members at all, and a fleet-wide sweep /// would report every other cluster's row as -/// forever. The actor therefore loads -/// only rows whose ClusterId matches the admin node's own -/// () before -/// calling , so it never false-reports a cluster it cannot observe. A -/// legacy admin node carrying no cluster role (null ClusterId) still sees every member -/// via gossip and reconciles the full fleet unchanged. The pure function below is untouched by -/// this — scoping happens entirely in the actor's DB queries. +/// forever. The actor therefore scopes +/// both inputs to this function to the admin node's own cluster: the rows/enabled +/// set to rows whose ClusterId matches +/// , and the +/// observed membership to members carrying its own +/// +/// (FilterOwnClusterDriverMembers). Scoping only the rows would swap the +/// false positive for a worse +/// Error-level during the mixed-mesh +/// window, where the DB scope is live but central still sees foreign members via gossip. A +/// legacy admin node carrying no cluster role (null ClusterId/ClusterRole) still +/// sees every member via gossip and reconciles the full fleet unchanged. The pure function +/// below is untouched — scoping happens entirely in the actor's DB queries and its +/// membership filter. /// /// public static class ClusterNodeAddressReconciler diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs index cc546e60..61ebbc3d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs @@ -31,6 +31,7 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer private readonly IDbContextFactory _dbFactory; private readonly string? _ownClusterId; + private readonly string? _ownClusterRole; private readonly Akka.Cluster.Cluster _cluster; private readonly TimeSpan _sweepInterval; private readonly ILoggingAdapter _log = Context.GetLogger(); @@ -43,33 +44,47 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer /// Factory for the config database context. /// /// The admin node's own ClusterId (e.g. MAIN), from - /// . Non-null/empty scopes the reconcile to rows in that - /// cluster — the only members central's own mesh can see after the Phase 6 split. Null/empty - /// (a legacy admin node with no cluster role) reconciles the whole fleet, unchanged. + /// . When non-null-and-non-empty this scopes the row + /// side of the reconcile (rows/enabled) to that cluster — the only members + /// central's own mesh can see after the Phase 6 split. A null or empty value (a legacy admin + /// node with no cluster role) reconciles the whole fleet, unchanged. + /// + /// + /// The admin node's own cluster role (e.g. cluster-MAIN), from + /// . Scopes the membership (observed) side to + /// match the row side, so a foreign-cluster driver member central still sees via gossip in the + /// mixed-mesh window is not mis-reported. Null/empty keeps every driver member (legacy). /// /// Periodic sweep interval; defaults to . /// Props for actor creation. public static Props Props( IDbContextFactory dbFactory, string? ownClusterId = null, + string? ownClusterRole = null, TimeSpan? sweepInterval = null) => Akka.Actor.Props.Create(() => - new ClusterNodeAddressReconcilerActor(dbFactory, ownClusterId, sweepInterval)); + new ClusterNodeAddressReconcilerActor(dbFactory, ownClusterId, ownClusterRole, sweepInterval)); /// Initializes a new instance of the class. /// Factory for the config database context. /// - /// The admin node's own ClusterId; scopes the reconcile to that cluster's rows when - /// non-null/empty, else reconciles the whole fleet (legacy behaviour). + /// The admin node's own ClusterId; scopes the row side to that cluster's rows when + /// non-null-and-non-empty, else reconciles the whole fleet (legacy behaviour). + /// + /// + /// The admin node's own cluster role; scopes the observed-membership side to that role when + /// non-null-and-non-empty, else keeps every driver member (legacy behaviour). /// /// Periodic sweep interval; defaults to . public ClusterNodeAddressReconcilerActor( IDbContextFactory dbFactory, string? ownClusterId = null, + string? ownClusterRole = null, TimeSpan? sweepInterval = null) { _dbFactory = dbFactory; _ownClusterId = string.IsNullOrEmpty(ownClusterId) ? null : ownClusterId; + _ownClusterRole = string.IsNullOrEmpty(ownClusterRole) ? null : ownClusterRole; _cluster = Akka.Cluster.Cluster.Get(Context.System); _sweepInterval = sweepInterval ?? DefaultSweepInterval; @@ -93,11 +108,7 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer private void Reconcile() { - var observed = _cluster.State.Members - .Where(m => m.Status == MemberStatus.Up && m.Roles.Contains(DriverRole)) - .Select(m => (Host: m.Address.Host ?? string.Empty, Port: m.Address.Port ?? 0)) - .Where(a => !string.IsNullOrWhiteSpace(a.Host) && a.Port > 0) - .ToList(); + var observed = FilterOwnClusterDriverMembers(_cluster.State.Members, _ownClusterRole); List rows; List enabled; @@ -151,6 +162,35 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer } } + /// + /// Projects the Up, driver-role members central should reconcile against, scoped to the admin + /// node's OWN cluster role when it carries one. Extracted so the observed-membership filter — + /// the membership half of the Phase 6 own-cluster scope, mirroring the ClusterId row + /// scope in — is unit-testable without seeding a live cluster. + /// + /// The live cluster members (from Cluster.State.Members). + /// + /// The admin node's own cluster role (e.g. cluster-MAIN). When non-null-and-non-empty, + /// driver members that do not also carry it are excluded — matching the row scope so a foreign + /// driver member central still sees via gossip in the mixed-mesh window (row filtered out, but + /// member still visible) is NOT mis-reported as an Error-level RunningNodeHasNoRow. A + /// null or empty value (a legacy node with no cluster role) keeps every driver member — + /// reconcile-all, unchanged. + /// + /// The host/port of each in-scope Up driver member. + public static IReadOnlyList<(string Host, int Port)> FilterOwnClusterDriverMembers( + IEnumerable members, string? ownClusterRole) + { + var driverMembers = members + .Where(m => m.Status == MemberStatus.Up && m.Roles.Contains(DriverRole)); + if (!string.IsNullOrEmpty(ownClusterRole)) + driverMembers = driverMembers.Where(m => m.Roles.Contains(ownClusterRole)); + return driverMembers + .Select(m => (Host: m.Address.Host ?? string.Empty, Port: m.Address.Port ?? 0)) + .Where(a => !string.IsNullOrWhiteSpace(a.Host) && a.Port > 0) + .ToList(); + } + /// Self-message: run a reconcile pass now. public sealed class ReconcileNow { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index f1baa621..86788b02 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -153,12 +153,16 @@ public static class ServiceCollectionExtensions (system, registry, resolver) => { var dbFactory = resolver.GetService>(); - // Per-cluster mesh Phase 6: scope the reconcile to the admin node's OWN cluster's rows. - // After the mesh split central sees only its own pair via gossip, so a fleet-wide sweep - // would false-report every other cluster's rows as EnabledRowNotInCluster forever. A - // legacy node with no cluster role has null ClusterId ⇒ full-fleet reconcile, unchanged. + // Per-cluster mesh Phase 6: scope BOTH halves of the reconcile to the admin node's OWN + // cluster — rows by ClusterId, observed membership by cluster role. After the mesh split + // central sees only its own pair via gossip, so an unscoped sweep would false-report + // every other cluster's rows (EnabledRowNotInCluster). Scoping only the rows would swap + // that for a WORSE Error-level RunningNodeHasNoRow in the mixed-mesh window (rows already + // filtered, but foreign members still visible via gossip). A legacy node with no cluster + // role has null ClusterId/ClusterRole ⇒ full-fleet reconcile, unchanged. var roleInfo = resolver.GetService(); - return ClusterNodeAddressReconcilerActor.Props(dbFactory, roleInfo.ClusterId); + return ClusterNodeAddressReconcilerActor.Props( + dbFactory, roleInfo.ClusterId, roleInfo.ClusterRole); }, singletonOptions, createProxyToo: false); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerTests.cs index 3d5416f0..022abd72 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerTests.cs @@ -1,3 +1,8 @@ +using System.Collections.Immutable; +using System.Reflection; +using Akka.Actor; +using Akka.Cluster; +using Akka.Util; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.ControlPlane.Fleet; @@ -123,4 +128,90 @@ public sealed class ClusterNodeAddressReconcilerTests [("SITE-A-1", 4053)], [Row("site-a-1", 4053)], ["site-a-1:4053"]) .ShouldBeEmpty(); } + + private static int _memberUid; + + // Akka's Member.Create factories are internal, so reflect the Up-status overload + // Create(UniqueAddress, int upNumber, MemberStatus, ImmutableHashSet roles, AppVersion). + private static readonly MethodInfo MemberCreate = typeof(Member).GetMethod( + "Create", + BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public, + binder: null, + types: + [ + typeof(UniqueAddress), typeof(int), typeof(MemberStatus), + typeof(ImmutableHashSet), typeof(AppVersion), + ], + modifiers: null) + ?? throw new InvalidOperationException("Akka.Cluster.Member.Create(5-arg) not found — Akka API changed"); + + /// + /// Builds an Up cluster at : + /// carrying , for exercising + /// directly. + /// + private static Member UpMember(string host, int port, params string[] roles) => + (Member)MemberCreate.Invoke(null, + [ + new UniqueAddress(new Address("akka.tcp", "otopcua", host, port), Interlocked.Increment(ref _memberUid)), + 1, + MemberStatus.Up, + roles.ToImmutableHashSet(), + AppVersion.Zero, + ])!; + + /// + /// Per-cluster mesh Phase 6: scoped to cluster-MAIN, a driver member carrying a FOREIGN + /// cluster role (cluster-SITE-A) — the mixed-mesh window, where central still sees it via + /// gossip while its row is already filtered out by ClusterId — is excluded from observed. Left + /// in, it would hit the Error-level branch. + /// + [Fact] + public void Foreign_cluster_role_member_is_excluded_when_scoped() + { + var observed = ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers( + [UpMember("site-a-1", 4053, "driver", "cluster-SITE-A")], + ownClusterRole: "cluster-MAIN"); + + observed.ShouldBeEmpty(); + } + + /// Scoped to cluster-MAIN, a member carrying that same role is kept. + [Fact] + public void Own_cluster_role_member_is_kept_when_scoped() + { + var observed = ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers( + [UpMember("main-1", 4053, "driver", "cluster-MAIN")], + ownClusterRole: "cluster-MAIN"); + + observed.ShouldHaveSingleItem().ShouldBe(("main-1", 4053)); + } + + /// + /// Legacy: a null cluster role keeps every Up driver member regardless of cluster — an admin + /// node with no cluster role genuinely sees the whole fleet via gossip (reconcile-all). + /// + [Fact] + public void Null_cluster_role_keeps_all_driver_members() + { + var observed = ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers( + [ + UpMember("main-1", 4053, "driver", "cluster-MAIN"), + UpMember("site-a-1", 4053, "driver", "cluster-SITE-A"), + ], + ownClusterRole: null); + + observed.Count.ShouldBe(2); + observed.ShouldContain(("main-1", 4053)); + observed.ShouldContain(("site-a-1", 4053)); + } + + /// Non-driver members (e.g. admin-only) are excluded whether or not a scope is set. + [Fact] + public void Non_driver_members_are_always_excluded() + { + ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers( + [UpMember("central-1", 4053, "admin", "cluster-MAIN")], ownClusterRole: "cluster-MAIN") + .ShouldBeEmpty(); + } }