From 4550486144aa1a43618716f48673a83911d4cf18 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 12:43:27 -0400 Subject: [PATCH] fix(health): active tier reports the cluster Primary, not the admin role leader Closes the remaining half of #494, and corrects the half fixed in 8dd9da7d. The shared ActiveNodeHealthCheck(role: "admin") answered the wrong question twice over. It returns Healthy for any node LACKING the role, so all four driver-only site nodes called themselves active and no consumer could find the Primary of a site Cluster. And it selects by RoleLeader - the lowest-ADDRESSED member - which is not where Akka places singletons. Replaced with ClusterPrimaryHealthCheck ("cluster-primary"). One rule: this node is active iff it is the OLDEST Up member carrying its own active role, where the active role is admin when the node has it and driver otherwise. That serves both consumers correctly - a fused admin node answers for admin, so Traefik pins the AdminUI to the node hosting the singletons; a driver-only site node answers for driver, which is exactly SelectDriverPrimary, the same election behind IsDriverPrimary and the OPC UA ServiceLevel 250/240 split. Per-mesh scoping is free after Phase 6: ClusterState.Members already contains only this node's own Cluster. SelectDriverPrimary is generalised to SelectOldestUpMemberOfRole so the age-ordering rule has one implementation. Two copies of "oldest Up member of a role" would be two chances to silently disagree about who is in charge, and the tier and the redundancy snapshot must never disagree. THE ROLE-LEADER HALF MATTERED IN PRACTICE, not just in theory. On the rebuilt rig the two orderings diverge right now: akka leader (lowest address) = central-1 oldest admin member = central-2 <- hosts the singletons 8dd9da7d made the tier a real 503 but still selected by RoleLeader, so it would have pinned Traefik to central-1 - the node NOT hosting the work. With this change Traefik correctly routes to central-2. Live-verified, exactly one 200 per mesh: MAIN central-2 200 central-1 503 traefik: central-2 UP, central-1 DOWN SITE-A site-a-1 200 site-a-2 503 SITE-B site-b-1 200 site-b-2 503 Tests: role-selection matrix and the startup-safe Degraded path in Host.Tests (26 pass); parity between the tier's selector and the redundancy snapshot's, plus role scoping, added to RedundancyPrimaryElectionTests, which forms a real two-node cluster deliberately built so the oldest member is not the lowest-addressed one (5 pass). --- .../Redundancy/RedundancyStateActor.cs | 22 ++- .../Health/ClusterPrimaryHealthCheck.cs | 145 ++++++++++++++++++ .../Health/HealthEndpoints.cs | 14 +- .../RedundancyPrimaryElectionTests.cs | 28 ++++ .../Health/ClusterPrimaryHealthCheckTests.cs | 65 ++++++++ 5 files changed, 267 insertions(+), 7 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ClusterPrimaryHealthCheckTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs index d8d69070..3ded7268 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs @@ -141,12 +141,30 @@ public sealed class RedundancyStateActor : ReceiveActor, IWithTimers /// /// The current cluster members, in any order. /// The oldest Up driver member's address, or null when there is none. - public static Address? SelectDriverPrimary(IEnumerable members) + public static Address? SelectDriverPrimary(IEnumerable members) => + SelectOldestUpMemberOfRole(members, DriverRole); + + /// + /// Selects the oldest Up member carrying — the node that + /// ClusterSingletonManager would place a role-scoped singleton on. + /// + /// The current cluster members, in any order. + /// The cluster role to scope the selection to. + /// The oldest Up member's address for that role, or null when there is none. + /// + /// The age-ordering rationale in applies verbatim to any role: + /// oldest, never RoleLeader. Factored out so the rule has exactly one implementation — + /// the health tier that reports which node is active needs the same answer the redundancy + /// snapshot does, and two copies of "oldest Up member of a role" would be two chances to + /// silently disagree about who is in charge. + /// + public static Address? SelectOldestUpMemberOfRole(IEnumerable members, string role) { ArgumentNullException.ThrowIfNull(members); + ArgumentException.ThrowIfNullOrWhiteSpace(role); return members - .Where(m => m.Status == MemberStatus.Up && m.Roles.Contains(DriverRole)) + .Where(m => m.Status == MemberStatus.Up && m.Roles.Contains(role)) .OrderBy(m => m, Member.AgeOrdering) .FirstOrDefault() ?.Address; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs new file mode 100644 index 00000000..25e88a99 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs @@ -0,0 +1,145 @@ +using Akka.Actor; +using Akka.Cluster; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Redundancy; + +namespace ZB.MOM.WW.OtOpcUa.Host.Health; + +/// +/// The active tier's answer to "is this the node in charge of this cluster?" — Healthy (200) on the +/// one node that owns the active work for its mesh, Unhealthy (503) on its partner. +/// +/// +/// +/// Replaces the shared ActiveNodeHealthCheck(role: "admin"), which answered a +/// different question and got it wrong twice. It was scoped to the admin role and +/// returns Healthy for any node that lacks that role, so all four driver-only site nodes +/// reported themselves active; and it selected by RoleLeader, the +/// lowest-addressed member, which is not where Akka places singletons. See +/// lmxopcua#494. +/// +/// +/// The rule here is one sentence: this node is active iff it is the oldest Up member +/// carrying its own active role, where the active role is admin when the node has +/// it and driver otherwise. That single rule serves both consumers correctly: +/// +/// +/// +/// A fused admin node answers for the admin role, so Traefik pins the AdminUI to +/// the node actually hosting the cluster singletons — which +/// ClusterSingletonManager places on the oldest member, not the role +/// leader. The two diverge after any restart. +/// +/// +/// A driver-only site node answers for the driver role, which is exactly +/// — the same election that +/// drives IsDriverPrimary and the OPC UA ServiceLevel 250/240 split. So +/// the tier and the ServiceLevel can no longer disagree about which node is Primary. +/// +/// +/// +/// Scoping is per-mesh for free: after per-cluster mesh Phase 6 a node's +/// ClusterState.Members contains only its own application Cluster, so +/// "oldest Up member" is already "oldest Up member of this Cluster" — exactly one +/// node answers 200 per Cluster, by construction rather than by filtering. +/// +/// +/// Startup-safe, matching the shared check it replaces: before the +/// or the cluster is available the result is Degraded, not Unhealthy, so a node that is +/// merely still booting is not reported as a failed standby. +/// +/// +public sealed class ClusterPrimaryHealthCheck : IHealthCheck +{ + private readonly IServiceProvider _serviceProvider; + + /// Initializes a new . + /// + /// The application service provider. The is resolved lazily so the + /// check is startup-safe. + /// + public ClusterPrimaryHealthCheck(IServiceProvider serviceProvider) => + _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); + + /// + /// The role whose oldest Up member owns the active work for . + /// + /// The local member's cluster roles. + /// + /// when the node carries it, otherwise + /// when it carries that, otherwise null for a node in + /// neither role. + /// + /// + /// Admin wins on a fused node because that is the role the cluster singletons — and the AdminUI + /// that Traefik routes to — are pinned to. + /// + public static string? ActiveRoleFor(IEnumerable selfRoles) + { + ArgumentNullException.ThrowIfNull(selfRoles); + + var roles = selfRoles as IReadOnlyCollection ?? selfRoles.ToList(); + + if (roles.Contains(RoleParser.Admin)) + return RoleParser.Admin; + + return roles.Contains(RoleParser.Driver) ? RoleParser.Driver : null; + } + + /// + public Task CheckHealthAsync( + HealthCheckContext context, + CancellationToken cancellationToken = default) + { + var system = _serviceProvider.GetService(); + if (system is null) + return Task.FromResult(HealthCheckResult.Degraded("ActorSystem not yet available.")); + + Member self; + IEnumerable members; + try + { + var cluster = Akka.Cluster.Cluster.Get(system); + self = cluster.SelfMember; + members = cluster.State.Members; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + return Task.FromResult(HealthCheckResult.Degraded("Akka cluster state not yet accessible.", ex)); + } + + var role = ActiveRoleFor(self.Roles); + if (role is null) + { + // Neither admin nor driver. RoleParser will not produce such a node today, so this is a + // guard rather than a supported topology — reported Healthy because the tier has no + // opinion about a node that owns no active work, not because the node is in charge. + return Task.FromResult(HealthCheckResult.Healthy("Node carries neither the admin nor the driver role.")); + } + + var primary = RedundancyStateActor.SelectOldestUpMemberOfRole(members, role); + + var data = new Dictionary(StringComparer.Ordinal) + { + ["activeRole"] = role, + ["selfAddress"] = self.Address.ToString(), + }; + + if (primary is not null) + data["primary"] = primary.ToString(); + + if (primary is null) + { + // No Up member holds the role — during a cold start, or while every holder is Joining or + // Leaving. Nobody is active, so nobody may claim to be. + return Task.FromResult(HealthCheckResult.Unhealthy( + $"No Up member currently carries the '{role}' role.", exception: null, data)); + } + + return Task.FromResult(primary == self.Address + ? HealthCheckResult.Healthy($"Primary for role '{role}' (oldest Up member).", data) + : HealthCheckResult.Unhealthy($"Standby: '{role}' primary is {primary}.", exception: null, data)); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs index 903fb92e..7c9bbad0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs @@ -17,7 +17,7 @@ public static class HealthEndpoints { /// /// Registers the shared ZB.MOM.WW health probes. Tier semantics preserved: configdb + akka on - /// ready+active; admin-leader on active only. The configdb probe is admin-only (per-cluster mesh + /// ready+active; cluster-primary on active only. The configdb probe is admin-only (per-cluster mesh /// Phase 4): a driver-only node holds no ConfigDb, so it is registered iff . /// /// The service collection to register the health checks on. @@ -53,11 +53,15 @@ public static class HealthEndpoints failureStatus: null, tags: new[] { ZbHealthTags.Ready, ZbHealthTags.Active }, args: AkkaClusterStatusPolicy.OtOpcUaCompat) - .AddTypeActivatedCheck( - "admin-leader", + // "Is this node in charge of its mesh?" — the question the active tier is supposed to + // answer, and the one the shared ActiveNodeHealthCheck(role: "admin") did not. That check + // returned Healthy for any node lacking the admin role, so every driver-only site node + // called itself active, and it selected by RoleLeader (lowest address) rather than by + // age, which is not where Akka places singletons. See lmxopcua#494. + .AddTypeActivatedCheck( + "cluster-primary", failureStatus: null, - tags: new[] { ZbHealthTags.Active }, - args: "admin") + tags: new[] { ZbHealthTags.Active }) // Registered on every node regardless of role (unlike the admin-only configdb probe above): // the check itself resolves ISyncStatus optionally and reports Healthy when LocalDb is absent // (admin-only graphs) or replication is default-OFF, so a plain node is never degraded by it. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/RedundancyPrimaryElectionTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/RedundancyPrimaryElectionTests.cs index 4c36156d..73be4973 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/RedundancyPrimaryElectionTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/RedundancyPrimaryElectionTests.cs @@ -122,6 +122,34 @@ public sealed class RedundancyPrimaryElectionTests : IAsyncLifetime "the fixture is only meaningful if the lowest-addressed node is the YOUNGER one"); } + /// + /// The generalised selector agrees with the driver-specific one, and is scoped to the role it is + /// asked about. + /// + /// + /// /health/active now answers from SelectOldestUpMemberOfRole while the redundancy + /// snapshot answers from SelectDriverPrimary. If those two ever disagreed, the tier would + /// report a different node as active than the one whose OPC UA ServiceLevel reads 250 — so the + /// parity is the property worth pinning, not either answer alone. + /// + [Fact] + public void OldestUpMemberOfRole_agrees_with_SelectDriverPrimary_and_is_role_scoped() + { + var members = Akka.Cluster.Cluster.Get(_oldest!).State.Members; + + RedundancyStateActor.SelectOldestUpMemberOfRole(members, "driver") + .ShouldBe(RedundancyStateActor.SelectDriverPrimary(members)); + + // Both nodes here carry admin AND driver, so the admin-scoped answer is the same oldest node + // — and, critically, still the oldest rather than the lowest-addressed role leader. + RedundancyStateActor.SelectOldestUpMemberOfRole(members, "admin")!.Port + .ShouldBe(OldestPort); + + // A role nobody carries has no owner. The health check maps this to Unhealthy: during a cold + // start nobody is active, and nobody may claim to be. + RedundancyStateActor.SelectOldestUpMemberOfRole(members, "no-such-role").ShouldBeNull(); + } + /// /// The elected Primary is the oldest driver member — the node that hosts the cluster singletons — /// not the lowest-addressed one. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ClusterPrimaryHealthCheckTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ClusterPrimaryHealthCheckTests.cs new file mode 100644 index 00000000..0206ddec --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ClusterPrimaryHealthCheckTests.cs @@ -0,0 +1,65 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Host.Health; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Health; + +/// +/// Pins the rule the active tier now uses to decide which node is in charge, and the startup path +/// that must not report a booting node as a failed standby. +/// +/// +/// The cluster-membership half of the rule — oldest Up member, never RoleLeader — is pinned +/// against a real two-node cluster in RedundancyPrimaryElectionTests, including parity with +/// the redundancy snapshot's own election. What is left to cover here is the role-selection rule and +/// the startup guard. +/// +public sealed class ClusterPrimaryHealthCheckTests +{ + [Fact] + public void Fused_node_answers_for_the_admin_role() + { + // Admin wins on a node carrying both, because the cluster singletons and the AdminUI that + // Traefik routes to are pinned to that role. Answering for 'driver' here could name a + // different node the moment an admin-only or driver-only member joins the mesh. + ClusterPrimaryHealthCheck.ActiveRoleFor(["admin", "driver", "cluster-MAIN"]) + .ShouldBe("admin"); + } + + [Fact] + public void Driver_only_node_answers_for_the_driver_role() + { + // The regression this whole change exists for. Under the previous admin-scoped check every + // site node returned Healthy — "or not a role member" — so all four called themselves + // active and no consumer could find the Primary of a site Cluster. lmxopcua#494. + ClusterPrimaryHealthCheck.ActiveRoleFor(["driver", "cluster-SITE-A"]) + .ShouldBe("driver"); + } + + [Fact] + public void Admin_only_node_answers_for_the_admin_role() + { + ClusterPrimaryHealthCheck.ActiveRoleFor(["admin"]).ShouldBe("admin"); + } + + [Fact] + public void Node_in_neither_role_has_no_active_role() + { + ClusterPrimaryHealthCheck.ActiveRoleFor(["cluster-MAIN"]).ShouldBeNull(); + } + + [Fact] + public async Task No_actor_system_is_degraded_not_unhealthy() + { + // A node that is still booting has not lost an election. Reporting Unhealthy here would make + // every start look like a failed standby, and — now that the tier is a real 503 — would pull + // the node out of Traefik's pool on the way up. + var check = new ClusterPrimaryHealthCheck(new ServiceCollection().BuildServiceProvider()); + + var result = await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + } +}