2bae1b4c9f
Never registered in DI and dead since Phase 6.1. Keeping two unwired cache designs invites the next reader to wire the wrong one. Deletes all three test files - LiteDbConfigCacheTests.cs was missing from the plan's list and would have failed to compile (it calls new LiteDB.LiteDatabase directly). StaleConfigFlagTests lives inside ResilientConfigReaderTests.cs, so it goes too. The XML-doc reference in ILdapGroupRoleMappingService is rewritten rather than removed: it now records that no sign-in fallback exists and that reviving one means an admin-side cache on LocalDb.
618 lines
34 KiB
Markdown
618 lines
34 KiB
Markdown
# LocalDb Phase 1 — Task 0 Recon Findings
|
|
|
|
**Date:** 2026-07-20
|
|
**Branch:** `feat/localdb-phase1` (based on `master`)
|
|
**Plan:** `docs/plans/2026-07-20-localdb-adoption-phase1.md`
|
|
**Design authority:** `~/Desktop/scadaproj/docs/plans/2026-07-20-otopcua-localdb-design.md`
|
|
|
|
All `file:line` citations are against the branch base commit.
|
|
|
|
---
|
|
|
|
## Step 1 — Feed verification: PASS
|
|
|
|
```
|
|
$ dotnet package search ZB.MOM.WW.LocalDb --source dohertj2-gitea --format json
|
|
ZB.MOM.WW.LocalDb 0.1.1
|
|
ZB.MOM.WW.LocalDb.Contracts 0.1.1
|
|
ZB.MOM.WW.LocalDb.Replication 0.1.1
|
|
```
|
|
|
|
All three present at **0.1.1**. No STOP. (Note: the local NuGet cache holds both `0.1.0` and `0.1.1`
|
|
— pin exactly `0.1.1` so a stale cache cannot win.)
|
|
|
|
---
|
|
|
|
## Step 3 — STOP conditions: NONE TRIGGERED
|
|
|
|
| STOP condition | Verdict |
|
|
|---|---|
|
|
| Artifact apply path cannot take a post-apply hook without restructuring the actor | **Clear** — `ApplyAndAck` has a clean insertion point (§2.2) |
|
|
| `DeploymentId` / cluster identity are not stable strings/GUIDs | **Clear** — both are stable strings, but cluster identity is **not resolvable at boot**; see D-1 |
|
|
| LiteDB LocalCache is referenced by live code | **Clear** — zero live-code references (§7) |
|
|
|
|
Three STOP conditions clear. **Eight deviations** from the plan's stated assumptions are recorded in
|
|
§9 — four of them (D-1, D-3, D-5, D-6) change the work materially.
|
|
|
|
---
|
|
|
|
## 1. Deploy fetch path
|
|
|
|
### 1.1 Blob-load sites — there are **five** `CreateDbContext()` calls, not two
|
|
|
|
| Line | Method | Purpose |
|
|
|---|---|---|
|
|
| `DriverHostActor.cs:513` | `Bootstrap()` | `NodeDeploymentStates` + `Deployments` at PreStart |
|
|
| `DriverHostActor.cs:1472` | `ReconcileDrivers(DeploymentId)` | **loads `ArtifactBlob`** |
|
|
| `DriverHostActor.cs:1543` | `PushDesiredSubscriptions(DeploymentId)` | **loads `ArtifactBlob` again** |
|
|
| `DriverHostActor.cs:2000` | `TryRecoverFromStale()` | latest `Sealed` deployment (id + hash only, no blob) |
|
|
| `DriverHostActor.cs:2029` | `UpsertNodeDeploymentState(...)` | writes `NodeDeploymentState` |
|
|
|
|
`DriverHostActor.cs:1472-1476`:
|
|
```csharp
|
|
using var db = _dbFactory.CreateDbContext();
|
|
blob = db.Deployments.AsNoTracking()
|
|
.Where(d => d.DeploymentId == deploymentId.Value)
|
|
.Select(d => d.ArtifactBlob)
|
|
.FirstOrDefault() ?? Array.Empty<byte>();
|
|
```
|
|
`:1543-1547` is byte-identical apart from the local. **The same blob is read twice per apply.**
|
|
|
|
### 1.2 Artifact runtime type — there is no instance type
|
|
|
|
`DeploymentArtifact` (`DeploymentArtifact.cs:48`) is a **static parser class** over
|
|
`ReadOnlySpan<byte>`. The blob is never materialised into an object graph.
|
|
|
|
- Storage: `byte[]` — `Deployment.cs:29` `public byte[] ArtifactBlob { get; init; } = Array.Empty<byte>();`
|
|
- Format: **UTF-8 JSON**, `JsonDocument.Parse` (`DeploymentArtifact.cs:66`, `:490`). Not MessagePack.
|
|
- Leniency contract: empty/malformed blob → empty result, **never throws** (`:62`, `:537-541`).
|
|
|
|
**Consequence for the cache:** we cache raw `byte[]`. No serializer coupling. Good.
|
|
|
|
### 1.3 Identity types
|
|
|
|
| Type | Definition | Shape |
|
|
|---|---|---|
|
|
| `DeploymentId` | `Commons/Types/DeploymentId.cs:3` — `readonly record struct DeploymentId(Guid Value)` | `ToString()` → `Value.ToString("N")`, 32 hex chars, **no hyphens** (`:10`) |
|
|
| `RevisionHash` | `Commons/Types/RevisionHash.cs:8` — `readonly record struct RevisionHash(string Value)` | SHA-256 hex, **lowercase 64 chars**, no `0x`; empty invalid (`:3-6`) |
|
|
|
|
Both are stable, filesystem/SQL-safe strings. Use `DeploymentId.ToString()` (the "N" form) as the
|
|
`deployment_id` TEXT column value.
|
|
|
|
---
|
|
|
|
## 2. Post-apply point
|
|
|
|
### 2.1 `ApplyAndAck` control flow — `DriverHostActor.cs:1413-1458`
|
|
|
|
```
|
|
1415 _applyingDeploymentId = deploymentId;
|
|
1416 Become(Applying);
|
|
1425 UpsertNodeDeploymentState(..., Applying, null)
|
|
1427 try {
|
|
1429 ReconcileDrivers(deploymentId); // blob fetch #1 — SWALLOWS DB errors
|
|
1430 _currentRevision = revision;
|
|
1431 UpsertNodeDeploymentState(..., Applied, null);
|
|
1432 SendAck(..., ApplyAckOutcome.Applied, null, correlation);
|
|
1436 _opcUaPublishActor?.Tell(RebuildAddressSpace(...));
|
|
1439 PushDesiredSubscriptions(deploymentId); // blob fetch #2 — SWALLOWS DB errors
|
|
1440 OtOpcUaTelemetry.DeploymentApplied.Add(1, "outcome"="ack");
|
|
1441 _log.Info("applied deployment ...");
|
|
1443 }
|
|
1444 catch (Exception ex) { ...Failed ACK... }
|
|
1452 finally { _applyingDeploymentId = null; Become(Steady); }
|
|
```
|
|
|
|
### 2.2 Where the cache write belongs
|
|
|
|
**Immediately after `DriverHostActor.cs:1439`**, before `:1440` — but **wrapped in its own
|
|
try/catch**. Reason: `:1432` already sent an `Applied` ACK to the coordinator. An unguarded cache
|
|
write that throws would fall into the `catch` at `:1444` and send a *second*, contradictory
|
|
`Failed` ACK for a deployment the fleet already believes is applied.
|
|
|
|
### 2.3 Paths that must NOT write to the cache
|
|
|
|
1. **`catch` at `:1444`** — any apply failure.
|
|
2. **Idempotent short-circuit at `:1402-1409`** — `HandleDispatchFromSteady` returns early with an
|
|
`Applied` ACK when `_currentRevision == msg.RevisionHash`. `ApplyAndAck` is never entered and no
|
|
blob is read.
|
|
3. **⚠ THE SILENT-DEGRADATION TRAP.** `ReconcileDrivers:1478-1483` and
|
|
`PushDesiredSubscriptions:1549-1553` catch DB failures, log a **warning**, and `return` **without
|
|
rethrowing**. `ApplyAndAck` therefore reaches `:1440` and declares success having applied **zero
|
|
drivers** when the blob load failed. Caching "whatever we read" would **persist an empty artifact
|
|
as a successful apply** — poisoning the boot-from-cache path with a config that starts no drivers.
|
|
|
|
**Mitigation (load-bearing):** the cache write must be gated on the blob having actually loaded
|
|
non-empty. Have `ReconcileDrivers` surface the loaded blob upward (a field or an out-param)
|
|
rather than re-reading it a third time, and skip the cache write when it is
|
|
`Array.Empty<byte>()`. This also removes the duplicate read noted in §1.1.
|
|
|
|
4. **`RestoreApplied` (`:1514-1528`)** — the bootstrap replay path. Restoring from central; caching
|
|
here is harmless and arguably desirable, but it is a distinct seam. **Phase 1 scope: skip it.**
|
|
5. **`TryRecoverFromStale` (`:1996-2022`)** — never reads the artifact at all (see §3.4).
|
|
|
|
---
|
|
|
|
## 3. Cold-boot path
|
|
|
|
### 3.1 PreStart — `DriverHostActor.cs:415-436`
|
|
|
|
Ctor already did `Become(Steady)` at `:411`. PreStart subscribes to topics, spawns the virtual-tag
|
|
and scripted-alarm hosts, then calls `Bootstrap()` at `:435`.
|
|
|
|
### 3.2 `Bootstrap()` — `DriverHostActor.cs:506-565` — **yes, it queries central SQL at boot**
|
|
|
|
Two queries inside one `try`: latest `NodeDeploymentState` for self (`:514-518`), then the matching
|
|
`Deployment` row (`:527-532`). Four outcomes: no prior deployments → Steady; `Applied` →
|
|
`RestoreApplied` (`:544`); `Applying` orphan → `ApplyAndAck` replay (`:549`); `Failed` → Steady.
|
|
|
|
### 3.3 **THE SEAM** — `DriverHostActor.cs:560-564`
|
|
|
|
```csharp
|
|
catch (Exception ex)
|
|
{
|
|
_log.Warning(ex, "DriverHost {Node}: ConfigDb unreachable on bootstrap; entering Stale", _localNode);
|
|
Become(Stale);
|
|
}
|
|
```
|
|
|
|
**This is the exact point where "central fetch failed at boot" is known.** The cache fallback goes
|
|
between `:562` and `:563`: attempt a local-cache load, and only `Become(Stale)` if the cache is also
|
|
a miss.
|
|
|
|
⚠ The catch also fires if the *second* query throws after the first succeeded, so `latest` is not in
|
|
scope-valid state from the catch. **The cache must be self-describing** — it must carry its own
|
|
`DeploymentId` + `RevisionHash`, because at this seam the actor has no idea what it should be
|
|
restoring. The planned `deployment_pointer` shape already satisfies this.
|
|
|
|
### 3.4 The `Stale` state — `DriverHostActor.cs:1343-1379`
|
|
|
|
`ReconnectInterval = 30s` (`:55`), retry timer started at `:1378`.
|
|
|
|
The dispatch-ignore at **`:1345-1348`** discards `DispatchDeployment` entirely — no ACK, no
|
|
buffering (contrast `Applying:599` which does `Self.Forward(msg)`). A Stale node looks like a
|
|
timeout to the coordinator. Phase 1 does **not** change this (per design §3.3: a *new* deployment
|
|
still requires central).
|
|
|
|
`RouteNodeWrite` fast-fails at `:1359-1360` with `"driver host stale (config DB unreachable)"`.
|
|
|
|
### 3.5 Pre-existing gap worth recording (not Phase 1 scope)
|
|
|
|
`TryRecoverFromStale` (`:1996-2022`) sets `_currentRevision` and marks `Applied` **without calling
|
|
`ReconcileDrivers` or `PushDesiredSubscriptions`** — documented at `:1692-1695` and `:1706`. A
|
|
Stale-recovered node reports Applied while running **zero drivers**, and
|
|
`HandleDispatchFromSteady:1402` then short-circuits on revision match. Boot-from-cache largely
|
|
neutralises this in practice, but the bookkeeping remains optimistic. **Logged as a follow-up, not
|
|
fixed here.**
|
|
|
|
### 3.6 `DbHealthProbeActor`
|
|
|
|
**No interaction with `DriverHostActor` whatsoever.** Spawned as a sibling
|
|
(`ServiceCollectionExtensions.cs:243-246`), consumed only by `OpcUaPublishActor`
|
|
(`OpcUaPublishActor.cs:105`, `:256`, `:528`, `:543-544`). `DriverHostActor` runs its own independent
|
|
30s `retry-db` timer. If running-from-cache should later influence OPC UA ServiceLevel,
|
|
`OpcUaPublishActor`'s DB-health seam is the precedent to mirror.
|
|
|
|
---
|
|
|
|
## 4. Cluster identity — **the one real design problem (see D-1)**
|
|
|
|
**There is no `ClusterId` field on `DriverHostActor` and no accessor for it.** The actor knows only
|
|
`_localNode` (`DriverHostActor.cs:61`, type `Commons.Types.NodeId`, aliased at `:28`).
|
|
|
|
`ClusterId` is resolved **per-artifact**, by `DeploymentArtifact.ResolveClusterScope(blob, nodeId)`
|
|
(`DeploymentArtifact.cs:485-516`) — it scans the artifact's `Nodes[]` array for a matching `NodeId`
|
|
and reads that element's `ClusterId`. Types: `ClusterScope` (`:46`), `ClusterFilterMode` (`:28`).
|
|
|
|
`IClusterRoleInfo` (`Commons/Interfaces/IClusterRoleInfo.cs:12`) exposes `LocalNode`, `LocalRoles`,
|
|
`HasRole`, `MembersWithRole`, `RoleLeader`, `RoleLeaderChanged` — **no `ClusterId`**.
|
|
`docker-dev/docker-compose.yml` has `Cluster__Hostname`/`Port`/`Roles`/`SeedNodes` — **no
|
|
`Cluster__ClusterId`**. The `ClusterNode` entity
|
|
(`Configuration/Entities/ClusterNode.cs:7,10`) carries both, but lives in central SQL — exactly
|
|
what is unreachable at the seam that needs it.
|
|
|
|
> **The bind:** the design keys the cache by `ClusterId` so a replicated pair shares one cache
|
|
> entry. But `ClusterId` is only discoverable *from a blob you already have* or *from central SQL*.
|
|
> At the §3.3 boot-failure seam we have neither.
|
|
|
|
**Resolution adopted (D-1):** keep `cluster_id` as the `deployment_pointer` PK — pair sharing is the
|
|
whole point of replication and must not be given up. Resolve it at each end as follows:
|
|
|
|
- **Write path** (`ApplyAndAck`, §2.2): call `DeploymentArtifact.ResolveClusterScope(blob, nodeId)`
|
|
on the blob just applied. `ClusterFilterMode.ScopeTo` → use its `ClusterId`. `None` (single-cluster
|
|
artifact) or `Suppress` → fall back to the literal `"__single"` sentinel, matching the actor's own
|
|
existing degenerate-case convention at `:1827-1830`.
|
|
- **Read path** (boot seam, §3.3): we cannot compute the key, so **do not key the read**. Select the
|
|
single `deployment_pointer` row, ordered by `applied_at_utc DESC`, `LIMIT 1`. A node belongs to
|
|
exactly one cluster and its pair peer replicates the same cluster's row, so the table holds one
|
|
row in every real topology. If more than one row is present (a node was re-homed between
|
|
clusters), take the newest and **log a warning naming both cluster ids** — an operator-visible
|
|
signal rather than a silent wrong-config boot.
|
|
- Optional escape hatch: honour a `Cluster:ClusterId` config key when present. **Deferred** — not
|
|
needed for the rig, and adding a required key contradicts the design's "storage ships
|
|
unconditionally" posture.
|
|
|
|
This preserves the replication payoff (design §3.3) and the schema exactly as specified, and
|
|
confines the change to the read query's `WHERE` clause.
|
|
|
|
---
|
|
|
|
## 5. `DriverHostActor` construction — the `Props` expression tree
|
|
|
|
### 5.1 The factory — `DriverHostActor.cs:326-346`
|
|
|
|
`static Props(...)` takes **16 parameters**, 14 optional, and forwards them **fully positionally**
|
|
into `Akka.Actor.Props.Create(() => new DriverHostActor(...))` at **`:343-346`**. Ctor at
|
|
`:375-412`, assignments `:393-408`.
|
|
|
|
> ⚠ Six `IActorRef?` parameters and three interface-typed parameters mean many mis-bindings are
|
|
> **type-compatible and therefore compile clean**. Callers use *named* arguments (safe); the
|
|
> internal forwarding at `:343-346` is *positional* (unsafe).
|
|
>
|
|
> **Rule for Task 7: append the new parameter LAST in the `Props` signature, LAST in the ctor
|
|
> signature, and LAST in the positional forwarding list at `:346`. Change nothing else.**
|
|
|
|
### 5.2 Construction sites — **27 total** (1 production + 26 test)
|
|
|
|
**Production (1):** `Runtime/ServiceCollectionExtensions.cs:332`, inside `WithOtOpcUaRuntimeActors`
|
|
(`:193`); registered as `DriverHostActorKey` (`:344`). All args named except the first three. Does
|
|
not pass `virtualTagHostOverride`, `scriptedAlarmHostOverride`, or `driverMemberCountProvider`.
|
|
|
|
`WithOtOpcUaRuntimeActors` callers: `Host/Program.cs:331`,
|
|
`Host.IntegrationTests/TwoNodeClusterHarness.cs:398`, `ServiceCollectionExtensionsTests.cs:42,144`.
|
|
|
|
**Tests (26)**, all in `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/`:
|
|
|
|
| File | Lines |
|
|
|---|---|
|
|
| `Drivers/DriverHostActorTests.cs` | 28, 60, 81, 118, 143, 189 |
|
|
| `Drivers/DriverHostActorReconcileTests.cs` | 37, 59, 90, 119, 156, 193 |
|
|
| `Drivers/DriverHostActorDiscoveryTests.cs` | 385, 671, 748, 1023 |
|
|
| `Drivers/DriverHostActorWriteRoutingTests.cs` | 148, 208 |
|
|
| `Drivers/DriverHostActorPrimaryGateTests.cs` | 228, 247 |
|
|
| `Drivers/DriverHostActorVirtualTagTests.cs` | 44, 76 |
|
|
| `Drivers/DriverHostActorNativeAlarmTests.cs` | 350 |
|
|
| `Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs` | 126 |
|
|
| `Drivers/DriverHostActorProbeResultDropTests.cs` | 66 |
|
|
| `Drivers/DriverHostActorHistoryWriterTests.cs` | 59 |
|
|
| `Drivers/DiscoveryInjectionEndToEndTests.cs` | 219 |
|
|
| `VirtualTags/DependencyMuxActorTests.cs` | 139 |
|
|
|
|
No site calls `Props.Create<DriverHostActor>()` or `new DriverHostActor(...)` directly — the only
|
|
`new DriverHostActor` in the tree is inside the factory at `:343`. Because the new parameter is
|
|
optional and appended last, **no test site should need editing.**
|
|
|
|
### 5.3 Status object for the running-from-cache signal
|
|
|
|
`NodeDiagnosticsSnapshot` (`Commons/Interfaces/NodeDiagnosticsSnapshot.cs:17-22`) — built in
|
|
`HandleGetDiagnostics` (`DriverHostActor.cs:1381-1398`), a positional record of
|
|
`(NodeId, RevisionHash? CurrentRevision, IReadOnlyList<DriverInstanceDiagnostics> Drivers, DateTime AsOfUtc)`.
|
|
`GetDiagnostics` is handled in **all three states** (`Steady:570`, `Applying:601`, `Stale:1349`), so
|
|
a flag here is observable regardless of state. **This is the natural home for `RunningFromCache`**
|
|
(append last to the record). Consumer: `IFleetDiagnosticsClient` → AdminUI.
|
|
|
|
`IDriverHealthPublisher` (`:73`, defaulted `:402`) is **not** a fit — the host never calls it; it
|
|
only passes it down to `DriverInstanceActor` children (`:1845`, `:1859`). Per-driver, not per-node.
|
|
|
|
### 5.4 Async / side-effect IO convention
|
|
|
|
The actor is **overwhelmingly synchronous-blocking on DB IO** — all five `CreateDbContext()` sites
|
|
use `using var db = ...` with synchronous LINQ on the actor thread. No `async`/`await` in the file,
|
|
no `Task.Run`.
|
|
|
|
The single async pattern is `ContinueWith(..., TaskScheduler.Default).PipeTo(replyTo)` in
|
|
`HandleRouteNodeWrite` (`:1188-1200`), with an explicit `Sender` capture before the continuation.
|
|
|
|
**Recommendation for Task 7:** a **synchronous** cache write at the §2.2 point is *consistent with
|
|
the file's existing style* and is correct there — the actor is mid-apply, holds `Applying`, and
|
|
every other DB call on that path already blocks. A fire-and-forget `PipeTo(Self)` would need a
|
|
handler registered in `Steady`, `Applying` **and** `Stale`, or it dead-letters after the
|
|
`Become(Steady)` at `:1456`. **Prefer synchronous + try/catch.** (This deviates from the plan's
|
|
"fire-and-forget"; see D-4.)
|
|
|
|
`IWithTimers` is already implemented (`:50`, `:282`); `"retry-db"` key in use at `:1378`/`:2009`.
|
|
|
|
---
|
|
|
|
## 6. Host composition root
|
|
|
|
### 6.1 Role flag — `Program.cs:47-50`
|
|
|
|
```csharp
|
|
var roles = RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
|
|
var hasAdmin = roles.Contains("admin");
|
|
var hasDriver = roles.Contains("driver");
|
|
```
|
|
Allowed roles: `admin`, `driver`, `dev` (`Cluster/RoleParser.cs:5-8`); unknown throws (`:25-27`).
|
|
|
|
### 6.2 `hasDriver` branch — `Program.cs:131-310`
|
|
|
|
Anchor for the new call: **`Program.cs:173-175`** `builder.Services.AddAlarmHistorian(...)`, inside
|
|
the config-gated cluster at `:154-195`. Must land **before** `AddAkka` at `:314`.
|
|
|
|
### 6.3 Pipeline — `Program.cs:368-404`
|
|
|
|
`:383` `MapStaticAssets` · `:385-396` `if (hasAdmin) { ... }` · **`:398` `MapOtOpcUaHealth()`** ·
|
|
`:399` `MapOtOpcUaMetrics()` · `:404` `RunAsync()`.
|
|
|
|
⚠ **There is no `hasDriver` block in the pipeline half** — only `hasAdmin` is branched on. A
|
|
driver-gated `app.Map*` is a new construct; it goes between `:396` and `:398`.
|
|
|
|
`Program.cs:406-410` re-exports `public partial class Program`.
|
|
|
|
### 6.4 Kestrel / URLs — **CONFIRMED: no Kestrel configuration exists**
|
|
|
|
- `Program.cs:57` `builder.WebHost.UseStaticWebAssets();` — the **only** `builder.WebHost` call.
|
|
- **Zero** `ConfigureKestrel` / `UseUrls` / `ListenAnyIP` / `UseKestrel` anywhere in `src/`.
|
|
- Binding is exclusively via `ASPNETCORE_URLS`: `docker-compose.yml:173` (central-1) and `:240`
|
|
(central-2) set `http://+:9000`; `launchSettings.json:7` sets `http://localhost:9000`.
|
|
|
|
Two traps for Task 5:
|
|
|
|
1. ⚠ **`scripts/install/Install-Services.ps1:186`** (`$hostEnv += "ASPNETCORE_URLS=..."`) is inside
|
|
`if ($hasAdmin)` at `:185`. **A driver-only Windows-service node gets no `ASPNETCORE_URLS` at
|
|
all** and binds the ASP.NET default. The plan's parse-URLs-then-rebind snippet would re-bind
|
|
`http://+:9000` there — a port that node never listened on. The `?? "http://+:9000"` fallback in
|
|
the plan's snippet is therefore **wrong for driver-only nodes**; see D-6.
|
|
2. ⚠ **`Host.IntegrationTests/TwoNodeClusterHarness.cs:331`** already calls
|
|
`builder.WebHost.UseKestrel(o => o.Listen(IPAddress.Parse(LoopbackHost), 0));`. Adding a
|
|
production `ConfigureKestrel` gives two competing explicit configurations in integration tests —
|
|
last-one-wins. The `syncPort > 0` gate keeps this dormant by default (harness sets no sync port),
|
|
but any harness that *does* set one will fight the port-0 binding.
|
|
|
|
**Health route for smoke tests:** `/healthz` — liveness, runs no checks, always 200 while the
|
|
process is up, `AllowAnonymous`. (`Health/HealthEndpoints.cs:48` → `MapZbHealth()`; routes from the
|
|
`ZB.MOM.WW.Health` package.) Also `/health/ready`, `/health/active`, and `/metrics` (`Program.cs:399`).
|
|
|
|
### 6.5 `SecretsRegistration` template — `Host/Configuration/SecretsRegistration.cs`
|
|
|
|
Shape for `LocalDbRegistration` to mirror: `public static class` in
|
|
`ZB.MOM.WW.OtOpcUa.Host.Configuration`; `public const string` section/key path constants; a
|
|
`public static bool IsXEnabled(IConfiguration)` predicate using
|
|
`GetValue(key, defaultValue: false)` (**default-deny**) that `Program.cs` can also call;
|
|
`AddOtOpcUaX(this IServiceCollection, IConfiguration)` returning `IServiceCollection`, both args
|
|
`ArgumentNullException.ThrowIfNull`-guarded; XML docs with `<remarks><para>` explaining *why* the
|
|
gate is default-deny.
|
|
|
|
Call-site precedent: `Program.cs:363` `AddOtOpcUaSecrets(...)`; predicate reused inside the `AddAkka`
|
|
lambda at `Program.cs:323`.
|
|
|
|
### 6.6 Telemetry meter allowlist — **EXISTS. Task 10 step 3 is REQUIRED, not conditional.**
|
|
|
|
`Host/Observability/ObservabilityExtensions.cs:25-38`:
|
|
```csharp
|
|
return services.AddZbTelemetry(o =>
|
|
{
|
|
o.ServiceName = "otopcua";
|
|
o.Meters = [OtOpcUaTelemetry.MeterName]; // <-- ObservabilityExtensions.cs:30
|
|
o.ActivitySources = [OtOpcUaTelemetry.ActivitySourceName];
|
|
```
|
|
|
|
`o.Meters` is a **strict allowlist** — `ZbTelemetryExtensions.cs:51-54` iterates it calling
|
|
`metrics.AddMeter(name)`; **no wildcard support**. Anything unlisted is silently not exported.
|
|
Single element today: `OtOpcUaTelemetry.MeterName` = `"ZB.MOM.WW.OtOpcUa"`
|
|
(`Commons/Observability/OtOpcUaTelemetry.cs:19`).
|
|
|
|
**Action:** add `LocalDbMetrics.MeterName` at `ObservabilityExtensions.cs:30`. This is exactly the
|
|
omission the ScadaBridge live gate caught.
|
|
|
|
### 6.7 Health registration — `Host/Health/HealthEndpoints.cs:19-41`
|
|
|
|
`AddOtOpcUaHealth()` **takes no parameters** and is called **unconditionally** at `Program.cs:365`,
|
|
outside both role blocks. Three `.AddTypeActivatedCheck<>` calls (`configdb`, `akka`,
|
|
`admin-leader`); **no registration-time role gating exists** — `ActiveNodeHealthCheck` does runtime
|
|
role scoping instead (returns `Healthy` when the node lacks the role).
|
|
|
|
⚠ **There are zero `IHealthCheck` implementations in this repo** — all three come from the
|
|
`ZB.MOM.WW.Health.*` packages (`Directory.Packages.props:124-126`, v0.1.0). A LocalDb check would be
|
|
the **first health-check class in the tree**; `Host/Health/` holds only `HealthEndpoints.cs` today.
|
|
|
|
**Consequence for Task 10:** driver-gating the new check requires changing
|
|
`AddOtOpcUaHealth()`'s signature (add `bool hasDriver` or `IConfiguration`) and its `Program.cs:365`
|
|
call site. **Preferred alternative:** follow the `ActiveNodeHealthCheck` precedent — register
|
|
unconditionally and have the check return `Healthy` when LocalDb is not registered. That keeps the
|
|
signature and matches the "default-OFF must not degrade a plain node" requirement for free. See D-7.
|
|
|
|
### 6.8 Config files
|
|
|
|
| File | Top-level sections |
|
|
|---|---|
|
|
| `appsettings.json` | `Serilog`, `Security`, `Secrets`, `ServerHistorian`, `ContinuousHistorization`, `AlarmHistorian`, `Deployment` |
|
|
| `appsettings.driver.json` / `.admin.json` / `.admin-driver.json` / `.Development.json` | `Serilog`, `Security` only |
|
|
|
|
Overlay selection: `Program.cs:67` sorts roles ordinally and joins with `-` → `admin,driver`
|
|
becomes `appsettings.admin-driver.json`, added `optional: true` at `:70`; env vars (`:77`) and CLI
|
|
args (`:78`) are re-appended after, so deployment overrides outrank the overlay.
|
|
|
|
**`LocalDb` key search: CONFIRMED ABSENT.** Case-insensitive `localdb` across all `*.json`,
|
|
`*.yml`, `*.ps1`, `*.csproj`, `*.props` matches only the two Phase-1/Phase-2 planning artifacts. No
|
|
conflicting keys in any overlay or in `Install-Services.ps1`.
|
|
|
|
**Placement convention:** each feature section opens with a `"_comment"` string then `"Enabled":
|
|
false`; per-field caveats use sibling `"_<Field>Comment"` keys (`appsettings.json:34`, `:45`). Place
|
|
`LocalDb` between `AlarmHistorian` (ends `:61`) and `Deployment` (`:62`).
|
|
|
|
---
|
|
|
|
## 7. LiteDB LocalCache — dormant, safe to delete
|
|
|
|
### 7.1 Files (6) under `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/`
|
|
|
|
`ILocalConfigCache.cs`, `GenerationSnapshot.cs`, `StaleConfigFlag.cs`, `LiteDbConfigCache.cs`
|
|
(also declares `LocalConfigCacheCorruptException` at `:128`), `GenerationSealedCache.cs`,
|
|
`ResilientConfigReader.cs`. Single namespace `...Configuration.LocalCache`. The only two
|
|
`using LiteDB;` statements in `src/` are `LiteDbConfigCache.cs:1` and `GenerationSealedCache.cs:1`.
|
|
|
|
### 7.2 Reference classification — **no live-code references. NO STOP.**
|
|
|
|
- **(c) live code elsewhere: ZERO.** Also confirms full dormancy: `SealedBootstrap` (referenced by
|
|
`docs/v2/v2-release-readiness.md:46`) has **zero hits** in `src/` and `tests/` — it no longer exists.
|
|
- Two benign near-misses:
|
|
- `Configuration/Services/ILdapGroupRoleMappingService.cs:14` — the string `ResilientConfigReader`
|
|
inside an **XML doc comment**. Prose only. Needs a comment edit or the doc goes stale.
|
|
- `Configuration/ZB.MOM.WW.OtOpcUa.Configuration.csproj:29` — `<PackageReference Include="LiteDB"/>`,
|
|
the only one in the repo.
|
|
- Dozens of `LocalConfigCacheCorruptException` hits under `src/**/bin/**/*.xml` are **generated doc
|
|
build artifacts** (`GenerateDocumentationFile=true`), not source.
|
|
|
|
### 7.3 ⚠ Test-file drift — the plan's delete list is incomplete
|
|
|
|
Plan `…phase1.md:484-485` names only `GenerationSealedCacheTests.cs` and
|
|
`ResilientConfigReaderTests.cs`. Actual test files in
|
|
`tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/`:
|
|
|
|
| File | Status |
|
|
|---|---|
|
|
| `GenerationSealedCacheTests.cs` | in plan ✅ |
|
|
| `ResilientConfigReaderTests.cs` | in plan ✅ — note `StaleConfigFlagTests` is co-located here at `:323` |
|
|
| **`LiteDbConfigCacheTests.cs`** | **MISSING from the plan** — will fail to compile if the source is deleted (`:125` calls `new LiteDB.LiteDatabase(...)` directly) |
|
|
|
|
See D-3.
|
|
|
|
### 7.3b Outcome (Task 9, executed 2026-07-20)
|
|
|
|
Deleted: the 6 `LocalCache/` sources and **all three** test files (including
|
|
`LiteDbConfigCacheTests.cs`, which the plan omitted — D-3). `LiteDB` removed from both
|
|
`Configuration.csproj` and `Directory.Packages.props`. The stale XML-doc reference at
|
|
`ILdapGroupRoleMappingService.cs:14` was rewritten to state the present truth (no fallback exists;
|
|
reviving it means an admin-side cache on LocalDb, not restoring the old pipeline) rather than
|
|
deleted, per the DoD's "explanatory prose may remain". Configuration tests: 92/92 green.
|
|
|
|
**Follow-up found while executing:** `Polly.Core` in `Configuration.csproj` is now orphaned too —
|
|
`ResilientConfigReader` was its only real consumer in that project (remaining `Polly` mentions are
|
|
comments). Left in place deliberately: removing it is outside Task 9's scope and could break a
|
|
consumer relying on the transitive flow. Worth a separate cleanup.
|
|
|
|
### 7.4 Package removal
|
|
|
|
- `PackageReference`: `Configuration.csproj:29` — only consumer.
|
|
- `PackageVersion`: `Directory.Packages.props:35` — `LiteDB` `5.0.21`.
|
|
|
|
Both safe to delete; no transitive consumer.
|
|
|
|
---
|
|
|
|
## 8. Test projects, rig, packages
|
|
|
|
### 8.1 Test project layout
|
|
|
|
| Question | Answer |
|
|
|---|---|
|
|
| `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/` | **DOES NOT EXIST** — there is no Host *unit*-test project at all |
|
|
| Closest Host project | `Host.IntegrationTests` — despite the name it carries plenty of pure unit tests (`LdapOptionsValidatorTests`, `ServerHistorianOptionsValidatorTests`, `ResilienceInvokerFactoryRegistrationTests`, …) |
|
|
| `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/` | **EXISTS** |
|
|
| DriverHostActor tests | `Runtime.Tests/Drivers/` — 11 `DriverHostActor*Tests.cs` files. **xunit v2** (`xunit` 2.9.3), references **`Akka.TestKit.Xunit2`** ✅ |
|
|
| `Host.IntegrationTests` xunit version | **xunit.v3** 1.1.0; SDK `Microsoft.NET.Sdk.Web` |
|
|
|
|
`Directory.Packages.props` documents that AdminUI.Tests / ControlPlane.Tests / **Runtime.Tests** are
|
|
the three xunit-v2 holdouts, held there by `Akka.TestKit.Xunit2` (verified 2026-07-13: no xunit.v3
|
|
Akka TestKit exists). This matches the design's "actor tests live in an xunit2 TestKit project".
|
|
|
|
### 8.2 ⚠ `WebApplicationFactory<Program>` is **deliberately not used** — Task 11 must be rewritten
|
|
|
|
The only mention of the type in the entire `tests/` tree is a comment explaining why it is avoided:
|
|
|
|
`Host.IntegrationTests/TwoNodeClusterHarness.cs:48`
|
|
```
|
|
/// <para>Why not <c>WebApplicationFactory<Program></c>? Program.cs reads <c>OTOPCUA_ROLES</c> ...
|
|
```
|
|
|
|
The actual pattern builds the host directly — `TwoNodeClusterHarness.cs:329`:
|
|
```csharp
|
|
var builder = WebApplication.CreateBuilder(new WebApplicationOptions { Args = [] });
|
|
```
|
|
`Microsoft.AspNetCore.Mvc.Testing` is still referenced (csproj `:15`) but only for transitive
|
|
test-host plumbing. **Copy `TwoNodeClusterHarness`, not `WebApplicationFactory`.** See D-5.
|
|
|
|
`Host.IntegrationTests` also has its own `docker-compose.yml` (sql `14331`, ldap `3894`) gated by
|
|
`DockerFixtureAvailability.cs`.
|
|
|
|
### 8.3 docker-dev rig topology — plan's site-a/site-b assumption is **correct**
|
|
|
|
Single Akka mesh (`otopcua`), seeded by `central-1`. Tenant separation is by `ServerCluster.ClusterId`
|
|
rows (MAIN / SITE-A / SITE-B) inside **one shared ConfigDb** — not separate meshes.
|
|
|
|
| Service | Roles | Cluster |
|
|
|---|---|---|
|
|
| `central-1`, `central-2` | `admin,driver` | MAIN |
|
|
| `site-a-1`, `site-a-2` | `driver` | SITE-A |
|
|
| `site-b-1`, `site-b-2` | `driver` | SITE-B |
|
|
| `sql`, `migrator`, `cluster-seed`, `traefik` | — | infra |
|
|
|
|
**There is no admin-only service** — the "admin" nodes are fused `admin,driver`. All six host nodes
|
|
share the `&otopcua-host` YAML anchor.
|
|
|
|
⚠ **NO HOST NODE HAS A WRITABLE VOLUME.** The only `volumes:` in the file are `sql` →
|
|
`otopcua-mssql-data`, plus two read-only binds (`./seed:ro`, `./traefik-dynamic.yml:ro`). All six
|
|
nodes write to the container's ephemeral layer, destroyed on recreate. This is already an accepted
|
|
pattern (per-container `Secrets:SqlitePath=otopcua-secrets.db`, replicated over Akka rather than
|
|
persisted). **Task 13 must add six named volumes** — there is nothing to piggyback on. See D-8.
|
|
|
|
**Env style: confirmed double-underscore `Section__Key`**, with array indices and dictionary keys as
|
|
a third segment (`Cluster__SeedNodes__0`, `Security__Ldap__GroupToRole__ReadOnly`). Exceptions that
|
|
are plain SCREAMING_SNAKE (read directly, not config-bound): `OTOPCUA_ROLES`,
|
|
`ZB_SECRETS_MASTER_KEY`, `GALAXY_MXGW_API_KEY`, `OTOPCUA_CONFIG_CONNECTION`, `ASPNETCORE_URLS`.
|
|
|
|
**Ports.** Host-published: `14330`→sql, `4840`-`4845`→the six nodes' OPC UA, `9200`→traefik web,
|
|
`8089`→traefik dashboard. Container-internal on every node: `4053` (Akka remoting), `9000`
|
|
(Kestrel), `4840` (OPC UA). Also reserved on this machine: `14331`/`3894` (Host.IntegrationTests
|
|
compose), `80`/`8080` (sibling scadabridge/scadalink stack), `10.100.0.35:3893` (shared GLAuth).
|
|
|
|
**`9001` (the plan's choice) is free** container-internal and need not be published. ✅
|
|
|
|
### 8.4 Package versions — `Directory.Packages.props`
|
|
|
|
| Package | Version | Line |
|
|
|---|---|---|
|
|
| `Grpc.Core.Api` | 2.76.0 | 32 |
|
|
| `Grpc.Net.Client` | 2.76.0 | 33 |
|
|
| **`Grpc.AspNetCore`** | **NOT PRESENT** | — |
|
|
| `Google.Protobuf` | 3.34.1 | 31 |
|
|
| `Microsoft.Data.Sqlite` | 10.0.7 | ~55 |
|
|
| `SQLitePCLRaw.bundle_e_sqlite3` | 2.1.12 | ~108 |
|
|
| `LiteDB` | 5.0.21 | 35 |
|
|
|
|
Both Grpc floors and the Protobuf floor are already satisfied — **no bumps needed**, only the new
|
|
`Grpc.AspNetCore` entry (use 2.76.0 to match the train; watch for NU1605/NU1608 if it wants a
|
|
Protobuf newer than the pinned 3.34.1).
|
|
|
|
⚠ **`SQLitePCLRaw` is a surgical direct pin, not a transitive one.**
|
|
`CentralPackageTransitivePinningEnabled` is deliberately **off** (it breaks the Roslyn version
|
|
split). The 2.1.12 pin is promoted by a **direct `PackageReference` in `Core.AlarmHistorian`**
|
|
(`Core.AlarmHistorian.csproj:15` + `:21`) — today the only Sqlite consumer in `src/`.
|
|
|
|
> **Any new project that pulls `Microsoft.Data.Sqlite` MUST also take a direct
|
|
> `<PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3"/>`,** or it silently resolves the
|
|
> vulnerable 2.1.11 native bundle. This applies to **Host** and **Runtime** (both gain `ZB.MOM.WW.LocalDb`,
|
|
> which depends on `Microsoft.Data.Sqlite`) and to every test project that builds a real `ILocalDb`.
|
|
> Copy the `Core.AlarmHistorian.csproj:15,21` pair exactly. The plan's Task 1 mentions
|
|
> `SQLitePCLRaw.lib.e_sqlite3` "if the audit flags it" — the correct package is
|
|
> **`bundle_e_sqlite3`**, and it is **required, not conditional**.
|
|
|
|
---
|
|
|
|
## 9. Deviations from the plan
|
|
|
|
Recorded per the tasks.json `deviation` convention. **D-1, D-3, D-5, D-6 change the work
|
|
materially.**
|
|
|
|
| # | Plan says | Reality | Resolution |
|
|
|---|---|---|---|
|
|
| **D-1** | Cache keyed by `ClusterId`; "cluster identity" is a recon item with a STOP condition | `ClusterId` is stable, but **not resolvable at the boot seam** — it lives inside the artifact or in the unreachable central DB (§4) | Keep `cluster_id` as PK (pair sharing is the point). **Write:** resolve via `DeploymentArtifact.ResolveClusterScope`, `"__single"` sentinel for the degenerate case. **Read:** unkeyed `ORDER BY applied_at_utc DESC LIMIT 1`, warn if >1 row. No STOP. |
|
|
| **D-2** | Cache write "fire-and-forget (`PipeTo`-style or guarded `Task.Run`)" | The actor has **no async DB IO at all**; a `PipeTo(Self)` needs handlers in all three states or it dead-letters after `Become(Steady)` (§5.4) | Synchronous write in its own try/catch at `:1439`. Matches the file's style; failure still cannot fail the apply. |
|
|
| **D-3** | Task 9 deletes 2 test files | **3** test files reference the subsystem — `LiteDbConfigCacheTests.cs` is missing from the list (§7.3) | Delete all three. Also fix the stale XML-doc mention at `ILdapGroupRoleMappingService.cs:14`. |
|
|
| **D-4** | Cache the artifact after a successful apply | `ReconcileDrivers`/`PushDesiredSubscriptions` **swallow** DB errors, so a "successful" apply can have loaded an **empty blob** and started zero drivers (§2.3) | Gate the write on a non-empty blob; surface the blob from `ReconcileDrivers` instead of a third read. **Load-bearing — without it the cache can be poisoned with an empty config.** |
|
|
| **D-5** | Task 11 uses `WebApplicationFactory<Program>` | Deliberately **not used** in this repo; `TwoNodeClusterHarness.cs:48` documents why (Program.cs reads `OTOPCUA_ROLES` from the environment) (§8.2) | Rewrite Task 11 against the `TwoNodeClusterHarness` direct-host-build pattern. |
|
|
| **D-6** | Task 5 re-binds `ASPNETCORE_URLS ?? "http://+:9000"` | `Install-Services.ps1:185-186` sets `ASPNETCORE_URLS` **only when `hasAdmin`** — the fallback would bind a driver-only node to a port it never served (§6.4) | Re-bind only when a URL is actually configured; when absent, add **only** the sync listener. Also note `TwoNodeClusterHarness.cs:331` already calls `UseKestrel(Listen(…, 0))`. |
|
|
| **D-7** | Task 10 driver-gates the health check | `AddOtOpcUaHealth()` takes no args and is called unconditionally at `Program.cs:365`; no registration-time gating exists anywhere (§6.7) | Register unconditionally; return `Healthy` when LocalDb is absent — the `ActiveNodeHealthCheck` precedent. Avoids a signature change and satisfies "default-OFF must not degrade a plain node". |
|
|
| **D-8** | Task 13 "existing data mount" | **No host node has any writable volume** (§8.3) | Add six named volumes. Note: `9001` is free container-internal. Meter allowlist (§6.6) is **required**, not conditional. |
|
|
|
|
### Follow-ups logged, not fixed here
|
|
|
|
1. **`TryRecoverFromStale` marks `Applied` without reconciling drivers** (§3.5) — pre-existing,
|
|
documented in-code at `:1692-1695`. Out of Phase 1 scope.
|
|
2. **Duplicate blob read per apply** (`:1472` and `:1543`, §1.1) — D-4's fix removes one of them as a
|
|
side effect.
|
|
3. **`Install-Services.ps1` gives driver-only nodes no `ASPNETCORE_URLS`** (§6.4) — pre-existing;
|
|
D-6 works around it rather than fixing it.
|