fix(mesh): scope ClusterNode address reconcile to the admin node's own cluster
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
This commit is contained in:
Joseph Doherty
@@ -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<RedundancyStateActorKey>(
|
||||
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.
|
||||
|
||||
@@ -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<AkkaClusterOptions>, 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]},
|
||||
|
||||
@@ -58,10 +58,16 @@ public sealed record ClusterNodeAddress(string NodeId, string Host, int AkkaPort
|
||||
/// to be deleted again.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Phase 2 must revisit this.</b> 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
|
||||
/// <see cref="AddressMismatchKind.EnabledRowNotInCluster"/> forever. The check then has to
|
||||
/// move to whatever transport replaces gossip — or be scoped to the central mesh.
|
||||
/// <b>Scoped to the admin node's own cluster (Phase 6).</b> 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
|
||||
/// <see cref="AddressMismatchKind.EnabledRowNotInCluster"/> forever. The actor therefore loads
|
||||
/// only rows whose <c>ClusterId</c> matches the admin node's own
|
||||
/// (<see cref="ZB.MOM.WW.OtOpcUa.Commons.Interfaces.IClusterRoleInfo.ClusterId"/>) before
|
||||
/// calling <see cref="Reconcile"/>, so it never false-reports a cluster it cannot observe. A
|
||||
/// legacy admin node carrying no cluster role (null <c>ClusterId</c>) 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.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public static class ClusterNodeAddressReconciler
|
||||
|
||||
+29
-5
@@ -30,6 +30,7 @@ public sealed class ClusterNodeAddressReconcilerActor : ReceiveActor, IWithTimer
|
||||
public static readonly TimeSpan DefaultSweepInterval = TimeSpan.FromMinutes(5);
|
||||
|
||||
private readonly IDbContextFactory<OtOpcUaConfigDbContext> _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
|
||||
|
||||
/// <summary>Creates Props for the reconciler.</summary>
|
||||
/// <param name="dbFactory">Factory for the config database context.</param>
|
||||
/// <param name="ownClusterId">
|
||||
/// The admin node's own <c>ClusterId</c> (e.g. <c>MAIN</c>), from
|
||||
/// <see cref="IClusterRoleInfo.ClusterId"/>. 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.
|
||||
/// </param>
|
||||
/// <param name="sweepInterval">Periodic sweep interval; defaults to <see cref="DefaultSweepInterval"/>.</param>
|
||||
/// <returns>Props for actor creation.</returns>
|
||||
public static Props Props(
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory, TimeSpan? sweepInterval = null) =>
|
||||
Akka.Actor.Props.Create(() => new ClusterNodeAddressReconcilerActor(dbFactory, sweepInterval));
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory,
|
||||
string? ownClusterId = null,
|
||||
TimeSpan? sweepInterval = null) =>
|
||||
Akka.Actor.Props.Create(() =>
|
||||
new ClusterNodeAddressReconcilerActor(dbFactory, ownClusterId, sweepInterval));
|
||||
|
||||
/// <summary>Initializes a new instance of the <see cref="ClusterNodeAddressReconcilerActor"/> class.</summary>
|
||||
/// <param name="dbFactory">Factory for the config database context.</param>
|
||||
/// <param name="ownClusterId">
|
||||
/// The admin node's own <c>ClusterId</c>; scopes the reconcile to that cluster's rows when
|
||||
/// non-null/empty, else reconciles the whole fleet (legacy behaviour).
|
||||
/// </param>
|
||||
/// <param name="sweepInterval">Periodic sweep interval; defaults to <see cref="DefaultSweepInterval"/>.</param>
|
||||
public ClusterNodeAddressReconcilerActor(
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory, TimeSpan? sweepInterval = null)
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> 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)
|
||||
|
||||
@@ -153,7 +153,12 @@ public static class ServiceCollectionExtensions
|
||||
(system, registry, resolver) =>
|
||||
{
|
||||
var dbFactory = resolver.GetService<IDbContextFactory<OtOpcUaConfigDbContext>>();
|
||||
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<IClusterRoleInfo>();
|
||||
return ClusterNodeAddressReconcilerActor.Props(dbFactory, roleInfo.ClusterId);
|
||||
},
|
||||
singletonOptions,
|
||||
createProxyToo: false);
|
||||
|
||||
+67
-11
@@ -70,24 +70,80 @@ public sealed class ClusterNodeAddressReconcilerActorTests : ControlPlaneActorTe
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>MAIN</c>, a <c>SITE-A</c> enabled row is filtered
|
||||
/// out before <see cref="ClusterNodeAddressReconciler.Reconcile"/> ever sees it, so it produces
|
||||
/// no <see cref="AddressMismatchKind.EnabledRowNotInCluster"/> warning. This is the false
|
||||
/// positive the split would otherwise print forever.
|
||||
/// </summary>
|
||||
[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));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Scoping does not silence the admin node's own cluster. Scoped to <c>MAIN</c>, an enabled
|
||||
/// <c>MAIN</c> row with no matching driver member is still warned about — proving the filter
|
||||
/// keeps in-cluster rows (and with them, within-cluster drift detection).
|
||||
/// </summary>
|
||||
[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")));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Legacy backward-compat: an admin node with no cluster role (null <c>ownClusterId</c>) still
|
||||
/// reconciles the WHOLE fleet — such a node genuinely sees every member via gossip. A
|
||||
/// <c>SITE-A</c> enabled row with no member is warned about, exactly as before the Phase 6 scope.
|
||||
/// </summary>
|
||||
[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<OtOpcUaConfigDbContext> dbFactory, string nodeId, bool enabled)
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> 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,
|
||||
|
||||
Reference in New Issue
Block a user