feat(fleet): reconcile ClusterNode dial targets against live membership
ClusterNode.AkkaPort and the node's own Cluster:Port are the same fact in two places and nothing made them agree. Phase 2 dials the row instead of gossiping, so a node binding 4054 while its row says 4053 becomes unreachable from central — and the symptom is a silent absence of acks rather than an error. That is the shape of the Modbus/ModbusTcp and TwinCat/Focas drifts already in this repo. Since Phase 1 also made the rows the deploy path's expected-ack set, drift already costs a failed deployment today. Implemented as the plan's preferred option: an admin-role singleton comparing rows against the membership an admin node can already see, rather than each driver node asserting its own row — Phase 4 removes the driver nodes' ConfigDb connection, so a self-assertion written there would have to be deleted again. Three shapes, split by severity: a row whose dial target disagrees with its own NodeId and a running node with no row are Errors; an enabled row with no matching member is a Warning, because a node down for maintenance is a legitimate state. Findings are logged only when the set changes — a check that reprints the same warning every sweep trains operators to filter it out. Documented limitation: Phase 2 must revisit this. Once the fleet splits into one mesh per cluster an admin node cannot see site members, and every site row would report EnabledRowNotInCluster forever. Sabotage: removing the change-detection guard turns the repeat test red. ControlPlane.Tests 100/100. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
+98
@@ -0,0 +1,98 @@
|
||||
using Akka.Actor;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
||||
using ZB.MOM.WW.OtOpcUa.ControlPlane.Fleet;
|
||||
using ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Harness;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Proves the reconciler is actually wired to something — subscribes to membership, reads the DB,
|
||||
/// and emits. <see cref="ClusterNodeAddressReconcilerTests"/> covers the comparison logic, which a
|
||||
/// dormant actor would leave perfectly correct and completely inert.
|
||||
/// </summary>
|
||||
public sealed class ClusterNodeAddressReconcilerActorTests : ControlPlaneActorTestBase
|
||||
{
|
||||
/// <summary>
|
||||
/// An enabled row with no matching driver member is warned about. The harness ActorSystem
|
||||
/// joins as <c>admin</c> with no driver members, so the seeded row is unmatched by
|
||||
/// construction.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Enabled_row_with_no_matching_member_is_logged()
|
||||
{
|
||||
var dbFactory = NewInMemoryDbFactory();
|
||||
SeedNode(dbFactory, "site-a-1:4053", enabled: true);
|
||||
|
||||
EventFilter.Warning(contains: "site-a-1:4053").ExpectOne(TimeSpan.FromSeconds(10), () =>
|
||||
Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props(dbFactory)));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The finding set is logged once, not once per sweep. A check that reprints the same warning
|
||||
/// every five minutes trains operators to filter it out, which costs more than it catches —
|
||||
/// so this runs a deliberately fast sweep and asserts the count stays at one.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Unchanged_findings_are_not_repeated_on_every_sweep()
|
||||
{
|
||||
var dbFactory = NewInMemoryDbFactory();
|
||||
SeedNode(dbFactory, "site-a-1:4053", enabled: true);
|
||||
|
||||
// ~6 sweeps inside the window; without the change-detection guard this logs 6 times.
|
||||
EventFilter.Warning(contains: "site-a-1:4053").Expect(1, TimeSpan.FromSeconds(3), () =>
|
||||
{
|
||||
Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props(
|
||||
dbFactory, sweepInterval: TimeSpan.FromMilliseconds(400)));
|
||||
// Hold the filter open past several sweeps rather than returning immediately.
|
||||
ExpectNoMsg(TimeSpan.FromSeconds(2.5));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A disabled row is silence — the maintenance hatch must not trade a failed deployment for a
|
||||
/// permanent warning. Positive control for the test above: same setup, one flag flipped.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Disabled_row_produces_no_warning()
|
||||
{
|
||||
var dbFactory = NewInMemoryDbFactory();
|
||||
SeedNode(dbFactory, "site-a-1:4053", enabled: false);
|
||||
|
||||
EventFilter.Warning(contains: "site-a-1:4053").Expect(0, TimeSpan.FromSeconds(2), () =>
|
||||
{
|
||||
Sys.ActorOf(ClusterNodeAddressReconcilerActor.Props(
|
||||
dbFactory, sweepInterval: TimeSpan.FromMilliseconds(400)));
|
||||
ExpectNoMsg(TimeSpan.FromSeconds(1.5));
|
||||
});
|
||||
}
|
||||
|
||||
private static void SeedNode(
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory, string nodeId, bool enabled)
|
||||
{
|
||||
using var db = dbFactory.CreateDbContext();
|
||||
db.ServerClusters.Add(new ServerCluster
|
||||
{
|
||||
ClusterId = "SITE-A",
|
||||
Name = "Site A",
|
||||
Enterprise = "zb",
|
||||
Site = "site-a",
|
||||
NodeCount = 2,
|
||||
RedundancyMode = RedundancyMode.Warm,
|
||||
CreatedBy = "test",
|
||||
});
|
||||
db.ClusterNodes.Add(new ClusterNode
|
||||
{
|
||||
NodeId = nodeId,
|
||||
ClusterId = "SITE-A",
|
||||
Host = nodeId.Split(':')[0],
|
||||
ApplicationUri = $"urn:OtOpcUa:{nodeId}",
|
||||
Enabled = enabled,
|
||||
CreatedBy = "test",
|
||||
});
|
||||
db.SaveChanges();
|
||||
}
|
||||
}
|
||||
+126
@@ -0,0 +1,126 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.ControlPlane.Fleet;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Per-cluster mesh Phase 1 Task 4: <c>ClusterNode.AkkaPort</c> and the node's own
|
||||
/// <c>Cluster:Port</c> are the same fact in two places, and nothing makes them agree. These pin
|
||||
/// the shapes that drift takes.
|
||||
/// </summary>
|
||||
public sealed class ClusterNodeAddressReconcilerTests
|
||||
{
|
||||
private static ClusterNodeAddress Row(string host, int akkaPort, string? nodeId = null) =>
|
||||
new(nodeId ?? $"{host}:{akkaPort}", host, akkaPort);
|
||||
|
||||
/// <summary>A fleet whose rows match membership reports nothing.</summary>
|
||||
[Fact]
|
||||
public void Matching_rows_and_membership_produce_no_findings()
|
||||
{
|
||||
var findings = ClusterNodeAddressReconciler.Reconcile(
|
||||
[("site-a-1", 4053), ("site-a-2", 4053)],
|
||||
[Row("site-a-1", 4053), Row("site-a-2", 4053)],
|
||||
["site-a-1:4053", "site-a-2:4053"]);
|
||||
|
||||
findings.ShouldBeEmpty();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The drift the plan names: a node binds 4054 while its row says 4053. Membership shows it
|
||||
/// at an address no row claims, and the row it should have matched looks absent — both halves
|
||||
/// are reported, because either alone reads as a different problem.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Node_bound_to_the_wrong_port_is_reported_from_both_sides()
|
||||
{
|
||||
var findings = ClusterNodeAddressReconciler.Reconcile(
|
||||
[("site-a-1", 4054)],
|
||||
[Row("site-a-1", 4053)],
|
||||
["site-a-1:4053"]);
|
||||
|
||||
findings.Count.ShouldBe(2);
|
||||
findings.ShouldContain(f =>
|
||||
f.Kind == AddressMismatchKind.RunningNodeHasNoRow && f.NodeId == "site-a-1:4054");
|
||||
findings.ShouldContain(f =>
|
||||
f.Kind == AddressMismatchKind.EnabledRowNotInCluster && f.NodeId == "site-a-1:4053");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The Phase 2 shape: the row's NodeId still matches membership, but its <c>Host</c> /
|
||||
/// <c>AkkaPort</c> columns — the values central will actually dial — have been edited away
|
||||
/// from it. Gossip still works today, so nothing else would notice.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Row_whose_dial_target_disagrees_with_its_own_NodeId_is_reported()
|
||||
{
|
||||
var findings = ClusterNodeAddressReconciler.Reconcile(
|
||||
[("site-a-1", 4053)],
|
||||
[Row("site-a-1", 4054, nodeId: "site-a-1:4053")],
|
||||
["site-a-1:4053"]);
|
||||
|
||||
var only = findings.ShouldHaveSingleItem();
|
||||
only.Kind.ShouldBe(AddressMismatchKind.RowDialTargetDisagrees);
|
||||
only.NodeId.ShouldBe("site-a-1:4053");
|
||||
only.Detail.ShouldContain("site-a-1:4054");
|
||||
}
|
||||
|
||||
/// <summary>A running driver node nobody registered — the coordinator discards its acks.</summary>
|
||||
[Fact]
|
||||
public void Running_node_with_no_row_is_reported()
|
||||
{
|
||||
var findings = ClusterNodeAddressReconciler.Reconcile(
|
||||
[("ghost-9", 4053)], [], []);
|
||||
|
||||
findings.ShouldHaveSingleItem().Kind.ShouldBe(AddressMismatchKind.RunningNodeHasNoRow);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A disabled row for an absent node is silence, not a finding — being absent is the entire
|
||||
/// point of disabling it. Without this the maintenance hatch would trade a failed deploy for
|
||||
/// a permanent warning.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Disabled_row_for_an_absent_node_is_not_reported()
|
||||
{
|
||||
var findings = ClusterNodeAddressReconciler.Reconcile(
|
||||
[("site-a-1", 4053)],
|
||||
[Row("site-a-1", 4053), Row("site-a-2", 4053)],
|
||||
["site-a-1:4053"]);
|
||||
|
||||
findings.ShouldBeEmpty();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// An admin-only member has no <c>ClusterNode</c> row by design, so the caller filters
|
||||
/// membership to driver-role members. Passing an unfiltered list would report correct
|
||||
/// configuration as broken — pinned here because the filter lives at the call site.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Admin_only_members_are_the_callers_responsibility_to_exclude()
|
||||
{
|
||||
// What the actor passes: driver members only.
|
||||
ClusterNodeAddressReconciler.Reconcile(
|
||||
[("central-1", 4053)], [Row("central-1", 4053)], ["central-1:4053"])
|
||||
.ShouldBeEmpty();
|
||||
|
||||
// What an unfiltered call would produce — the false positive this guards against.
|
||||
ClusterNodeAddressReconciler.Reconcile(
|
||||
[("central-1", 4053), ("admin-only-1", 4053)],
|
||||
[Row("central-1", 4053)],
|
||||
["central-1:4053"])
|
||||
.ShouldHaveSingleItem().Kind.ShouldBe(AddressMismatchKind.RunningNodeHasNoRow);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Host matching is case-insensitive, matching the coordinator's <c>NodeIdComparer</c> — DNS
|
||||
/// and SQL collation are both case-insensitive, so the same node surfaces with either casing.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Host_matching_is_case_insensitive()
|
||||
{
|
||||
ClusterNodeAddressReconciler.Reconcile(
|
||||
[("SITE-A-1", 4053)], [Row("site-a-1", 4053)], ["site-a-1:4053"])
|
||||
.ShouldBeEmpty();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user