diff --git a/Directory.Packages.props b/Directory.Packages.props index d111b772..9ef663e2 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -90,9 +90,9 @@ to mark tests as Skipped (not silently Passed) when MSSQL is unreachable. --> - - - + + + diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/ClusterState/ActiveNodeEvaluator.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/ClusterState/ActiveNodeEvaluator.cs index 43ba93a7..58058ba8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/ClusterState/ActiveNodeEvaluator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/ClusterState/ActiveNodeEvaluator.cs @@ -1,4 +1,5 @@ using Akka.Cluster; +using ZB.MOM.WW.Health.Akka; namespace ZB.MOM.WW.ScadaBridge.Communication.ClusterState; @@ -32,17 +33,13 @@ public static class ActiveNodeEvaluator /// The Akka cluster to evaluate. /// Optional role scope; when set, only members with this role are considered. /// true when self is Up and the oldest Up member in the role scope. - public static bool SelfIsOldestUp(Cluster cluster, string? role = null) - { - var self = cluster.SelfMember; - if (self.Status != MemberStatus.Up) - return false; - if (role != null && !self.HasRole(role)) - return false; - - return cluster.State.Members - .Where(m => m.Status == MemberStatus.Up) - .Where(m => role == null || m.HasRole(role)) - .All(m => m.UniqueAddress.Equals(self.UniqueAddress) || self.IsOlderThan(m)); - } + /// + /// Delegates to the shared + /// (ZB.MOM.WW.Health.Akka 0.3.0), which is this evaluator promoted into the shared library + /// after OtOpcUa independently needed the same rule and wrote its own third copy. The rule now has + /// one implementation family-wide; this method survives as Communication's entry point into it, so + /// the delivery gate and heartbeat keep their existing call sites and layering. + /// + public static bool SelfIsOldestUp(Cluster cluster, string? role = null) => + ClusterActiveNode.SelfIsActive(cluster, role); } diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/ZB.MOM.WW.ScadaBridge.Communication.csproj b/src/ZB.MOM.WW.ScadaBridge.Communication/ZB.MOM.WW.ScadaBridge.Communication.csproj index ccc4bf18..329f237f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/ZB.MOM.WW.ScadaBridge.Communication.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/ZB.MOM.WW.ScadaBridge.Communication.csproj @@ -28,6 +28,13 @@ + + diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Health/ActiveNodeGate.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Health/ActiveNodeGate.cs index db95aedf..efe6e911 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Health/ActiveNodeGate.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Health/ActiveNodeGate.cs @@ -14,7 +14,8 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Health; /// — NOT /// the cluster leader, whose address-ordered definition diverges from singleton /// placement after a restart (review 01 [High]). This is the same evaluator -/// backing , so +/// backing the shared ActiveNodeHealthCheck registered on the active tier +/// — both route to ClusterActiveNode — so /// can return HTTP 503 on a standby. /// /// Registered only in the Central-role branch of Program.cs. The gate diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Health/ClusterActivityEvaluator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Health/ClusterActivityEvaluator.cs index b4729cb5..926049d0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Health/ClusterActivityEvaluator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Health/ClusterActivityEvaluator.cs @@ -1,4 +1,5 @@ using Akka.Cluster; +using ZB.MOM.WW.Health.Akka; namespace ZB.MOM.WW.ScadaBridge.Host.Health; @@ -27,14 +28,10 @@ public static class ClusterActivityEvaluator /// The Akka cluster to evaluate. /// Optional role scope; when set, only members with this role are considered. /// The oldest Up member in the role scope, or null when none is Up. - public static Member? OldestUpMember(Cluster cluster, string? role = null) - { - Member? oldest = null; - foreach (var m in cluster.State.Members.Where(m => m.Status == MemberStatus.Up)) - { - if (role != null && !m.HasRole(role)) continue; - if (oldest == null || m.IsOlderThan(oldest)) oldest = m; - } - return oldest; - } + /// + /// Delegates to the shared , so + /// the label a node shows and the answer gates on come from one rule. + /// + public static Member? OldestUpMember(Cluster cluster, string? role = null) => + ClusterActiveNode.OldestUpMember(cluster, role); } diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Health/OldestNodeActiveHealthCheck.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Health/OldestNodeActiveHealthCheck.cs deleted file mode 100644 index c20a1f81..00000000 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Health/OldestNodeActiveHealthCheck.cs +++ /dev/null @@ -1,34 +0,0 @@ -using Akka.Actor; -using Microsoft.Extensions.Diagnostics.HealthChecks; - -namespace ZB.MOM.WW.ScadaBridge.Host.Health; - -/// -/// /health/active check: Healthy only on the oldest Up member — the node that -/// hosts the cluster singletons. Replaces the shared package's leader-based -/// ActiveNodeHealthCheck (review 01 [High]): leadership is address-ordered and -/// diverges from singleton placement after a restart, and during a partition -/// BOTH nodes compute themselves leader, so Traefik served both. Oldest-member -/// semantics keep exactly one active node through a partition (the non-oldest -/// side still sees the old oldest member until SBR resolves membership). -/// -public sealed class OldestNodeActiveHealthCheck : IHealthCheck -{ - private readonly ActorSystem _system; - - /// Initializes a new bound to the running actor system. - /// The live cluster actor system. - public OldestNodeActiveHealthCheck(ActorSystem system) => _system = system; - - /// Reports Healthy only when this node is the oldest Up cluster member (the singleton host); otherwise reports Unhealthy. - /// The health check context (unused; the result depends only on cluster membership). - /// Cancellation token. - /// A task that resolves to the health check result. - public Task CheckHealthAsync(HealthCheckContext context, CancellationToken ct = default) - { - var cluster = Akka.Cluster.Cluster.Get(_system); - return Task.FromResult(ClusterActivityEvaluator.SelfIsOldest(cluster) - ? HealthCheckResult.Healthy("oldest Up member (singleton host)") - : HealthCheckResult.Unhealthy("not the oldest Up member (standby)")); - } -} diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Health/SitePairActiveNodeHealthCheck.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Health/SitePairActiveNodeHealthCheck.cs index c17e0bfd..1ed049ea 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Health/SitePairActiveNodeHealthCheck.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Health/SitePairActiveNodeHealthCheck.cs @@ -9,7 +9,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Health; /// consumer reads one contract fleet-wide (200 = Active, 503 = Standby). /// /// -/// Deliberately NOT central's : that one calls +/// Deliberately NOT central's unscoped shared ActiveNodeHealthCheck: that one calls /// ClusterActivityEvaluator.SelfIsOldest(cluster) with no role argument, so on a mesh that /// carries more than one site's members it computes "oldest" across the wrong member set and a site /// node could report Active because it happens to be the oldest member overall. Delegating to diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 831506fe..6e5e1df7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -344,9 +344,10 @@ try // Ready tier (/health/ready): database + akka-cluster + required-singletons. // Active tier (/health/active): active-node = oldest Up member (singleton host) AND // database reachable (review 01 [High]) — a DB-dead active node drops from Traefik - // rotation, and the active-node check is the host-local OldestNodeActiveHealthCheck, - // NOT the shared leader-based ActiveNodeHealthCheck (leadership diverges from singleton - // placement after a restart and both sides claim leadership during a partition). + // rotation. The active-node check is the shared ActiveNodeHealthCheck, which selects by age + // as of Health 0.3.0; through 0.2.1 it selected by leadership, which diverges from singleton + // placement after a restart and names both sides during a partition, so this host kept a + // private copy until the shared one was fixed. // The Akka checks resolve ActorSystem from DI via the singleton bridge registered below; // the DatabaseHealthCheck resolves a scoped ScadaBridgeDbContext (no factory). builder.Services.AddHealthChecks() @@ -371,9 +372,12 @@ try "required-singletons", failureStatus: null, tags: new[] { ZbHealthTags.Ready }) - // Host-local oldest-member check (review 01 [High]) — see OldestNodeActiveHealthCheck. - // Resolves ActorSystem from DI, like the akka-cluster check above. - .AddTypeActivatedCheck( + // Oldest-Up-member check (review 01 [High]), unscoped: every central member competes for + // the one active slot. Since ZB.MOM.WW.Health 0.3.0 this IS the shared check — it selects + // by age, so the host-local OldestNodeActiveHealthCheck that existed only to avoid the old + // leader-based selection is gone. Resolves ActorSystem from DI, like the akka-cluster + // check above. + .AddTypeActivatedCheck( "active-node", failureStatus: null, tags: new[] { ZbHealthTags.Active }); @@ -399,9 +403,9 @@ try // consults IActiveNodeGate and defaults to "allow" when none is registered, // which leaves the design's "central cluster only (active node)" guarantee // unenforced in deployed binaries). The gate is backed by the same oldest-member - // evaluator (ClusterActivityEvaluator) as OldestNodeActiveHealthCheck above, so the - // inbound API and the /health/active endpoint Traefik routes against agree on - // which node is active. + // evaluator (ClusterActivityEvaluator, which delegates to the shared ClusterActiveNode) as + // the active-node check above, so the inbound API and the /health/active endpoint Traefik + // routes against agree on which node is active. builder.Services.AddSingleton(); // Admin-triggered manual failover of the central pair (Health page control, diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs index 83794565..46f3f924 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs @@ -218,7 +218,7 @@ public static class SiteServiceRegistration "localdb", failureStatus: null, tags: new[] { ZbHealthTags.Ready }) - // NOT central's OldestNodeActiveHealthCheck — that one is role-less; see + // NOT central's unscoped shared ActiveNodeHealthCheck — that one is role-less; see // SitePairActiveNodeHealthCheck for why a site pair needs the role-scoped answer. .AddTypeActivatedCheck( "active-node", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs index c730038e..b73843f7 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Options; using ZB.MOM.WW.Health; +using ZB.MOM.WW.Health.Akka; using ZB.MOM.WW.ScadaBridge.Host.Health; namespace ZB.MOM.WW.ScadaBridge.Host.Tests; @@ -207,11 +208,13 @@ public class HealthCheckTests : IDisposable } [Fact] - public void ActiveNodeCheck_IsHostLocalOldestNodeCheck() + public void ActiveNodeCheck_IsTheSharedOldestUpMemberCheck() { - // The shared package's ActiveNodeHealthCheck is LEADER-based (review 01 [High]): - // during a partition both nodes think they lead and Traefik serves both. The - // active-node check must be the host-local oldest-member check instead. + // The property under test is that the active tier selects by AGE, not by leadership: during a + // partition both sides think they lead and Traefik would serve both (review 01 [High]). + // Through ZB.MOM.WW.Health 0.2.1 the shared check was leader-based, so this host carried a + // private OldestNodeActiveHealthCheck; 0.3.0 made the shared check age-based, so the private + // copy is gone and the shared type is now the correct answer here. var previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT"); try { @@ -222,7 +225,7 @@ public class HealthCheckTests : IDisposable Assert.Contains(ZbHealthTags.Active, active.Tags); var check = active.Factory(factory.Services); - Assert.IsType(check); + Assert.IsType(check); } finally { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthCheckTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthCheckTests.cs index 850902f3..aa6ac71b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthCheckTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthCheckTests.cs @@ -122,14 +122,18 @@ public class SiteHealthCheckTests : IDisposable } [Fact] - public void Site_ActiveNodeCheck_IsNotCentralsRoleLessOldestCheck() + public void Site_ActiveNodeCheck_IsRoleScoped_NotTheUnscopedCentralCheck() { - // Central's OldestNodeActiveHealthCheck calls SelfIsOldest(cluster) with no role argument. - // On a mesh carrying more than one site's members that computes "oldest" across the wrong - // member set, so a site node could report Active purely for being the oldest member overall. + // Central registers the shared ActiveNodeHealthCheck UNSCOPED — oldest Up member of the whole + // cluster. On a mesh carrying more than one site's members that computes "oldest" across the + // wrong member set, so a site node could report Active purely for being the oldest member + // overall. A site must answer for its own site-{SiteId} role, which SitePairActiveNodeHealthCheck + // does by delegating to the site's IClusterNodeProvider (itself now backed by the shared + // ClusterActiveNode, so the rule is shared even though the scoping is local). var check = Assert.Single(Registrations, r => r.Name == "active-node").Factory(_host.Services); - Assert.IsNotType(check); + Assert.IsType(check); + Assert.IsNotType(check); } [Theory]