diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index be3f3c59..b6b33a7d 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -344,6 +344,15 @@ services: AlarmHistorian__Enabled: "true" AlarmHistorian__MaxAttempts: "1000000" ServerHistorian__Endpoint: "${OTOPCUA_HISTORIAN_ENDPOINT:-http://histgw-absent.invalid:5222}" + # Required whenever the section is consumed: the gateway client validates its own options at + # construction and throws on an empty key, so a keyless node crash-loops during Akka startup + # rather than degrading. DEV-ONLY placeholder — it authenticates nothing, because the endpoint + # above resolves nowhere. NEVER reuse this shape for a real gateway key; supply those via + # ServerHistorian__ApiKey from the environment. + ServerHistorian__ApiKey: "${OTOPCUA_HISTORIAN_APIKEY:-histgw_dev_rig-no-gateway-present}" + # Must agree with the endpoint scheme above — the client throws on a mismatch in EITHER + # direction, so an http endpoint with the default UseTls=true crash-loops the host. + ServerHistorian__UseTls: "false" LocalDb__Path: "/app/data/otopcua-localdb.db" LocalDb__SyncListenPort: "9001" LocalDb__Replication__PeerAddress: "http://site-a-2:9001" @@ -397,6 +406,15 @@ services: AlarmHistorian__Enabled: "true" AlarmHistorian__MaxAttempts: "1000000" ServerHistorian__Endpoint: "${OTOPCUA_HISTORIAN_ENDPOINT:-http://histgw-absent.invalid:5222}" + # Required whenever the section is consumed: the gateway client validates its own options at + # construction and throws on an empty key, so a keyless node crash-loops during Akka startup + # rather than degrading. DEV-ONLY placeholder — it authenticates nothing, because the endpoint + # above resolves nowhere. NEVER reuse this shape for a real gateway key; supply those via + # ServerHistorian__ApiKey from the environment. + ServerHistorian__ApiKey: "${OTOPCUA_HISTORIAN_APIKEY:-histgw_dev_rig-no-gateway-present}" + # Must agree with the endpoint scheme above — the client throws on a mismatch in EITHER + # direction, so an http endpoint with the default UseTls=true crash-loops the host. + ServerHistorian__UseTls: "false" LocalDb__Path: "/app/data/otopcua-localdb.db" LocalDb__SyncListenPort: "9001" LocalDb__Replication__ApiKey: "dev-site-a-localdb-sync-key" @@ -434,6 +452,15 @@ services: AlarmHistorian__Enabled: "true" AlarmHistorian__MaxAttempts: "1000000" ServerHistorian__Endpoint: "${OTOPCUA_HISTORIAN_ENDPOINT:-http://histgw-absent.invalid:5222}" + # Required whenever the section is consumed: the gateway client validates its own options at + # construction and throws on an empty key, so a keyless node crash-loops during Akka startup + # rather than degrading. DEV-ONLY placeholder — it authenticates nothing, because the endpoint + # above resolves nowhere. NEVER reuse this shape for a real gateway key; supply those via + # ServerHistorian__ApiKey from the environment. + ServerHistorian__ApiKey: "${OTOPCUA_HISTORIAN_APIKEY:-histgw_dev_rig-no-gateway-present}" + # Must agree with the endpoint scheme above — the client throws on a mismatch in EITHER + # direction, so an http endpoint with the default UseTls=true crash-loops the host. + ServerHistorian__UseTls: "false" LocalDb__Path: "/app/data/otopcua-localdb.db" ports: - "4844:4840" @@ -464,6 +491,15 @@ services: AlarmHistorian__Enabled: "true" AlarmHistorian__MaxAttempts: "1000000" ServerHistorian__Endpoint: "${OTOPCUA_HISTORIAN_ENDPOINT:-http://histgw-absent.invalid:5222}" + # Required whenever the section is consumed: the gateway client validates its own options at + # construction and throws on an empty key, so a keyless node crash-loops during Akka startup + # rather than degrading. DEV-ONLY placeholder — it authenticates nothing, because the endpoint + # above resolves nowhere. NEVER reuse this shape for a real gateway key; supply those via + # ServerHistorian__ApiKey from the environment. + ServerHistorian__ApiKey: "${OTOPCUA_HISTORIAN_APIKEY:-histgw_dev_rig-no-gateway-present}" + # Must agree with the endpoint scheme above — the client throws on a mismatch in EITHER + # direction, so an http endpoint with the default UseTls=true crash-loops the host. + ServerHistorian__UseTls: "false" LocalDb__Path: "/app/data/otopcua-localdb.db" ports: - "4845:4840" diff --git a/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json b/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json index 27c13a64..2eb2139a 100644 --- a/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json +++ b/docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json @@ -75,10 +75,11 @@ { "id": 8, "subject": "Task 8: Live gate on the docker-dev rig (needs explicit user go-ahead)", - "status": "pending", + "status": "completed", "blockedBy": [ 7 - ] + ], + "note": "Gate doc: docs/plans/2026-07-20-localdb-phase2-live-gate.md. Checks 1/2/5/6 PASS; checks 3+4 NOT SATISFIED (see below). FOUR production defects found + fixed, three of which crash-looped every driver node before check 1 could run: (1) empty ServerHistorian:ApiKey crashes the host - the validator had explicitly classified it as 'degrades', which is false because the gateway client validates its own options at construction; (2) UseTls vs endpoint scheme mismatch crashes likewise, both directions; (3) plaintext h2c was UNREACHABLE - the adapter forwarded TLS-only options unconditionally so every documented http:// deployment crashed; (4) THE BLOCKER - the drain gate deferred to a CLUSTER-WIDE elected Primary while the queue is PAIR-LOCAL. On the rig that Primary is central-1 (carries the driver Akka role, replicates nobody, runs no historian), so ALL FOUR driver nodes suspended their drains including unpaired site-b: silent permanent loss where pre-Phase-2 drained fine. Fixed in 3 layers (separate ShouldDrainAlarmHistory policy; peer-host matching in DriverHostActor; gate short-circuited entirely when replication is unconfigured, testing BOTH PeerAddress and SyncListenPort since only the dialer sets the former). Also fixed a THIRD vacuous test: AwaitAssert on the seeded-open value passed with the guard deleted; now asserts the sequence of PUBLISHED values via a recording view. Migration evidence: 11 legacy rows across two overlapping files -> exactly 9 identical rows on both nodes, proving D-6's payload-hash identity live. OPEN DESIGN FORK (in the gate doc): a pair cannot identify its own Primary, so both halves drain - safe (no loss, duplicates only) but the gate's de-dup benefit is unrealised." } ], "lastUpdated": "2026-07-21T00:00:00Z" diff --git a/docs/plans/2026-07-20-localdb-phase2-live-gate.md b/docs/plans/2026-07-20-localdb-phase2-live-gate.md new file mode 100644 index 00000000..8016bf18 --- /dev/null +++ b/docs/plans/2026-07-20-localdb-phase2-live-gate.md @@ -0,0 +1,151 @@ +# LocalDb Phase 2 — live gate (docker-dev rig) + +> Task 8 of `2026-07-20-localdb-adoption-phase2.md`. Evidence log. +> Rig: local `docker-dev/docker-compose.yml` — 2 central + site-a pair (replication ON) + site-b pair +> (default-OFF), one Akka cluster, one shared SQL. +> +> **Safety rules followed** (same as the Phase 1 gate): all DB inspection via `docker cp` of the +> `db`/`-wal`/`-shm` triplet out of the container, querying the copy — never host `sqlite3` on the +> live WAL file. + +## Headline + +**The gate did not get past its own first boot before finding real defects.** Four production bugs, +all fixed on-branch with regression tests, plus a fifth finding in the test suite itself. Three of +the four crash-looped every driver node; the fourth silently stopped alarm history everywhere. + +Checks 1, 2, 5 and 6 pass. **Checks 3 and 4 are NOT satisfied** — see "The blocker" below; they +cannot be run as written on a rig that has no per-pair redundancy roles, and the defect they were +meant to confirm turned out to be the opposite of what the plan assumed. + +## Seeding the migration + +The rig had no pre-consolidation `alarm-historian.db`, so one was synthesised per node and `docker +cp`-ed to `/app/alarm-historian.db` (the WORKDIR-relative default) into created-but-not-started +containers — the exact upgrade shape. + +Deliberately overlapping content, to test D-6 in production: **6 rows on site-a-1, 5 on site-a-2, +2 of them byte-identical payloads** (the warm-pair duplication `HistorianAdapterActor` produces in +every boot window). 11 legacy rows, **9 distinct**. + +## Defects found and fixed + +### 1. Empty `ServerHistorian:ApiKey` crash-loops the host (was classified "degrades") + +`AlarmHistorian:Enabled=true` requires the gateway alarm writer, which requires a key. +`HistorianGatewayClientOptions.Validate()` throws `The gateway API key must not be empty` inside the +client factory during Akka startup, so every driver node crash-looped. + +`ServerHistorianOptionsValidator` exists precisely to convert that class of failure into a named +`OptionsValidationException` — but its documented fail tier explicitly excluded `ApiKey`, reasoning +that a keyless client "degrades rather than crashes (the gateway rejects calls)". **That premise is +false**: the client validates its own options at construction, so the process dies before any call. +Fixed; the key is never echoed in the message. + +### 2. `UseTls` / endpoint-scheme mismatch crash-loops the host + +Immediately after fixing #1, the rig crash-looped again: `UseTls` defaults to `true`, the rig +endpoint is `http://`, and the client throws `UseTls requires an https gateway endpoint`. The +inverse (`https://` + `UseTls=false`) throws too — both strings confirmed in the shipped client +assembly. Setting an `http` endpoint without clearing `UseTls` is an ordinary migration slip that +takes the host down. Now validated in both directions. + +### 3. Plaintext h2c was *unreachable* — every `http://` deployment crashed + +Third crash-loop, and a genuine product bug rather than operator config. +`HistorianGatewayClientAdapter.Create` forwarded the TLS-only options unconditionally, and +`AllowUntrustedServerCertificate` defaults to `false`, so it always sent +`RequireCertificateValidation=true` — which the client rejects outright when `UseTls=false` +(`RequireCertificateValidation is a TLS-only option and requires UseTls=true`). + +CLAUDE.md and `docs/Historian.md` both document `http://` as the supported way to select h2c, but +**no h2c configuration could start**, and the only workaround was to claim +`AllowUntrustedServerCertificate=true` — a certificate posture for a connection with no certificate. +Fixed: TLS-only options stay at their defaults when `UseTls=false`. + +### 4. THE BLOCKER — the drain gate deferred to a Primary that cannot deliver + +With the rig finally up, **every driver node logged `Historian drain suspended`. Nothing drained +anywhere** — including the two site-b nodes, which have no redundancy peer and no replication at all. +Before Phase 2 this deployment drained fine, because the drain was ungated. + +Root cause, established from the rig rather than guessed: + +- `RedundancyStateActor` derives roles from Akka's **cluster-wide** `RoleLeader("driver")`. +- `central-1`/`central-2` carry **both** `admin` and `driver` Akka roles (`OTOPCUA_ROLES=admin,driver`). +- So the elected driver Primary is **central-1** — a node that replicates nobody's LocalDb and does + not even run the alarm historian. +- Every site node was `Secondary`, saw that a Primary existed, and stood down in its favour. + +**The redundancy role is a cluster-wide election; the alarm queue is pair-local.** Deferring on +"somebody is Primary" is therefore wrong in principle: in a multi-pair fleet the Primary is usually +in another pair and holds none of this node's rows. The consequence is not a duplicate — it is the +buffer growing on every node until the capacity wall evicts the oldest events, i.e. silent permanent +loss of the audit trail the queue exists to protect. + +Fixed in three layers, each with a positive control: + +1. `PrimaryGatePolicy.ShouldDrainAlarmHistory(role, queueSharingPeerIsPrimary)` — a **separate** + policy from `ShouldServiceAsPrimary`. Unknown role ⇒ drain. A node stands down only for a Primary + that holds its rows. The two gates now deliberately disagree, and a test pins that disagreement so + nobody "harmonises" them back. +2. `DriverHostActor` publishes that verdict, matching the snapshot's Primary against this node's + configured LocalDb replication peer host. +3. `AddAlarmHistorian` short-circuits the gate entirely when replication is not configured — a node + whose rows exist nowhere else must never defer. The predicate tests **both** `Replication:PeerAddress` + and `SyncListenPort`, because only the dialing half sets the former and both halves share the queue. + +The asymmetry of the error costs is what drives every one of these: a false allow costs a duplicate +history row, which the design already accepts (*at-least-once across a failover, by design*, and +payload-hash ids collapse identical events); a false deny loses data silently. + +### 5. A third vacuous test — `AwaitAssert` on the seeded value + +Deleting the guard (publishing the old write-gate verdict) left `DriverHostActorRoleViewTests` +**green**. `AwaitAssert` polls until an assertion passes, and the assertion was "the view reads +open" — which is also the value the view is *seeded* with, so it succeeded at the first poll, before +the actor had processed anything. It could not distinguish a correct publish from no publish at all. + +Fixed by asserting on the **sequence of published values** via a recording view, never on current +state. Re-run under the control: red, for exactly the cases that matter. This is the same +"absence-assertion passes by race" trap recorded for #485/#486, in a new disguise. + +## Checks + +| # | Check | Result | Evidence | +|---|---|---|---| +| 1 | Migration ran; `.migrated` sidecar on both site-a nodes; row counts match | ✅ PASS | `alarm-historian.db.migrated` present on both. 11 legacy rows across the two files → **exactly 9 rows on each node**, id sets byte-identical — the 2 shared payloads collapsed, proving D-6's payload-hash identity in production. Each node holds the *other's* rows (`legacy/a1-only-*` present on a-2 and vice versa). Legacy `attempt_count` distinction survived the copy (the 3-attempt row still leads the 0-attempt rows), `last_error` carried, 0 dead-lettered | +| 2 | Alarm burst converges: identical rowsets, oplog drains to 0, dead letters 0 | ✅ PASS | Identical id sets on both nodes; `__localdb_oplog` depth **0** on both; **0** dead-lettered. Drain visibly live against the deliberately unresolvable gateway — `attempt_count` climbing 33 → 42 → 168 with `last_error='retry-please'`, which is exactly the buffering behaviour the queue exists for | +| 3 | Only the primary drains | ❌ **NOT SATISFIED** — see defect 4 | As written the check assumes a per-pair Primary exists. On this rig the elected Primary is a central node outside both pairs, so the honest post-fix behaviour is that **all four nodes drain**. No node can currently identify a queue-sharing Primary: site-b is unpaired, and of the site-a pair only the dialing half knows its peer. Safe (no loss, duplicates only), but the check's intent is unproven | +| 4 | Failover: standby resumes draining; count double-delivered events | ⛔ **NOT RUN** | Superseded by defect 4. A meaningful failover test needs a rig where the pair itself elects a Primary; measuring "double delivery" is meaningless while both halves drain unconditionally | +| 5 | Both nodes restarted together: clean rejoin, identical counts, no I/O errors | ✅ PASS | `docker compose restart site-a-1 site-a-2` → **0** `disk I/O error` / `SQLITE_IOERR` / `corrupt` lines and 0 fatals in either log; both back to 9 rows / 0 dead / oplog 0, still identical | +| 6 | site-b default-OFF pin: sink works locally, no sync traffic | ✅ PASS | `alarm_sf_events` created and registered on both site-b nodes; port **9001 not bound** (`/proc/net/tcp`), no sync listener, no replication config; both drain their own queue (defect 4's third fix) with 0 fatals | + +## The open design question + +The gate leaves one question that is a design fork, not a bug to patch: + +**A pair cannot currently identify its own Primary.** Redundancy roles are elected cluster-wide, but +the queue is pair-local. Only the dialing half of a pair knows its peer (`Replication:PeerAddress`); +the listening half sets `SyncListenPort` only. So the drain gate can never fully engage, and both +halves of a replicated pair drain — safe, but duplicated. + +Three ways out, in increasing order of scope: + +- **(A) Configure both halves to dial.** Smallest change; makes the peer known on both sides so the + gate engages symmetrically. Needs confirmation that the LocalDb library accepts bidirectional dial, + and an operator-facing config change. +- **(B) Accept duplicates for unpaired-role deployments.** Change nothing further, document that the + gate only engages when redundancy roles are configured per pair. Cheapest; leaves the steady-state + double-delivery the gate was introduced to remove. +- **(C) Scope redundancy roles per pair in `RedundancyStateActor`.** Fixes the root cause for *every* + consumer of the role (ServiceLevel, scripted-alarm emit gate, inbound-write gate), all of which + inherit the same cluster-wide assumption. Largest blast radius, and the only one that makes + "Primary" mean what the docs say it means in a multi-pair fleet. + +The branch is safe in every topology as it stands — no configuration loses data — so this is a +question of when to reclaim the de-duplication benefit, not an outstanding hazard. + +## Not merged + +Per the plan, nothing on this branch is merged. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/HistorianGatewayClientAdapter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/HistorianGatewayClientAdapter.cs index 5f429f30..c7a323d1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/HistorianGatewayClientAdapter.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/HistorianGatewayClientAdapter.cs @@ -57,16 +57,23 @@ public sealed class HistorianGatewayClientAdapter : IHistorianGatewayClient, IDi $"ServerHistorian:Endpoint must be an absolute http(s) URI (e.g. https://host:5222); got '{options.Endpoint}'."); } + // TLS-only options must stay at their defaults on a plaintext h2c connection: the client rejects + // each of them outright when UseTls=false (" is a TLS-only option and requires + // UseTls=true"). Forwarding them unconditionally made h2c UNREACHABLE — AllowUntrustedServer- + // Certificate defaults to false, so RequireCertificateValidation was always sent as true and + // every http:// deployment crashed at startup, even though the scheme is documented as the + // supported way to select h2c. There is no certificate to have a posture about here. var clientOptions = new HistorianGatewayClientOptions { Endpoint = endpointUri, ApiKey = options.ApiKey, UseTls = options.UseTls, - CaCertificatePath = options.CaCertificatePath, - // INVERTED mapping: ServerHistorianOptions.AllowUntrustedServerCertificate (opt-in to accept - // a self-signed cert) is the negation of the client's RequireCertificateValidation. Allowing - // an untrusted cert == not requiring validation; a pinned CaCertificatePath always verifies. - RequireCertificateValidation = !options.AllowUntrustedServerCertificate, + CaCertificatePath = options.UseTls ? options.CaCertificatePath : null, + // INVERTED mapping (TLS only): ServerHistorianOptions.AllowUntrustedServerCertificate + // (opt-in to accept a self-signed cert) is the negation of the client's + // RequireCertificateValidation. Allowing an untrusted cert == not requiring validation; a + // pinned CaCertificatePath always verifies. + RequireCertificateValidation = options.UseTls && !options.AllowUntrustedServerCertificate, DefaultCallTimeout = options.CallTimeout, LoggerFactory = loggerFactory, }; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs index ca727a5f..a9ebcc86 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs @@ -24,12 +24,25 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; /// in the Host, so Enabled covers it. /// /// -/// Fail tier = provably-crashing configs only. Only an empty / non-absolute / non-http(s) -/// Endpoint fails here (it throws in the factory). Empty ApiKey and non-positive -/// MaxTieClusterOverfetch degrade rather than crash (the gateway rejects calls / the node -/// manager surfaces a Bad read), so they stay operator warnings in +/// Fail tier = provably-crashing configs only. Three settings qualify, and they all fail +/// the same way — HistorianGatewayClientOptions.Validate() throws inside the client +/// factory before any call is attempted, so the host dies at startup: +/// an empty / non-absolute / non-http(s) Endpoint; an empty ApiKey ("The gateway +/// API key must not be empty"); and a UseTls that disagrees with the endpoint scheme +/// ("UseTls requires an https gateway endpoint" / "An https gateway endpoint requires UseTls"). +/// A non-positive MaxTieClusterOverfetch genuinely degrades rather than crashes (the node +/// manager surfaces a Bad read), so it stays an operator warning in /// . The Endpoint value is not a secret and -/// is echoed to make the error actionable; the ApiKey is never surfaced. +/// is echoed to make each error actionable; the ApiKey is never surfaced. +/// +/// +/// The ApiKey and UseTls checks were added after two consecutive live-rig +/// crash-loops (LocalDb Phase 2 gate). Both had been classified as degrading, on the +/// assumption that a bad client configuration would connect and be rejected by the gateway — but +/// the client validates its own options at construction, so the process dies during Akka startup +/// instead, which is precisely the failure this validator exists to convert into a named, +/// aggregated error. The lesson generalises: "degrades" is only true of settings the client +/// does not itself validate. /// /// public sealed class ServerHistorianOptionsValidator : OptionsValidatorBase @@ -62,5 +75,26 @@ public sealed class ServerHistorianOptionsValidator : OptionsValidatorBase private readonly IRedundancyRoleView? _redundancyRoleView; + /// + /// Host of this node's LocalDb replication partner — the only node that holds a copy of this + /// node's alarm queue, and therefore the only node it may ever stand down in favour of. + /// null when this node dials nobody (unpaired, or the listening half of a pair). + /// + private readonly string? _replicationPeerHost; + /// Debounces the S5 "snapshot omitted this node" Warning to once per process — a persistent /// identity mismatch (03/S5) would otherwise log on every snapshot. private bool _warnedSnapshotMissingLocalNode; @@ -372,7 +379,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers IDriverCapabilityInvokerFactory? invokerFactory = null, Func? driverMemberCountProvider = null, IDeploymentArtifactCache? deploymentArtifactCache = null, - IRedundancyRoleView? redundancyRoleView = null) => + IRedundancyRoleView? redundancyRoleView = null, + string? replicationPeerHost = null) => // WARNING: this forwarding list is POSITIONAL, and Props.Create compiles it into an // expression tree. Six IActorRef? parameters and several interface-typed ones mean a // mis-ordered argument is usually type-compatible and therefore compiles clean, then binds @@ -382,7 +390,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers dbFactory, localNode, coordinator, driverFactory, localRoles, dependencyMux, opcUaPublishActor, healthPublisher, virtualTagEvaluator, historyWriter, virtualTagHostOverride, loggerFactory, scriptRootLogger, scriptedAlarmHostOverride, invokerFactory, driverMemberCountProvider, - deploymentArtifactCache, redundancyRoleView)); + deploymentArtifactCache, redundancyRoleView, replicationPeerHost)); /// Initializes a new DriverHostActor with the specified dependencies. /// Database context factory for configuration database access. @@ -433,10 +441,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers IDriverCapabilityInvokerFactory? invokerFactory = null, Func? driverMemberCountProvider = null, IDeploymentArtifactCache? deploymentArtifactCache = null, - IRedundancyRoleView? redundancyRoleView = null) + IRedundancyRoleView? redundancyRoleView = null, + string? replicationPeerHost = null) { _deploymentArtifactCache = deploymentArtifactCache; _redundancyRoleView = redundancyRoleView; + _replicationPeerHost = replicationPeerHost; _dbFactory = dbFactory; _localNode = localNode; _coordinatorOverride = coordinator; @@ -1465,6 +1475,14 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers } } + /// Host part of a host:port node id. + private static string HostOf(NodeId nodeId) + { + var text = nodeId.ToString(); + var colon = text.LastIndexOf(':'); + return colon < 0 ? text : text[..colon]; + } + /// The Primary-gate deny reason tag for the denial meter (secondary|detached|role-unknown). private string PrimaryGateDenyReason() => _localRole switch { @@ -1496,11 +1514,23 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _localNode, string.Join(",", msg.Nodes.Select(n => n.NodeId))); } - // Publish on EVERY snapshot, including ones that leave the cached role untouched: the other - // half of the decision is the driver member count, which moves with cluster membership and - // not with this message. Snapshots are re-published on a heartbeat, so a membership change - // is picked up within one heartbeat rather than waiting for a role change that may never come. - _redundancyRoleView?.Publish(ShouldServiceAsPrimary()); + // Publish on EVERY snapshot, including ones that leave the cached role untouched, so a role + // that changes without this node being named still reaches the drain within one heartbeat. + // + // NOTE the different policy: the drain gate keys on the role ALONE and opens when it is + // unknown, where the data-plane gate above resolves an unknown role by member count and + // denies. Publishing ShouldServiceAsPrimary() here suspended the drain on every node of a + // multi-driver cluster that had no redundancy configured — see ShouldDrainAlarmHistory. + // Defer ONLY to the node that holds a copy of our rows. The redundancy election is + // cluster-wide and the queue is pair-local, so "somebody is Primary" is not a licence to stop + // draining — that Primary is usually in another pair entirely, and cannot deliver our events. + var peerIsPrimary = + _replicationPeerHost is not null + && msg.Nodes.Any(n => n.Role == RedundancyRole.Primary + && HostOf(n.NodeId) == _replicationPeerHost); + + _redundancyRoleView?.Publish( + PrimaryGatePolicy.ShouldDrainAlarmHistory(_localRole, queueSharingPeerIsPrimary: peerIsPrimary)); } private void Stale() diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs index 90486a19..86a0e3bf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs @@ -25,4 +25,50 @@ public static class PrimaryGatePolicy RedundancyRole.Secondary or RedundancyRole.Detached => false, _ => driverMemberCount <= 1, // unknown: allow only when no driver peer exists }; + + /// + /// Decide whether this node should drain the replicated alarm store-and-forward queue. + /// + /// + /// + /// Deliberately not . That gate protects a shared + /// field device, where two nodes acting at once is dangerous and irreversible, so an unknown + /// role with a peer present must deny. This gate protects an append-only historian, and the + /// delivery contract is already at-least-once across a failover, by design — identical + /// events even collapse to a single row, because ids hash the payload. + /// + /// + /// The two error costs are therefore asymmetric: a false allow costs a duplicate history row; + /// a false deny silently stops the alarm audit trail and eventually evicts it at the capacity + /// wall. So this decision keys on the role alone and opens when the role is unknown — + /// no membership tie-break, because cluster-wide driver count says nothing about whether + /// this node has a redundant partner. + /// + /// + /// Found by the LocalDb Phase 2 live gate: on a rig of four driver nodes in one cluster with + /// no Redundancy section, borrowing the write gate's verdict suspended the drain on + /// every node — including unpaired ones — so nothing drained anywhere. + /// + /// + /// This node's last-known redundancy role, or null when unknown. + /// + /// Whether the node holding in the same snapshot is a node + /// that shares this node's queue — i.e. its LocalDb replication partner. + /// + /// Merely knowing that a Primary exists is not enough, because the redundancy role is + /// a CLUSTER-WIDE election while the alarm queue is PAIR-LOCAL. A fleet runs many pairs in + /// one cluster, so the elected driver Primary is usually in some other pair — and on the + /// docker-dev rig it is a central node, which carries the driver Akka role, replicates + /// nobody's LocalDb and does not even run the alarm historian. Deferring to it means this + /// node's events are delivered by no one, ever. + /// + /// + /// true to drain; false only when a node holding these same rows is doing it. + public static bool ShouldDrainAlarmHistory(RedundancyRole? localRole, bool queueSharingPeerIsPrimary) => + localRole switch + { + RedundancyRole.Primary => true, + RedundancyRole.Secondary or RedundancyRole.Detached => !queueSharingPeerIsPrimary, + _ => true, // unknown role ⇒ drain; a duplicate row beats a silent gap + }; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs index a74797f5..a9c41dff 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs @@ -3,44 +3,55 @@ using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; namespace ZB.MOM.WW.OtOpcUa.Runtime.Redundancy; /// -/// A singleton snapshot of the Primary-gate decision, so code that lives outside an actor can -/// ask "should this node be servicing Primary-only work right now?". +/// A singleton snapshot of the alarm-history drain decision, so code that lives outside an actor +/// can ask "should this node be draining the alarm queue right now?". /// /// /// /// Exists for the alarm store-and-forward drain. That drain runs on a timer owned by the /// sink, not on an actor mailbox, so it cannot receive RedundancyStateChanged -/// directly — but it must be Primary-scoped, because the queue it drains replicates to the +/// directly — but it must be role-scoped, because the queue it drains replicates to the /// pair peer and two draining nodes would deliver every event twice. /// /// -/// Deliberately publishes the decision rather than the role and member count that -/// produced it, so stays the single place that decides. +/// Deliberately publishes the decision rather than the role that produced it, so +/// stays the single place that decides. +/// +/// +/// This is not the device-write gate. It is fed by +/// , which opens on an unknown role +/// where closes. The naming is +/// explicit about the consumer for that reason: an earlier revision published the +/// write-gate verdict here, and on a cluster with several driver nodes but no configured +/// redundancy it suspended the drain everywhere, so alarm history accumulated on every node +/// and left none. /// /// public interface IRedundancyRoleView { - /// Whether this node should service Primary-only work right now. - bool ShouldServiceAsPrimary { get; } + /// Whether this node should be draining the alarm store-and-forward queue right now. + bool ShouldDrainAlarmHistory { get; } /// Records a fresh decision. Called by DriverHostActor on every redundancy snapshot. - /// The decision produced. - void Publish(bool shouldServiceAsPrimary); + /// + /// The decision produced. + /// + void Publish(bool shouldDrainAlarmHistory); } /// Thread-safe backed by a volatile field. public sealed class RedundancyRoleView : IRedundancyRoleView { - // Seeded with the decision for "role unknown, no driver peer" rather than a bare false. An - // unpublished view and a node with no peer are genuinely the same situation, and they must - // behave the same: a deployment that runs no redundancy at all never publishes here, and - // defaulting closed would silently stop its alarm history forever. - private volatile bool _shouldServiceAsPrimary = - PrimaryGatePolicy.ShouldServiceAsPrimary(localRole: null, driverMemberCount: 0); + // Seeded with the decision for "role unknown" rather than a bare false. An unpublished view and a + // node whose role nothing ever reports are genuinely the same situation, and they must behave the + // same: a deployment that runs no redundancy at all never publishes here, and defaulting closed + // would silently stop its alarm history forever. + private volatile bool _shouldDrainAlarmHistory = + PrimaryGatePolicy.ShouldDrainAlarmHistory(localRole: null, queueSharingPeerIsPrimary: false); /// - public bool ShouldServiceAsPrimary => _shouldServiceAsPrimary; + public bool ShouldDrainAlarmHistory => _shouldDrainAlarmHistory; /// - public void Publish(bool shouldServiceAsPrimary) => _shouldServiceAsPrimary = shouldServiceAsPrimary; + public void Publish(bool shouldDrainAlarmHistory) => _shouldDrainAlarmHistory = shouldDrainAlarmHistory; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index f00f9663..9cdbeddb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -107,6 +107,34 @@ public static class ServiceCollectionExtensions // posture for a deployment that runs no redundancy at all. services.TryAddSingleton(); + // THE GATE ONLY APPLIES TO A REPLICATED QUEUE. Standing down is only ever safe because some + // other node holds the same rows and will send them instead — and the only node that does is + // this node's LocalDb replication peer. Without replication configured, these rows exist here + // and nowhere else, so deferring to anyone means they are never delivered by anyone. + // + // This is not hypothetical. The redundancy role is a CLUSTER-WIDE election + // (RedundancyStateActor keys on Akka's RoleLeader("driver")), while the queue is PAIR-LOCAL. + // On the docker-dev rig the elected driver Primary is a central node — which carries the + // driver Akka role, replicates nobody's LocalDb, and does not even run the alarm historian — + // so every site node dutifully suspended its drain in favour of a node that could not + // possibly deliver its events. Scoping the gate to "is my queue actually shared?" is what + // keeps the two scopes from disagreeing. + // BOTH halves of a pair share the queue, but only one of them dials: the initiator sets + // Replication:PeerAddress, its partner only sets SyncListenPort and waits. Testing the dial + // side alone would leave the listening half permanently ungated — one drainer per pair by + // accident rather than by role, and the wrong one whenever the roles swap. + var replicated = + !string.IsNullOrWhiteSpace(configuration["LocalDb:Replication:PeerAddress"]) + || !string.IsNullOrWhiteSpace(configuration["LocalDb:SyncListenPort"]); + + if (!replicated) + { + Serilog.Log.Logger.ForContext().Information( + "Alarm historian: LocalDb replication is not configured, so this node's queue is not " + + "shared with any peer and the Primary drain gate does not apply — this node always " + + "drains its own alarm queue."); + } + services.AddSingleton(sp => { // LocalDbStoreAndForwardSink takes a Serilog ILogger (not Microsoft.Extensions.Logging). @@ -121,7 +149,7 @@ public static class ServiceCollectionExtensions capacity: opts.Capacity, deadLetterRetention: TimeSpan.FromDays(opts.DeadLetterRetentionDays), maxAttempts: opts.MaxAttempts, - drainGate: () => roleView.ShouldServiceAsPrimary); + drainGate: () => !replicated || roleView.ShouldDrainAlarmHistory); sink.StartDrainLoop(TimeSpan.FromSeconds(opts.DrainIntervalSeconds)); return sink; }); @@ -252,6 +280,16 @@ public static class ServiceCollectionExtensions // Registered by AddAlarmHistorian; absent when no durable sink is configured, in which // case there is nothing downstream to inform. var redundancyRoleView = resolver.GetService(); + // Host of this node's LocalDb replication partner: the ONLY node that holds a copy of this + // node's alarm queue, and so the only node it may stand down in favour of. Null when this + // node dials nobody, which correctly means "never stand down". + var replicationPeerHost = + Uri.TryCreate( + resolver.GetService()?["LocalDb:Replication:PeerAddress"], + UriKind.Absolute, + out var peerUri) + ? peerUri.Host + : null; // Root script logger backs the ScriptedAlarm host's engine + script logging. Registered in // Host DI inside the hasDriver block; may be absent in some role configs / test harnesses, // in which case the DriverHostActor gracefully skips spawning the ScriptedAlarm host. @@ -371,7 +409,8 @@ public static class ServiceCollectionExtensions scriptRootLogger: scriptRootLogger, invokerFactory: invokerFactory, deploymentArtifactCache: deploymentArtifactCache, - redundancyRoleView: redundancyRoleView), + redundancyRoleView: redundancyRoleView, + replicationPeerHost: replicationPeerHost), DriverHostActorName); registry.Register(driverHost); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/HistorianGatewayClientAdapterTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/HistorianGatewayClientAdapterTests.cs index 1a86a92e..55719278 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/HistorianGatewayClientAdapterTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/HistorianGatewayClientAdapterTests.cs @@ -22,6 +22,61 @@ public sealed class HistorianGatewayClientAdapterTests Assert.NotNull(adapter); } + /// + /// A plaintext h2c gateway (an http:// endpoint with UseTls=false) must + /// construct, using nothing but documented defaults. + /// + /// + /// + /// Found by the LocalDb Phase 2 live gate, on the third consecutive crash-loop of the rig. + /// docs/Historian.md and CLAUDE.md both document http:// as a supported + /// transport ("scheme selects transport: https = TLS, http = h2c"), but it was in fact + /// unreachable: this factory forwarded the TLS-only options unconditionally, and + /// defaults to + /// , so it always sent RequireCertificateValidation=true — + /// which the client rejects outright when UseTls=false + /// ("RequireCertificateValidation is a TLS-only option and requires UseTls=true"). + /// + /// + /// So every h2c deployment crashed at startup, and the only way to avoid it was to + /// set AllowUntrustedServerCertificate=true — asserting a certificate posture for a + /// connection that has no certificate at all. + /// + /// + [Fact] + public void Adapter_constructs_for_plaintext_h2c_endpoint() + { + var opts = new ServerHistorianOptions + { + Enabled = true, Endpoint = "http://localhost:5222", ApiKey = "histgw_x_y", UseTls = false, + }; + + using var adapter = HistorianGatewayClientAdapter.Create(opts, NullLoggerFactory.Instance); + + adapter.ShouldNotBeNull(); + } + + /// + /// A pinned CA path is likewise TLS-only, and must not leak into an h2c client — the same + /// rejection, reached by a different field. + /// + [Fact] + public void Adapter_ignores_tls_only_settings_on_a_plaintext_endpoint() + { + var opts = new ServerHistorianOptions + { + Enabled = true, + Endpoint = "http://localhost:5222", + ApiKey = "histgw_x_y", + UseTls = false, + CaCertificatePath = "/etc/ssl/certs/leftover-from-a-tls-config.pem", + }; + + using var adapter = HistorianGatewayClientAdapter.Create(opts, NullLoggerFactory.Instance); + + adapter.ShouldNotBeNull(); + } + /// /// archreview 06/S-11 — an enabled historian with an empty Endpoint must fail with a /// named, config-key-carrying (defense-in-depth for any diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ServerHistorianOptionsValidatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ServerHistorianOptionsValidatorTests.cs index d374c6b9..54baf356 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ServerHistorianOptionsValidatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ServerHistorianOptionsValidatorTests.cs @@ -62,11 +62,18 @@ public sealed class ServerHistorianOptionsValidatorTests result.Failures.ShouldContain(f => f.Contains("ServerHistorian:Endpoint")); } - /// Enabled with a valid absolute https endpoint succeeds. + /// + /// Enabled with a valid absolute https endpoint and a key succeeds. The key is not + /// incidental: this case previously omitted it and still asserted success, which pinned a config + /// that actually crash-loops the host — see . + /// [Fact] public void Enabled_with_valid_https_endpoint_succeeds() { - var result = Sut().Validate(null, new ServerHistorianOptions { Enabled = true, Endpoint = "https://host:5222" }); + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "https://host:5222", ApiKey = "histgw_id_secret", + }); result.Succeeded.ShouldBeTrue(); } @@ -96,16 +103,145 @@ public sealed class ServerHistorianOptionsValidatorTests f.Contains("ServerHistorian:Endpoint") && f.Contains("AlarmHistorian:Enabled")); } - /// Alarm-only mode with a valid endpoint succeeds. + /// Alarm-only mode with a valid endpoint and a key succeeds. [Fact] public void Disabled_but_alarm_historian_enabled_with_valid_endpoint_succeeds() { var result = Sut(alarmHistorianEnabled: true) - .Validate(null, new ServerHistorianOptions { Enabled = false, Endpoint = "https://host:5222" }); + .Validate(null, new ServerHistorianOptions + { + Enabled = false, Endpoint = "https://host:5222", ApiKey = "histgw_id_secret", + }); result.Succeeded.ShouldBeTrue(); } + /// + /// An empty ApiKey on a consumed section fails, because it crashes — it does not + /// degrade. + /// + /// + /// + /// Found by the LocalDb Phase 2 live gate. The rig booted with + /// AlarmHistorian:Enabled=true and a valid endpoint but no key, and every driver + /// node crash-looped on + /// ArgumentException: The gateway API key must not be empty (Parameter 'ApiKey') + /// thrown by HistorianGatewayClientOptions.Validate() — reached through + /// GatewayHistorian.CreateAlarmWriter during Akka startup. + /// + /// + /// That is the *same* factory path, and the same crash-loop-under-a-restart-policy + /// failure mode, this validator was built to convert into a named + /// OptionsValidationException. The previous "empty ApiKey degrades, the gateway + /// just rejects calls" premise was simply wrong: the client validates its own options + /// at construction, so the process never reaches the point of making a call. + /// + /// + [Fact] + public void Consumed_with_empty_api_key_fails() + { + var result = Sut(alarmHistorianEnabled: true) + .Validate(null, new ServerHistorianOptions + { + Enabled = false, Endpoint = "https://host:5222", ApiKey = "", + }); + + result.Failed.ShouldBeTrue(); + result.Failures.ShouldContain(f => + f.Contains("ServerHistorian:ApiKey") && f.Contains("AlarmHistorian:Enabled")); + } + + /// The read path has the identical crash, so it is gated identically. + [Fact] + public void Enabled_with_empty_api_key_fails() + { + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "https://host:5222", ApiKey = "", + }); + + result.Failed.ShouldBeTrue(); + result.Failures.ShouldContain(f => f.Contains("ServerHistorian:ApiKey")); + } + + /// + /// The key must never be echoed. An endpoint is not a secret and is quoted to make the error + /// actionable; a key is, so the failure names only the setting. + /// + [Fact] + public void Api_key_failure_never_echoes_the_key() + { + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "https://host:5222", ApiKey = " ", + }); + + result.Failed.ShouldBeTrue(); + result.Failures.ShouldNotContain(f => f.Contains(" ", StringComparison.Ordinal)); + } + + /// A section nobody consumes may be keyless — no failure. + [Fact] + public void Disabled_with_empty_api_key_succeeds() + { + var result = Sut().Validate(null, new ServerHistorianOptions { Enabled = false, ApiKey = "" }); + + result.Succeeded.ShouldBeTrue(); + } + + /// + /// UseTls=true against an http:// endpoint fails — the third member of the + /// crash-at-construction family, and the easiest of the three for an operator to trip. + /// + /// + /// Also found by the Phase 2 live gate, immediately after the ApiKey fix: the rig points + /// at a deliberately unresolvable http:// endpoint, and UseTls defaults to + /// , so every node crash-looped a second time on + /// ArgumentException: UseTls requires an https gateway endpoint. Switching an endpoint + /// from https to http without clearing UseTls is an ordinary migration slip, and it takes + /// the host down rather than degrading it. + /// + [Fact] + public void Consumed_with_use_tls_against_http_endpoint_fails() + { + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "http://host:5222", ApiKey = "histgw_id_secret", UseTls = true, + }); + + result.Failed.ShouldBeTrue(); + result.Failures.ShouldContain(f => + f.Contains("ServerHistorian:UseTls") && f.Contains("http://host:5222")); + } + + /// An http endpoint with TLS explicitly off is the valid h2c shape — no failure. + [Fact] + public void Consumed_with_http_endpoint_and_tls_off_succeeds() + { + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "http://host:5222", ApiKey = "histgw_id_secret", UseTls = false, + }); + + result.Succeeded.ShouldBeTrue(); + } + + /// + /// The mirror slip — UseTls=false against an https:// endpoint — also fails, so the + /// scheme and the flag are pinned to agree in both directions rather than in one. + /// + [Fact] + public void Consumed_with_tls_off_against_https_endpoint_fails() + { + var result = Sut().Validate(null, new ServerHistorianOptions + { + Enabled = true, Endpoint = "https://host:5222", ApiKey = "histgw_id_secret", UseTls = false, + }); + + result.Failed.ShouldBeTrue(); + result.Failures.ShouldContain(f => f.Contains("ServerHistorian:UseTls")); + } + /// /// Wiring guard (the "register-AND-consume" trap): resolving IOptions<ServerHistorianOptions>.Value /// after throws diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRoleViewTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRoleViewTests.cs index b5c3ed6b..c46952fa 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRoleViewTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorRoleViewTests.cs @@ -21,10 +21,10 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// every alarm event twice. /// /// -/// These cases mirror 's matrix on purpose: -/// the view must reach exactly the same verdict as the inbound-write and native-ack gates, -/// because a second, subtly different notion of "am I the Primary" is how a pair ends up -/// with one node writing and neither draining. +/// These cases deliberately DIVERGE from in the +/// unknown-role case: the write gate denies when a peer may exist, this one allows. A duplicate +/// history row is cheap and already contemplated by the at-least-once contract; a silently +/// un-drained queue is not. See AlarmHistoryDrainGatePolicyTests. /// /// public sealed class DriverHostActorRoleViewTests : RuntimeActorTestBase @@ -43,15 +43,37 @@ public sealed class DriverHostActorRoleViewTests : RuntimeActorTestBase [Fact] public void An_unpublished_view_reads_as_primary() { - new RedundancyRoleView().ShouldServiceAsPrimary.ShouldBeTrue(); + new RedundancyRoleView().ShouldDrainAlarmHistory.ShouldBeTrue(); } + /// A Secondary closes the view — but only because the snapshot also names a Primary. [Fact] - public void Secondary_role_closes_the_view() + public void Secondary_role_closes_the_view_when_a_primary_exists() { var view = SpawnAndTellRole(RedundancyRole.Secondary, driverMemberCount: 2); - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeFalse(), duration: Timeout); + AwaitAssert(() => view.Published.ShouldBe([false]), duration: Timeout); + } + + /// + /// A Secondary whose Primary is some OTHER pair's node keeps draining: that node holds none of + /// this node's rows, so deferring to it would deliver them never. + /// + [Fact] + public void Secondary_role_keeps_the_view_open_when_the_primary_is_not_its_peer() + { + var view = new RecordingRoleView(); + SpawnHost(view, driverMemberCount: 4).Tell(new RedundancyStateChanged( + [ + new NodeRedundancyState(TestNode, RedundancyRole.Secondary, + IsClusterLeader: false, IsRoleLeaderForDriver: false, AsOfUtc: DateTime.UtcNow), + // A Primary DOES exist — in another pair. It is not this node's replication peer. + new NodeRedundancyState(NodeId.Parse("some-other-pair:4053"), RedundancyRole.Primary, + IsClusterLeader: true, IsRoleLeaderForDriver: true, AsOfUtc: DateTime.UtcNow), + ], + CorrelationId.NewId())); + + AwaitAssert(() => view.Published.ShouldBe([true]), duration: Timeout); } [Fact] @@ -59,67 +81,118 @@ public sealed class DriverHostActorRoleViewTests : RuntimeActorTestBase { var view = SpawnAndTellRole(RedundancyRole.Primary, driverMemberCount: 2); - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeTrue(), duration: Timeout); + // Published, not current — `true` is the seed. See the note in the unknown-role theory. + AwaitAssert(() => view.Published.ShouldBe([true]), duration: Timeout); } - /// Unknown role + a real driver peer ⇒ closed, matching the write gate's default-DENY. - [Fact] - public void Unknown_role_with_a_driver_peer_closes_the_view() + /// + /// Unknown role ⇒ the view stays open, even with driver peers present — the deliberate + /// divergence from the write gate's default-DENY. + /// + /// + /// + /// This case previously asserted the opposite, on the stated goal of mirroring the write + /// gate. The Phase 2 live gate showed that goal to be wrong for this consumer: the rig runs + /// four driver nodes in one cluster with no Redundancy section, so every role was + /// unknown and the driver count was 4 — and every node logged "Historian drain suspended", + /// including the two unpaired site-b nodes with no peer and no replication. Alarm history + /// drained nowhere, where before Phase 2 it drained fine. + /// + /// + /// See AlarmHistoryDrainGatePolicyTests for why the asymmetry of the two error costs + /// makes fail-open correct here and fail-closed correct for device writes. + /// + /// + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(4)] + public void Unknown_role_leaves_the_view_open_whatever_the_peer_count(int driverMemberCount) { // A snapshot that never names this node is how the role stays unknown in practice — the // documented identity-mismatch shape, not a contrived case. - var view = SpawnAndTellForeignSnapshot(driverMemberCount: 2); + var view = SpawnAndTellForeignSnapshot(driverMemberCount); - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeFalse(), duration: Timeout); - } - - /// Unknown role + no driver peer ⇒ open, preserving the single-node posture. - [Fact] - public void Unknown_role_without_a_driver_peer_leaves_the_view_open() - { - var view = SpawnAndTellForeignSnapshot(driverMemberCount: 1); - - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeTrue(), duration: Timeout); + // Assert on what was PUBLISHED, never on the current value. `true` is also the seeded + // default, so `AwaitAssert(() => view.ShouldDrainAlarmHistory.ShouldBeTrue())` passes at the + // first poll — before the actor has processed the snapshot — and therefore passes just as + // happily when the publish is wrong or absent. Verified: with the old write-gate verdict + // restored, the current-value form stayed green and only this form goes red. + AwaitAssert(() => view.Published.ShouldBe([true]), duration: Timeout); } /// A demotion must move the view, not just an initial promotion. [Fact] public void A_demotion_closes_a_previously_open_view() { - var view = new RedundancyRoleView(); + var view = new RecordingRoleView(); var host = SpawnHost(view, driverMemberCount: 2); host.Tell(Snapshot(TestNode, RedundancyRole.Primary)); - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeTrue(), duration: Timeout); + AwaitAssert(() => view.Published.ShouldBe([true]), duration: Timeout); host.Tell(Snapshot(TestNode, RedundancyRole.Secondary)); - AwaitAssert(() => view.ShouldServiceAsPrimary.ShouldBeFalse(), duration: Timeout); + AwaitAssert(() => view.Published.ShouldBe([true, false]), duration: Timeout); } // ---------------- helpers ---------------- - private RedundancyRoleView SpawnAndTellRole(RedundancyRole role, int driverMemberCount) + /// + /// A view that remembers every published decision, so a test can assert on the actor's + /// action rather than on a state the seed already satisfies. + /// + private sealed class RecordingRoleView : IRedundancyRoleView { - var view = new RedundancyRoleView(); + private readonly List _published = []; + private readonly Lock _gate = new(); + private readonly RedundancyRoleView _inner = new(); + + /// Every value published so far, in order. + public IReadOnlyList Published + { + get { lock (_gate) return _published.ToArray(); } + } + + public bool ShouldDrainAlarmHistory => _inner.ShouldDrainAlarmHistory; + + public void Publish(bool shouldDrainAlarmHistory) + { + lock (_gate) _published.Add(shouldDrainAlarmHistory); + _inner.Publish(shouldDrainAlarmHistory); + } + } + + private RecordingRoleView SpawnAndTellRole(RedundancyRole role, int driverMemberCount) + { + var view = new RecordingRoleView(); SpawnHost(view, driverMemberCount).Tell(Snapshot(TestNode, role)); return view; } - private RedundancyRoleView SpawnAndTellForeignSnapshot(int driverMemberCount) + private RecordingRoleView SpawnAndTellForeignSnapshot(int driverMemberCount) { - var view = new RedundancyRoleView(); + var view = new RecordingRoleView(); SpawnHost(view, driverMemberCount).Tell(Snapshot(NodeId.Parse("some-other-node"), RedundancyRole.Primary)); return view; } + /// The peer this node replicates with — the only node it may ever stand down for. + private const string PeerHost = "the-peer"; + private IActorRef SpawnHost(IRedundancyRoleView view, int driverMemberCount) => Sys.ActorOf(DriverHostActor.Props( NewInMemoryDbFactory(), TestNode, coordinator: null, driverMemberCountProvider: () => driverMemberCount, - redundancyRoleView: view)); + redundancyRoleView: view, + replicationPeerHost: PeerHost)); + /// + /// A snapshot of a real pair: this node in , plus a peer holding the + /// other half. The peer matters — a Secondary only stands down when a Primary exists, so a + /// single-entry snapshot would silently change what these cases test. + /// private static RedundancyStateChanged Snapshot(NodeId node, RedundancyRole role) => new( [ @@ -127,6 +200,12 @@ public sealed class DriverHostActorRoleViewTests : RuntimeActorTestBase IsClusterLeader: role == RedundancyRole.Primary, IsRoleLeaderForDriver: role == RedundancyRole.Primary, AsOfUtc: DateTime.UtcNow), + new NodeRedundancyState( + NodeId.Parse($"{PeerHost}:4053"), + role == RedundancyRole.Primary ? RedundancyRole.Secondary : RedundancyRole.Primary, + IsClusterLeader: role != RedundancyRole.Primary, + IsRoleLeaderForDriver: role != RedundancyRole.Primary, + AsOfUtc: DateTime.UtcNow), ], CorrelationId.NewId()); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs index 07dac1d6..7a1262a9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs @@ -36,3 +36,86 @@ public sealed class PrimaryGatePolicyTests public void Unknown_role_resolves_by_membership(int members, bool expected) => PrimaryGatePolicy.ShouldServiceAsPrimary(null, members).ShouldBe(expected); } + +/// +/// The alarm-history drain gate, which deliberately does not mirror the device-write gate. +/// +/// +/// +/// Why a second policy is correct here. The write gate protects a shared field device: +/// two nodes driving one PLC is dangerous and irreversible, so an unknown role with a peer +/// present must deny. The drain writes to an append-only historian instead, and Phase 2's +/// delivery contract is already at-least-once across a failover, by design — identical +/// events even collapse to one row, because ids are a hash of the payload. +/// +/// +/// So the two error costs are wildly asymmetric: a false allow costs a duplicate history row, +/// while a false deny silently stops the alarm audit trail and eventually evicts it at the +/// capacity wall. This gate therefore keys on the role alone and opens when the role is +/// unknown. +/// +/// +/// Found by the LocalDb Phase 2 live gate. The rig runs four driver nodes in one Akka +/// cluster with no Redundancy section, so every node's role was unknown and the +/// cluster-wide driver count was 4 — and every node, including the two unpaired site-b nodes +/// that have no peer and no replication at all, logged "Historian drain suspended". Nothing +/// drained anywhere. Before Phase 2 that deployment drained fine, because the drain was ungated. +/// +/// +public sealed class AlarmHistoryDrainGatePolicyTests +{ + [Theory] + // A Primary always drains its own queue. + [InlineData(RedundancyRole.Primary, true, true)] + [InlineData(RedundancyRole.Primary, false, true)] + // A Secondary stands down ONLY when a Primary actually exists to stand up. + [InlineData(RedundancyRole.Secondary, true, false)] + [InlineData(RedundancyRole.Detached, true, false)] + [InlineData(RedundancyRole.Secondary, false, true)] + [InlineData(RedundancyRole.Detached, false, true)] + public void A_known_role_stands_down_only_for_a_primary_that_exists( + RedundancyRole role, bool queueSharingPeerIsPrimary, bool expected) + => PrimaryGatePolicy.ShouldDrainAlarmHistory(role, queueSharingPeerIsPrimary).ShouldBe(expected); + + /// + /// Unknown role ⇒ drain, whoever else is out there. The first case the live gate broke on, and + /// the one that separates this policy from . + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void An_unknown_role_drains(bool queueSharingPeerIsPrimary) + => PrimaryGatePolicy.ShouldDrainAlarmHistory(null, queueSharingPeerIsPrimary).ShouldBeTrue(); + + /// + /// A Secondary whose Primary is NOT its queue-sharing peer keeps draining — the deepest of the + /// live gate's findings. + /// + /// + /// RedundancyStateActor derives roles from Akka's cluster-wide RoleLeader("driver"), + /// but the alarm queue is pair-local. On the docker-dev rig the elected Primary is a CENTRAL node + /// — it carries the driver Akka role, replicates nobody's LocalDb, and does not even run the + /// alarm historian — so under a plain "a Primary exists ⇒ stand down" rule every site node + /// suspended its drain in favour of a node that could not possibly deliver its events. The + /// buffer then grows on every node until the capacity wall evicts the oldest: silent, permanent + /// loss of the audit trail the queue exists to protect. + /// + [Fact] + public void A_secondary_whose_peer_is_not_the_primary_keeps_draining() + { + // A Primary exists somewhere in the cluster, but it does not hold this node's rows. + PrimaryGatePolicy.ShouldDrainAlarmHistory(RedundancyRole.Secondary, queueSharingPeerIsPrimary: false) + .ShouldBeTrue("a Secondary must not defer to a Primary that cannot deliver its events"); + } + + /// + /// The divergence, pinned explicitly: with a peer present and no role known, the write gate + /// denies and the drain gate allows. If someone later "harmonises" the two, this fails. + /// + [Fact] + public void The_two_gates_deliberately_disagree_when_the_role_is_unknown_and_a_peer_exists() + { + PrimaryGatePolicy.ShouldServiceAsPrimary(localRole: null, driverMemberCount: 4).ShouldBeFalse(); + PrimaryGatePolicy.ShouldDrainAlarmHistory(localRole: null, queueSharingPeerIsPrimary: true).ShouldBeTrue(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/AlarmHistorianRegistrationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/AlarmHistorianRegistrationTests.cs index 59d375e7..32531012 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/AlarmHistorianRegistrationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/AlarmHistorianRegistrationTests.cs @@ -7,6 +7,7 @@ using Xunit; using ZB.MOM.WW.LocalDb; using ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian; using ZB.MOM.WW.OtOpcUa.Runtime.Historian; +using ZB.MOM.WW.OtOpcUa.Runtime.Redundancy; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Historian; @@ -102,6 +103,109 @@ public sealed class AlarmHistorianRegistrationTests } } + /// + /// A node whose LocalDb is not replicated drains its own queue regardless of redundancy role. + /// + /// + /// + /// Standing down is only safe because a peer holds the same rows and will send them + /// instead. Without LocalDb:Replication:PeerAddress there is no such peer: the rows + /// are here and nowhere else, so any deferral means nobody ever delivers them. + /// + /// + /// Found by the Phase 2 live gate. The redundancy role is a CLUSTER-WIDE election while the + /// queue is PAIR-LOCAL, and on the docker-dev rig the elected driver Primary turned out to + /// be a central node — one that carries the driver Akka role, replicates nobody's LocalDb + /// and does not even run the alarm historian. Every site node suspended its drain in favour + /// of a node that could not possibly deliver its events. + /// + /// + [Theory] + // Neither key: an unpaired node. Its rows exist nowhere else. + [InlineData(null, null, true)] + // The dialing half of a pair — gated, because its partner holds the same rows. + [InlineData("http://peer:9001", null, false)] + // The LISTENING half. Same shared queue, but it never dials, so a PeerAddress-only test would + // wrongly class it as unpaired and leave it permanently ungated. + [InlineData(null, "9001", false)] + public async Task Only_an_unreplicated_node_ignores_the_role_view( + string? peerAddress, string? syncListenPort, bool expectDrain) + { + var tempDir = Path.Combine(Path.GetTempPath(), "otopcua-alarmhist-test-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDir); + var dbPath = Path.Combine(tempDir, "node-local.db"); + + try + { + var config = ConfigFrom(new Dictionary + { + ["AlarmHistorian:Enabled"] = "true", + ["LocalDb:Path"] = dbPath, + ["LocalDb:Replication:PeerAddress"] = peerAddress, + ["LocalDb:SyncListenPort"] = syncListenPort, + }); + + var services = BaseServices(); + services.AddZbLocalDb(config, db => + { + using var connection = db.CreateConnection(); + AlarmSfSchema.Apply(connection); + db.RegisterReplicated(AlarmSfSchema.EventsTable); + }); + + // A role view pinned SHUT: this node is a Secondary and a Primary exists elsewhere. + services.AddSingleton(new StandDownRoleView()); + services.AddAlarmHistorian(config, (_, _) => new FakeWriter()); + + using (var provider = services.BuildServiceProvider()) + { + var sink = provider.GetRequiredService(); + await sink.EnqueueAsync(SampleEvent, CancellationToken.None); + + await ((LocalDbStoreAndForwardSink)sink).DrainOnceAsync(CancellationToken.None); + + var status = sink.GetStatus(); + if (expectDrain) + { + // Delivered and removed — not parked behind a gate that nobody else will open. + status.DrainState.ShouldNotBe(HistorianDrainState.NotPrimary); + status.QueueDepth.ShouldBe(0); + } + else + { + // Held for the peer that shares this queue. + status.DrainState.ShouldBe(HistorianDrainState.NotPrimary); + status.QueueDepth.ShouldBe(1); + } + } + } + finally + { + SqliteConnection.ClearAllPools(); + try { Directory.Delete(tempDir, recursive: true); } catch (IOException) { /* best effort */ } + } + } + + /// A view that always says "some other node is the Primary". + private sealed class StandDownRoleView : IRedundancyRoleView + { + public bool ShouldDrainAlarmHistory => false; + + public void Publish(bool shouldDrainAlarmHistory) { } + } + + private static AlarmHistorianEvent SampleEvent => new( + AlarmId: "eq/alarm-1", + EquipmentPath: "line-1/eq-1", + AlarmName: "temp-high", + AlarmTypeName: "LimitAlarm", + Severity: ZB.MOM.WW.OtOpcUa.Core.Abstractions.AlarmSeverity.High, + EventKind: "Activated", + Message: "temperature high", + User: "system", + Comment: null, + TimestampUtc: new DateTime(2026, 7, 21, 0, 0, 1, DateTimeKind.Utc)); + /// /// An enabled historian with no LocalDb registered must fail at resolution, loudly. ///