From c0475f2df49d6a484e28ee9ebd429e88373ec1ea Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 07:55:31 -0400 Subject: [PATCH] =?UTF-8?q?fix(mesh):=20resolve=20dialer=20+=20PSK=20provi?= =?UTF-8?q?der=20lazily=20=E2=80=94=20central=20boots=20again?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1A live gate defect 4. Neither central node booted on this branch, in ANY transport mode: Akka.Hosting.MissingActorRegistryEntryException: No actor registered for key ...ControlPlane.AuditWriterActorKey at AdminUI.Audit.ActorAuditWriter..ctor(ActorRegistry registry) at ControlPlane.ServiceCollectionExtensions. WithOtOpcUaControlPlaneSingletons' first WithActors starter resolved IMeshCommandDialer and IMeshPskProvider eagerly for the Props. Their graph reaches back into the registry — dialer -> IMeshPskProvider -> ISecretResolver -> AddZbSecrets -> IAuditWriter -> ActorAuditWriter, whose ctor calls registry.Get() — and WithSingleton is registered ~60 lines LATER in the same builder. Boot died before the entry existed. Mode-independent: the resolve happened before anything consulted MeshTransport:Mode, so Dps (the default, and the rollback target) died exactly like Grpc. Both dependencies now arrive as Func<> and are invoked in PreStart, by which point the host is fully built — the same shape, for the same class of reason, as the coordinator proxy Func one argument earlier. PreStart re-resolves on restart rather than caching across one; both are container singletons, so a restart gets the same instances. A resolution throw is caught and logged rather than propagated: a throw out of PreStart is an actor failure, the supervisor answers it with a restart, and a broken DI graph is not transient — that is an unbounded restart loop that would take the transport-independent ack path down with it. Regression pin: ControlPlaneStarterOrderTests boots a REAL admin host through WithOtOpcUaClusterBootstrap + WithOtOpcUaControlPlaneSingletons in production starter order, with the production graph from IMeshCommandDialer down to a registry-bound IAuditWriter, and asserts StartAsync itself — the failure occurs while the builder's starters execute, so building a ServiceProvider and resolving by hand cannot reproduce it. Verified RED against the eager resolve: 3/3 modes fail with MissingActorRegistryEntryException. A second fact pins that the deferral did not become 'never resolved' — the actor must resolve the provider from the container and drive it through the contact refresh, signalled by a TaskCompletionSource (PreStart runs on the actor thread and the contact load is a piped DB read, so no synchronous assertion after StartAsync is correct). Why nothing caught this: the dialer's container-activation test resolves from a collection with no secrets registration, so its graph stops one hop in; the actor facts inject fakes and never touch DI; and the existing whole-host boots register no mesh provider, so the eager lookups returned null and walked nothing. Two aggravators recorded by the gate, no code here: the failed process did not exit (docker Up while /health/ready 503s — Phase 2 readiness concern, tracked), and this is the third instance of the same eager-resolve-during-AddAkka class after scadaproj#1 and mesh Phase 6's IClusterRoleInfo boot crash. ControlPlane.Tests 236 -> 240, Host.Tests 92, Cluster.Tests 184, 0 warnings in every touched project. --- .../CentralCommunicationActor.cs | 103 ++++++-- .../ServiceCollectionExtensions.cs | 14 +- .../CentralCommunicationActorTests.cs | 8 +- .../ControlPlaneStarterOrderTests.cs | 233 ++++++++++++++++++ 4 files changed, 333 insertions(+), 25 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs index 60f3b7c2..79166703 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs @@ -64,8 +64,15 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers private readonly MeshTransportOptions _options; private readonly IMeshClusterClientFactory _clientFactory; private readonly Func _coordinator; - private readonly IMeshCommandDialer? _dialer; - private readonly IMeshPskProvider? _pskProvider; + + // Resolved in PreStart, never at construction. See the Props doc: the DI graph behind these two + // reaches actor-registry entries that do not exist yet while the Akka builder's starters are + // still running, and touching it there is what stopped every central node from booting. + private readonly Func? _dialerFactory; + private readonly Func? _pskProviderFactory; + private IMeshCommandDialer? _dialer; + private IMeshPskProvider? _pskProvider; + private readonly ILoggingAdapter _log = Context.GetLogger(); private readonly bool _useClusterClient; private readonly bool _useGrpc; @@ -100,24 +107,45 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// time — it resolves on first use and caches. /// /// - /// Places one command call on one node, under . - /// Optional so the two older transports need nothing new wired; a - /// dialer under Grpc is a mis-wiring and is reported loudly rather than silently - /// dropping every command. + /// Resolves the thing that places one command call on one node, under + /// . Optional so the two older transports need + /// nothing new wired; a dialer under Grpc is a mis-wiring and + /// is reported loudly rather than silently dropping every command. /// /// - /// Central's per-cluster key lookup, told on every contact refresh which clusters exist (the - /// known-cluster gate) and to drop its cache (so a rotated key takes effect within one - /// refresh). Optional for the same reason as . + /// Resolves central's per-cluster key lookup, told on every contact refresh which clusters + /// exist (the known-cluster gate) and to drop its cache (so a rotated key takes effect within + /// one refresh). Optional for the same reason as . /// /// The props. + /// + /// + /// Both dependencies arrive as , and that is not a style + /// choice. Passing instances meant the caller resolved them while the Akka builder's + /// starters were still running, and the DI graph behind them reaches back into the actor + /// registry: dialer → IMeshPskProvider → ISecretResolver → AddZbSecrets + /// → IAuditWriter → ActorAuditWriter, whose constructor calls + /// registry.Get<AuditWriterActorKey>() — a singleton registered dozens of + /// lines LATER in the very same builder. Every central node failed to boot, in every + /// transport mode, on a MissingActorRegistryEntryException. Deferring to + /// moves the first resolve to a point where the host is fully + /// built, and matches the the coordinator proxy already uses + /// one argument earlier for the same class of reason. + /// + /// + /// This is the third instance of the same failure mode in this family — eager resolution + /// inside an AddAkka configurator, previously scadaproj#1 (a DI cycle that + /// deadlocked) and mesh Phase 6's IClusterRoleInfo boot StackOverflow. The + /// rule it keeps re-teaching: inside an Akka builder, resolve nothing you can defer. + /// + /// public static Props Props( IDbContextFactory dbFactory, MeshTransportOptions options, IMeshClusterClientFactory clientFactory, Func coordinator, - IMeshCommandDialer? dialer = null, - IMeshPskProvider? pskProvider = null) => + Func? dialer = null, + Func? pskProvider = null) => Akka.Actor.Props.Create(() => new CentralCommunicationActor( dbFactory, options, clientFactory, coordinator, dialer, pskProvider)); @@ -127,22 +155,22 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// The bound mesh-transport options. /// Creates the ClusterClient. /// Lazily resolves the deploy-coordinator singleton proxy. - /// Places one gRPC command call on one node; Grpc mode only. - /// Central's per-cluster key lookup; Grpc mode only. + /// Resolves the gRPC command dialer; invoked in . + /// Resolves the PSK provider; invoked in . public CentralCommunicationActor( IDbContextFactory dbFactory, MeshTransportOptions options, IMeshClusterClientFactory clientFactory, Func coordinator, - IMeshCommandDialer? dialer = null, - IMeshPskProvider? pskProvider = null) + Func? dialer = null, + Func? pskProvider = null) { _dbFactory = dbFactory; _options = options; _clientFactory = clientFactory; _coordinator = coordinator; - _dialer = dialer; - _pskProvider = pskProvider; + _dialerFactory = dialer; + _pskProviderFactory = pskProvider; _useClusterClient = string.Equals( options.Mode, MeshTransportOptions.ModeClusterClient, StringComparison.OrdinalIgnoreCase); _useGrpc = string.Equals( @@ -183,6 +211,12 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// protected override void PreStart() { + // First point at which the host is fully built — see the Props remark. Re-runs on every + // actor restart, which re-reads the container rather than caching a stale reference; both + // dependencies are container singletons, so a restart gets the same instances back. + _dialer = Resolve(_dialerFactory, nameof(IMeshCommandDialer)); + _pskProvider = Resolve(_pskProviderFactory, nameof(IMeshPskProvider)); + if (!_useClusterClient && !_useGrpc) { _log.Info( @@ -229,6 +263,41 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers TimeSpan.FromSeconds(_options.ContactRefreshSeconds)); } + /// + /// Invokes a dependency factory, converting a resolution failure into a logged absence. + /// + /// The dependency's type. + /// The factory, or when nothing was supplied. + /// The dependency's name, for the log. + /// The resolved dependency, or . + /// + /// Catching is the point. A throw out of is an actor failure, and the + /// supervisor's answer to that is to restart — which calls again, and + /// a broken DI graph is not a transient condition. That is an unbounded restart loop on the + /// node's command boundary, taking the transport-independent ack path down with it. Logging + /// and continuing leaves the ack path serving and hands the two null guards below a genuine + /// absence to report, which is the same outcome as never having been wired. + /// + private T? Resolve(Func? factory, string name) + where T : class + { + if (factory is null) + return null; + + try + { + return factory(); + } + catch (Exception ex) + { + _log.Error( + ex, + "Failed to resolve {Dependency} for the mesh command boundary; continuing without " + + "it. The guards below report what that costs", name); + return null; + } + } + /// protected override void PostStop() { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index 32a04231..1801da38 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -66,14 +66,20 @@ public static class ServiceCollectionExtensions var dbFactory = resolver.GetService>(); var options = resolver.GetService>().Value; + // Captured, NOT resolved. Resolving the mesh dialer or PSK provider here made every + // central node fail to boot in every transport mode: their graph is dialer → + // IMeshPskProvider → ISecretResolver → AddZbSecrets → IAuditWriter → ActorAuditWriter, + // whose constructor calls registry.Get() — and WithSingleton + // is registered further down THIS builder, so the entry does not + // exist while this starter runs. The actor invokes these in PreStart instead, by which + // point the host is built. Same shape, same reason, as the coordinator Func below. + // // Through IServiceProvider rather than the Akka resolver, deliberately: these two are // OPTIONAL (Grpc mode only, and admin-role only within that), and GetService's // null-for-absent contract is the thing being relied on. The Akka resolver's // GetService makes no such promise, and this same block runs in test harnesses whose // service collections register neither. var serviceProvider = resolver.GetService(); - var dialer = serviceProvider.GetService(); - var pskProvider = serviceProvider.GetService(); var actor = system.ActorOf( CentralCommunicationActor.Props( @@ -85,8 +91,8 @@ public static class ServiceCollectionExtensions () => registry.TryGet(out var coordinator) ? coordinator : null, - dialer, - pskProvider), + () => serviceProvider.GetService(), + () => serviceProvider.GetService()), MeshPaths.CentralCommunicationName); // Registered unconditionally, in both transport modes. Under Dps nothing dials it, and diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/CentralCommunicationActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/CentralCommunicationActorTests.cs index 5556295d..9004bf2d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/CentralCommunicationActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/CentralCommunicationActorTests.cs @@ -742,8 +742,8 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase new MeshTransportOptions { Mode = mode, ContactRefreshSeconds = 60 }, clients, () => null, - new RecordingDialer(), - psk)); + () => new RecordingDialer(), + () => psk)); AwaitCondition( () => psk.KnownClusterPublications.Count >= 1, TimeSpan.FromSeconds(5)); @@ -855,8 +855,8 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase GrpcMode(), clientFactory ?? new RecordingClientFactory(() => CreateTestProbe()), () => coordinator, - dialer, - psk)); + () => dialer, + () => psk)); return new GrpcRig(actor, dialer, psk); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs new file mode 100644 index 00000000..589c34f2 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/ControlPlaneStarterOrderTests.cs @@ -0,0 +1,233 @@ +using System.Diagnostics.CodeAnalysis; +using Akka.Actor; +using Akka.Cluster.Hosting; +using Akka.Hosting; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; +using ZB.MOM.WW.Audit; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.Commons.Interfaces; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Communication; + +/// +/// Guards the one thing every other test in this project structurally cannot see: whether an +/// admin node boots with the real registration graph behind the mesh command boundary. +/// +/// +/// +/// The Phase 1A live gate failed here. WithOtOpcUaControlPlaneSingletons's first +/// starter resolved IMeshCommandDialer and IMeshPskProvider eagerly, and their +/// graph reaches back into the actor registry — dialer → IMeshPskProvider → +/// ISecretResolver → AddZbSecrets → IAuditWriter → +/// ActorAuditWriter, whose constructor calls +/// registry.Get<AuditWriterActorKey>() for a singleton registered further down +/// the SAME builder. Every central node died on +/// MissingActorRegistryEntryException, in every transport mode, before serving +/// anything. +/// +/// +/// Why the existing tests were blind. MeshGrpcCommandDialerTests activates the +/// dialer from a ServiceCollection that has no secrets registration at all, so its +/// graph stops one hop in; the CentralCommunicationActorTests facts inject fakes +/// directly and never touch DI; and the existing whole-host boots register no mesh provider, +/// so the eager lookups returned and resolved nothing. Every one of +/// them passed while no central node could start. +/// +/// +/// So the assertion is StartAsync itself. The failure happens while the Akka +/// builder's starters execute, which is inside StartAsync — building the +/// ServiceProvider and resolving from it by hand would not reproduce it. The graph +/// below is the production one from IMeshCommandDialer down to the registry-bound +/// audit writer; only the secret store itself is a stand-in, because a real one would add a +/// KEK and a SQLite file to a test whose subject is starter ORDER. +/// +/// +public sealed class ControlPlaneStarterOrderTests +{ + [Theory] + [InlineData(MeshTransportOptions.ModeDps)] + [InlineData(MeshTransportOptions.ModeClusterClient)] + [InlineData(MeshTransportOptions.ModeGrpc)] + public async Task An_admin_node_boots_with_the_real_mesh_dependency_graph(string mode) + { + // Every mode, because the eager resolve was mode-independent: the starter resolved both + // dependencies before anything looked at MeshTransport:Mode, so Dps — the DEFAULT, and the + // rollback target — died exactly like Grpc did. + using var host = BuildAdminHost(mode); + + await Should.NotThrowAsync(() => host.StartAsync()); + + try + { + var registry = host.Services.GetRequiredService(); + registry.TryGet(out _).ShouldBeTrue( + "the command boundary must be registered on a booted admin node"); + registry.TryGet(out _).ShouldBeTrue( + "the audit singleton the eager resolve reached for too early must exist by now"); + } + finally + { + await host.StopAsync(); + } + } + + [Fact] + public async Task The_deferred_dependencies_are_resolved_and_used_once_the_host_is_up() + { + // Deferring must not become "never resolved" — that would trade a loud boot failure for a + // silent one, with the known-cluster gate permanently unpublished and the dialer permanently + // absent. So this asserts the far end of the deferral: the actor resolved the PSK provider + // FROM THE CONTAINER and drove it, through the ordinary contact-refresh path. + // + // The signal is a TaskCompletionSource rather than a counter, because PreStart runs on the + // actor's own thread and the contact load is a piped DB read: there is no instant after + // StartAsync at which a synchronous assertion is correct. + var signal = new SignallingPskProvider(); + using var host = BuildAdminHost(MeshTransportOptions.ModeGrpc, signal); + + await host.StartAsync(); + + try + { + await signal.Published.WaitAsync(TimeSpan.FromSeconds(20)); + } + finally + { + await host.StopAsync(); + } + } + + /// + /// Builds an admin host through the production cluster bootstrap and the production control-plane + /// starter registration, with the mesh dependency graph wired as it is on a real central node. + /// + /// The mesh transport mode to boot under. + /// + /// Substitute for the real . Only the resolution test supplies + /// one; the boot test deliberately keeps the production type so the graph it walks is the + /// production graph. + /// + /// The unstarted host. + private static IHost BuildAdminHost(string mode, IMeshPskProvider? pskProvider = null) + { + var clusterOptions = new AkkaClusterOptions + { + Port = 0, + Hostname = "127.0.0.1", + PublicHostname = "127.0.0.1", + SeedNodes = [], + Roles = ["admin"], + }; + + return Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton>(Options.Create(clusterOptions)); + services.AddSingleton>(Options.Create( + new MeshTransportOptions { Mode = mode, DispatchTimeoutSeconds = 10 })); + services.AddDbContextFactory( + o => o.UseInMemoryDatabase($"starter-order-{Guid.NewGuid():N}")); + services.AddSingleton(new StubClusterRoleInfo()); + + // THE GRAPH. Production types from the dialer down to the registry-bound writer; the + // order of these registrations is deliberately irrelevant, because the defect was + // never about registration order — it was about WHEN the graph is first walked. + services.AddSingleton(); + services.AddSingleton(); + if (pskProvider is null) + services.AddSingleton(); + else + services.AddSingleton(pskProvider); + services.AddSingleton(); + + services.AddAkka("otopcua-starter-order-test", (ab, sp) => + { + ab.WithOtOpcUaClusterBootstrap(sp); + ab.WithOtOpcUaControlPlaneSingletons(); + }); + }) + .Build(); + } + + /// + /// Stands in for ActorAuditWriter: an that resolves a + /// cluster singleton from the in its constructor. That + /// constructor is the tripwire — it throws + /// MissingActorRegistryEntryException for exactly as long as the audit singleton has + /// not been registered yet. + /// + private sealed class RegistryBoundAuditWriter : IAuditWriter + { + private readonly IActorRef _proxy; + + public RegistryBoundAuditWriter(ActorRegistry registry) => _proxy = registry.Get(); + + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + _proxy.Tell(evt); + return Task.CompletedTask; + } + } + + /// + /// An that completes a task the first time the actor publishes + /// the known-cluster set. Takes it never uses, purely so that + /// resolving it still walks the same registry-bound chain the real provider does. + /// + private sealed class SignallingPskProvider : IMeshPskProvider + { + private readonly TaskCompletionSource _published = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + /// Completes once has been called. + public Task Published => _published.Task; + + public bool TryGetPsk(string clusterId, [NotNullWhen(true)] out string? psk) + { + psk = null; + return false; + } + + public bool Matches(string clusterId, string presented) => false; + + public void SetKnownClusters(IReadOnlyCollection clusterIds) => _published.TrySetResult(); + + public void Invalidate(string clusterId) { } + + public void InvalidateAll() { } + } + + /// + /// Stands in for the shared secret store's resolver. Its only load-bearing property is the + /// one that broke boot: it depends on , exactly as + /// AddZbSecrets' resolver registration does. + /// + private sealed class AuditingSecretResolver : ISecretResolver + { + public AuditingSecretResolver(IAuditWriter auditWriter) => ArgumentNullException.ThrowIfNull(auditWriter); + + public Task GetAsync(SecretName name, CancellationToken ct) => Task.FromResult(null); + } + + /// Minimal ; the admin singleton factories read it. + private sealed class StubClusterRoleInfo : IClusterRoleInfo + { + public string? ClusterRole => null; + public string? ClusterId => null; + public NodeId LocalNode => NodeId.Parse("127.0.0.1:0"); + public IReadOnlySet LocalRoles => new HashSet(); + public bool HasRole(string role) => false; + public IReadOnlyList MembersWithRole(string role) => []; + public NodeId? RoleLeader(string role) => null; + public event EventHandler? RoleLeaderChanged { add { } remove { } } + } +}