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); + } +}