diff --git a/Directory.Packages.props b/Directory.Packages.props index ccb9daa8..a30e3e2d 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -140,7 +140,7 @@ - + - - + + 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 3ded7268..2c43f34a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Redundancy/RedundancyStateActor.cs @@ -2,6 +2,7 @@ using Akka.Actor; using Akka.Cluster; using Akka.Cluster.Tools.PublishSubscribe; using Akka.Event; +using ZB.MOM.WW.Health.Akka; using ZB.MOM.WW.OtOpcUa.Cluster; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Redundancy; using ZB.MOM.WW.OtOpcUa.Commons.Types; @@ -152,22 +153,28 @@ public sealed class RedundancyStateActor : ReceiveActor, IWithTimers /// 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. + /// + /// The age-ordering rationale in applies verbatim to any + /// role: oldest, never RoleLeader. The rule itself lives in the shared + /// (ZB.MOM.WW.Health.Akka 0.3.0) so it has exactly + /// one implementation family-wide — this method is the control plane's entry point into it. + /// + /// + /// Delegating rather than re-implementing is what keeps the redundancy snapshot and the + /// /health/active tier structurally in agreement. They answer for different + /// consumers — the OPC UA ServiceLevel 250/240 split reads this election, while an + /// orchestrator routes admin traffic by the health tier — so a divergence would advertise + /// one node as authoritative while gating the data plane on another. Both apps in the + /// family previously kept private copies of this rule, and both got it wrong in a + /// different way. + /// /// 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(role)) - .OrderBy(m => m, Member.AgeOrdering) - .FirstOrDefault() - ?.Address; + return ClusterActiveNode.OldestUpMember(members, role)?.Address; } private IReadOnlyList BuildSnapshot() diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj index d2e52480..a20477da 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj @@ -19,6 +19,14 @@ Google.Protobuf + Grpc.Core.Api flow transitively from Commons (the generated client). --> + + diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs deleted file mode 100644 index 25e88a99..00000000 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/ClusterPrimaryHealthCheck.cs +++ /dev/null @@ -1,145 +0,0 @@ -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 7c9bbad0..f7a740bd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/HealthEndpoints.cs @@ -8,6 +8,7 @@ using ZB.MOM.WW.Health; using ZB.MOM.WW.Health.Akka; using ZB.MOM.WW.Health.EntityFrameworkCore; using ZB.MOM.WW.LocalDb.Replication; +using ZB.MOM.WW.OtOpcUa.Cluster; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Host.Configuration; @@ -53,15 +54,33 @@ public static class HealthEndpoints failureStatus: null, tags: new[] { ZbHealthTags.Ready, ZbHealthTags.Active }, args: AkkaClusterStatusPolicy.OtOpcUaCompat) - // "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( + // "Is this node in charge of its mesh?" — Healthy on the one node that owns the active + // work for its Cluster, Unhealthy (503) on its partner, so Traefik pins the AdminUI to the + // node actually hosting the cluster singletons. + // + // Role preference admin-then-driver: a fused central node answers for `admin` (the role + // the singletons and the AdminUI are pinned to), while a driver-only site node answers for + // `driver` — which is exactly RedundancyStateActor.SelectDriverPrimary, the election behind + // IsDriverPrimary and the OPC UA ServiceLevel 250/240 split. Both now route through the + // shared ClusterActiveNode, so the tier and the ServiceLevel cannot disagree. + // + // Scoping is per-mesh for free: after per-cluster mesh Phase 6 a node's ClusterState + // 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. + // + // Before Health 0.3.0 this was ActiveNodeHealthCheck(role: "admin"), which answered a + // different question and got it wrong twice: it 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 }) + tags: new[] { ZbHealthTags.Active }, + args: new ActiveNodeHealthCheckOptions + { + RolePreference = new[] { RoleParser.Admin, RoleParser.Driver }, + }) // 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.Host.Tests/Health/ActiveTierRegistrationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ActiveTierRegistrationTests.cs new file mode 100644 index 00000000..1a9730ce --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ActiveTierRegistrationTests.cs @@ -0,0 +1,59 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; +using ZB.MOM.WW.Health; +using ZB.MOM.WW.Health.Akka; +using ZB.MOM.WW.OtOpcUa.Host.Health; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Health; + +/// +/// Pins OtOpcUa's active-tier wiring. The rule (oldest Up member of a role) now lives in the +/// shared ClusterActiveNode and is tested there against a real two-node cluster; what stays +/// OtOpcUa's own decision — and what regressed in lmxopcua#494 — is which check is +/// registered on the active tag, and with which roles. +/// +public sealed class ActiveTierRegistrationTests +{ + private static HealthCheckRegistration ClusterPrimaryRegistration(bool hasAdmin) => + new ServiceCollection() + .AddOtOpcUaHealth(hasAdmin) + .BuildServiceProvider() + .GetRequiredService>() + .Value.Registrations + .Where(r => r.Name == "cluster-primary") + .ShouldHaveSingleItem(); + + [Fact] + public void Active_tier_uses_the_shared_ActiveNodeHealthCheck() + { + // Not a bespoke copy. Two apps in the family independently hand-rolled this check because the + // shared one selected by RoleLeader; since Health 0.3.0 the shared one is the correct rule, so + // a private reimplementation reappearing here would be the regression. + var registration = ClusterPrimaryRegistration(hasAdmin: true); + + registration.Factory(new ServiceCollection().BuildServiceProvider()) + .ShouldBeOfType(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Active_tier_check_is_registered_on_every_node_role(bool hasAdmin) + { + // Registered on driver-only nodes too, unlike the admin-only configdb probe. A site pair needs + // an active tier just as much as the central pair does — under the old admin-scoped check all + // four driver-only site nodes answered 200 and no consumer could find a site's Primary. + ClusterPrimaryRegistration(hasAdmin).Tags.ShouldContain(ZbHealthTags.Active); + } + + [Fact] + public void Active_tier_check_is_not_on_the_ready_tier() + { + // Readiness must not depend on being the active node: a healthy standby is ready to serve, and + // tagging it ready would take the whole pair out of rotation whenever one node is standby. + ClusterPrimaryRegistration(hasAdmin: true).Tags.ShouldNotContain(ZbHealthTags.Ready); + } +} 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 deleted file mode 100644 index 0206ddec..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Health/ClusterPrimaryHealthCheckTests.cs +++ /dev/null @@ -1,65 +0,0 @@ -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); - } -}