refactor(health): adopt the shared active-node check (Health 0.3.0)

OldestNodeActiveHealthCheck existed because the shared ActiveNodeHealthCheck
selected by cluster leadership (review 01 [High]): leadership is address-ordered
and diverges from singleton placement after a restart, and during a partition
both sides compute themselves leader so Traefik served both. Health 0.3.0 makes
the shared check age-based — it is now this host's own rule, promoted — so the
private copy is deleted and central registers the shared type.

ActiveNodeEvaluator, the "THE single definition of active node" this repo already
maintained, now delegates to the shared ClusterActiveNode rather than
re-implementing it. That keeps the delivery gate, the heartbeat IsActive stamp,
the inbound-API gate and the /health/active tier on one rule, and it means the
rule is shared with OtOpcUa, which had independently written a third copy.
Communication takes a ZB.MOM.WW.Health.Akka reference for it; the layering
trade-off is recorded at the PackageReference.

SitePairActiveNodeHealthCheck is deliberately KEPT. It is not a duplicate of the
rule — it is a thin adapter over the site's own IClusterNodeProvider, which
scopes to site-{SiteId} and is itself now backed by ClusterActiveNode. Registering
the shared check directly would have to re-derive the site role and would lose the
property that the tier and singleton placement come from the same provider.
Central stays unscoped: every central member competes for one active slot.

No behaviour change on any node — same rule, one implementation instead of two.

Verified: Host.Tests 439/439, Communication ActiveNode 2/2.
This commit is contained in:
Joseph Doherty
2026-07-24 13:21:58 -04:00
parent 47850c0f53
commit 5d4853a1f0
11 changed files with 61 additions and 82 deletions
+3 -3
View File
@@ -90,9 +90,9 @@
to mark tests as Skipped (not silently Passed) when MSSQL is unreachable.
-->
<PackageVersion Include="Xunit.SkippableFact" Version="1.5.61" />
<PackageVersion Include="ZB.MOM.WW.Health" Version="0.2.0" />
<PackageVersion Include="ZB.MOM.WW.Health.Akka" Version="0.2.0" />
<PackageVersion Include="ZB.MOM.WW.Health.EntityFrameworkCore" Version="0.2.0" />
<PackageVersion Include="ZB.MOM.WW.Health" Version="0.3.0" />
<PackageVersion Include="ZB.MOM.WW.Health.Akka" Version="0.3.0" />
<PackageVersion Include="ZB.MOM.WW.Health.EntityFrameworkCore" Version="0.3.0" />
<PackageVersion Include="ZB.MOM.WW.Telemetry" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Telemetry.Serilog" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.MxGateway.Client" Version="0.1.1" />
@@ -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
/// <param name="cluster">The Akka cluster to evaluate.</param>
/// <param name="role">Optional role scope; when set, only members with this role are considered.</param>
/// <returns><c>true</c> when self is Up and the oldest Up member in the role scope.</returns>
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));
}
/// <remarks>
/// Delegates to the shared <see cref="ClusterActiveNode.SelfIsActive"/>
/// (<c>ZB.MOM.WW.Health.Akka</c> 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.
/// </remarks>
public static bool SelfIsOldestUp(Cluster cluster, string? role = null) =>
ClusterActiveNode.SelfIsActive(cluster, role);
}
@@ -28,6 +28,13 @@
<PackageReference Include="Grpc.Net.Client" />
<PackageReference Include="Grpc.Tools" PrivateAssets="All" />
<PackageReference Include="ZB.MOM.WW.Configuration" />
<!-- For ClusterActiveNode — the shared "oldest Up member" primitive that ActiveNodeEvaluator was
promoted into. The store-and-forward delivery gate, the heartbeat IsActive stamp and the
/health/active tier must all name the SAME active node; delegating to one implementation is
what makes that structural. It lives in Health.Akka because that is the family's Akka-cluster
utility package (it already hosts AkkaClusterStatusPolicy and an endpoint gate); a dedicated
cluster package would be a better home if a third such primitive appears. -->
<PackageReference Include="ZB.MOM.WW.Health.Akka" />
</ItemGroup>
<ItemGroup>
@@ -14,7 +14,8 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Health;
/// <see cref="ClusterActivityEvaluator.SelfIsOldest(Cluster, string?)"/> — NOT
/// the cluster leader, whose address-ordered definition diverges from singleton
/// placement after a restart (review 01 [High]). This is the same evaluator
/// backing <see cref="OldestNodeActiveHealthCheck"/>, so
/// backing the shared <c>ActiveNodeHealthCheck</c> registered on the active tier
/// — both route to <c>ClusterActiveNode</c> — so
/// <see cref="InboundApiEndpointFilter"/> can return HTTP 503 on a standby.
///
/// Registered only in the Central-role branch of <c>Program.cs</c>. The gate
@@ -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
/// <param name="cluster">The Akka cluster to evaluate.</param>
/// <param name="role">Optional role scope; when set, only members with this role are considered.</param>
/// <returns>The oldest Up member in the role scope, or null when none is Up.</returns>
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;
}
/// <remarks>
/// Delegates to the shared <see cref="ClusterActiveNode.OldestUpMember(Cluster, string?)"/>, so
/// the label a node shows and the answer <see cref="SelfIsOldest"/> gates on come from one rule.
/// </remarks>
public static Member? OldestUpMember(Cluster cluster, string? role = null) =>
ClusterActiveNode.OldestUpMember(cluster, role);
}
@@ -1,34 +0,0 @@
using Akka.Actor;
using Microsoft.Extensions.Diagnostics.HealthChecks;
namespace ZB.MOM.WW.ScadaBridge.Host.Health;
/// <summary>
/// /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).
/// </summary>
public sealed class OldestNodeActiveHealthCheck : IHealthCheck
{
private readonly ActorSystem _system;
/// <summary>Initializes a new <see cref="OldestNodeActiveHealthCheck"/> bound to the running actor system.</summary>
/// <param name="system">The live cluster actor system.</param>
public OldestNodeActiveHealthCheck(ActorSystem system) => _system = system;
/// <summary>Reports Healthy only when this node is the oldest Up cluster member (the singleton host); otherwise reports Unhealthy.</summary>
/// <param name="context">The health check context (unused; the result depends only on cluster membership).</param>
/// <param name="ct">Cancellation token.</param>
/// <returns>A task that resolves to the health check result.</returns>
public Task<HealthCheckResult> 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)"));
}
}
@@ -9,7 +9,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Health;
/// consumer reads one contract fleet-wide (200 = Active, 503 = Standby).
/// </summary>
/// <remarks>
/// Deliberately NOT central's <see cref="OldestNodeActiveHealthCheck"/>: that one calls
/// Deliberately NOT central's unscoped shared <c>ActiveNodeHealthCheck</c>: that one calls
/// <c>ClusterActivityEvaluator.SelfIsOldest(cluster)</c> 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
+13 -9
View File
@@ -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<TContext> 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<OldestNodeActiveHealthCheck>(
// 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<ActiveNodeHealthCheck>(
"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<ZB.MOM.WW.ScadaBridge.InboundAPI.IActiveNodeGate, ActiveNodeGate>();
// Admin-triggered manual failover of the central pair (Health page control,
@@ -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<SitePairActiveNodeHealthCheck>(
"active-node",
@@ -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<OldestNodeActiveHealthCheck>(check);
Assert.IsType<ActiveNodeHealthCheck>(check);
}
finally
{
@@ -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<OldestNodeActiveHealthCheck>(check);
Assert.IsType<SitePairActiveNodeHealthCheck>(check);
Assert.IsNotType<ActiveNodeHealthCheck>(check);
}
[Theory]