From 413bc5c04fb3398b93d231d4e00932d763ef718e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 06:52:54 -0400 Subject: [PATCH] fix(host): fatal-log, flush and exit non-zero when the host fails to start (#618) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/Redundancy.md | 10 ++++ docs/security.md | 8 ++++ .../OpcUa/OtOpcUaServerHostedService.cs | 5 +- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 25 ++++++++++ .../Startup/FailStartExitTests.cs | 47 +++++++++++++++++++ 5 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Startup/FailStartExitTests.cs diff --git a/docs/Redundancy.md b/docs/Redundancy.md index a8dc0ec3..e310c190 100644 --- a/docs/Redundancy.md +++ b/docs/Redundancy.md @@ -108,6 +108,16 @@ plane (writes, alarm acks, the alerts emit, the alarm-history drain) on the wron *higher* address so the two orderings genuinely disagree. `NodeRedundancyState.IsRoleLeaderForDriver` was renamed `IsDriverPrimary` to stop the name asserting the old derivation. +**Accepted residual (#620): the election's role match is case-sensitive.** `RedundancyStateActor` +delegates to `ZB.MOM.WW.Health.Akka`'s `ClusterActiveNode.OldestUpMember`, which matches roles with +Akka's own ordinal `Member.HasRole`. A peer gossiping a miscased plain role (`Driver`) is therefore +invisible to the election — which only happens in a mixed rolling-upgrade window with a pre-R3-04 +build (fixed builds normalize roles before gossip). Deliberately **not** patched: the library is a +family-shared package, and making either it or a local role pre-filter case-insensitive would split the +Primary election from `ClusterSingletonManager` placement (also ordinal) — recreating the "Primary not +on the singleton node" defect above. Case-sensitive on both sides is the consistent behaviour; finish +the rolling upgrade rather than working around it. + > **RESOLVED by Phase 6 — the election is now per application `Cluster`, not per Akka mesh.** > Phase 6 of the per-cluster mesh program split the single fleet-wide Akka mesh into one independent > 2-node mesh per application `Cluster` (see [Per-cluster meshes (Phase 6)](#per-cluster-meshes-phase-6) diff --git a/docs/security.md b/docs/security.md index 3883f281..2213c93f 100644 --- a/docs/security.md +++ b/docs/security.md @@ -453,6 +453,14 @@ takes no realm and no node. ### Node ACL enforcement (`Security:Acl:Enabled`) +> **ns=0 standard nodes stay ungated, deliberately (#617).** A session with no grants can still connect +> and read the standard OPC UA server nodes (`ServerStatus`, `NamespaceArray`, `ServiceLevel`, ...) and +> browse ns=0. Those are served by the SDK's core node manager outside our authorizer, and gating them +> would break session setup (clients read them for connection and redundancy health). They carry no plant +> data; every raw/UNS node, event subscription and HistoryRead is still gated. The boot warning says so +> ("can still connect and read the standard server nodes ... but sees no plant data") — earlier wording +> ("can connect and see nothing") overstated it. + | Key | Default | Effect | |---|---|---| | `Security:Acl:Enabled` | `false` | `true` makes scoped `NodeAcl` grants decide **every** data-plane operation — read, browse, subscribe, write, alarm commands and HistoryRead | diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs index adea5467..5d4b93b8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs @@ -262,7 +262,10 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl _logger.LogWarning( "OtOpcUaServerHostedService: node ACL enforcement is ENABLED — EVERY data-plane operation " + "(read, browse, subscribe, write, alarm commands, HistoryRead) now requires a matching " - + "NodeAcl grant. A principal with no grant can connect and see nothing."); + + "NodeAcl grant. A principal with no grant can still connect and read the standard OPC UA " + + "server nodes (ns=0: ServerStatus, NamespaceArray, ServiceLevel, ...) — those are served by " + + "the SDK core node manager and are deliberately ungated because clients need them for " + + "session setup — but it sees no plant data (raw/UNS namespaces)."); } // Propagate the configured tie-cluster over-fetch bound onto the node manager so HistoryRead-Raw diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index 33277f90..6f7e00e0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -57,6 +57,14 @@ using ZB.MOM.WW.Secrets.DependencyInjection; using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet; using ZB.MOM.WW.Secrets.Sqlite; +// #618 — the composition root runs inside one try/catch (not re-indented, to keep the diff reviewable) +// so ANY failure to start — an unknown role, a ValidateOnStart rejection, a hosted service that +// throws from StartAsync — is logged Fatal, flushed, and ends the process with a NON-ZERO exit code. +// Left as an unhandled exception the host could stay up as a zombie under a supervisor/container +// (partially-started Akka threads), reporting "running" while serving nothing. See the catch at the +// bottom of this file. +try +{ var builder = WebApplication.CreateBuilder(args); // The SINGLE role authority: Cluster:Roles first, OTOPCUA_ROLES as the fallback (archreview @@ -740,6 +748,23 @@ if (LogPipelineDiagnostics.CheckFileSinkGrowth(app.Configuration, } await app.RunAsync(); +} +catch (Exception ex) +{ + // Log.Logger is still the silent bootstrap logger when the failure happened before AddZbSerilog + // ran (e.g. EffectiveRoles.Resolve), so ALSO write to stderr — the one channel that always exists. + Log.Fatal(ex, "OtOpcUa.Host terminated during startup/run"); + Console.Error.WriteLine($"FATAL: OtOpcUa.Host terminated: {ex}"); + Log.CloseAndFlush(); + // Exit explicitly: non-background threads left by a partially-started host (Akka, Kestrel) would + // otherwise keep the process alive after the failure. Failure path only — a clean shutdown returns + // from RunAsync normally and never reaches here, so CoordinatedShutdown is not skipped. + Environment.Exit(1); +} +finally +{ + Log.CloseAndFlush(); +} namespace ZB.MOM.WW.OtOpcUa.Host { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Startup/FailStartExitTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Startup/FailStartExitTests.cs new file mode 100644 index 00000000..e1ea58a3 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Startup/FailStartExitTests.cs @@ -0,0 +1,47 @@ +using System.Diagnostics; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Startup; + +/// +/// #618 — a host that fails to start must END the process with a non-zero exit code and a Fatal line, +/// not linger as a half-started zombie. Spawns the built host assembly as a real child process. +/// +public sealed class FailStartExitTests +{ + [Fact] + public async Task A_failed_start_exits_non_zero_and_logs_fatal() + { + var hostDll = typeof(ZB.MOM.WW.OtOpcUa.Host.Program).Assembly.Location; + var dir = Path.GetDirectoryName(hostDll)!; + + var psi = new ProcessStartInfo("dotnet", $"\"{hostDll}\"") + { + WorkingDirectory = dir, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }; + // An invalid Akka seed address makes the cluster wiring throw before the host can ever run; + // no external service is touched. + psi.Environment["Cluster__Roles"] = "driver"; + psi.Environment["Cluster__SeedNodes__0"] = "not-a-valid-address"; + + using var proc = Process.Start(psi)!; + var stdout = proc.StandardOutput.ReadToEndAsync(); + var stderr = proc.StandardError.ReadToEndAsync(); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); + try { await proc.WaitForExitAsync(cts.Token); } + catch (OperationCanceledException) + { + proc.Kill(entireProcessTree: true); + Assert.Fail("host did not exit within 60 s of a failed start (#618): " + await stdout + await stderr); + } + + var output = await stdout + await stderr; + proc.ExitCode.ShouldNotBe(0, output); + output.ShouldContain("terminated during startup", Case.Insensitive, "the failure must be logged as Fatal"); + } +}