From 344bdc4da5b272b4ea3d6fdd79902f1b4249dec1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 13:27:07 -0400 Subject: [PATCH] fix(overview): rank activeNode over leader for the Active chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard derived every leader chip from AkkaClusterHealthCheck's `leader` data — ClusterState.Leader, the LOWEST-ADDRESSED Up member. That is not the node in charge. The node holding the singletons, gated for writes, and answering 200 on /health/active is the OLDEST Up member. The two agree only until some node restarts, after which the restarted node rejoins youngest but keeps its address. Observed live on the rebuilt OtOpcUa rig during the Health 0.3.0 work: leader central-1, active node central-2. The dashboard would have put the Active chip on central-1 while /health/active said central-2 — the chip contradicting the tier it exists to visualise. The Phase 4 acceptance run missed it because the rig had just been formed, the one state in which the two orderings agree. Health 0.3.0's active tier now publishes `activeNode` on every member, including standbys, so the fix is to prefer it and keep `leader` as the fallback for an instance on an older Health or whose active tier was not probed. The instance card now shows `active` and `leader` side by side rather than swapping one for the other: they are legitimately different nodes, and showing both turns a silent contradiction into something readable at 3am. Tests: 177 (+3) — precedence when the two disagree, a standby naming the active node rather than itself, and the fallback still working with no activeNode. --- .../Components/Widgets/InstanceCard.razor | 35 +++++++---- .../Polling/LeaderResolver.cs | 56 ++++++++++++++---- .../LeaderResolverTests.cs | 59 +++++++++++++++++++ .../PollingTestSupport.cs | 25 ++++++++ 4 files changed, 153 insertions(+), 22 deletions(-) diff --git a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Components/Widgets/InstanceCard.razor b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Components/Widgets/InstanceCard.razor index 70206d8..2ef9ce7 100644 --- a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Components/Widgets/InstanceCard.razor +++ b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Components/Widgets/InstanceCard.razor @@ -214,18 +214,32 @@ /// Renders the cluster check's data as the mockup's monospace evidence line. /// /// - /// Located by the data it carries rather than by check name: the two apps register the same - /// shared cluster check under different names (akka-cluster vs akka), so keying on - /// either one silently drops the evidence line for the other. + /// + /// Located by the data it carries rather than by check name: the two apps register the + /// same shared checks under different names (akka-cluster vs akka, + /// active-node vs cluster-primary), so keying on either one silently drops + /// the evidence line for the other. + /// + /// + /// active and leader are shown side by side on purpose. They come + /// from different checks and are different nodes whenever any node has restarted — the + /// leader is the lowest-addressed Up member, the active node is the oldest. Showing both + /// turns the confusion that produced lmxopcua#494 into something an operator can read + /// straight off the card, instead of a silent disagreement between the chip and the tier. + /// /// private string? ClusterDataLine { get { - var entry = Instance.Report?.Entries.Values - .FirstOrDefault(e => e.DataString(LeaderResolver.LeaderDataKey) is { Length: > 0 }); + var entries = Instance.Report?.Entries.Values; + if (entries is null) + return null; - if (entry is null) + var cluster = entries.FirstOrDefault(e => e.DataString(LeaderResolver.LeaderDataKey) is { Length: > 0 }); + var active = entries.FirstOrDefault(e => e.DataString(LeaderResolver.ActiveNodeDataKey) is { Length: > 0 }); + + if (cluster is null && active is null) return null; var parts = new List(); @@ -236,10 +250,11 @@ parts.Add($"{Escape(label)} {Escape(value)}"); } - Add("leader", entry.DataString(LeaderResolver.LeaderDataKey)); - Add("members", entry.DataInt("memberCount")?.ToString()); - Add("unreachable", entry.DataInt("unreachableCount")?.ToString()); - Add("self", entry.DataString("selfAddress")); + Add("active", active?.DataString(LeaderResolver.ActiveNodeDataKey)); + Add("leader", cluster?.DataString(LeaderResolver.LeaderDataKey)); + Add("members", cluster?.DataInt("memberCount")?.ToString()); + Add("unreachable", cluster?.DataInt("unreachableCount")?.ToString()); + Add("self", (cluster ?? active)?.DataString("selfAddress")); return parts.Count == 0 ? null : string.Join(" · ", parts); } diff --git a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/LeaderResolver.cs b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/LeaderResolver.cs index 175b113..79f751a 100644 --- a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/LeaderResolver.cs +++ b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/LeaderResolver.cs @@ -11,7 +11,16 @@ namespace ZB.MOM.WW.Overview.Polling; /// public static class LeaderResolver { - /// The data key holding the Akka leader address. + /// + /// The data key holding the address of the node actually in charge — the oldest Up member, + /// published by the active tier's ActiveNodeHealthCheck from ZB.MOM.WW.Health 0.3.0. + /// + public const string ActiveNodeDataKey = "activeNode"; + + /// + /// The data key holding the Akka cluster leader address, published by + /// AkkaClusterHealthCheck. Fallback only — see . + /// public const string LeaderDataKey = "leader"; /// Resolves leader state for every cluster group present in . @@ -79,29 +88,52 @@ public static class LeaderResolver distinct); } - /// Finds the leader address one instance reports, whatever check carries it. + /// Finds the address of the in-charge node one instance reports. /// The instance whose report to search. - /// The reported leader address, or null when this instance publishes none. + /// The reported address, or null when this instance publishes none. /// - /// Matched on the DATA KEY, not on the check name. The two apps register the same shared - /// AkkaClusterHealthCheck under different names — ScadaBridge calls it - /// akka-cluster, OtOpcUa calls it akka — and a resolver keyed on either name - /// silently renders no leader chip for the other, which is exactly how this was missed until a - /// live rig showed OtOpcUa's groups bare while ScadaBridge's were fully populated. The data key - /// is the part of the contract the shared check actually owns. + /// + /// activeNode first, leader only as a fallback. They are different + /// nodes and the chip must show the first. The Akka cluster leader is the + /// lowest-addressed Up member — address order, no relationship to time — while the node + /// that actually holds the singletons and answers 200 on /health/active is the + /// oldest Up member. They agree only until some node restarts, after which the + /// restarted node rejoins youngest but keeps its address; observed live on the rebuilt + /// OtOpcUa rig, where the leader was central-1 while central-2 was the + /// active node. Ranking leader first would put the Active chip on the standby. + /// + /// + /// The fallback still matters: it is what an instance running Health < 0.3.0, or one + /// whose active tier was not probed, publishes. Showing the leader there is better than + /// showing nothing, and it is the pre-0.3.0 behaviour unchanged. + /// + /// + /// Matched on the DATA KEY, not on the check name. The two apps register the same shared + /// checks under different names — ScadaBridge calls its cluster check + /// akka-cluster, OtOpcUa calls it akka; the active check is + /// active-node in one and cluster-primary in the other — and a resolver + /// keyed on any of those names silently renders no chip for the other app, which is + /// exactly how this was missed until a live rig showed OtOpcUa's groups bare while + /// ScadaBridge's were fully populated. The data key is the part of the contract the + /// shared checks actually own. + /// /// internal static string? LeaderReportedBy(InstanceSnapshot instance) { if (instance.Report?.Entries is not { } entries) return null; + string? leaderFallback = null; + foreach (var entry in entries.Values) { - if (entry.DataString(LeaderDataKey) is { Length: > 0 } leader) - return leader; + if (entry.DataString(ActiveNodeDataKey) is { Length: > 0 } activeNode) + return activeNode; + + leaderFallback ??= entry.DataString(LeaderDataKey) is { Length: > 0 } leader ? leader : null; } - return null; + return leaderFallback; } /// diff --git a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/LeaderResolverTests.cs b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/LeaderResolverTests.cs index 49b6026..8704dc5 100644 --- a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/LeaderResolverTests.cs +++ b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/LeaderResolverTests.cs @@ -221,4 +221,63 @@ public class LeaderResolverTests Assert.Equal("akka.tcp://otopcua@node-a:4053", LeaderResolver.LeaderReportedBy(instance)); } + + [Fact] + public void ActiveNode_WinsOverLeader_WhenTheyDisagree() + { + // The two are DIFFERENT NODES after any restart: leader is the lowest-addressed Up member, + // the active node is the oldest. Observed live on the rebuilt OtOpcUa rig — leader central-1, + // active central-2 — so ranking leader first would put the Active chip on the standby, which + // is the same class of error as lmxopcua#494 rendered in the UI instead of in routing. + var body = HealthBody.WithData( + "Healthy", + ("akka", "Healthy", new Dictionary + { + ["leader"] = "akka.tcp://otopcua@central-1:4053", + }), + ("cluster-primary", "Healthy", new Dictionary + { + ["activeNode"] = "akka.tcp://otopcua@central-2:4053", + })); + var report = System.Text.Json.JsonSerializer.Deserialize(body)!; + var instance = Snapshots.Instance("central-1", "MAIN", "http://central-1:8080") with { Report = report }; + + Assert.Equal("akka.tcp://otopcua@central-2:4053", LeaderResolver.LeaderReportedBy(instance)); + } + + [Fact] + public void ActiveNode_ReportedByAStandby_NamesTheActiveNode_NotItself() + { + // Every member publishes who IS active, so a standby votes for the same address the active + // node does. That is what lets the group resolve with one unanimous chip instead of a + // manufactured disagreement, and it is why the standby's payload alone is enough. + var body = HealthBody.WithData( + "Unhealthy", + ("cluster-primary", "Unhealthy", new Dictionary + { + ["activeNode"] = "akka.tcp://otopcua@site-a-1:4053", + ["selfAddress"] = "akka.tcp://otopcua@site-a-2:4053", + })); + var report = System.Text.Json.JsonSerializer.Deserialize(body)!; + var instance = Snapshots.Instance("site-a-2", "SITE-A", "http://site-a-2:8080") with { Report = report }; + + Assert.Equal("akka.tcp://otopcua@site-a-1:4053", LeaderResolver.LeaderReportedBy(instance)); + } + + [Fact] + public void Leader_IsStillUsed_WhenNoActiveNodeIsPublished() + { + // An instance on Health < 0.3.0, or one whose active tier was not probed, publishes only + // `leader`. Showing it beats showing nothing, and it keeps the pre-0.3.0 behaviour intact. + var body = HealthBody.WithData( + "Healthy", + ("akka-cluster", "Healthy", new Dictionary + { + ["leader"] = "akka.tcp://scadabridge@central-a:4053", + })); + var report = System.Text.Json.JsonSerializer.Deserialize(body)!; + var instance = Snapshots.Instance("central-a", "central", "http://central-a:8080") with { Report = report }; + + Assert.Equal("akka.tcp://scadabridge@central-a:4053", LeaderResolver.LeaderReportedBy(instance)); + } } diff --git a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/PollingTestSupport.cs b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/PollingTestSupport.cs index 77774ca..8ac9f18 100644 --- a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/PollingTestSupport.cs +++ b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/PollingTestSupport.cs @@ -139,6 +139,31 @@ internal static class HealthBody return JsonSerializer.Serialize(payload); } + + /// + /// Builds a body whose entries carry arbitrary data, for the cases where the key under + /// test is not leader — notably the active tier's activeNode. + /// + public static string WithData(string status, params (string Name, string Status, Dictionary? Data)[] entries) + { + var payload = new + { + status, + totalDurationMs = 1.5, + entries = entries.ToDictionary( + e => e.Name, + e => (object)new + { + status = e.Status, + description = $"{e.Name} says {e.Status}", + durationMs = 0.5, + data = e.Data, + }, + StringComparer.Ordinal), + }; + + return JsonSerializer.Serialize(payload); + } } /// Builds values for the pure-logic tests.