From 2535df019b134d0914f90b58109ad66d4edc0e8e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 01:39:06 -0400 Subject: [PATCH] fix(mesh): scope ClusterNode address reconcile to the admin node's own cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 splits the fleet into one mesh per Cluster, so an admin (central) node's Cluster.State.Members shows only its own pair — it no longer sees site members via gossip. The ClusterNodeAddressReconciler singleton compared live membership against EVERY ClusterNode row fleet-wide, so post-split every foreign-cluster row would log EnabledRowNotInCluster forever. Scope the actor's two DB queries to rows whose ClusterId matches the admin node's own (IClusterRoleInfo.ClusterId, sourced at registration). A legacy admin node with no cluster role (null ClusterId) still reconciles the whole fleet — it genuinely sees every member via gossip. The pure Reconcile function and AddressMismatchKind semantics are unchanged; scoping lives entirely in the actor's queries. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- ...24-mesh-phase6-partition-and-colocation.md | 52 +++++++++---- ...se6-partition-and-colocation.md.tasks.json | 4 +- .../Fleet/ClusterNodeAddressReconciler.cs | 14 +++- .../ClusterNodeAddressReconcilerActor.cs | 34 ++++++-- .../ServiceCollectionExtensions.cs | 7 +- .../ClusterNodeAddressReconcilerActorTests.cs | 78 ++++++++++++++++--- 6 files changed, 149 insertions(+), 40 deletions(-) diff --git a/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md b/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md index e6efe04f..6ccfd179 100644 --- a/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md +++ b/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md @@ -176,10 +176,14 @@ DPS. **De-risked:** `RedundancyStateActor` has zero injected deps (verified — public static AkkaConfigurationBuilder WithOtOpcUaClusterRedundancySingleton( this AkkaConfigurationBuilder builder, IClusterRoleInfo roleInfo) { - var clusterRole = roleInfo.ClusterRole - ?? throw new InvalidOperationException( - "A driver node must carry a cluster-{ClusterId} role to host the redundancy singleton."); - var opts = new ClusterSingletonOptions { Role = clusterRole }; + // Cluster-scoped when the node carries a cluster-{ClusterId} role (Phase-6 split: one + // redundancy singleton per pair, robust even if two meshes accidentally merged). Falls + // back to the driver role for a legacy single-mesh node or a test harness with no cluster + // role — where driver-scope is the pre-Phase-6 fleet-wide behavior, and on a genuinely + // split 2-node mesh the driver role is ALREADY pair-local because the mesh IS the pair. + // No throw: decision 2 requires legacy/harness nodes to keep booting unchanged. + var role = roleInfo.ClusterRole ?? RoleParser.Driver; + var opts = new ClusterSingletonOptions { Role = role }; return builder.WithSingleton( RedundancyStateActor.Topic, RedundancyStateActor.Props(), opts /* createProxyToo per existing pattern */); } @@ -190,11 +194,12 @@ DPS. **De-risked:** `RedundancyStateActor` has zero injected deps (verified — `WithOtOpcUaRuntimeActors`), call `ab.WithOtOpcUaClusterRedundancySingleton(clusterRoleInfo)`. Resolve `IClusterRoleInfo` from `sp`. Ensure it runs on the fused central node too (central has `hasDriver=true`). Do NOT also register it in the `hasAdmin` block. -4. **Guard: a driver node with no cluster role must fail fast** (the new extension throws) — this is - caught more helpfully by the Task 5 validator, but the throw is the backstop. +4. **No fail-fast on a missing cluster role** — the fallback keeps legacy/harness nodes booting + (decision 2). A *misconfigured* Phase-6 node (split transports, no cluster role) is a non-issue for + the redundancy singleton: driver-scope is pair-local on a split mesh anyway. 5. Tests: assert the admin singleton set no longer contains `redundancy-state`; assert the new - extension registers a singleton scoped to `Role == clusterRole`; a node with null `ClusterRole` - throws. + extension registers a singleton scoped to `Role == "cluster-SITE-A"` when the roleInfo reports that; + assert that a roleInfo with null `ClusterRole` falls back to `Role == "driver"` (no throw). **Acceptance:** each mesh hosts exactly one `RedundancyStateActor` scoped to its cluster role; a driver-only site node elects its own Primary; admin path no longer owns it. Unit tests green. @@ -229,16 +234,29 @@ not assert anything about their rows. finding); an own-cluster enabled row with no member still yields `EnabledRowNotInCluster`; drift detection within own cluster still works. +**The pure `ClusterNodeAddressReconciler.Reconcile` function is UNCHANGED** — the fix is entirely in +the actor's two DB queries (`ClusterNodeAddressReconcilerActor.Reconcile`, lines 89-97), which filter +rows by the admin node's own `ClusterId` before projecting. `ClusterNode` has a `ClusterId` column. + **Steps:** -1. Thread the reconciler's own `ClusterId` (from `IClusterRoleInfo.ClusterId`, which for a fused - central node is `"MAIN"`) into `Reconcile`. Filter `rows` to `row.ClusterId == ownClusterId` before - any comparison. (`ClusterNode` has a `ClusterId` column — confirmed.) -2. Keep `RowDialTargetDisagrees` / `RunningNodeHasNoRow` / `EnabledRowNotInCluster` semantics for - own-cluster rows only. -3. If `IClusterRoleInfo.ClusterId` is null on an admin node (shouldn't happen post-Phase-6, but guard), - log one Warning and skip the sweep rather than reconciling the whole fleet. -4. Update the class doc: replace the deferred-to-Phase-6 note with the delivered behavior. -5. Tests as above. +1. Inject the admin node's own `ClusterId` into the actor: add `IClusterRoleInfo` (or just a + `string? ownClusterId`) to `Props`/ctor, sourced from `IClusterRoleInfo.ClusterId` at registration + (`ControlPlane/ServiceCollectionExtensions.cs` where the reconciler singleton is registered). +2. In the actor's `Reconcile`, add `.Where(n => n.ClusterId == ownClusterId)` to BOTH the `rows` and + `enabled` queries — but ONLY when `ownClusterId` is non-null. +3. **Null `ownClusterId` (legacy admin node with no cluster role) ⇒ reconcile ALL rows** — this is the + correct single-mesh behavior (a legacy admin node genuinely sees every driver member via gossip). + Do NOT skip. This preserves backward-compat, mirroring the Task 2 fallback philosophy. +4. `RowDialTargetDisagrees` / `RunningNodeHasNoRow` / `EnabledRowNotInCluster` semantics are unchanged; + they now simply operate on the own-cluster row subset (post-split) or the full set (legacy). +5. Update the class doc comment in `ClusterNodeAddressReconciler.cs:60-65`: replace the "Phase 2 must + revisit" note with the delivered own-cluster-scoped behavior. +6. Tests (`ClusterNodeAddressReconcilerActorTests` or the existing reconciler tests — pick the level + that can inject `ownClusterId`): a SITE-A row is **ignored** when `ownClusterId=="MAIN"` (no + `EnabledRowNotInCluster`); an own-cluster (MAIN) enabled row with no member still yields + `EnabledRowNotInCluster`; own-cluster drift still caught; `ownClusterId==null` reconciles the full + set (legacy). If the actor is hard to unit-test directly, a focused query-filter test + the + unchanged pure-function tests suffice — say which you chose. **Acceptance:** central reconciles only MAIN rows; site rows never produce false `EnabledRowNotInCluster`; own-cluster drift still caught. diff --git a/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md.tasks.json b/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md.tasks.json index afb2b443..8d29181d 100644 --- a/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md.tasks.json +++ b/docs/plans/2026-07-24-mesh-phase6-partition-and-colocation.md.tasks.json @@ -9,8 +9,8 @@ "rigScope": "OtOpcUa-only three 2-node meshes on one shared network" }, "tasks": [ - {"id": 0, "subject": "Task 0: RoleParser accepts cluster-{ClusterId}; consolidate driver literal", "classification": "small", "status": "pending"}, - {"id": 1, "subject": "Task 1: ClusterRoleInfo exposes node's own ClusterRole/ClusterId", "classification": "small", "status": "pending", "blockedBy": [0]}, + {"id": 0, "subject": "Task 0: RoleParser accepts cluster-{ClusterId}; consolidate driver literal", "classification": "small", "status": "completed", "commit": "87ff00fd"}, + {"id": 1, "subject": "Task 1: ClusterRoleInfo exposes node's own ClusterRole/ClusterId", "classification": "small", "status": "completed", "commit": "2d2f1dec", "note": "interface in Commons/Interfaces/IClusterRoleInfo.cs; ctor (ActorSystem, IOptions, ILogger)"}, {"id": 2, "subject": "Task 2: re-home RedundancyStateActor to cluster-role singleton on drivers", "classification": "high-risk", "status": "pending", "blockedBy": [1], "parallelizableWith": [3, 4]}, {"id": 3, "subject": "Task 3: re-scope ClusterNodeAddressReconciler to own-cluster rows", "classification": "standard", "status": "pending", "blockedBy": [1], "parallelizableWith": [2, 4]}, {"id": 4, "subject": "Task 4: one ClusterClient per cluster in CentralCommunicationActor", "classification": "high-risk", "status": "pending", "blockedBy": [], "parallelizableWith": [2, 3]}, 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 342408dc..de4972dc 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconciler.cs @@ -58,10 +58,16 @@ public sealed record ClusterNodeAddress(string NodeId, string Host, int AkkaPort /// to be deleted again. /// /// -/// Phase 2 must revisit this. Once the fleet splits into one mesh per cluster, an -/// admin node no longer sees site members at all, and every site row would report -/// forever. The check then has to -/// move to whatever transport replaces gossip — or be scoped to the central mesh. +/// 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. /// /// 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 bb7c476e..cc546e60 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Fleet/ClusterNodeAddressReconcilerActor.cs @@ -30,6 +30,7 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer public static readonly TimeSpan DefaultSweepInterval = TimeSpan.FromMinutes(5); private readonly IDbContextFactory _dbFactory; + private readonly string? _ownClusterId; private readonly Akka.Cluster.Cluster _cluster; private readonly TimeSpan _sweepInterval; private readonly ILoggingAdapter _log = Context.GetLogger(); @@ -40,19 +41,35 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer /// Creates Props for the reconciler. /// 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. + /// /// Periodic sweep interval; defaults to . /// Props for actor creation. public static Props Props( - IDbContextFactory dbFactory, TimeSpan? sweepInterval = null) => - Akka.Actor.Props.Create(() => new ClusterNodeAddressReconcilerActor(dbFactory, sweepInterval)); + IDbContextFactory dbFactory, + string? ownClusterId = null, + TimeSpan? sweepInterval = null) => + Akka.Actor.Props.Create(() => + new ClusterNodeAddressReconcilerActor(dbFactory, ownClusterId, 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). + /// /// Periodic sweep interval; defaults to . public ClusterNodeAddressReconcilerActor( - IDbContextFactory dbFactory, TimeSpan? sweepInterval = null) + IDbContextFactory dbFactory, + string? ownClusterId = null, + TimeSpan? sweepInterval = null) { _dbFactory = dbFactory; + _ownClusterId = string.IsNullOrEmpty(ownClusterId) ? null : ownClusterId; _cluster = Akka.Cluster.Cluster.Get(Context.System); _sweepInterval = sweepInterval ?? DefaultSweepInterval; @@ -87,13 +104,20 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer try { using var db = _dbFactory.CreateDbContext(); - rows = db.ClusterNodes.AsNoTracking() + // Per-cluster mesh Phase 6: scope to the admin node's OWN cluster's rows. After the split, + // central's mesh sees only its own pair, so a row in another cluster it cannot observe would + // false-report EnabledRowNotInCluster forever. A legacy admin node with no cluster role + // (_ownClusterId == null) still sees every member via gossip, so it reconciles the full fleet. + var scoped = db.ClusterNodes.AsNoTracking(); + if (_ownClusterId is not null) + scoped = scoped.Where(n => n.ClusterId == _ownClusterId); + rows = scoped .Select(n => new ClusterNodeAddress(n.NodeId, n.Host, n.AkkaPort)) .ToList(); // Same predicate the coordinator's expected-ack set uses — a node the deploy path will // not wait for must not be reported as missing either, or the maintenance hatch trades a // failed deployment for a permanent warning. - enabled = db.ClusterNodes.AsNoTracking() + enabled = scoped .Where(n => n.Enabled && !n.MaintenanceMode).Select(n => n.NodeId).ToList(); } catch (Exception ex) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index 149e03dd..f1baa621 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -153,7 +153,12 @@ public static class ServiceCollectionExtensions (system, registry, resolver) => { var dbFactory = resolver.GetService>(); - return ClusterNodeAddressReconcilerActor.Props(dbFactory); + // 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. + var roleInfo = resolver.GetService(); + return ClusterNodeAddressReconcilerActor.Props(dbFactory, roleInfo.ClusterId); }, singletonOptions, createProxyToo: false); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerActorTests.cs index a25cf368..27299c63 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ClusterNodeAddressReconcilerActorTests.cs @@ -70,24 +70,80 @@ public sealed class ClusterNodeAddressReconcilerActorTests : ControlPlaneActorTe }); } + /// + /// Per-cluster mesh Phase 6: an admin node scoped to its OWN cluster ignores rows in a cluster + /// its mesh can no longer see. Scoped to MAIN, a SITE-A enabled row is filtered + /// out before ever sees it, so it produces + /// no warning. This is the false + /// positive the split would otherwise print forever. + /// + [Fact] + public void Row_in_a_foreign_cluster_is_ignored_when_scoped() + { + var dbFactory = NewInMemoryDbFactory(); + SeedNode(dbFactory, "site-a-1:4053", enabled: true, clusterId: "SITE-A"); + + EventFilter.Warning(contains: "site-a-1:4053").Expect(0, TimeSpan.FromSeconds(2), () => + { + Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props( + dbFactory, ownClusterId: "MAIN", sweepInterval: TimeSpan.FromMilliseconds(400))); + ExpectNoMsg(TimeSpan.FromSeconds(1.5)); + }); + } + + /// + /// Scoping does not silence the admin node's own cluster. Scoped to MAIN, an enabled + /// MAIN row with no matching driver member is still warned about — proving the filter + /// keeps in-cluster rows (and with them, within-cluster drift detection). + /// + [Fact] + public void Enabled_row_in_own_cluster_is_still_logged_when_scoped() + { + var dbFactory = NewInMemoryDbFactory(); + SeedNode(dbFactory, "main-1:4053", enabled: true, clusterId: "MAIN"); + + EventFilter.Warning(contains: "main-1:4053").ExpectOne(TimeSpan.FromSeconds(10), () => + Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props(dbFactory, ownClusterId: "MAIN"))); + } + + /// + /// Legacy backward-compat: an admin node with no cluster role (null ownClusterId) still + /// reconciles the WHOLE fleet — such a node genuinely sees every member via gossip. A + /// SITE-A enabled row with no member is warned about, exactly as before the Phase 6 scope. + /// + [Fact] + public void Foreign_cluster_row_is_still_logged_when_unscoped() + { + var dbFactory = NewInMemoryDbFactory(); + SeedNode(dbFactory, "site-a-1:4053", enabled: true, clusterId: "SITE-A"); + + EventFilter.Warning(contains: "site-a-1:4053").ExpectOne(TimeSpan.FromSeconds(10), () => + Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props(dbFactory, ownClusterId: null))); + } + private static void SeedNode( - IDbContextFactory dbFactory, string nodeId, bool enabled) + IDbContextFactory dbFactory, string nodeId, bool enabled, + string clusterId = "SITE-A") { using var db = dbFactory.CreateDbContext(); - db.ServerClusters.Add(new ServerCluster + if (!db.ServerClusters.Any(c => c.ClusterId == clusterId)) { - ClusterId = "SITE-A", - Name = "Site A", - Enterprise = "zb", - Site = "site-a", - NodeCount = 2, - RedundancyMode = RedundancyMode.Warm, - CreatedBy = "test", - }); + db.ServerClusters.Add(new ServerCluster + { + ClusterId = clusterId, + Name = clusterId, + Enterprise = "zb", + Site = clusterId.ToLowerInvariant(), + NodeCount = 2, + RedundancyMode = RedundancyMode.Warm, + CreatedBy = "test", + }); + } + db.ClusterNodes.Add(new ClusterNode { NodeId = nodeId, - ClusterId = "SITE-A", + ClusterId = clusterId, Host = nodeId.Split(':')[0], ApplicationUri = $"urn:OtOpcUa:{nodeId}", Enabled = enabled,