From 0db40a5e7f16406ac43769d3212a00b1d15ab79c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:35:54 -0400 Subject: [PATCH 01/11] =?UTF-8?q?refactor(mesh):=20step=201=20=E2=80=94=20?= =?UTF-8?q?delete=20the=20central=20ClusterClient=20factory=20and=20per-cl?= =?UTF-8?q?uster=20client=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMeshClusterClientFactory/DefaultMeshClusterClientFactory are gone, along with CentralCommunicationActor's _clients map, RebuildClient, the SendToAll fan and the receptionist-address half of the contact load. LoadContactsFromDb SURVIVES — it feeds the gRPC dialer's endpoint resolution and the known-cluster gate — but now projects only the columns those two need (the AkkaPort/ActorPath.TryParse parse and its Malformed list went with the receptionist addresses). Ripples fixed in this step because the build must be green at every commit: - ControlPlane/ServiceCollectionExtensions drops the factory argument. - CentralCommunicationActorTests: the six ClusterClient-only facts are deleted (each has a Grpc counterpart that already asserted the same property independently), the three transport-agnostic facts move to Grpc mode, and the Dps fact keeps its own coverage. - MeshClusterClientFactoryTests deleted (its subject is gone). - MeshClusterClientBoundaryTests deleted here rather than in step 7: it drives DefaultMeshClusterClientFactory directly, so it cannot compile past this commit. Its replacement, MeshGrpcBoundaryTests, lands in step 7 and keeps the load-bearing assertion (two separate meshes, no cross-mesh Akka association). --- .../CentralCommunicationActor.cs | 306 ++---------- .../IMeshClusterClientFactory.cs | 67 --- .../ServiceCollectionExtensions.cs | 11 +- .../CentralCommunicationActorTests.cs | 274 ++--------- .../MeshClusterClientFactoryTests.cs | 82 ---- .../MeshClusterClientBoundaryTests.cs | 446 ------------------ 6 files changed, 78 insertions(+), 1108 deletions(-) delete mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshClusterClientFactory.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshClusterClientFactoryTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshClusterClientBoundaryTests.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 316a1c4a..c3443d67 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/CentralCommunicationActor.cs @@ -1,6 +1,4 @@ -using System.Collections.Immutable; using Akka.Actor; -using Akka.Cluster.Tools.Client; using Akka.Cluster.Tools.PublishSubscribe; using Akka.Event; using Grpc.Core; @@ -20,39 +18,30 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; /// /// Central-side end of the central↔node command boundary (per-cluster mesh Phase 2). Routes -/// every central→node command out — over DistributedPubSub, ClusterClient or gRPC, per -/// — and forwards inbound s to the -/// deploy coordinator singleton. +/// every central→node command out — over DistributedPubSub (pair-local) or gRPC (cross-mesh), +/// per — and forwards inbound s to +/// the deploy coordinator singleton. /// /// /// -/// Registered per node, NOT as a cluster singleton. A driver node's ClusterClient -/// rotates across contact points and must find a live comm actor at whichever central node -/// answers. A singleton would be reachable only through the one node hosting it, and the -/// rotation would fail silently against the other. +/// Registered per node, NOT as a cluster singleton. A driver node posts its ack to +/// whichever central node answers first, so a live comm actor must exist on both. A singleton +/// would be reachable only through the one node hosting it, and the other central node's ack +/// ingress would have nothing to forward to. /// /// -/// One ClusterClient per application Cluster (Phase 6). The fleet is now one -/// Akka mesh per cluster, so a single fleet-wide client's SendToAll would reach only -/// the one mesh whose receptionist answered. This actor therefore holds a client per cluster -/// and fans SendToAll across all of them. See — this is the -/// single most important thing to understand before changing this actor. +/// The gRPC branch fans node by node over the contact load's target set. That load +/// selects exactly the enabled, non-maintenance ClusterNode rows carrying a +/// GrpcPort, and the fan dials every one of them, one call per node. It is deliberately +/// NOT narrower: the node side has no ClusterId or node filter at all — scoping happens +/// later, inside the artifact — so a node left out here silently keeps its old configuration +/// while the deployment seals green. /// /// -/// The gRPC branch fans over the same recipient set, node by node -/// (ClusterClient→gRPC Phase 1A). SendToAll reaches every node registered on a mesh's -/// receptionist, and the contact load already selects exactly the enabled, non-maintenance -/// rows those nodes come from; the gRPC branch dials that same row set directly, one call per -/// node, so the recipient set is identical by construction rather than by coincidence. It is -/// deliberately NOT narrower: the node side has no ClusterId or node filter at all — -/// scoping happens later, inside the artifact — so a node left out here silently keeps its -/// old configuration while the deployment seals green. -/// -/// -/// Fire-and-forget, on every transport. The gRPC branch pipes per-node results back to +/// Fire-and-forget, on both transports. The gRPC branch pipes per-node results back to /// itself for logging ONLY: a failure names the node and the status and is dropped. No retry, /// no queue, no ordering guarantee across nodes. That is not an omission — it is the -/// ClusterClient semantic preserved exactly, and the backstops are unchanged (the deploy +/// DistributedPubSub semantic preserved exactly, and the backstops are unchanged (the deploy /// coordinator's apply deadline, and a node's boot-time replay). /// /// @@ -62,7 +51,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers private readonly IDbContextFactory _dbFactory; private readonly MeshTransportOptions _options; - private readonly IMeshClusterClientFactory _clientFactory; private readonly Func _coordinator; // Resolved in PreStart, never at construction. See the Props doc: the DI graph behind these two @@ -88,15 +76,8 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers _dialer ??= Resolve(_dialerFactory, nameof(IMeshCommandDialer), firstUse: true); private readonly ILoggingAdapter _log = Context.GetLogger(); - private readonly bool _useClusterClient; private readonly bool _useGrpc; - // One client per application Cluster, keyed by ClusterId. A ClusterClient reaches only the mesh - // whose receptionist it dialled, and after Phase 6 every cluster is its own mesh — so central must - // hold a client per cluster and fan SendToAll across all of them. - private readonly Dictionary Contacts)> _clients = - new(); - // Node ids already warned about for a null GrpcPort. A persistently portless row would otherwise // produce one warning per refresh forever; same warn-once shape as TelemetryNodeSource. Actor // thread only. @@ -121,7 +102,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// Creates the props for this actor. /// Factory for the config database holding ClusterNode rows. /// The bound mesh-transport options. - /// Creates the ClusterClient; substituted in tests. /// /// Resolves the deploy-coordinator singleton proxy. A rather than /// an so this actor never depends on registration order at wiring @@ -129,9 +109,9 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// /// /// 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. + /// . Optional so the Dps transport needs nothing + /// new wired; a dialer under Grpc is a mis-wiring and is + /// reported loudly rather than silently dropping every command. /// /// /// Resolves central's per-cluster key lookup, told on every contact refresh which clusters @@ -163,37 +143,30 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers public static Props Props( IDbContextFactory dbFactory, MeshTransportOptions options, - IMeshClusterClientFactory clientFactory, Func coordinator, Func? dialer = null, Func? pskProvider = null) => Akka.Actor.Props.Create(() => - new CentralCommunicationActor( - dbFactory, options, clientFactory, coordinator, dialer, pskProvider)); + new CentralCommunicationActor(dbFactory, options, coordinator, dialer, pskProvider)); /// Initializes a new instance of the class. /// Factory for the config database. /// The bound mesh-transport options. - /// Creates the ClusterClient. /// Lazily resolves the deploy-coordinator singleton proxy. /// Resolves the gRPC command dialer; invoked in . /// Resolves the PSK provider; invoked in . public CentralCommunicationActor( IDbContextFactory dbFactory, MeshTransportOptions options, - IMeshClusterClientFactory clientFactory, Func coordinator, Func? dialer = null, Func? pskProvider = null) { _dbFactory = dbFactory; _options = options; - _clientFactory = clientFactory; _coordinator = coordinator; _dialerFactory = dialer; _pskProviderFactory = pskProvider; - _useClusterClient = string.Equals( - options.Mode, MeshTransportOptions.ModeClusterClient, StringComparison.OrdinalIgnoreCase); _useGrpc = string.Equals( options.Mode, MeshTransportOptions.ModeGrpc, StringComparison.OrdinalIgnoreCase); @@ -246,13 +219,13 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers _dialer = Resolve(_dialerFactory, nameof(IMeshCommandDialer), firstUse: false); _pskProvider = Resolve(_pskProviderFactory, nameof(IMeshPskProvider), firstUse: false); - if (!_useClusterClient && !_useGrpc) + if (!_useGrpc) { _log.Info( - "Mesh transport is {Mode}: commands are published on DistributedPubSub and no " - + "ClusterClient is created. The contact refresh still runs — it is what keeps the " - + "known-cluster gate current, and that gate belongs to the inbound ack endpoint, " - + "which is mapped by port and does not care about this mode", _options.Mode); + "Mesh transport is {Mode}: commands are published on DistributedPubSub and nothing " + + "is dialled. The contact refresh still runs — it is what keeps the known-cluster " + + "gate current, and that gate belongs to the inbound ack endpoint, which is mapped " + + "by port and does not care about this mode", _options.Mode); } // Deliberately NO null guard for either dependency here. At this point a still-null value @@ -266,7 +239,7 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers // Started in EVERY mode. The refresh is not transport state: it is central's ClusterNode // inventory, and the known-cluster gate it feeds protects an endpoint mapped by PORT, not by // MeshTransport:Mode. Gating the timer on the mode would mean a dark-switch rollback to - // ClusterClient or Dps left that endpoint live with the gate permanently inactive — silently + // Dps left that endpoint live with the gate permanently inactive — silently // reopening both the timing oracle and the pre-auth store/audit flood. Under Dps this is a // ConfigDb query every ContactRefreshSeconds that did not run before; central always has a // ConfigDb, and one indexed read a minute is the cheaper half of the trade. @@ -357,9 +330,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// protected override void PostStop() { - foreach (var (client, _) in _clients.Values) Context.Stop(client); - _clients.Clear(); - // Cancelling the lifecycle token also cancels every gRPC dial in flight. The dialer itself is // deliberately NOT disposed here: it is a container-owned singleton shared with anything else // that may dial, and an actor restart must not tear its channel pool down. @@ -369,43 +339,15 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers private void HandleMeshCommand(MeshCommand cmd) { - // Checked before the DPS arm: _useClusterClient is false under Grpc too, so the order is - // what keeps a Grpc-mode command off DistributedPubSub. + // Checked first, so a Grpc-mode command never reaches DistributedPubSub. DPS is mesh-wide and + // cannot cross an application-Cluster boundary at all in the split topology. if (_useGrpc) { DispatchOverGrpc(cmd); return; } - if (!_useClusterClient) - { - DistributedPubSub.Get(Context.System).Mediator.Tell(new Publish(cmd.Topic, cmd.Message)); - return; - } - - if (_clients.Count == 0) - { - // App-level drop, matching the sister project's decision and this repo's buffer-size = 0. - // The command is NOT queued and will NOT be retried: a deploy fails at the coordinator's - // apply deadline naming the silent nodes, which is a better operator signal than a - // command arriving minutes late against a config the DB already recorded as failed. - _log.Warning( - "No mesh ClusterClient — dropping {MessageType}. The last contact refresh produced " - + "no usable contact point from the enabled, non-maintenance ClusterNode rows in any " - + "cluster; the command is not buffered and will not be retried", - cmd.Message.GetType().Name); - return; - } - - // SendToAll, NEVER Send, fanned across EVERY cluster's client. Today's DPS publish reaches - // EVERY DriverHostActor — there is no ClusterId or node filter on the node side at all; scoping - // happens later, inside the artifact (DeploymentArtifact.ResolveClusterScope). Each ClusterClient - // reaches only the one mesh it dialled, so central must fan across all of them to preserve the - // fleet-wide reach DPS gives. ClusterClient.Send delivers to exactly ONE registered actor, which - // would deploy to a single node while every other node silently kept its old configuration — and - // the deployment would still seal green if the ack set happened to be satisfied. - foreach (var (client, _) in _clients.Values) - client.Tell(new ClusterClient.SendToAll(MeshPaths.NodeCommunication, cmd.Message)); + DistributedPubSub.Get(Context.System).Mediator.Tell(new Publish(cmd.Topic, cmd.Message)); } /// @@ -453,10 +395,10 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers var targets = _targets; if (targets.Count == 0) { - // App-level drop, exactly as the ClusterClient arm does with no client. The command is - // NOT queued and will NOT be retried: a deploy fails at the coordinator's apply deadline - // naming the silent nodes, which is a better operator signal than a command arriving - // minutes late against a config the DB already recorded as failed. + // App-level drop. The command is NOT queued and will NOT be retried: a deploy fails at + // the coordinator's apply deadline naming the silent nodes, which is a better operator + // signal than a command arriving minutes late against a config the DB already recorded + // as failed. _log.Warning( "No mesh gRPC targets — dropping {MessageType}. The last contact refresh produced no " + "enabled, non-maintenance ClusterNode row carrying a GrpcPort in any cluster; the " @@ -632,7 +574,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers return; // Stopping. } - var systemName = Context.System.Name; var dbFactory = _dbFactory; // Off the actor thread, result piped back as a message. A synchronous DB read here would @@ -641,29 +582,21 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers { await using var db = await dbFactory.CreateDbContextAsync(ct).ConfigureAwait(false); // UNFILTERED, with the eligibility flags projected and applied per row below. The two - // outputs of this read want different row sets: the contact/dispatch sets are the - // enabled, non-maintenance nodes central talks TO, while the known-cluster gate answers - // "is this a cluster central has ever heard of" and must include a cluster whose every - // node is disabled or in maintenance — such a cluster can still send an ack, and denying - // it would make maintenance mode an authentication switch. + // outputs of this read want different row sets: the dispatch set is the enabled, + // non-maintenance nodes central talks TO, while the known-cluster gate answers "is this a + // cluster central has ever heard of" and must include a cluster whose every node is + // disabled or in maintenance — such a cluster can still send an ack, and denying it would + // make maintenance mode an authentication switch. var rows = await db.ClusterNodes .AsNoTracking() .Select(n => new { - n.NodeId, n.ClusterId, n.Host, n.AkkaPort, n.GrpcPort, n.Enabled, n.MaintenanceMode, + n.NodeId, n.ClusterId, n.Host, n.GrpcPort, n.Enabled, n.MaintenanceMode, }) .ToListAsync(ct) .ConfigureAwait(false); - // Grouped BY ClusterId: central now dials one ClusterClient per application Cluster, so the - // contact set must be partitioned the same way. A row's contacts join only its own cluster's - // client. - var byCluster = new Dictionary>(); - var malformed = new List(); - - // The gRPC dispatch set, built from the SAME row set in the SAME pass. Sharing the read is - // what makes "the gRPC fan reaches exactly the ClusterClient recipient set" structural - // rather than a claim two separate queries would have to keep agreeing on. + // The gRPC dispatch set: one entry per eligible row that carries a port. var targets = new List(rows.Count); var portless = new List(); @@ -677,8 +610,7 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers clusterIds.Add(row.ClusterId); // The eligibility filter that used to be the query's WHERE clause. Everything below - // this line — receptionist contacts and gRPC targets — sees exactly the rows it saw - // before. + // this line — the gRPC targets — sees exactly the rows it saw before. if (!row.Enabled || row.MaintenanceMode) continue; @@ -687,29 +619,9 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers row.NodeId, row.ClusterId, $"http://{row.Host}:{grpcPort}")); else portless.Add(row.NodeId); - - var address = $"akka.tcp://{systemName}@{row.Host}:{row.AkkaPort}/system/receptionist"; - - // Parse up front, per row, inside the loop's own guard. A single malformed row must - // not abort the whole refresh and leave the contact set half-built — the sister - // project shipped that bug and its regression test deliberately orders the bad row - // first. - if (ActorPath.TryParse(address, out _)) - { - if (!byCluster.TryGetValue(row.ClusterId, out var list)) - byCluster[row.ClusterId] = list = new List(); - list.Add(address); - } - else - { - malformed.Add($"{row.NodeId} ({row.ClusterId}) -> {address}"); - } } - var contactsByCluster = byCluster.ToDictionary( - kv => kv.Key, kv => (IReadOnlyList)kv.Value); - return new ContactsLoaded( - contactsByCluster, malformed, targets, portless, clusterIds.ToList()); + return new ContactsLoaded(targets, portless, clusterIds.ToList()); }, ct).PipeTo(self); } @@ -722,66 +634,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers // EVERY mode. See PreStart — the gate protects an endpoint mapped by port, not by mode. PublishKnownClusters(msg); - - if (!_useClusterClient) - return; // Dps or Grpc: no ClusterClient to rebuild. - - foreach (var bad in msg.Malformed) - { - _log.Warning( - "ClusterNode {Row} does not yield a usable receptionist address; skipping it in this " - + "refresh (other nodes are unaffected). Check the row's Host and AkkaPort", bad); - } - - var byCluster = msg.ContactsByCluster.ToDictionary( - kv => kv.Key, kv => kv.Value.ToImmutableHashSet()); - - // Clusters that dropped out of the DB entirely, or whose only rows were malformed this refresh: - // stop and forget their clients so a vanished cluster stops receiving. LoadContactsFromDb only - // creates a ByCluster key when at least one of its rows parsed, so an all-malformed cluster has - // NO key at all and is caught here by the absent-key (!TryGetValue) arm — not by an empty value. - var gone = _clients.Keys - .Where(id => !byCluster.TryGetValue(id, out var c) || c.IsEmpty) - .ToList(); - foreach (var clusterId in gone) - { - _log.Info( - "Cluster {ClusterId} no longer yields any usable contact point; stopping its " - + "ClusterClient", clusterId); - Context.Stop(_clients[clusterId].Client); - _clients.Remove(clusterId); - } - - var anyUsable = false; - foreach (var (clusterId, contacts) in byCluster) - { - if (contacts.IsEmpty) - { - // Defensive: LoadContactsFromDb never emits an empty-but-present cluster (a key is - // created only on a first successfully-parsed row), so this arm is unreachable for the - // DB producer — an all-malformed cluster is handled by the absent-key branch above. It - // guards a ContactsLoaded (a public record) constructed elsewhere with an empty list. - _log.Warning( - "Cluster {ClusterId} has no usable ClusterNode contact points; no ClusterClient " - + "was created for it and its nodes' commands will be dropped until a refresh finds " - + "one", clusterId); - continue; - } - - anyUsable = true; - - if (_clients.TryGetValue(clusterId, out var existing) && existing.Contacts.SetEquals(contacts)) - continue; - - RebuildClient(clusterId, contacts); - } - - if (!anyUsable && _clients.Count == 0) - { - _log.Warning( - "No usable ClusterNode contact points in any cluster; no mesh ClusterClient was " - + "created and every central→node command will be dropped until a refresh finds one"); - } } /// @@ -790,11 +642,9 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// /// The load result. /// - /// The malformed-receptionist-address warnings are deliberately NOT logged here: nothing on - /// this transport dials a receptionist, and reporting an unusable Akka address as a - /// command-plane problem would send an operator after the wrong column. A host that is - /// unusable as a gRPC endpoint surfaces instead as a named per-node dial failure on every - /// dispatch, which is a truer signal. + /// A host that is unusable as a gRPC endpoint is deliberately NOT screened here: it surfaces + /// as a named per-node dial failure on every dispatch, which is a truer signal than a + /// refresh-time warning about a string that may well be fine. /// private void AdoptGrpcTargets(ContactsLoaded msg) { @@ -837,7 +687,7 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// Not gated on , on purpose. The gate feeds /// CentralCommandAuthInterceptor, which is registered on every admin node, and the /// ack service it protects is mapped by listener port — neither consults the mode. Gating - /// this on Grpc would mean a dark-switch rollback to ClusterClient or + /// this on Grpc would mean a dark-switch rollback to /// Dps left that endpoint serving with a permanently inactive gate: the timing /// oracle and the pre-authentication store/audit flood would both come back, silently, /// as a side effect of a rollback nobody would connect to authentication. @@ -887,75 +737,13 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers pskProvider.InvalidateAll(); } - /// Stops a cluster's existing client and creates a replacement for its new contact set. - /// - /// - /// One client per application Cluster (delivered in Phase 6). This actor now - /// holds one ClusterClient per cluster and fans - /// SendToAll across all of them. Once the fleet is split into one Akka mesh per - /// cluster, a ClusterClientReceptionist serves only its own mesh, so a single - /// client's SendToAll would reach the one mesh whose receptionist answered and leave - /// every other cluster silently on its old configuration. Fanning across a client per - /// cluster restores the fleet-wide reach the pre-split DPS broadcast gave. - /// - /// - /// Why this is not N× duplicate dispatch: each client dials only its own cluster's - /// receptionists, so a command is delivered once per mesh, not once per cluster into - /// every mesh. (While the fleet was a single mesh, the same code would have duplicated — - /// which is exactly why the fan-out could not land until the meshes were partitioned.) - /// - /// - /// Corollary worth stating because it is easy to assume otherwise: the contact set - /// does not scope delivery within a mesh. Excluding a maintenance-mode node from a - /// cluster's contacts does not stop it receiving commands — its receptionist still delivers - /// to every registered node-comm actor in that mesh. The filter is about which receptionists - /// are worth dialling (a node in maintenance may be switched off), not about who gets - /// the message. - /// - /// - /// The application Cluster whose client is being (re)built. - /// The receptionist addresses to dial for that cluster. - private void RebuildClient(string clusterId, ImmutableHashSet contacts) - { - if (_clients.TryGetValue(clusterId, out var existing)) - { - _log.Info("Contact set for cluster {ClusterId} changed; replacing its ClusterClient", clusterId); - Context.Stop(existing.Client); - - // Removed now: if the create below throws, a stale ref would route commands into a - // stopping actor and they would vanish without a warning. - _clients.Remove(clusterId); - } - - try - { - var paths = contacts.Select(ActorPath.Parse).ToImmutableHashSet(); - var client = _clientFactory.Create(Context.System, clusterId, paths); - _clients[clusterId] = (client, contacts); - _log.Info( - "Mesh ClusterClient for cluster {ClusterId} created with {Count} contact point(s)", - clusterId, contacts.Count); - } - catch (Exception ex) - { - _log.Error(ex, - "Failed to create the mesh ClusterClient for cluster {ClusterId}; its nodes' commands " - + "are dropped until the next contact refresh succeeds", clusterId); - } - } - /// Internal tick asking for a contact-point refresh. public sealed record RefreshContacts; /// Result of a contact-point load, piped back from the DB read. - /// - /// Receptionist addresses that parsed cleanly, grouped by the owning application - /// Cluster's id. One ClusterClient is built per key. - /// - /// Rows that did not parse, described for the log. /// /// The gRPC dispatch set: one entry per enabled, non-maintenance row that carries a - /// GrpcPort. Built from the same rows as . + /// GrpcPort. /// /// /// Node ids skipped because their row has no GrpcPort. Carried rather than logged in @@ -968,8 +756,6 @@ public sealed class CentralCommunicationActor : ReceiveActor, IWithTimers /// still send an ack, and refusing it would make maintenance mode an authentication switch. /// public sealed record ContactsLoaded( - IReadOnlyDictionary> ContactsByCluster, - IReadOnlyList Malformed, IReadOnlyList Targets, IReadOnlyList PortlessNodes, IReadOnlyList ClusterIds); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshClusterClientFactory.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshClusterClientFactory.cs deleted file mode 100644 index 83b2b2ee..00000000 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshClusterClientFactory.cs +++ /dev/null @@ -1,67 +0,0 @@ -using System.Collections.Immutable; -using Akka.Actor; -using Akka.Cluster.Tools.Client; - -namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; - -/// -/// Creates the ClusterClient central uses to reach driver nodes. Exists as a seam so -/// CentralCommunicationActor can be tested against a TestProbe without standing up -/// a second cluster. -/// -public interface IMeshClusterClientFactory -{ - /// Creates a client for the given receptionist contact points. - /// The actor system to spawn the client in. - /// - /// The application Cluster these contacts belong to. Central now creates one client per - /// cluster (Phase 6), so the id is woven into the client's actor name to keep the per-cluster - /// clients distinct and diagnosable. - /// - /// Receptionist actor paths, one per reachable node. - /// The new client's . - IActorRef Create(ActorSystem system, string clusterId, ImmutableHashSet contacts); -} - -/// -public sealed class DefaultMeshClusterClientFactory : IMeshClusterClientFactory -{ - /// - /// Per-incarnation generation counter, appended to the actor name. - /// - /// - /// Context.Stop of the previous client is asynchronous — its actor name stays - /// reserved until termination completes — so recreating the same name while handling a single - /// message throws . That is exactly what a contact-set - /// change does: stop the old client, create the replacement, both in one handler. A - /// generation suffix makes every incarnation's name unique by construction. Ported from the - /// sister project, where it was a shipped bug fix rather than foresight. - /// - private long _generation; - - /// - public IActorRef Create(ActorSystem system, string clusterId, ImmutableHashSet contacts) - { - ArgumentNullException.ThrowIfNull(system); - ArgumentException.ThrowIfNullOrEmpty(clusterId); - ArgumentNullException.ThrowIfNull(contacts); - - var settings = ClusterClientSettings.Create(system).WithInitialContacts(contacts); - var name = $"mesh-client-{SanitizeForActorName(clusterId)}-{Interlocked.Increment(ref _generation)}"; - return system.ActorOf(ClusterClient.Props(settings), name); - } - - /// - /// Reduces a cluster id to characters safe in an Akka actor name. The id is operator-authored - /// (e.g. a GUID or a slug), so it may carry characters Akka rejects; anything outside - /// [A-Za-z0-9-] becomes -. The trailing generation counter still guarantees - /// uniqueness, so a lossy sanitisation cannot cause a name collision. - /// - /// The raw cluster id. - /// An actor-name-safe rendering of the id. - private static string SanitizeForActorName(string clusterId) - { - var chars = clusterId.Select(c => char.IsAsciiLetterOrDigit(c) || c == '-' ? c : '-'); - return new string(chars.ToArray()); - } -} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index 1801da38..472f753e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -56,11 +56,11 @@ public static class ServiceCollectionExtensions // Per-cluster mesh Phase 2: the central end of the central↔node command boundary. // - // A plain WithActors, NOT a singleton — and that is the load-bearing part. A driver node's - // ClusterClient rotates across contact points and must find a live comm actor at whichever - // central node answers. As a singleton it would exist on one central node only, and the - // rotation would fail silently against the other: acks would land sometimes, depending on - // which contact the client happened to settle on. + // A plain WithActors, NOT a singleton — and that is the load-bearing part. A driver node + // walks MeshTransport:CentralGrpcEndpoints in order and posts its ack to whichever central + // node answers first, so a live comm actor must exist on both. As a singleton it would exist + // on one central node only, and the other node's ack ingress would have nothing to forward + // to: acks would land sometimes, depending on which endpoint answered. builder.WithActors((system, registry, resolver) => { var dbFactory = resolver.GetService>(); @@ -85,7 +85,6 @@ public static class ServiceCollectionExtensions CentralCommunicationActor.Props( dbFactory, options, - new DefaultMeshClusterClientFactory(), // Resolved lazily, per message, so this actor never depends on the order in // which Akka.Hosting materialises the singleton proxy relative to this block. () => registry.TryGet(out var coordinator) 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 4a0bdc57..c93cabd8 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 @@ -1,9 +1,6 @@ -using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using Akka.Actor; -using Akka.Cluster.Tools.Client; using Akka.Cluster.Tools.PublishSubscribe; -using Akka.TestKit; using Microsoft.EntityFrameworkCore; using Shouldly; using Xunit; @@ -25,40 +22,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase { private const string DeploymentsTopic = "deployments"; - /// Records every client the actor asks for, and hands back a probe in its place. - private sealed class RecordingClientFactory : IMeshClusterClientFactory - { - private readonly Func _newProbe; - - public RecordingClientFactory(Func newProbe) => _newProbe = newProbe; - - public List> Calls { get; } = []; - - public List ClusterIds { get; } = []; - - public List Probes { get; } = []; - - /// Probe created for a given cluster id, for per-cluster fan-out assertions. - public Dictionary ProbeByCluster { get; } = []; - - public IActorRef Create(ActorSystem system, string clusterId, ImmutableHashSet contacts) - { - Calls.Add(contacts); - ClusterIds.Add(clusterId); - var probe = _newProbe(); - Probes.Add(probe); - ProbeByCluster[clusterId] = probe; - return probe.Ref; - } - } - - private static MeshTransportOptions ClusterClientMode(int refreshSeconds = 60) => new() - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["akka.tcp://otopcua@central-1:4053"], - ContactRefreshSeconds = refreshSeconds, - }; - private static MeshTransportOptions GrpcMode(int refreshSeconds = 60) => new() { Mode = MeshTransportOptions.ModeGrpc, @@ -66,47 +29,7 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase DispatchTimeoutSeconds = 10, }; - private static void SeedCluster( - IDbContextFactory factory, - string clusterId, - params (string NodeId, string Host, int AkkaPort, bool Enabled, bool Maintenance)[] nodes) - { - using var db = factory.CreateDbContext(); - db.ServerClusters.Add(new ServerCluster - { - ClusterId = clusterId, - Name = clusterId, - Enterprise = "ZB", - Site = "Site", - // Warm/Hot require NodeCount = 2 (CK_ServerCluster_RedundancyMode_NodeCount); these - // fixtures only need the row to exist, so use the standalone shape. - RedundancyMode = RedundancyMode.None, - NodeCount = 1, - CreatedBy = "test", - }); - foreach (var n in nodes) - { - db.ClusterNodes.Add(new ClusterNode - { - NodeId = n.NodeId, - ClusterId = clusterId, - Host = n.Host, - AkkaPort = n.AkkaPort, - ApplicationUri = $"urn:{n.NodeId}", - Enabled = n.Enabled, - MaintenanceMode = n.Maintenance, - CreatedBy = "test", - }); - } - - db.SaveChanges(); - } - - /// - /// Seeds a cluster whose nodes carry a GrpcPort — the gRPC command plane's shape. - /// Kept separate from so the ClusterClient facts, which have no - /// opinion about the port, stay exactly as they were. - /// + /// Seeds a cluster whose nodes carry a GrpcPort — the command plane's shape. private static void SeedGrpcCluster( IDbContextFactory factory, string clusterId, @@ -145,15 +68,16 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase private IActorRef Spawn( IDbContextFactory factory, MeshTransportOptions options, - IMeshClusterClientFactory clientFactory, IActorRef? coordinator = null) => - Sys.ActorOf(CentralCommunicationActor.Props(factory, options, clientFactory, () => coordinator)); + Sys.ActorOf(CentralCommunicationActor.Props(factory, options, () => coordinator)); [Fact] - public void Dps_mode_publishes_on_the_carried_topic_and_creates_no_client() + public void Dps_mode_publishes_on_the_carried_topic() { + // Dps is the pair-local default and stays in the tree: it is the correct transport for a + // single-mesh topology, which every integration test and every profile that leaves + // MeshTransport:Mode unset still runs. var factory = NewInMemoryDbFactory(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); var mediatorProbe = CreateTestProbe(); // Subscribe on the probe's behalf: SubscribeAck goes to the SENDER, so telling the mediator // without an explicit sender would ack to TestActor and this wait would hang. @@ -161,150 +85,22 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase new Subscribe(DeploymentsTopic, mediatorProbe.Ref), mediatorProbe.Ref); mediatorProbe.ExpectMsg(TimeSpan.FromSeconds(5)); - var actor = Spawn(factory, new MeshTransportOptions(), clients); + var actor = Spawn(factory, new MeshTransportOptions()); var dispatch = new DispatchDeployment( new DeploymentId(Guid.NewGuid()), new RevisionHash("abc"), new CorrelationId(Guid.NewGuid())); actor.Tell(new MeshCommand(DeploymentsTopic, dispatch)); mediatorProbe.ExpectMsg(TimeSpan.FromSeconds(5)).ShouldBe(dispatch); - clients.Calls.ShouldBeEmpty(); - } - - [Fact] - public void ClusterClient_mode_sends_to_all_not_to_one() - { - // THE assertion of this phase. ClusterClient.Send delivers to exactly ONE registered actor; - // today's DPS publish reaches EVERY DriverHostActor, and the node side has no ClusterId or - // node filter to compensate. Send would deploy to a single node while every other node - // silently kept its old configuration. - var factory = NewInMemoryDbFactory(); - SeedCluster(factory, "C1", ("host-a:4053", "host-a", 4053, true, false)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - - var actor = Spawn(factory, ClusterClientMode(), clients); - AwaitCondition(() => clients.Probes.Count == 1, TimeSpan.FromSeconds(5)); - - var dispatch = new DispatchDeployment( - new DeploymentId(Guid.NewGuid()), new RevisionHash("abc"), new CorrelationId(Guid.NewGuid())); - actor.Tell(new MeshCommand(DeploymentsTopic, dispatch)); - - var sent = clients.Probes[0].ExpectMsg(TimeSpan.FromSeconds(5)); - sent.Path.ShouldBe(MeshPaths.NodeCommunication); - sent.Message.ShouldBe(dispatch); - } - - [Fact] - public void One_client_per_cluster_is_created_for_a_multi_cluster_fleet() - { - // THE Phase 6 inversion. Each application Cluster is now its own Akka mesh, and a ClusterClient - // reaches only the mesh whose receptionist it dialled. Central must therefore build one client - // per cluster, each dialling only that cluster's nodes. - var factory = NewInMemoryDbFactory(); - SeedCluster(factory, "C1", ("host-a:4053", "host-a", 4053, true, false)); - SeedCluster(factory, "C2", ("host-b:4053", "host-b", 4053, true, false)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - - Spawn(factory, ClusterClientMode(), clients); - AwaitCondition(() => clients.Calls.Count == 2, TimeSpan.FromSeconds(5)); - - // Give a second refresh a chance to (wrongly) create more. - ExpectNoMsg(TimeSpan.FromMilliseconds(300)); - clients.Calls.Count.ShouldBe(2); - clients.ClusterIds.ShouldBe(new[] { "C1", "C2" }, ignoreOrder: true); - - // Each client dials ONLY its own cluster's node. - clients.Calls[clients.ClusterIds.IndexOf("C1")].Single().ToString().ShouldContain("host-a"); - clients.Calls[clients.ClusterIds.IndexOf("C2")].Single().ToString().ShouldContain("host-b"); - } - - [Fact] - public void A_command_fans_out_a_send_to_all_to_every_cluster_client() - { - // The other half of the inversion: one MeshCommand must reach EVERY cluster's mesh, so central - // fans SendToAll across all per-cluster clients. A single Send/one-client shape would leave - // every cluster but one silently on its old configuration. - var factory = NewInMemoryDbFactory(); - SeedCluster(factory, "C1", ("host-a:4053", "host-a", 4053, true, false)); - SeedCluster(factory, "C2", ("host-b:4053", "host-b", 4053, true, false)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - - var actor = Spawn(factory, ClusterClientMode(), clients); - AwaitCondition(() => clients.Probes.Count == 2, TimeSpan.FromSeconds(5)); - - var dispatch = new DispatchDeployment( - new DeploymentId(Guid.NewGuid()), new RevisionHash("abc"), new CorrelationId(Guid.NewGuid())); - actor.Tell(new MeshCommand(DeploymentsTopic, dispatch)); - - foreach (var (clusterId, probe) in clients.ProbeByCluster) - { - var sent = probe.ExpectMsg( - TimeSpan.FromSeconds(5), $"cluster {clusterId} must receive the fanned SendToAll"); - sent.Path.ShouldBe(MeshPaths.NodeCommunication); - sent.Message.ShouldBe(dispatch); - } - } - - [Fact] - public void Disabled_and_maintenance_nodes_are_not_dialled() - { - var factory = NewInMemoryDbFactory(); - SeedCluster( - factory, - "C1", - ("host-a:4053", "host-a", 4053, true, false), - ("host-b:4053", "host-b", 4053, false, false), - ("host-c:4053", "host-c", 4053, true, true)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - - Spawn(factory, ClusterClientMode(), clients); - AwaitCondition(() => clients.Calls.Count == 1, TimeSpan.FromSeconds(5)); - - clients.Calls[0].Count.ShouldBe(1); - clients.Calls[0].Single().ToString().ShouldContain("host-a"); - } - - [Fact] - public void A_malformed_node_row_does_not_abort_the_refresh() - { - // The bad row is ordered FIRST on purpose: the failure this guards against is an exception - // that aborts the loop and leaves the contact set half-built. - var factory = NewInMemoryDbFactory(); - SeedCluster( - factory, - "C1", - ("bad", "not a host!!", 4053, true, false), - ("host-a:4053", "host-a", 4053, true, false)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - - Spawn(factory, ClusterClientMode(), clients); - AwaitCondition(() => clients.Calls.Count == 1, TimeSpan.FromSeconds(5)); - - clients.Calls[0].ShouldContain(p => p.ToString().Contains("host-a")); - } - - [Fact] - public void A_command_with_no_client_is_dropped_with_a_warning() - { - // No ClusterNode rows at all: nothing to dial, so the command must be dropped loudly rather - // than queued. buffer-size = 0 covers the case where a client exists but cannot connect; - // this covers the case where there is no client at all. - var factory = NewInMemoryDbFactory(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - var actor = Spawn(factory, ClusterClientMode(), clients); - - EventFilter.Warning(contains: "dropping").ExpectOne(TimeSpan.FromSeconds(5), () => - actor.Tell(new MeshCommand(DeploymentsTopic, "anything"))); } [Fact] public void ApplyAck_is_forwarded_to_the_coordinator_preserving_the_sender() { var factory = NewInMemoryDbFactory(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); var coordinator = CreateTestProbe(); var node = CreateTestProbe(); - var actor = Spawn(factory, ClusterClientMode(), clients, coordinator.Ref); + var actor = Spawn(factory, GrpcMode(), coordinator.Ref); var ack = new ApplyAck( new DeploymentId(Guid.NewGuid()), @@ -324,8 +120,7 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase public void An_unresolvable_coordinator_logs_rather_than_losing_the_ack_silently() { var factory = NewInMemoryDbFactory(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - var actor = Spawn(factory, ClusterClientMode(), clients, coordinator: null); + var actor = Spawn(factory, GrpcMode(), coordinator: null); var ack = new ApplyAck( new DeploymentId(Guid.NewGuid()), @@ -342,12 +137,11 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase public void A_faulted_contact_load_warns_and_leaves_the_actor_alive() { var factory = NewInMemoryDbFactory(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); - var actor = Spawn(factory, ClusterClientMode(), clients); + var actor = Spawn(factory, GrpcMode()); // Filter on a phrase unique to the DB-failure warning. EventFilter matches case-INSENSITIVELY, - // so a looser "was NOT" also caught this actor's other warning ("the mesh ClusterClient was - // not created") and the assertion failed with two matches instead of one. + // so a looser "was NOT" would also catch this actor's other warnings and the assertion would + // fail with two matches instead of one. EventFilter.Warning(contains: "contact set was NOT refreshed").ExpectOne( TimeSpan.FromSeconds(5), () => actor.Tell(new Status.Failure(new InvalidOperationException("db down")))); @@ -359,16 +153,15 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase // ------------------------------------------------------------------ Mode = Grpc // - // The third transport. The fan-out must reach EXACTLY the recipient set SendToAll reaches today - // — every enabled, non-maintenance node in every cluster — because the node side still has no - // ClusterId or node filter; scoping happens later, inside the artifact. A narrower fan would - // leave nodes silently on their old configuration while the deployment sealed green. + // The cross-mesh transport. The fan-out must reach EVERY enabled, non-maintenance node in every + // cluster, because the node side has no ClusterId or node filter at all; scoping happens later, + // inside the artifact. A narrower fan would leave nodes silently on their old configuration + // while the deployment sealed green. [Fact] public void Grpc_mode_fans_one_command_to_every_enabled_node_across_all_clusters() { - // THE assertion of this task, and the direct counterpart of - // A_command_fans_out_a_send_to_all_to_every_cluster_client. + // THE assertion of this transport: one command, one call per node, across every cluster. var factory = NewInMemoryDbFactory(); SeedGrpcCluster( factory, @@ -392,31 +185,32 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase } [Fact] - public void Grpc_mode_creates_no_ClusterClient_and_publishes_nothing_on_pubsub() + public void Grpc_mode_publishes_nothing_on_pubsub() { + // DPS is mesh-wide and cannot cross an application-Cluster boundary, so a command that also + // went out on the topic would be delivered twice inside central's own mesh and not at all + // anywhere else. var factory = NewInMemoryDbFactory(); SeedGrpcCluster(factory, "C1", ("host-a:4053", "host-a", 4056, true, false)); - var clients = new RecordingClientFactory(() => CreateTestProbe()); var mediatorProbe = CreateTestProbe(); DistributedPubSub.Get(Sys).Mediator.Tell( new Subscribe(DeploymentsTopic, mediatorProbe.Ref), mediatorProbe.Ref); mediatorProbe.ExpectMsg(TimeSpan.FromSeconds(5)); - var rig = SpawnGrpcRig(factory, clientFactory: clients); + var rig = SpawnGrpcRig(factory); AwaitContactsLoaded(rig); rig.Actor.Tell(new MeshCommand(DeploymentsTopic, NewDispatch())); AwaitCondition(() => rig.Dialer.Calls.Count == 1, TimeSpan.FromSeconds(5)); - clients.Calls.ShouldBeEmpty(); mediatorProbe.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); } [Fact] public void Grpc_mode_does_not_dial_disabled_or_maintenance_nodes() { - // Same recipient filter as the ClusterClient contact load, asserted independently so the two - // transports cannot drift on who counts as a node. + // Disabled and maintenance-mode rows are not nodes central talks to; the same filter the + // contact load applies, asserted here so the fan cannot drift from it. var factory = NewInMemoryDbFactory(); SeedGrpcCluster( factory, @@ -725,22 +519,20 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase [InlineData(MeshTransportOptions.ModeGrpc)] public void The_known_cluster_gate_is_published_in_every_transport_mode(string mode) { - // Replaces an earlier fact asserting the OPPOSITE, whose premise did not survive 1B: the + // Replaces an earlier fact asserting the OPPOSITE, whose premise did not survive Phase 1B: the // gate feeds CentralCommandAuthInterceptor, which is registered on every admin node, and the // ack service it protects is mapped by LISTENER PORT — neither consults MeshTransport:Mode. - // Gating the publication on Grpc meant a dark-switch rollback to ClusterClient or Dps left - // that endpoint serving with a permanently inactive gate, silently restoring both the timing + // Gating the publication on Grpc meant a dark-switch rollback to Dps left that endpoint + // serving with a permanently inactive gate, silently restoring both the timing // oracle and the pre-authentication store/audit flood. The set is central's cluster // inventory, not transport state. var factory = NewInMemoryDbFactory(); SeedGrpcCluster(factory, "C1", ("host-a:4053", "host-a", 4056, true, false)); var psk = new RecordingPskProvider(); - var clients = new RecordingClientFactory(() => CreateTestProbe()); Sys.ActorOf(CentralCommunicationActor.Props( factory, new MeshTransportOptions { Mode = mode, ContactRefreshSeconds = 60 }, - clients, () => null, () => new RecordingDialer(), () => psk)); @@ -749,12 +541,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase () => psk.KnownClusterPublications.Count >= 1, TimeSpan.FromSeconds(5)); psk.KnownClusterPublications[0].ShouldBe(new[] { "C1" }); psk.InvalidateAllCount.ShouldBeGreaterThanOrEqualTo(1); - - // The one load-bearing line the publication itself cannot witness: with the refresh - // timer now running in every mode, only the ClusterClient branch may construct clients. - // With rows seeded, deleting the !_useClusterClient guard turns this red for Dps/Grpc. - if (mode != MeshTransportOptions.ModeClusterClient) - clients.Calls.ShouldBeEmpty(); } [Fact] @@ -815,7 +601,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase var actor = Sys.ActorOf(CentralCommunicationActor.Props( factory, GrpcMode(), - new RecordingClientFactory(() => CreateTestProbe()), () => null, dialer: null, pskProvider: null)); @@ -871,7 +656,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase actor = Sys.ActorOf(CentralCommunicationActor.Props( factory, GrpcMode(), - new RecordingClientFactory(() => CreateTestProbe()), () => null, DialerFactory, PskFactory)); @@ -924,7 +708,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase actor = Sys.ActorOf(CentralCommunicationActor.Props( factory, GrpcMode(), - new RecordingClientFactory(() => CreateTestProbe()), () => null, () => new RecordingDialer(), pskProvider: PskFactory)); @@ -969,7 +752,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase actor = Sys.ActorOf(CentralCommunicationActor.Props( factory, GrpcMode(), - new RecordingClientFactory(() => CreateTestProbe()), () => null, dialer: () => throw new InvalidOperationException("permanently broken"), pskProvider: () => psk)); @@ -1006,7 +788,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase private GrpcRig SpawnGrpcRig( IDbContextFactory factory, RecordingDialer? dialer = null, - IMeshClusterClientFactory? clientFactory = null, IActorRef? coordinator = null) { dialer ??= new RecordingDialer(); @@ -1014,7 +795,6 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase var actor = Sys.ActorOf(CentralCommunicationActor.Props( factory, GrpcMode(), - clientFactory ?? new RecordingClientFactory(() => CreateTestProbe()), () => coordinator, () => dialer, () => psk)); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshClusterClientFactoryTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshClusterClientFactoryTests.cs deleted file mode 100644 index 3c188719..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshClusterClientFactoryTests.cs +++ /dev/null @@ -1,82 +0,0 @@ -using System.Collections.Immutable; -using Akka.Actor; -using Akka.Cluster.Tools.Client; -using Akka.Configuration; -using Akka.TestKit.Xunit2; -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; - -namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Communication; - -/// -/// Guards the one non-obvious part of client creation: recreating a client with the same name -/// inside a single message handler throws, because Context.Stop is asynchronous and the -/// name stays reserved until termination completes. -/// -public class MeshClusterClientFactoryTests : TestKit -{ - private static readonly Config TestConfig = ConfigurationFactory.ParseString(@" - akka.actor.provider = cluster - akka.remote.dot-netty.tcp.port = 0 - akka.remote.dot-netty.tcp.hostname = 127.0.0.1") - .WithFallback(ClusterClientReceptionist.DefaultConfig()); - - public MeshClusterClientFactoryTests() - : base(TestConfig) - { - } - - // A real ClusterClient pointed at an unreachable contact is safe in TestKit — it simply retries - // the initial-contact rotation and never fails the test. - private static ImmutableHashSet UnreachableContact => ImmutableHashSet.Create( - ActorPath.Parse("akka.tcp://otopcua@127.0.0.1:65501/system/receptionist")); - - [Fact] - public void Recreating_a_client_in_one_pass_does_not_collide_on_the_actor_name() - { - var factory = new DefaultMeshClusterClientFactory(); - - var first = factory.Create(Sys, "C1", UnreachableContact); - Sys.Stop(first); - - // Stop is asynchronous: `first`'s name is still reserved right here. Without the generation - // suffix this throws InvalidActorNameException — which is precisely what a contact-set - // change would do in production, where stop-then-recreate happens in one handler. - var second = factory.Create(Sys, "C1", UnreachableContact); - - second.Path.Name.ShouldNotBe(first.Path.Name); - } - - [Fact] - public void Every_incarnation_gets_a_distinct_name() - { - var factory = new DefaultMeshClusterClientFactory(); - - var names = Enumerable.Range(0, 5) - .Select(_ => factory.Create(Sys, "C1", UnreachableContact).Path.Name) - .ToList(); - - names.Distinct().Count().ShouldBe(5); - } - - [Fact] - public void The_cluster_id_is_woven_into_the_client_actor_name() - { - // Per-cluster clients must be distinguishable in logs and the actor tree, and — more - // importantly — two clusters' clients created on one system must never collide on a name. - var factory = new DefaultMeshClusterClientFactory(); - - var a = factory.Create(Sys, "site-a", UnreachableContact); - var b = factory.Create(Sys, "site-b", UnreachableContact); - - a.Path.Name.ShouldContain("site-a"); - b.Path.Name.ShouldContain("site-b"); - a.Path.Name.ShouldNotBe(b.Path.Name); - } - - // Contact-set propagation is deliberately NOT asserted here: reading initial contacts back - // off a created ClusterClient is not possible, and asserting ClusterClientSettings directly - // would test Akka rather than this factory. It is covered where it can actually fail — the - // real two-ActorSystem boundary test, which only delivers if the contacts reached the client. -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshClusterClientBoundaryTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshClusterClientBoundaryTests.cs deleted file mode 100644 index 267dd3ce..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshClusterClientBoundaryTests.cs +++ /dev/null @@ -1,446 +0,0 @@ -using System.Collections.Immutable; -using Akka.Actor; -using Akka.Cluster; -using Akka.Cluster.Tools.Client; -using Akka.Configuration; -using Microsoft.EntityFrameworkCore; -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Cluster; -using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; -using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; -using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; -using ZB.MOM.WW.OtOpcUa.Commons.Types; -using ZB.MOM.WW.OtOpcUa.Configuration; -using ZB.MOM.WW.OtOpcUa.Configuration.Entities; -using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; -using ZB.MOM.WW.OtOpcUa.Runtime.Communication; -using AkkaCluster = Akka.Cluster.Cluster; - -namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; - -/// -/// Drives a real ClusterClient across two genuinely separate s — -/// the only test in the repo where the mesh boundary carries a message over the wire. -/// -/// -/// -/// The sister project substitutes an in-process relay that unwraps -/// and forwards. That tests actor logic and cannot -/// fail for any transport reason: not a missing receptionist extension, not a service that -/// was never registered, not a malformed contact path, not a serialization break. Those are -/// precisely the Phase 2 failure modes, so they are what this test exercises. -/// -/// -/// Two clusters, one ActorSystem name, joined ONLY by ClusterClient. The shared name -/// is mandatory — Akka.Remote matches on the full address, so a contact path naming -/// otopcua cannot reach a system called anything else. Each system seeds itself and -/// neither lists the other, and -/// asserts the -/// separation outright: if they ever gossiped into one mesh, every delivery below could be -/// plain local messaging and the whole class would be testing nothing. -/// -/// -/// Scope limit. With exactly one node per side, "registered per node" and "registered -/// as a cluster singleton" are indistinguishable by construction, so this class does NOT -/// cover the per-node property whose structural assertions MeshCommActorPathTests -/// dropped. That property's discriminating coverage is live-gate step 8: stop the OLDEST -/// central and the survivor's comm actor must still receive a node ack, which a singleton -/// hosted on the stopped node cannot. -/// -/// -public sealed class MeshClusterClientBoundaryTests : IAsyncLifetime -{ - private const string SharedSystemName = "otopcua"; - private static readonly TimeSpan Arrival = TimeSpan.FromSeconds(30); - - /// - /// How long the falsifiability control waits before concluding nothing arrived. Deliberately - /// shorter than but far longer than a successful delivery takes in - /// practice, so a pass means "the boundary is shut", not "the clock ran out first". - /// - private static readonly TimeSpan Silence = TimeSpan.FromSeconds(10); - - /// - /// One factory for every client this class creates, because the generation counter that keeps - /// client actor names unique is per factory instance. A second factory on the same - /// system restarts at generation 1 and collides with the first factory's client — which is - /// exactly how this test first failed. Production has one instance per - /// CentralCommunicationActor, so the real code path is safe. - /// - private readonly DefaultMeshClusterClientFactory _clientFactory = new(); - - private ActorSystem _central = null!; - private ActorSystem _node = null!; - private IActorRef _nodeComm = null!; - private IActorRef _centralClient = null!; - private TaskCompletionSource _centralInbox = null!; - - /// - public async ValueTask InitializeAsync() - { - _central = StartSelfSeededSystem(withReceptionist: true); - _node = StartSelfSeededSystem(withReceptionist: true); - await Task.WhenAll(AwaitSingleMemberUpAsync(_central), AwaitSingleMemberUpAsync(_node)); - - // Central: the ack landing pad, at the path the node's comm actor sends to. - _centralInbox = NewInbox(); - var centralComm = _central.ActorOf( - CaptureActor.Props(_centralInbox), MeshPaths.CentralCommunicationName); - ClusterClientReceptionist.Get(_central).RegisterService(centralComm); - - // Node: the REAL comm actor, dialling central through the REAL production client factory. - var nodeToCentral = _clientFactory.Create(_node, "central", ReceptionistOf(_central)); - _nodeComm = _node.ActorOf( - NodeCommunicationActor.Props(nodeToCentral), MeshPaths.NodeCommunicationName); - ClusterClientReceptionist.Get(_node).RegisterService(_nodeComm); - - _centralClient = _clientFactory.Create(_central, "site-a", ReceptionistOf(_node)); - } - - /// - public async ValueTask DisposeAsync() - { - await ShutdownAsync(_central); - await ShutdownAsync(_node); - } - - [Fact] - public void The_two_systems_are_separate_clusters_joined_only_by_the_client() - { - // The load-bearing precondition of every other test here. Two systems sharing a name in one - // process are one gossip accident away from being one cluster, at which point ClusterClient - // is no longer the thing under test. - AkkaCluster.Get(_central).State.Members.Count.ShouldBe(1, - "central must be its own single-member cluster, or the boundary is not being crossed"); - AkkaCluster.Get(_node).State.Members.Count.ShouldBe(1, - "the node must be its own single-member cluster, or the boundary is not being crossed"); - } - - [Fact] - public async Task Central_send_to_all_reaches_the_node_comm_actor_and_lands_on_its_event_stream() - { - var inbox = NewInbox(); - _node.EventStream.Subscribe(_node.ActorOf(CaptureActor.Props(inbox)), typeof(RestartDriver)); - - var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); - var delivered = await SendUntilDeliveredAsync( - _centralClient, - () => new ClusterClient.SendToAll(MeshPaths.NodeCommunication, command), - inbox, - Arrival); - - delivered.ShouldNotBeNull( - "SendToAll must reach the node's registered comm actor and be re-emitted node-locally; " - + "this is the whole central→node leg of the mesh transport"); - delivered.ShouldBeOfType().DriverInstanceId.ShouldBe("driver-1"); - } - - [Fact] - public async Task A_node_ack_reaches_central_through_the_node_comm_actors_own_client() - { - // Told directly rather than routed from central, so a failure here can only be the outbound - // leg. The inbound leg has its own test above. - var ack = new ApplyAck( - new DeploymentId(Guid.NewGuid()), - new NodeId("node-a"), - ApplyAckOutcome.Applied, - null, - CorrelationId.NewId()); - - var delivered = await SendUntilDeliveredAsync( - _nodeComm, () => ack, _centralInbox, Arrival); - - delivered.ShouldNotBeNull( - "the node's ApplyAck must reach central's comm actor, or every deployment times out at " - + "the apply deadline naming nodes that applied successfully"); - delivered.ShouldBeOfType().NodeId.Value.ShouldBe("node-a"); - } - - [Fact] - public async Task Falsifiability_control_a_node_without_the_receptionist_extension_receives_nothing() - { - // Without this, the two tests above cannot distinguish "the ClusterClient boundary works" - // from "something else in the process delivered the message". Same message, same client - // factory, same paths, same comm actor — and it must NOT arrive. - // - // HONEST SCOPE: this removes the receptionist extension AND, unavoidably, the RegisterService - // call — resolving ClusterClientReceptionist.Get to register would materialise the very - // extension whose absence is the control. So it falsifies "delivery happens without a - // receptionist boundary", not the extension line in isolation. Verified falsifiable: giving - // this node the extension and the registration turns the test red, which also proves the send - // below is live rather than silently misaddressed. - var deaf = StartSelfSeededSystem(withReceptionist: false); - try - { - await AwaitSingleMemberUpAsync(deaf); - - var inbox = NewInbox(); - deaf.EventStream.Subscribe(deaf.ActorOf(CaptureActor.Props(inbox)), typeof(RestartDriver)); - - // Deliberately NOT ClusterClientReceptionist.Get(deaf) — resolving the extension would - // materialise the very receptionist whose absence is the control. - var comm = deaf.ActorOf(NodeCommunicationActor.Props(null), MeshPaths.NodeCommunicationName); - comm.ShouldNotBeNull(); - - var client = _clientFactory.Create(_central, "deaf", ReceptionistOf(deaf)); - var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); - - var deadline = DateTime.UtcNow + Silence; - while (DateTime.UtcNow < deadline && !inbox.Task.IsCompleted) - { - client.Tell(new ClusterClient.SendToAll(MeshPaths.NodeCommunication, command)); - await Task.Delay(500, TestContext.Current.CancellationToken); - } - - inbox.Task.IsCompleted.ShouldBeFalse( - "delivery survived removing the receptionist extension, so the two positive tests " - + "above are not measuring the ClusterClient boundary at all"); - } - finally - { - await ShutdownAsync(deaf); - } - } - - [Fact] - public async Task One_dispatch_through_central_reaches_a_node_in_every_cluster_mesh() - { - // THE Phase 6 assertion at the transport level, driving the REAL CentralCommunicationActor. - // Two genuinely separate site meshes (site-A = _node, site-B created here), each its own - // single-member cluster with its own receptionist. Central must build ONE ClusterClient per - // cluster and fan SendToAll across them, so a single MeshCommand reaches BOTH meshes. A single - // fleet-wide client would reach only the one mesh whose receptionist answered — the exact - // failure this test is red-before-green against. - var siteB = StartSelfSeededSystem(withReceptionist: true); - try - { - await AwaitSingleMemberUpAsync(siteB); - RegisterNodeComm(siteB); - - var inboxA = NewInbox(); - _node.EventStream.Subscribe(_node.ActorOf(CaptureActor.Props(inboxA)), typeof(RestartDriver)); - var inboxB = NewInbox(); - siteB.EventStream.Subscribe(siteB.ActorOf(CaptureActor.Props(inboxB)), typeof(RestartDriver)); - - var dbFactory = SeedTwoClusterDb(SiteAddress(_node), SiteAddress(siteB)); - var actor = _central.ActorOf(CentralCommunicationActor.Props( - dbFactory, ClusterClientOptions(), new DefaultMeshClusterClientFactory(), () => null)); - - var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); - var deadline = DateTime.UtcNow + Arrival; - while (DateTime.UtcNow < deadline && !(inboxA.Task.IsCompleted && inboxB.Task.IsCompleted)) - { - actor.Tell(new MeshCommand("deployments", command)); - await Task.Delay(250, TestContext.Current.CancellationToken); - } - - inboxA.Task.IsCompleted.ShouldBeTrue( - "site-A's mesh must receive the dispatch through its own per-cluster ClusterClient"); - inboxB.Task.IsCompleted.ShouldBeTrue( - "site-B's mesh must receive the SAME dispatch through its own per-cluster ClusterClient; " - + "a single fleet-wide client would have reached only one of the two meshes"); - } - finally - { - await ShutdownAsync(siteB); - } - } - - [Fact] - public async Task A_client_built_for_one_cluster_does_not_deliver_into_another_clusters_mesh() - { - // The partition proof for the fan-out: a client dialling ONLY site-A's contacts must land in - // site-A's mesh and NEVER cross into site-B's. This is why the per-cluster fan-out is correct - // rather than accidentally N x broadcasting — each client is scoped to its own mesh. - var siteB = StartSelfSeededSystem(withReceptionist: true); - try - { - await AwaitSingleMemberUpAsync(siteB); - RegisterNodeComm(siteB); - - var inboxA = NewInbox(); - _node.EventStream.Subscribe(_node.ActorOf(CaptureActor.Props(inboxA)), typeof(RestartDriver)); - var inboxB = NewInbox(); - siteB.EventStream.Subscribe(siteB.ActorOf(CaptureActor.Props(inboxB)), typeof(RestartDriver)); - - var clientA = _clientFactory.Create(_central, "site-a", ReceptionistOf(_node)); - var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); - - // Keep sending for the full Silence window so any erroneous cross-delivery into site-B has - // every chance to arrive — a pass means the boundary is shut, not that the clock ran out. - var deadline = DateTime.UtcNow + Silence; - while (DateTime.UtcNow < deadline) - { - clientA.Tell(new ClusterClient.SendToAll(MeshPaths.NodeCommunication, command)); - await Task.Delay(250, TestContext.Current.CancellationToken); - } - - inboxA.Task.IsCompleted.ShouldBeTrue("site-A's client must deliver into site-A's mesh"); - inboxB.Task.IsCompleted.ShouldBeFalse( - "a client built for site-A's contacts must NOT deliver into site-B's mesh — the " - + "per-cluster fan-out must stay partitioned, one client per mesh"); - } - finally - { - await ShutdownAsync(siteB); - } - } - - /// Registers a real node-comm actor with a system's receptionist so central can reach it. - /// The site system to register into. - private static void RegisterNodeComm(ActorSystem system) - { - var comm = system.ActorOf(NodeCommunicationActor.Props(null), MeshPaths.NodeCommunicationName); - ClusterClientReceptionist.Get(system).RegisterService(comm); - } - - private static (string Host, int Port) SiteAddress(ActorSystem system) - { - var addr = AkkaCluster.Get(system).SelfAddress; - return (addr.Host!, addr.Port!.Value); - } - - private static MeshTransportOptions ClusterClientOptions() => new() - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["akka.tcp://otopcua@127.0.0.1:4053"], - ContactRefreshSeconds = 1, - }; - - private static IDbContextFactory SeedTwoClusterDb( - (string Host, int Port) siteA, (string Host, int Port) siteB) - { - var factory = new InMemoryConfigDbFactory(Guid.NewGuid().ToString("N")); - using var db = factory.CreateDbContext(); - db.ClusterNodes.Add(NodeRow("SITE-A", "node-a", siteA)); - db.ClusterNodes.Add(NodeRow("SITE-B", "node-b", siteB)); - db.SaveChanges(); - return factory; - } - - private static ClusterNode NodeRow(string clusterId, string nodeId, (string Host, int Port) addr) => new() - { - NodeId = nodeId, - ClusterId = clusterId, - Host = addr.Host, - AkkaPort = addr.Port, - ApplicationUri = $"urn:{nodeId}", - Enabled = true, - MaintenanceMode = false, - CreatedBy = "test", - }; - - private sealed class InMemoryConfigDbFactory(string dbName) - : IDbContextFactory - { - public OtOpcUaConfigDbContext CreateDbContext() - { - var opts = new DbContextOptionsBuilder() - .UseInMemoryDatabase(dbName) - .Options; - return new OtOpcUaConfigDbContext(opts); - } - } - - /// - /// Starts a single-member cluster on a dynamic port, optionally without the receptionist. - /// - /// - /// drops the receptionist provider from akka.extensions, - /// leaving DistributedPubSub in place so only the one variable changes. - /// - /// The started system. - private static ActorSystem StartSelfSeededSystem(bool withReceptionist) - { - var extensions = withReceptionist - ? string.Empty - : "akka.extensions = [\"Akka.Cluster.Tools.PublishSubscribe.DistributedPubSubExtensionProvider, " - + "Akka.Cluster.Tools\"]\n"; - - // Port 0 + a post-start self-Join, rather than a seed-nodes list: the address is not known - // until the transport binds, and hard-coding ports makes the test collide with itself under - // parallel execution. - var config = ConfigurationFactory.ParseString( - "akka.remote.dot-netty.tcp.hostname = \"127.0.0.1\"\n" - + "akka.remote.dot-netty.tcp.public-hostname = \"127.0.0.1\"\n" - + "akka.remote.dot-netty.tcp.port = 0\n" - + "akka.cluster.seed-nodes = []\n" - + "akka.loglevel = \"WARNING\"\n" - + extensions) - .WithFallback(ZB.MOM.WW.OtOpcUa.Cluster.HoconLoader.LoadBaseConfig()); - - var system = ActorSystem.Create(SharedSystemName, config); - var cluster = AkkaCluster.Get(system); - cluster.Join(cluster.SelfAddress); - return system; - } - - private static async Task AwaitSingleMemberUpAsync(ActorSystem system) - { - var cluster = AkkaCluster.Get(system); - var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); - while (DateTime.UtcNow < deadline) - { - if (cluster.State.Members.Any(m => m.Status == MemberStatus.Up)) return; - await Task.Delay(100, TestContext.Current.CancellationToken); - } - - cluster.State.Members.Any(m => m.Status == MemberStatus.Up).ShouldBeTrue( - $"{cluster.SelfAddress} never reached Up as its own seed"); - } - - /// - /// Re-sends until the inbox completes or the deadline passes. - /// - /// - /// A ClusterClient establishes contact asynchronously and buffer-size = 0 means anything - /// sent before that lands is dropped on the floor — by design. A single send would therefore - /// race the handshake and fail intermittently for a reason that is not a defect. Re-sending - /// weakens nothing: the control test re-sends on exactly the same schedule and must still - /// receive nothing. - /// - /// Actor to send to. - /// Builds the message for each attempt. - /// Completion source the receiving capture actor resolves. - /// How long to keep trying. - /// The delivered message, or if none arrived. - private static async Task SendUntilDeliveredAsync( - IActorRef target, - Func message, - TaskCompletionSource inbox, - TimeSpan timeout) - { - var deadline = DateTime.UtcNow + timeout; - while (DateTime.UtcNow < deadline) - { - if (inbox.Task.IsCompleted) return await inbox.Task; - target.Tell(message()); - await Task.Delay(250, TestContext.Current.CancellationToken); - } - - return inbox.Task.IsCompleted ? await inbox.Task : null; - } - - private static TaskCompletionSource NewInbox() => - new(TaskCreationOptions.RunContinuationsAsynchronously); - - private static ImmutableHashSet ReceptionistOf(ActorSystem system) => - ImmutableHashSet.Create( - new RootActorPath(AkkaCluster.Get(system).SelfAddress) / "system" / "receptionist"); - - private static async Task ShutdownAsync(ActorSystem? system) - { - if (system is null) return; - await system.Terminate().WaitAsync(TimeSpan.FromSeconds(15)); - } - - /// Completes an inbox with the first message it receives. - private sealed class CaptureActor : ReceiveActor - { - public static Props Props(TaskCompletionSource sink) => - Akka.Actor.Props.Create(() => new CaptureActor(sink)); - - public CaptureActor(TaskCompletionSource sink) => ReceiveAny(m => sink.TrySetResult(m)); - } -} From ab5b958d47fe340b1d1007cccc5525cdea6f0d00 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:38:49 -0400 Subject: [PATCH 02/11] =?UTF-8?q?refactor(mesh):=20step=202=20=E2=80=94=20?= =?UTF-8?q?delete=20the=20node-side=20ClusterClient=20and=20its=20centralC?= =?UTF-8?q?lient=20parameter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WithOtOpcUaRuntimeActors no longer builds a ClusterClient from MeshTransport:CentralContactPoints, and NodeCommunicationActor.Props takes only the gRPC IApplyAckSender. The ackRouter gate narrows from `useClusterClient || useGrpc` to `useGrpc`, and the CentralClusterClientName constant is gone. NodeCommunicationActor's no-transport warning now names MeshTransport:CentralGrpcEndpoints and MeshTransport:Mode only — the old text pointed an operator at a key that no longer exists. Ripples in this step: the two ClusterClient-shaped facts in NodeCommunicationActorTests (Send-not-SendToAll, and sender-wins-over-a-client) are deleted — the first has no subject left, the second guarded a both-transports-present state that is now unconstructible. The Runtime ServiceCollectionExtensions fact keeps its ackRouter pin and drops its "no ClusterClient actor exists" half for the same reason. --- .../Communication/NodeCommunicationActor.cs | 66 +++++--------- .../ServiceCollectionExtensions.cs | 48 +++-------- .../EventStreamCommandDeliveryTests.cs | 4 +- .../NodeCommunicationActorTests.cs | 86 ++++--------------- .../ServiceCollectionExtensionsTests.cs | 26 +++--- 5 files changed, 63 insertions(+), 167 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs index 88702dc8..e54da301 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs @@ -1,5 +1,4 @@ using Akka.Actor; -using Akka.Cluster.Tools.Client; using Akka.Event; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy; @@ -9,17 +8,18 @@ using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// -/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2), registered -/// with the ClusterClientReceptionist at . +/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2), living at +/// — the path this node's NodeCommandService +/// gRPC ingress hands every decoded command to. /// /// /// -/// Registered per node, NOT as a cluster singleton — central's ClusterClient rotates -/// across contact points and must find a live comm actor at whichever node answers. +/// Registered per node, NOT as a cluster singleton — central dials each node's own +/// command endpoint, so every node needs its own live comm actor. /// /// /// Inbound commands are re-emitted on the node's , not on their -/// DistributedPubSub topic. DPS is mesh-wide, and central SendToAlls to every +/// DistributedPubSub topic. DPS is mesh-wide, and central delivers one copy to every /// node's comm actor — so a DPS re-publish would deliver N copies of every command to every /// subscriber, where N is the node count. The EventStream is node-local by construction, /// which is exactly the needed semantics. It also avoids a registration handshake with @@ -36,31 +36,23 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// public sealed class NodeCommunicationActor : ReceiveActor { - private readonly IActorRef? _centralClient; private readonly IApplyAckSender? _ackSender; private readonly ILoggingAdapter _log = Context.GetLogger(); /// Creates the props for this actor. - /// - /// ClusterClient dialling central's receptionist, or under - /// MeshTransport:Mode = Dps, where acks still go out over DistributedPubSub from - /// DriverHostActor and this actor never sees them — and under - /// Mode = Grpc, where no ClusterClient is created at all. - /// /// /// The gRPC ack transport under MeshTransport:Mode = Grpc, or - /// under Dps and ClusterClient. + /// under Dps, where acks still go out over DistributedPubSub from + /// DriverHostActor and this actor never sees them. /// /// The props. - public static Props Props(IActorRef? centralClient, IApplyAckSender? ackSender = null) => - Akka.Actor.Props.Create(() => new NodeCommunicationActor(centralClient, ackSender)); + public static Props Props(IApplyAckSender? ackSender = null) => + Akka.Actor.Props.Create(() => new NodeCommunicationActor(ackSender)); /// Initializes a new instance of the class. - /// ClusterClient dialling central, or . /// The gRPC ack transport, or . - public NodeCommunicationActor(IActorRef? centralClient, IApplyAckSender? ackSender = null) + public NodeCommunicationActor(IApplyAckSender? ackSender = null) { - _centralClient = centralClient; _ackSender = ackSender; // Inbound from central. Each type is listed explicitly rather than caught by a ReceiveAny: @@ -85,14 +77,10 @@ public sealed class NodeCommunicationActor : ReceiveActor { if (_ackSender is not null) { - // Fire-and-forget by construction, exactly as the ClusterClient Tell below is. The sender - // never throws and never retries (see IApplyAckSender), so there is nothing to await and - // nothing to pipe back: awaiting would stall this mailbox behind a network call to central, - // and an Ask would invent a request/response the one-way contract does not have. - // - // Checked BEFORE the ClusterClient. The registration arm builds exactly one transport, so - // both are never set — but were they, posting over both would deliver two acks for one - // apply and duplicate central's work for no benefit. + // Fire-and-forget by construction. The sender never throws and never retries (see + // IApplyAckSender), so there is nothing to await and nothing to pipe back: awaiting would + // stall this mailbox behind a network call to central, and an Ask would invent a + // request/response the one-way contract does not have. try { _ = _ackSender.SendAsync(ack); @@ -115,22 +103,12 @@ public sealed class NodeCommunicationActor : ReceiveActor return; } - if (_centralClient is null) - { - _log.Warning( - "No central ack transport — dropping ApplyAck for {DeploymentId}. The ack is not " - + "buffered and will not be retried, so the deployment will time out at central " - + "naming this node even though it applied successfully. Check " - + "MeshTransport:CentralContactPoints (ClusterClient mode) or " - + "MeshTransport:CentralGrpcEndpoints (Grpc mode)", - ack.DeploymentId); - return; - } - - // Send, not SendToAll: the deploy coordinator is a single singleton, and the central comm - // actor forwards to its proxy. Its NodeId-keyed upsert absorbs a duplicate idempotently, - // so single-target is defence in depth and a bound on wasted sends — mirroring the - // corrected Grpc-branch rationale above. - _centralClient.Tell(new ClusterClient.Send(MeshPaths.CentralCommunication, ack)); + _log.Warning( + "No central ack transport — dropping ApplyAck for {DeploymentId}. The ack is not " + + "buffered and will not be retried, so the deployment will time out at central " + + "naming this node even though it applied successfully. Check " + + "MeshTransport:CentralGrpcEndpoints, and that MeshTransport:Mode is Grpc on a node " + + "whose central lives in another mesh", + ack.DeploymentId); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index dd7c05ff..01b1a23b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -47,7 +47,6 @@ public static class ServiceCollectionExtensions public const string OpcUaPublishActorName = "opcua-publish"; public const string PeerProbeSupervisorName = "peer-probe-supervisor"; public const string ContinuousHistorizationRecorderActorName = "continuous-historization-recorder"; - public const string CentralClusterClientName = "central-cluster-client"; /// /// Registers shared runtime services. Currently binds @@ -101,8 +100,8 @@ public static class ServiceCollectionExtensions // with no gate at all, and both nodes would write every sample twice with nothing to say so. services.TryAddSingleton(sp => sp.GetRequiredService()); - // ClusterClient→gRPC migration Task 1B.2: the node→central ack transport under - // MeshTransport:Mode = Grpc. Registered unconditionally but resolved only by the Grpc arm of + // The node→central ack transport under MeshTransport:Mode = Grpc (mesh gRPC command plane, + // Task 1B.2). Registered unconditionally but resolved only by the Grpc arm of // WithOtOpcUaRuntimeActors — the mode is not knowable here (this runs before configuration is // read into options), and a singleton nobody resolves is never constructed, so a Dps node // opens no channel. @@ -404,35 +403,15 @@ public static class ServiceCollectionExtensions registry.Register(mux); // Per-cluster mesh Phase 2: the node end of the central↔node command boundary. Spawned - // BEFORE DriverHostActor because it is the host's ackRouter under ClusterClient mode. - // It has no dependency of its own on the host — inbound commands travel via the node's + // BEFORE DriverHostActor because it is the host's ackRouter under Grpc mode. It has no + // dependency of its own on the host — inbound commands travel via the node's // EventStream — so this ordering costs nothing. var meshOptions = resolver.GetService>().Value; - var useClusterClient = string.Equals( - meshOptions.Mode, MeshTransportOptions.ModeClusterClient, StringComparison.OrdinalIgnoreCase); var useGrpc = string.Equals( meshOptions.Mode, MeshTransportOptions.ModeGrpc, StringComparison.OrdinalIgnoreCase); - IActorRef? centralClient = null; - if (useClusterClient) - { - // Contacts come from appsettings, not the database — the deliberate asymmetry: - // central's node set changes as operators add and retire nodes, but central's own - // address is part of the deployment. MeshTransportOptionsValidator has already - // rejected an empty or malformed list at boot, so this cannot silently produce a - // client with no contacts. - var contacts = meshOptions.CentralContactPoints - .Select(cp => ActorPath.Parse($"{cp}/system/receptionist")) - .ToImmutableHashSet(); - centralClient = system.ActorOf( - ClusterClient.Props(ClusterClientSettings.Create(system).WithInitialContacts(contacts)), - CentralClusterClientName); - } - - // ClusterClient→gRPC migration Task 1B.2. Deliberately EXCLUSIVE with the branch above — - // under Grpc no ClusterClient is created at all, which is half the point of the migration - // (it is the cross-WAN Akka association this program removes). The two are never both - // handed to the actor, and it prefers the sender if they ever were. + // The node→central ack transport. Built ONLY under Grpc: under Dps the ack goes out on + // the deployment-acks topic from DriverHostActor and never reaches this actor at all. IApplyAckSender? ackSender = null; if (useGrpc) { @@ -451,7 +430,7 @@ public static class ServiceCollectionExtensions } var nodeComm = system.ActorOf( - NodeCommunicationActor.Props(centralClient, ackSender), + NodeCommunicationActor.Props(ackSender), MeshPaths.NodeCommunicationName); // Registered with the receptionist in ALL modes: under Dps and Grpc nothing dials it, and @@ -567,12 +546,11 @@ public static class ServiceCollectionExtensions : null; var driverHost = system.ActorOf( - // ackRouter: under ClusterClient AND Grpc the node comm actor relays ApplyAcks across - // the boundary (which transport it then uses is its own business — the ClusterClient - // ref or the gRPC sender it was built with); under Dps it stays null and the host - // publishes on the deployment-acks topic exactly as before. Grpc MUST be included - // here: routed to DPS instead, a Grpc node's ack would never reach the sender at all - // and every deployment to it would time out while it applied perfectly. + // ackRouter: under Grpc the node comm actor relays ApplyAcks across the mesh + // boundary through the gRPC sender it was built with; under Dps it stays null and + // the host publishes on the deployment-acks topic exactly as before. Routed to DPS + // instead, a Grpc node's ack would never reach the sender at all and every + // deployment to it would time out while it applied perfectly. // dbFactory is nullable (Phase 4): a driver-only node passes null here — ConfigDb is // gated on hasAdmin and driver-only nodes are forced to FetchAndCache. DriverHostActor // guards every ConfigDb-touching path on it (UpsertNodeDeploymentState no-ops; central @@ -580,7 +558,7 @@ public static class ServiceCollectionExtensions // alarmStateStore (there is no ConfigDb-backed EF fallback; the host skips the alarm // host when no store was wired). DriverHostActor.Props(dbFactory, roleInfo.LocalNode, - ackRouter: useClusterClient || useGrpc ? nodeComm : null, + ackRouter: useGrpc ? nodeComm : null, driverFactory: driverFactory, localRoles: roleInfo.LocalRoles, dependencyMux: mux, opcUaPublishActor: publishActor, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/EventStreamCommandDeliveryTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/EventStreamCommandDeliveryTests.cs index c9c66be1..aaa06b3c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/EventStreamCommandDeliveryTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/EventStreamCommandDeliveryTests.cs @@ -50,7 +50,7 @@ public class EventStreamCommandDeliveryTests : RuntimeActorTestBase var driverHost = Sys.ActorOf( DriverHostActor.Props(NewInMemoryDbFactory(), new NodeId("127.0.0.1:4053"), ackRouter: ackRouter.Ref), "driver-host"); - var comm = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null), "node-communication"); + var comm = Sys.ActorOf(NodeCommunicationActor.Props(), "node-communication"); // Warm-up: a direct Tell whose ack proves PreStart has completed, and with it the // EventStream subscription. Without this the relay below races the subscription. @@ -141,7 +141,7 @@ akka { ScriptedAlarmHostActor.Props( CreateTestProbe().Ref, CreateTestProbe().Ref, upstream, engine, new NodeId("node-A")), "scripted-alarm-host"); - var comm = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null), "node-communication"); + var comm = Sys.ActorOf(NodeCommunicationActor.Props(), "node-communication"); var cmd = new AlarmCommand("alarm-unowned", "Acknowledge", "alice", "c", null); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/NodeCommunicationActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/NodeCommunicationActorTests.cs index 473d2d02..e1bb1f9d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/NodeCommunicationActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/NodeCommunicationActorTests.cs @@ -1,5 +1,4 @@ using Akka.Actor; -using Akka.Cluster.Tools.Client; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; @@ -24,7 +23,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase { var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(DispatchDeployment)); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); var dispatch = NewDispatch(); actor.Tell(dispatch); @@ -40,7 +39,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase // subscriber. One publish per inbound command is the whole point. var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(DispatchDeployment)); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); actor.Tell(NewDispatch()); @@ -53,7 +52,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase { var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(RestartDriver)); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); var msg = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); actor.Tell(msg); @@ -66,7 +65,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase { var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(ReconnectDriver)); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); var msg = new ReconnectDriver("C1", "driver-1", "alice", Guid.NewGuid()); actor.Tell(msg); @@ -82,7 +81,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase // leg possible without a registration handshake. var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(AlarmCommand)); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); var msg = new AlarmCommand("alarm-1", "Acknowledge", "alice", "comment", null); actor.Tell(msg); @@ -91,32 +90,11 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase } [Fact] - public void ApplyAck_goes_to_central_as_a_Send_not_a_SendToAll() + public void An_ack_with_no_ack_sender_is_dropped_with_a_warning() { - // Send, not SendToAll: the deploy coordinator is one singleton behind the central comm - // actor's proxy. The coordinator's NodeId-keyed upsert absorbs a duplicate idempotently, - // so the single-target rule is defence in depth and a bound on wasted work, not a - // correctness requirement. - var client = CreateTestProbe(); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(client.Ref)); - - var ack = new ApplyAck( - new DeploymentId(Guid.NewGuid()), - new NodeId("host-a:4053"), - ApplyAckOutcome.Applied, - null, - new CorrelationId(Guid.NewGuid())); - actor.Tell(ack); - - var sent = client.ExpectMsg(TimeSpan.FromSeconds(5)); - sent.Path.ShouldBe(MeshPaths.CentralCommunication); - sent.Message.ShouldBe(ack); - } - - [Fact] - public void An_ack_with_no_central_client_is_dropped_with_a_warning() - { - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null)); + // Dps: DriverHostActor publishes the ack on the deployment-acks topic and never routes it + // here, so an ack arriving with no sender wired is a mis-wiring and must say so. + var actor = Sys.ActorOf(NodeCommunicationActor.Props()); var ack = new ApplyAck( new DeploymentId(Guid.NewGuid()), @@ -129,7 +107,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase TimeSpan.FromSeconds(5), () => actor.Tell(ack)); } - // ---- Mode=Grpc (ClusterClient→gRPC migration Task 1B.2) ------------------------------------- + // ---- Mode=Grpc (the cross-mesh ack leg) ----------------------------------------------------- private static ApplyAck NewAck() => new( new DeploymentId(Guid.NewGuid()), @@ -141,13 +119,12 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase [Fact] public void Under_Grpc_the_ack_goes_to_the_sender_exactly_once() { - // The gRPC counterpart of the ClusterClient.Send fact above, and it carries the same - // single-count obligation: one post per node per dispatch, because both central endpoints - // route to the SAME coordinator singleton proxy. Where the ack goes among the endpoints is the - // sender's business (GrpcApplyAckSenderTests); what this pins is that the actor hands it over - // once and does not also fan it out itself. + // One post per node per dispatch, because both central endpoints route to the SAME + // coordinator singleton proxy. Where the ack goes among the endpoints is the sender's + // business (GrpcApplyAckSenderTests); what this pins is that the actor hands it over once + // and does not also fan it out itself. var sender = new RecordingAckSender(); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null, ackSender: sender)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props(sender)); var ack = NewAck(); actor.Tell(ack); @@ -158,37 +135,6 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase sender.Sent.Count.ShouldBe(1); } - [Fact] - public void A_sender_wins_over_a_cluster_client_and_the_ack_is_not_sent_twice() - { - // Defence in depth. The registration arm builds exactly one transport — under Grpc no - // ClusterClient is created at all — and if both were ever present, posting over both would - // deliver two acks for one apply. The coordinator would absorb the duplicate (NodeId-keyed - // upsert), so this pins tidiness and bounded work, not seal correctness. - var client = CreateTestProbe(); - var sender = new RecordingAckSender(); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(client.Ref, sender)); - - actor.Tell(NewAck()); - - AwaitAssert(() => sender.Sent.Count.ShouldBe(1), TimeSpan.FromSeconds(5)); - client.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); - } - - [Fact] - public void The_ClusterClient_branch_never_touches_the_sender() - { - // Null-safe by construction: under Dps and ClusterClient the sender is not built at all, so the - // ApplyAck path must not depend on one existing. - var client = CreateTestProbe(); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(client.Ref)); - - var ack = NewAck(); - actor.Tell(ack); - - client.ExpectMsg(TimeSpan.FromSeconds(5)).Message.ShouldBe(ack); - } - [Fact] public void Inbound_commands_are_unaffected_by_the_presence_of_a_sender() { @@ -197,7 +143,7 @@ public class NodeCommunicationActorTests : RuntimeActorTestBase var subscriber = CreateTestProbe(); Sys.EventStream.Subscribe(subscriber.Ref, typeof(DispatchDeployment)); var sender = new RecordingAckSender(); - var actor = Sys.ActorOf(NodeCommunicationActor.Props(centralClient: null, ackSender: sender)); + var actor = Sys.ActorOf(NodeCommunicationActor.Props(sender)); var dispatch = NewDispatch(); actor.Tell(dispatch); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs index f649d30d..0b8965c7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs @@ -127,23 +127,22 @@ public sealed class ServiceCollectionExtensionsTests /// /// Under MeshTransport:Mode = Grpc an ApplyAck produced by the driver host reaches the - /// registered , and no ClusterClient actor exists - /// (ClusterClient→gRPC migration Task 1B.2). + /// registered . /// /// /// This is the fact that pins the ackRouter gate in WithOtOpcUaRuntimeActors - /// (useClusterClient || useGrpc ? nodeComm : null). Narrowed back to ClusterClient-only - /// that gate still compiles and every unit test still passes, while every ack on a Grpc node - /// goes out on the deployment-acks DPS topic instead — where, in the split-mesh topology - /// Grpc exists to serve, central is not subscribed. The whole transport would be dead and - /// nothing below this level of the stack could tell. + /// (useGrpc ? nodeComm : null). Drop the useGrpc arm and that gate still compiles + /// and every unit test still passes, while every ack on a Grpc node goes out on the + /// deployment-acks DPS topic instead — where, in the split-mesh topology Grpc exists to + /// serve, central is not subscribed. The whole transport would be dead and nothing below this + /// level of the stack could tell. /// /// The dispatch names a deployment that does not exist in the in-memory ConfigDb, so the apply /// fails and the host emits a Failed ack — deterministic, synchronous, and it exercises the /// real SendAck path rather than poking the router directly. /// [Fact] - public async Task Under_Grpc_a_driver_host_ack_reaches_the_ack_sender_and_no_ClusterClient_exists() + public async Task Under_Grpc_a_driver_host_ack_reaches_the_ack_sender() { var sender = new RecordingAckSender(); using var host = BuildRuntimeActorHost(services => @@ -160,7 +159,6 @@ public sealed class ServiceCollectionExtensionsTests await host.StartAsync(); try { - var system = host.Services.GetRequiredService(); var driverHost = host.Services.GetRequiredService>(); driverHost.ActorRef.Tell(new DispatchDeployment( @@ -168,18 +166,14 @@ public sealed class ServiceCollectionExtensionsTests new RevisionHash("rev-that-does-not-exist"), new CorrelationId(Guid.NewGuid()))); - // The ack travelled DriverHostActor -> NodeCommunicationActor -> IApplyAckSender. Under a - // ClusterClient-only gate it would have gone to DPS and this never completes. + // The ack travelled DriverHostActor -> NodeCommunicationActor -> IApplyAckSender. With the + // useGrpc arm dropped from the ackRouter gate it would have gone to DPS instead and this + // never completes. var deadline = DateTime.UtcNow.AddSeconds(15); while (sender.Sent.Count == 0 && DateTime.UtcNow < deadline) await Task.Delay(50); sender.Sent.Count.ShouldBe(1); - - // And the Akka cross-mesh association is genuinely gone, not merely unused. - await Should.ThrowAsync(() => system - .ActorSelection($"/user/{ServiceCollectionExtensions.CentralClusterClientName}") - .ResolveOne(TimeSpan.FromMilliseconds(500))); } finally { From 5d7ecdf73744109aed10c6b2ad485e42f2eb9f18 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:41:25 -0400 Subject: [PATCH 03/11] =?UTF-8?q?refactor(mesh):=20step=203=20=E2=80=94=20?= =?UTF-8?q?delete=20both=20ClusterClientReceptionist=20registrations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Central's CentralCommunicationActor and a node's NodeCommunicationActor no longer register with a receptionist; the ActorRegistry entry each already carried is now the whole reachability story, and it is what CentralCommandGrpcService / NodeCommandGrpcService resolve to hand an inbound message over. Both `using Akka.Cluster.Tools.Client` directives are gone with them. --- .../ServiceCollectionExtensions.cs | 9 +++------ .../ServiceCollectionExtensions.cs | 9 +++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index 472f753e..5a42f356 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -1,6 +1,5 @@ using Akka.Actor; using Akka.Cluster.Hosting; -using Akka.Cluster.Tools.Client; using Akka.Hosting; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Configuration; @@ -94,11 +93,9 @@ public static class ServiceCollectionExtensions () => serviceProvider.GetService()), MeshPaths.CentralCommunicationName); - // Registered unconditionally, in both transport modes. Under Dps nothing dials it, and - // an idle registration costs nothing — whereas registering only under ClusterClient - // would mean flipping the flag on a running fleet requires a central restart before any - // node can ack, turning a config change back into a deployment. - ClusterClientReceptionist.Get(system).RegisterService(actor); + // The registry entry is the whole reachability story: CentralCommandGrpcService resolves + // this key to hand an inbound ack over. Nothing registers with a receptionist any more — + // there is no Akka association between central and a site mesh to register into. registry.Register(actor); }); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index 01b1a23b..3ce81372 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -1,6 +1,4 @@ -using System.Collections.Immutable; using Akka.Actor; -using Akka.Cluster.Tools.Client; using Akka.Event; using Akka.Hosting; using Microsoft.EntityFrameworkCore; @@ -433,10 +431,9 @@ public static class ServiceCollectionExtensions NodeCommunicationActor.Props(ackSender), MeshPaths.NodeCommunicationName); - // Registered with the receptionist in ALL modes: under Dps and Grpc nothing dials it, and - // an idle registration costs nothing — whereas registering only under ClusterClient would - // make flipping the flag require a node restart before central could reach it. - ClusterClientReceptionist.Get(system).RegisterService(nodeComm); + // The registry entry is the whole reachability story: NodeCommandGrpcService resolves + // this key to hand an inbound command over. Nothing registers with a receptionist any + // more — there is no Akka association from central into this mesh to register into. registry.Register(nodeComm); // Continuous-historization recorder — gated on ContinuousHistorization:Enabled AND the From d67a7e5d0b777199aad7a946cbdb0acc6aef5f09 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:42:39 -0400 Subject: [PATCH 04/11] =?UTF-8?q?refactor(mesh):=20step=204=20=E2=80=94=20?= =?UTF-8?q?drop=20the=20receptionist=20extension=20and=20akka.cluster.clie?= =?UTF-8?q?nt=20from=20akka.conf?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit akka.extensions loses ClusterClientReceptionistExtensionProvider and the whole akka.cluster.client block (buffer-size / reconnect-timeout / receptionist.role) goes with it. The DistributedPubSub extension and the maximum-frame-size / log-frame-size-exceeding canary both stay — DPS is the pair-local transport and the frame canary guards every remoting message, neither of which the cross-mesh deletion touches. MeshTransportHoconTests drops its three ClusterClientSettings/ClusterReceptionistSettings asserts and keeps the frame-size pin and the DPS-extension pin, renamed since it no longer has a receptionist to survive alongside. --- .../Resources/akka.conf | 35 ++---------- .../MeshTransportHoconTests.cs | 55 ++----------------- 2 files changed, 10 insertions(+), 80 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf index b7e7a8a7..56648f0b 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/Resources/akka.conf @@ -13,14 +13,12 @@ akka { # since the Serilog logger can be slow to initialize at startup. logger-startup-timeout = 30s + # DistributedPubSub is PAIR-LOCAL and stays: it carries MeshTransport:Mode = Dps (the + # single-mesh default), the node-local alarm-command publishes and the node<->node + # redundancy-state probe leg. What is gone is the cross-mesh command transport, which is now + # gRPC and needs no Akka extension at all. extensions = [ "Akka.Cluster.Tools.PublishSubscribe.DistributedPubSubExtensionProvider, Akka.Cluster.Tools" - - # Per-cluster mesh Phase 2 — the central<->node command boundary. NOT auto-started: without - # this entry the receptionist materialises only if some code path happens to call - # ClusterClientReceptionist.Get(system) first, which makes "is the boundary listening?" - # depend on startup ordering rather than on configuration. - "Akka.Cluster.Tools.Client.ClusterClientReceptionistExtensionProvider, Akka.Cluster.Tools" ] actor { @@ -94,31 +92,6 @@ akka { down-removal-margin = 15s run-coordinated-shutdown-when-down = on - # CLUSTERCLIENT — the central<->node command boundary (per-cluster mesh Phase 2). - client { - # DELIBERATE, and NOT what the sister project runs: they never wrote this section, so - # they inherit buffer-size = 1000. Zero means "if the receptionist is unknown, drop the - # message now". Buffering would replay a DispatchDeployment on reconnect and apply a - # deployment ConfigPublishCoordinator had already sealed as TimedOut — the node would - # then be running a configuration the database says failed. Fail-fast keeps the DB the - # single record of truth, and reproduces today's DPS behaviour exactly: a node that is - # down misses the publish and self-heals from the DB on restart. - buffer-size = 0 - - # Retry forever rather than self-stopping the client. Restates Akka's default; stated - # explicitly because the behaviour is load-bearing — a central node down for an hour - # must not require a driver-node restart to become reachable again. - reconnect-timeout = off - - receptionist { - # Empty = every member hosts a receptionist. This is what makes the comm actors - # "registered per node, not as a singleton" work: contact rotation reaches whichever - # node answers. Restates Akka's default; do NOT scope this to a role, or the - # boundary becomes reachable through one node only and rotation silently fails. - role = "" - } - } - singleton { singleton-name = "singleton" } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportHoconTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportHoconTests.cs index 1960d0c3..9bdb1b0d 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportHoconTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportHoconTests.cs @@ -1,5 +1,4 @@ using Akka.Actor; -using Akka.Cluster.Tools.Client; using Akka.Configuration; using Shouldly; using Xunit; @@ -7,8 +6,8 @@ using Xunit; namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; /// -/// Asserts the ClusterClient / receptionist / frame-size settings that are actually in -/// force on a real built from the shipped base config. +/// Asserts the remoting frame-size canary and the DistributedPubSub extension that are actually +/// in force on a real built from the shipped base config. /// /// /// Asserting the akka.conf text instead would pass while a competing HOCON fragment @@ -29,38 +28,6 @@ public class MeshTransportHoconTests : IDisposable _sys = ActorSystem.Create("mesh-hocon-probe", config); } - [Fact] - public void Cluster_client_buffering_is_disabled() - { - // Asserted through ClusterClientSettings — the object the client is actually built from — - // NOT through Config.GetInt. GetInt returns 0 for a MISSING key, so a config assertion here - // passes identically whether the setting is explicitly 0 or absent with Akka's default of - // 1000 in force. It was green before this feature existed; that is the whole defect class. - // - // buffer-size = 0 means "drop now if the receptionist is unknown". Buffering would replay a - // DispatchDeployment on reconnect and apply a deployment the coordinator already sealed as - // TimedOut, leaving the node running a configuration the database says failed. This is a - // behaviour decision, not a tuning knob. - ClusterClientSettings.Create(_sys).BufferSize.ShouldBe(0); - } - - [Fact] - public void Cluster_client_retries_forever_rather_than_self_stopping() - { - // reconnect-timeout = off surfaces as a null TimeSpan on the settings object. A central node - // down for an hour must not require a driver-node restart to become reachable again. - ClusterClientSettings.Create(_sys).ReconnectTimeout.ShouldBeNull(); - } - - [Fact] - public void Receptionist_serves_every_member_regardless_of_role() - { - // An empty role is what makes "registered per node, not as a singleton" work: every member - // hosts a receptionist, so ClusterClient contact rotation reaches whichever node answers. - // Scoping this to a role would make the boundary reachable through one node only. - ClusterReceptionistSettings.Create(_sys).Role.ShouldBeNull(); - } - [Fact] public void Oversized_frames_are_logged_rather_than_dropped_silently() { @@ -73,21 +40,11 @@ public class MeshTransportHoconTests : IDisposable } [Fact] - public void Receptionist_extension_is_registered_at_startup() + public void Distributed_pub_sub_extension_is_registered_at_startup() { - // Not auto-started. Without this entry the receptionist materialises only if some code path - // happens to call ClusterClientReceptionist.Get(system) first, which makes "is the boundary - // listening?" depend on startup ordering rather than on configuration. - _sys.Settings.Config.GetStringList("akka.extensions") - .ShouldContain(s => s.Contains("ClusterClientReceptionistExtensionProvider")); - } - - [Fact] - public void Distributed_pub_sub_extension_survives_alongside_the_receptionist() - { - // The dark switch keeps both transports live for one phase, and DPS additionally carries - // traffic Phase 2 does not touch at all — node-local alarm-command publishes and the - // node<->node redundancy-state probe leg. Losing it here would break far more than deploys. + // DPS is PAIR-LOCAL and survives the cross-mesh transport's deletion: it carries + // MeshTransport:Mode = Dps (the single-mesh default), the node-local alarm-command publishes + // and the node<->node redundancy-state probe leg. Losing it would break far more than deploys. _sys.Settings.Config.GetStringList("akka.extensions") .ShouldContain(s => s.Contains("DistributedPubSubExtensionProvider")); } From 85ca86e9557330fb7ded36d2ba8b9c7ec7825fb0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:45:21 -0400 Subject: [PATCH 05/11] =?UTF-8?q?refactor(mesh):=20step=205=20=E2=80=94=20?= =?UTF-8?q?delete=20MeshTransport:CentralContactPoints,=20its=20rules=20an?= =?UTF-8?q?d=20its=20compose=20entries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The property, the four validator rules that graded it (empty / non-akka.tcp / actor-path suffix / one-bad-among-good) and all twelve docker-compose entries (6 nodes x 2) are gone. The compose comment blocks that explained "/system/receptionist is appended at construction time" and the client's contact rotation go with them, and the surrounding prose now describes the Grpc command plane it actually runs. CentralGrpcEndpoints' doc no longer says it "replaces" the Akka contact points — there is nothing left to replace — and ContactRefreshSeconds' failure message names the node dispatch set rather than the ClusterClient contact points. `rg CentralContactPoints src tests docker-dev` is now empty. --- docker-dev/docker-compose.yml | 93 ++++----------- .../MeshTransportOptions.cs | 26 +---- .../MeshTransportOptionsValidator.cs | 41 +------ .../MeshTransportOptionsValidatorTests.cs | 108 +----------------- 4 files changed, 34 insertions(+), 234 deletions(-) diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index 9e1069a7..3e6cbe4f 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -21,9 +21,10 @@ # # Since central no longer shares a gossip ring with the site meshes, it reaches # them over the three explicit split-topology transports instead of gossip: -# - Command plane: MeshTransport__Mode=ClusterClient (central dials each site -# mesh's ClusterClientReceptionist; DPS is gossip-only and -# would not cross the mesh boundary at all now) +# - Command plane: MeshTransport__Mode=Grpc (central dials each site node's +# NodeCommandService on :4056; a node posts its deploy ack +# back to central's CentralCommandService on :4055. DPS is +# gossip-only and would not cross the mesh boundary at all) # - Live telemetry: Telemetry__Mode=Grpc on every driver node (all six) + # TelemetryDial__Mode=Grpc on the admin pair (central-1/2) # — the site node hosts the gRPC server, central dials in @@ -53,8 +54,7 @@ # data-plane endpoints (4842-4845) and authenticate those # users against the same shared GLAuth (via the *ldap-env # anchor); central manages + deploys to them over the -# ClusterClient/gRPC/FetchAndCache transports above, not -# gossip. +# Grpc/FetchAndCache transports above, not gossip. # # Auth is real LDAP against the shared GLAuth on the Linux Docker host # (10.100.0.35:3893, dc=zb,dc=local) — there is no LDAP container here. @@ -329,33 +329,22 @@ services: Cluster__SeedNodes__1: "akka.tcp://otopcua@central-2:4053" # Per-cluster mesh Phase 6: cluster-MAIN marks this node as a member of the MAIN mesh # (RoleParser.ClusterRolePrefix = "cluster-"), read by SplitTopologyTransportValidator to - # require ClusterClient/Grpc transports, and by ClusterRoleInfo to scope redundancy + + # require the mesh-crossing Grpc transports, and by ClusterRoleInfo to scope redundancy + # ClusterNodeAddressReconcilerActor to ClusterId=MAIN. Cluster__Roles__0: "admin" Cluster__Roles__1: "driver" Cluster__Roles__2: "cluster-MAIN" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6). Central no # longer shares a gossip ring with the site meshes, so DPS (gossip-only) cannot reach them — - # ClusterClient is the only transport that can. SplitTopologyTransportValidator refuses to - # start any node carrying a cluster-* role that is left on Dps, so this is fixed, not an + # Grpc is the only transport that can. SplitTopologyTransportValidator refuses to start any + # node carrying a cluster-* role that is left on Dps, so this is fixed, not an # env-var-overridable default, on every node in this file. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" - # gRPC command transport (Phase 0 of the ClusterClient→gRPC migration, docs/plans/2026-08-06- - # clusterclient-to-grpc-design.md). SHIPPED DARK: Mode above stays ClusterClient, so none of - # this is dialled or served yet — only MeshTransportOptionsValidator's Grpc-mode rules read - # it. GrpcPsk is this node's OWN per-cluster key (MAIN, since central is cluster-MAIN); - # CentralGrpcEndpoints is the gRPC counterpart of CentralContactPoints above. ClusterPsks is - # CENTRAL-SIDE ONLY (IMeshPskProvider's override map, consulted before the Secrets store) — - # one entry per cluster central will dial once the switch flips, so only the central nodes - # carry it. + # gRPC command transport (docs/plans/2026-08-06-clusterclient-to-grpc-design.md). GrpcPsk is + # this node's OWN per-cluster key (MAIN, since central is cluster-MAIN); CentralGrpcEndpoints + # is where a node posts its deploy ack. ClusterPsks is CENTRAL-SIDE ONLY (IMeshPskProvider's + # override map, consulted before the Secrets store) — one entry per cluster central dials, so + # only the central nodes carry it. MeshTransport__GrpcPsk: "dev-mesh-psk-MAIN" MeshTransport__CentralGrpcEndpoints__0: "http://central-1:4055" MeshTransport__CentralGrpcEndpoints__1: "http://central-2:4055" @@ -478,16 +467,8 @@ services: Cluster__Roles__1: "driver" Cluster__Roles__2: "cluster-MAIN" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6) — see central-1. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" - # gRPC command transport (Phase 0) — see central-1. SHIPPED DARK (Mode stays ClusterClient). + # gRPC command transport — see central-1. # ClusterPsks is central-side only; both central nodes carry the same three-cluster map. MeshTransport__GrpcPsk: "dev-mesh-psk-MAIN" MeshTransport__CentralGrpcEndpoints__0: "http://central-1:4055" @@ -549,7 +530,7 @@ services: # ── Site A cluster (2-node driver-only) ───────────────────────────────────── # Per-cluster mesh Phase 6: site-a-1 and site-a-2 form their OWN 2-node "SITE-A" Akka mesh, # separate from MAIN and SITE-B — self-first seeded WITHIN the pair, never off central. No UI, - # no user auth; central manages + deploys to this mesh over ClusterClient/gRPC/FetchAndCache, + # no user auth; central manages + deploys to this mesh over Grpc/FetchAndCache, # not gossip (central and site-a share no seed list, so they never form one ring). site-a-1: @@ -592,7 +573,7 @@ services: Telemetry__Mode: "Grpc" Telemetry__ApiKey: "telemetry-docker-dev-key" # Per-cluster mesh Phase 6: site-a-1 seeds its OWN "SITE-A" mesh (itself first, site-a-2 - # second) — it no longer seeds off central-1. Central is reached over ClusterClient/gRPC/ + # second) — it no longer seeds off central-1. Central is reached over Grpc/ # FetchAndCache, never gossip; central and site-a share no seed list. Cluster__SeedNodes__0: "akka.tcp://otopcua@site-a-1:4053" Cluster__SeedNodes__1: "akka.tcp://otopcua@site-a-2:4053" @@ -605,20 +586,12 @@ services: Cluster__Roles__0: "driver" Cluster__Roles__1: "cluster-SITE-A" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6) — see - # central-1/central-2. Central dials this mesh's ClusterClientReceptionist via the contact - # points below; there is no gossip path anymore. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. + # central-1/central-2. Central dials this node's own NodeCommandService; there is no gossip + # path anymore. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" - # gRPC command transport (Phase 0) — see central-1. SHIPPED DARK (Mode stays ClusterClient). + # gRPC command transport — see central-1. # GrpcPsk is this node's own key for its cluster (SITE-A); CentralGrpcEndpoints is the gRPC - # counterpart of CentralContactPoints above. No ClusterPsks here — that map is central-side + # where this node posts its deploy ack. No ClusterPsks here — that map is central-side # only. MeshTransport__GrpcPsk: "dev-mesh-psk-SITE-A" MeshTransport__CentralGrpcEndpoints__0: "http://central-1:4055" @@ -721,15 +694,7 @@ services: Cluster__Roles__1: "cluster-SITE-A" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6) — see # central-1/site-a-1. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" # gRPC command transport (Phase 0) — see central-1/site-a-1. SHIPPED DARK. Same SITE-A key as # site-a-1 (must be byte-identical — both nodes authenticate against the same cluster's PSK). MeshTransport__GrpcPsk: "dev-mesh-psk-SITE-A" @@ -814,17 +779,9 @@ services: Cluster__Roles__1: "cluster-SITE-B" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6) — see # central-1/site-a-1. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" # gRPC command transport (Phase 0) — see central-1. SHIPPED DARK. GrpcPsk is this node's own - # key for its cluster (SITE-B); CentralGrpcEndpoints mirrors CentralContactPoints above. No + # key for its cluster (SITE-B); CentralGrpcEndpoints is where it posts its deploy ack. No # ClusterPsks here — that map is central-side only. MeshTransport__GrpcPsk: "dev-mesh-psk-SITE-B" MeshTransport__CentralGrpcEndpoints__0: "http://central-1:4055" @@ -892,15 +849,7 @@ services: Cluster__Roles__1: "cluster-SITE-B" # Mesh command transport (per-cluster mesh Phase 2, made mandatory by Phase 6) — see # central-1/site-a-1. - # - # Contact points are node ADDRESSES ONLY. "/system/receptionist" is appended at construction - # time, and MeshTransportOptionsValidator refuses to start a node whose contact carries an - # actor-path suffix, because a doubled suffix resolves to nothing and drops every ack in - # silence. Both central nodes are listed so a driver node's client can rotate to whichever one - # is up — that rotation is why the comm actors are per-node and NOT cluster singletons. MeshTransport__Mode: "${OTOPCUA_MESH_MODE:-Grpc}" - MeshTransport__CentralContactPoints__0: "akka.tcp://otopcua@central-1:4053" - MeshTransport__CentralContactPoints__1: "akka.tcp://otopcua@central-2:4053" # gRPC command transport (Phase 0) — see central-1/site-b-1. SHIPPED DARK. Same SITE-B key as # site-b-1 (must be byte-identical). MeshTransport__GrpcPsk: "dev-mesh-psk-SITE-B" diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs index a52c515d..7fa62de4 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs @@ -56,21 +56,7 @@ public sealed class MeshTransportOptions public string Mode { get; set; } = ModeDps; /// - /// Akka remoting addresses of the central nodes, e.g. - /// akka.tcp://otopcua@central-1:4053. Node addresses only — the - /// /system/receptionist path is appended at construction time. - /// - /// - /// Each entry must be a central node's remoting endpoint, never this node's own port. - /// The sister project shipped a site template whose second contact was the site's own - /// remoting port; it is a permanent failure in the initial-contact rotation and stays - /// invisible until a command goes missing. - /// rejects the malformed shapes at boot instead. - /// - public string[] CentralContactPoints { get; set; } = []; - - /// - /// How often central re-reads ClusterNode rows to rebuild its contact points. + /// How often central re-reads ClusterNode rows to rebuild its dispatch set. /// Refreshing on a timer (rather than only on admin change) means a row edited by any /// process — migration, direct SQL, a second admin node — is picked up without a restart. /// @@ -83,12 +69,10 @@ public sealed class MeshTransportOptions /// (scheme://host:port), not bare host:port — http:// is h2c. /// /// - /// Tried in order, stopping at the first success, which is what preserves the - /// receptionist's one-ack-per-node-per-dispatch semantic: either central node routes the ack - /// to the same ConfigPublishCoordinator singleton proxy, so which one answers does not - /// matter, but two answering would double-count. The gRPC counterpart of - /// , and it replaces it — a node running - /// never reads the Akka contact points. + /// Tried in order, stopping at the first success, which is what keeps the ack + /// one-per-node-per-dispatch: either central node routes it to the same + /// ConfigPublishCoordinator singleton proxy, so which one answers does not matter, + /// but two answering would double-count. /// public string[] CentralGrpcEndpoints { get; set; } = []; diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs index 5641672e..0bd78471 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs @@ -57,48 +57,13 @@ public sealed class MeshTransportOptionsValidator : IValidateOptions@:), not a bare host:port."); - } - else if (contactPoint.Contains("/user/", StringComparison.Ordinal) - || contactPoint.Contains("/system/", StringComparison.Ordinal)) - { - errors.Add( - $"Contact point '{contactPoint}' carries an actor-path suffix. Configure the " - + "node address only — '/system/receptionist' is appended at construction time, " - + "and a doubled suffix produces a path that never resolves."); - } - } + + "node dispatch set on this interval; a non-positive value would leave that set " + + "frozen at whatever it held when the node booted."); } // The first Grpc block below is node-side: the PSK a driver presents (and requires), the // central endpoints it posts its ack to, and the two call deadlines (the admin-side rule that - // follows it is a separate scope). Scoped to the driver role for the same reason the - // ClusterClient block above is scoped to its mode — an admin-only central node - // dials nodes rather than being dialled and resolves per-cluster keys at runtime through + // follows it is a separate scope). Scoped to the driver role because an admin-only central node // MeshTransportOptions.ClusterPsks / the Secrets store, so requiring a single GrpcPsk and an // endpoint list of it would demand configuration it never reads. Roles come from // configuration, not IClusterRoleInfo (see the ctor remark), and through diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs index 294897b2..d03e4b86 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs @@ -86,11 +86,7 @@ public class MeshTransportOptionsValidatorTests [Fact] public void Mode_matching_is_case_insensitive() { - Validate(new MeshTransportOptions - { - Mode = "clusterclient", - CentralContactPoints = ["akka.tcp://otopcua@central-1:4053"], - }).Succeeded.ShouldBeTrue(); + Validate(new MeshTransportOptions { Mode = "dps" }).Succeeded.ShouldBeTrue(); } [Fact] @@ -102,92 +98,7 @@ public class MeshTransportOptionsValidatorTests result.FailureMessage.ShouldContain(nameof(MeshTransportOptions.ContactRefreshSeconds)); } - [Fact] - public void ClusterClient_mode_requires_central_contact_points() - { - var result = Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = [], - }); - - result.Failed.ShouldBeTrue(); - result.FailureMessage.ShouldContain(nameof(MeshTransportOptions.CentralContactPoints)); - } - - [Fact] - public void Dps_mode_does_not_require_central_contact_points() - { - // An admin-only node has no reason to carry them, and requiring them under the default mode - // would make the section mandatory everywhere for a feature that is switched off. - Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeDps, - CentralContactPoints = [], - }).Succeeded.ShouldBeTrue(); - } - - [Fact] - public void A_contact_point_that_is_not_an_akka_address_fails() - { - var result = Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["central-1:4053"], - }); - - result.Failed.ShouldBeTrue(); - result.FailureMessage.ShouldContain("akka.tcp://"); - } - - [Fact] - public void A_contact_point_carrying_an_actor_path_suffix_fails() - { - // The sister project's shipped site template listed a full receptionist path, and separately - // listed the site's OWN remoting port as the second contact — "a permanent failure in the - // initial-contact rotation". Both are invisible until a command goes missing. - var result = Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["akka.tcp://otopcua@central-1:4053/system/receptionist"], - }); - - result.Failed.ShouldBeTrue(); - result.FailureMessage.ShouldContain("/system/receptionist"); - } - - [Fact] - public void One_bad_contact_point_among_good_ones_still_fails() - { - var result = Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = - [ - "akka.tcp://otopcua@central-1:4053", - "central-2:4053", - ], - }); - - result.Failed.ShouldBeTrue(); - result.FailureMessage.ShouldContain("central-2:4053"); - } - - [Fact] - public void Well_formed_ClusterClient_options_succeed() - { - Validate(new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = - [ - "akka.tcp://otopcua@central-1:4053", - "akka.tcp://otopcua@central-2:4053", - ], - }).Succeeded.ShouldBeTrue(); - } - - // ---- Grpc mode (ClusterClient→gRPC migration, Phase 0) -------------------------------------- + // ---- Grpc mode (the cross-mesh command plane) ----------------------------------------------- [Fact] public void Grpc_is_an_accepted_mode() @@ -385,14 +296,9 @@ public class MeshTransportOptionsValidatorTests [Fact] public void ClusterClient_admin_node_needs_no_config_serve_listener() { - // Contact points are required by the mode itself, independently of role — supplied so this - // test fails only if the ConfigServe rule wrongly fires outside Grpc mode. + // Fails only if the ConfigServe rule wrongly fires outside Grpc mode. Validate( - new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["akka.tcp://otopcua@central-1:4053"], - }, + new MeshTransportOptions { Mode = MeshTransportOptions.ModeClusterClient }, "admin", "cluster-MAIN").Succeeded.ShouldBeTrue(); } @@ -446,11 +352,7 @@ public class MeshTransportOptionsValidatorTests public void ClusterClient_mode_requires_none_of_the_new_Grpc_keys() { Validate( - new MeshTransportOptions - { - Mode = MeshTransportOptions.ModeClusterClient, - CentralContactPoints = ["akka.tcp://otopcua@central-1:4053"], - }, + new MeshTransportOptions { Mode = MeshTransportOptions.ModeClusterClient }, "driver", "cluster-SITE-A").Succeeded.ShouldBeTrue(); } From af3b0c314648b715315fc805687533cbdbb15bb3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 15:49:42 -0400 Subject: [PATCH 06/11] =?UTF-8?q?refactor(mesh):=20step=206=20=E2=80=94=20?= =?UTF-8?q?delete=20the=20ClusterClient=20mode=20value;=20split=20topology?= =?UTF-8?q?=20now=20requires=20Grpc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MeshTransportOptions.ModeClusterClient is gone. MeshTransportOptionsValidator accepts Dps or Grpc and nothing else, so a node left carrying the string "ClusterClient" is now REFUSED at boot naming MeshTransport:Mode and both remaining values — deliberately fail-closed rather than falling back to Dps, which on a split-topology node would deliver nothing at all. A new fact pins exactly that. SplitTopologyTransportValidator tightens from an allow-list of two to Grpc only, as its own comment said it would once ClusterClient was deleted. Dps on a cluster-role node stays refused; the rule is still an explicit allow-list, so a typo'd mode fails too. Tests: the two ClusterClient-mode facts in MeshTransportOptionsValidatorTests are deleted (their Dps siblings already assert the same rule for the one remaining non-Grpc mode), the SplitTopology fixtures move to Grpc, and the three-mode Theories in ControlPlaneStarterOrderTests / CentralCommunicationActorTests become two-mode. --- .../MeshTransportOptions.cs | 38 ++++++++++--------- .../MeshTransportOptionsValidator.cs | 15 +++++--- .../SplitTopologyTransportValidator.cs | 17 ++++----- .../MeshTransportOptionsValidatorTests.cs | 38 ++++++++----------- .../SplitTopologyTransportValidatorTests.cs | 33 ++++++++-------- .../CentralCommunicationActorTests.cs | 9 ++--- .../ControlPlaneStarterOrderTests.cs | 6 +-- 7 files changed, 74 insertions(+), 82 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs index 7fa62de4..33593bdc 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs @@ -6,9 +6,11 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster; /// /// /// -/// is a deliberate dark switch: the transports live in the tree -/// side by side so the docker-dev rig can run the same deployment over each and compare, and -/// so a bad rollout rolls back with a config change rather than a revert. +/// selects between a pair-local transport and a cross-mesh +/// one. is the default and cannot cross an application-Cluster +/// boundary; is the only mode that can, and +/// requires it on any node carrying a +/// cluster-{ClusterId} role. /// /// /// The DistributedPubSub branch is retained, not scheduled for deletion. An earlier @@ -16,15 +18,15 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster; /// deliberately reversed — see docs/Configuration.md § "The compiled transport-mode /// defaults were deliberately NOT flipped". DPS remains the correct and default transport for /// a single-mesh topology, which every integration test and every profile that leaves this -/// key unset still runs. What is being replaced is , by -/// . +/// key unset still runs. /// /// /// Discovery is deliberately asymmetric, matching the sister project. Central /// discovers nodes from the database — enabled, non-maintenance ClusterNode /// rows, rebuilt every . Nodes discover central from -/// this section: static, restart to change. The asymmetry follows the rate of change — -/// operators add and retire nodes, but central's own address is part of the deployment. +/// this section (): static, restart to change. The +/// asymmetry follows the rate of change — operators add and retire nodes, but central's own +/// address is part of the deployment. /// /// public sealed class MeshTransportOptions @@ -32,26 +34,26 @@ public sealed class MeshTransportOptions /// Configuration section name. public const string SectionName = "MeshTransport"; - /// Every command stays on DistributedPubSub — the pre-Phase-2 behaviour. + /// + /// Every command stays on DistributedPubSub — pair-local, and the default. Cannot cross an + /// application-Cluster boundary. + /// public const string ModeDps = "Dps"; - /// Commands cross the ClusterClient receptionist boundary. - public const string ModeClusterClient = "ClusterClient"; - /// /// Commands cross the mesh boundary as unary gRPC calls on the mesh_transport.v1 /// contract — central dials each node's NodeCommandService, a node posts its - /// deployment ack to central's CentralCommandService. The replacement for - /// ; unlike it, the command plane is authenticated - /// (). + /// deployment ack to central's CentralCommandService. The command plane is + /// authenticated per application Cluster (). /// public const string ModeGrpc = "Grpc"; /// - /// (default), or - /// . Any other value fails the host at startup rather than silently - /// falling back — a transport that is none of these delivers nothing, and the symptom would - /// be commands that vanish. + /// (default) or . Any other value fails the host + /// at startup rather than silently falling back — a transport that is neither delivers + /// nothing, and the symptom would be commands that vanish. That includes the retired + /// ClusterClient value: a node still carrying it is refused at boot rather than + /// quietly demoted to DPS, which on a split-topology node would deliver nothing at all. /// public string Mode { get; set; } = ModeDps; diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs index 0bd78471..65223102 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs @@ -40,16 +40,18 @@ public sealed class MeshTransportOptionsValidator : IValidateOptions(); var isDps = string.Equals(options.Mode, MeshTransportOptions.ModeDps, StringComparison.OrdinalIgnoreCase); - var isClusterClient = string.Equals( - options.Mode, MeshTransportOptions.ModeClusterClient, StringComparison.OrdinalIgnoreCase); var isGrpc = string.Equals(options.Mode, MeshTransportOptions.ModeGrpc, StringComparison.OrdinalIgnoreCase); - if (!isDps && !isClusterClient && !isGrpc) + // An unrecognised mode is refused, never defaulted. The retired 'ClusterClient' value lands + // here: a node left carrying it fails to start with the two remaining values named, rather + // than silently falling back to Dps — which on a split-topology node cannot cross the mesh + // boundary and would deliver nothing at all. + if (!isDps && !isGrpc) { errors.Add( $"{MeshTransportOptions.SectionName}:{nameof(MeshTransportOptions.Mode)} is " - + $"'{options.Mode}'. Expected '{MeshTransportOptions.ModeDps}', " - + $"'{MeshTransportOptions.ModeClusterClient}' or '{MeshTransportOptions.ModeGrpc}'."); + + $"'{options.Mode}'. Expected '{MeshTransportOptions.ModeDps}' or " + + $"'{MeshTransportOptions.ModeGrpc}'."); } if (options.ContactRefreshSeconds <= 0) @@ -64,6 +66,7 @@ public sealed class MeshTransportOptionsValidator : IValidateOptions Date: Thu, 6 Aug 2026 15:58:36 -0400 Subject: [PATCH 07/11] =?UTF-8?q?test(mesh):=20step=207=20=E2=80=94=20repl?= =?UTF-8?q?ace=20the=20boundary=20test=20with=20MeshGrpcBoundaryTests=20an?= =?UTF-8?q?d=20sweep=20the=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MeshGrpcBoundaryTests replaces the MeshClusterClientBoundaryTests deleted in step 1. Three self-seeded single-member clusters sharing the ActorSystem name, joined ONLY by gRPC, driving the REAL CentralCommunicationActor -> MeshGrpcCommandDialer -> Kestrel h2c -> MeshCommandAuthInterceptor -> NodeCommandGrpcService -> NodeCommunicationActor -> EventStream: 1. No_cross_mesh_Akka_association_forms_between_central_and_either_site — the program's own point, kept as a test rather than a live-gate note: each cluster stays single-member and none ever holds another's address as a member. 2. One_dispatch_through_central_reaches_a_node_in_every_cluster_mesh — the fan, with each site holding its OWN per-cluster PSK, so a dial that selected the wrong cluster's key would be refused at the far side. 3. Falsifiability_control_a_dial_carrying_the_wrong_cluster_key_delivers_nothing — without it, (2) cannot distinguish "the gRPC boundary carried it" from "something else in the process did". The same node then accepts the right key, so the silence is the auth gate and not a dead endpoint. All three run fully offline — no Docker fixture — and pass. Comment sweep across every remaining clusterclient|receptionist hit in src/ and tests/: the program label becomes "mesh gRPC command plane", and every comment that described the deleted transport as live now describes what actually runs (Dps = pair-local default, Grpc = cross-mesh). mesh_transport.proto's header/service comments are reworded; no message, field or enum is touched — the contract is locked. MeshCommActorPathTests is reshaped rather than retired: the two paths still exist and still matter, but what a mismatch costs is now a misleading Unavailable from the gRPC ingress rather than a silently dropped receptionist send. The only surviving mentions of the old names are the ones that must survive: the fail-closed rules that name "ClusterClient" as a retired value refused at boot, their tests, and this test class's own account of what it replaces. --- .../Messages/Mesh/MeshCommand.cs | 2 +- .../Messages/Mesh/MeshPaths.cs | 2 +- .../Messages/Mesh/MeshTransportDtoMapper.cs | 4 +- .../Protos/mesh_transport.proto | 14 +- .../Entities/ClusterNode.cs | 2 +- .../Grpc/CentralCommandGrpcService.cs | 8 +- .../AdminOperations/AdminOperationsActor.cs | 3 +- .../Communication/MeshGrpcCommandDialer.cs | 2 +- .../Coordinators/ConfigPublishCoordinator.cs | 2 +- .../ZB.MOM.WW.OtOpcUa.ControlPlane.csproj | 2 +- .../CentralCommandAuthInterceptor.cs | 2 +- .../MeshCommandAuthInterceptor.cs | 2 +- .../Grpc/NodeCommandGrpcService.cs | 6 +- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 13 +- .../Communication/GrpcApplyAckSender.cs | 10 +- .../Communication/IApplyAckSender.cs | 6 +- .../Drivers/DriverHostActor.cs | 30 +- .../ScriptedAlarms/ScriptedAlarmHostActor.cs | 2 +- .../MeshTransportOptionsValidatorTests.cs | 4 +- .../SplitTopologyTransportValidatorTests.cs | 2 +- .../MeshTransportDtoMapperTests.cs | 2 +- .../MeshTransportProtoContractTests.cs | 2 +- .../ClusterNodeTransportPortsTests.cs | 2 +- .../Grpc/CentralCommandGrpcServiceTests.cs | 4 +- .../CentralCommunicationActorTests.cs | 2 +- .../MeshGrpcCommandDialerTests.cs | 2 +- .../Communication/MeshPskProviderTests.cs | 2 +- .../MeshCommActorPathTests.cs | 19 +- .../MeshGrpcBoundaryTests.cs | 422 ++++++++++++++++++ .../CentralCommandAuthInterceptorTests.cs | 2 +- .../MeshCommandAuthInterceptorTests.cs | 2 +- .../Grpc/NodeCommandGrpcServiceTests.cs | 6 +- .../Communication/GrpcApplyAckSenderTests.cs | 6 +- 33 files changed, 505 insertions(+), 86 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcBoundaryTests.cs diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshCommand.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshCommand.cs index a12ab795..010b7c76 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshCommand.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshCommand.cs @@ -6,7 +6,7 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; /// /// The legacy DPS topic. Carried so the router can publish under /// MeshTransport:Mode = Dps without a second dispatch table; ignored entirely under -/// ClusterClient mode, where the destination is an actor path rather than a topic. +/// Grpc mode, where the destination is a node endpoint rather than a topic. /// /// /// The command itself, never wrapped. Node-side handlers see exactly the type they see today, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs index 308117f1..99337b79 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs @@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; /// Declared once, here, rather than as literals on each side. The deployments / /// deployment-acks topic names are declared twice — once in /// ConfigPublishCoordinator and again in DriverHostActor — and agree only by -/// convention; renaming one would compile, deploy, and deliver nothing. A ClusterClient path +/// convention; renaming one would compile, deploy, and deliver nothing. A mesh path /// has the same failure mode and no compiler to catch it, so the constant is shared. /// /// diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshTransportDtoMapper.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshTransportDtoMapper.cs index 1b83ef7d..f3d78255 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshTransportDtoMapper.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshTransportDtoMapper.cs @@ -9,13 +9,13 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; /// /// The one translation between the five cross-mesh command records and their -/// mesh_transport.v1 wire shapes (ClusterClient→gRPC migration Phase 0). Both directions live +/// mesh_transport.v1 wire shapes (mesh gRPC command plane, Phase 0). Both directions live /// here so a field can only be added to one side without the other by making this file /// visibly asymmetric. /// /// /// -/// Nothing here guesses. The transport it replaces — an Akka ClusterClient receptionist — +/// Nothing here guesses. The transport it replaced — an Akka ClusterClient receptionist — /// dropped an undeliverable command silently, and the entire point of the migration is to stop /// doing that. So every decode that cannot produce a faithful record throws: an unset oneof, an /// APPLY_OUTCOME_UNSPECIFIED ack, an enum value from a newer peer. A loud failure on one diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/mesh_transport.proto b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/mesh_transport.proto index d91c52f8..1dd09580 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/mesh_transport.proto +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/mesh_transport.proto @@ -6,9 +6,9 @@ option csharp_namespace = "ZB.MOM.WW.OtOpcUa.Commons.Protos.MeshTransport.V1"; import "google/protobuf/timestamp.proto"; -// ClusterClient→gRPC migration Phase 0: the cross-mesh command plane. The five messages that cross -// an application-Cluster boundary today ride an Akka ClusterClient receptionist; this contract is -// the replacement carrier. Four are central→node commands, one is the node→central deployment ack. +// The cross-mesh command plane (introduced in Phase 0, now the only transport that crosses an +// application-Cluster boundary). Five messages ride it: four central→node commands and one +// node→central deployment ack. // // The C# namespace deliberately says "MeshTransport", never "MeshCommand" — the latter is already // the CLR envelope record ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh.MeshCommand, and a generated type @@ -20,17 +20,17 @@ import "google/protobuf/timestamp.proto"; // MeshTransportProtoContract + its test in the Commons test project. // Central → node. Delivered to the node's NodeCommunicationActor, which re-emits the decoded message -// on the node's EventStream. ONE-WAY semantics, preserved verbatim from the ClusterClient transport: -// the ack means "received and re-emitted", nothing more — not applied, not acknowledged by any +// on the node's EventStream. ONE-WAY semantics, matching the node-local DistributedPubSub leg: the +// ack means "received and re-emitted", nothing more — not applied, not acknowledged by any // subscriber. No retry is layered on a failure; a failed Execute is logged per node and dropped, -// which is an honest upgrade over the receptionist's silent drop rather than a semantic change. +// which is an honest upgrade over a pub/sub publish nobody was listening for. service NodeCommandService { rpc Execute (CommandEnvelope) returns (ExecuteAck); } // Node → central. Delivered to central's CentralCommunicationActor, which forwards to the // ConfigPublishCoordinator singleton proxy — so an ack landing on either node of the central pair -// routes identically, exactly as the receptionist made arrival-node-agnostic. ONE-WAY semantics: the +// routes identically — which central node answered is deliberately not observable. ONE-WAY semantics: the // ack means "handed to the coordinator", not "the deployment is complete". service CentralCommandService { rpc ReportApply (ApplyAckMessage) returns (ReportAck); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs index 6214d08f..20653c20 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs @@ -22,7 +22,7 @@ public sealed class ClusterNode /// Akka remoting port (default 4053). This is central's dial target, not the node's own /// binding configuration — the node binds from Cluster:Port in its own appsettings, /// and this column duplicates that value for a reader that cannot see the node's config. - /// Per-cluster mesh Phase 2 builds its ClusterClient contact points from + /// Per-cluster mesh Phase 2 built its Akka contact points from /// + this port, at which point central and the site node no longer share a /// gossip ring and central has no other way to learn it. /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs index 5fe5ea78..d70834a4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs @@ -17,10 +17,10 @@ using Status = Grpc.Core.Status; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Grpc; /// -/// Central-side ingress for a node's deployment acknowledgment (ClusterClient→gRPC migration +/// Central-side ingress for a node's deployment acknowledgment (mesh gRPC command plane /// Phase 1B). Decodes an and hands it to the node-local -/// CentralCommunicationActor — the same actor an Akka ClusterClient receptionist delivered -/// to, whose forward-to-the-coordinator path is untouched. Registered only on admin-role nodes +/// CentralCommunicationActor — the same actor the node-local DistributedPubSub path +/// delivers to, whose forward-to-the-coordinator path is untouched. Registered only on admin-role nodes /// and only when ConfigServe:GrpcListenPort > 0. /// /// @@ -52,7 +52,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Grpc; /// /// One-way semantics, preserved. Tell, not Ask: the returned /// means "handed to the comm actor", never "the deployment is -/// complete" — the same promise the receptionist made. The comm actor forwards to the +/// complete" — the same promise the DistributedPubSub leg makes. The comm actor forwards to the /// coordinator singleton proxy, so an ack landing on either central node routes identically. /// /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs index 9a7de9d6..6f21f0e3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -92,8 +92,7 @@ public sealed class AdminOperationsActor : ReceiveActor /// retained, not scheduled for deletion — an earlier note here promised Phase 6 would /// remove it, which was deliberately reversed (docs/Configuration.md § "The compiled /// transport-mode defaults were deliberately NOT flipped"): DPS is still the correct and - /// default transport for a single-mesh topology. What is being replaced is the ClusterClient - /// branch, by gRPC. + /// default transport for a single-mesh topology; gRPC is what crosses a mesh boundary. /// /// The legacy DistributedPubSub topic. /// The command. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/MeshGrpcCommandDialer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/MeshGrpcCommandDialer.cs index 10d9761f..50b30fa5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/MeshGrpcCommandDialer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/MeshGrpcCommandDialer.cs @@ -10,7 +10,7 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; /// /// Places one central→node command call on one node's NodeCommandService -/// (ClusterClient→gRPC migration Phase 1A). The seam CentralCommunicationActor fans +/// (mesh gRPC command plane, Phase 1A). The seam CentralCommunicationActor fans /// across its target set, kept as an interface so the actor's tests never open a socket. /// /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs index f71c4c74..b52beb87 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs @@ -72,7 +72,7 @@ public sealed class ConfigPublishCoordinator : ReceiveActor, IWithTimers /// not scheduled for deletion — an earlier note here promised Phase 6 would remove it, which /// was deliberately reversed (docs/Configuration.md § "The compiled transport-mode /// defaults were deliberately NOT flipped"): DPS is still the correct and default transport - /// for a single-mesh topology. What is being replaced is the ClusterClient branch, by gRPC. + /// for a single-mesh topology; gRPC is what crosses a mesh boundary. /// private readonly IActorRef? _meshRouter; private readonly ILoggingAdapter _log = Context.GetLogger(); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj index 0ffdd314..05f6a470 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj @@ -19,7 +19,7 @@ Google.Protobuf + Grpc.Core.Api flow transitively from Commons (the generated client). --> - diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/CentralCommandAuthInterceptor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/CentralCommandAuthInterceptor.cs index 3b1f635a..a201f1a3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/CentralCommandAuthInterceptor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/CentralCommandAuthInterceptor.cs @@ -6,7 +6,7 @@ using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; /// -/// Gates the central-side command endpoint (ClusterClient→gRPC migration). The +/// Gates the central-side command endpoint (mesh gRPC command plane). The /// CentralCommandService.ReportApply RPC is how a node tells central its deployment /// applied; the coordinator counts those acks, so anything able to reach central's command port /// could otherwise forge a fleet-wide "everything applied". This interceptor is the ONLY inbound diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshCommandAuthInterceptor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshCommandAuthInterceptor.cs index 46a93954..273a3089 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshCommandAuthInterceptor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/MeshCommandAuthInterceptor.cs @@ -9,7 +9,7 @@ using ZB.MOM.WW.OtOpcUa.Cluster; namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; /// -/// Gates the node-side command endpoint (ClusterClient→gRPC migration). The +/// Gates the node-side command endpoint (mesh gRPC command plane). The /// NodeCommandService.Execute RPC is how central drives this node — restart a driver, /// acknowledge an alarm, dispatch a deployment — so anything able to reach a node's command port /// could otherwise operate the node. This interceptor is the ONLY inbound auth on that endpoint. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/NodeCommandGrpcService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/NodeCommandGrpcService.cs index 473c4366..961a0357 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/NodeCommandGrpcService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/NodeCommandGrpcService.cs @@ -15,11 +15,11 @@ using Status = Grpc.Core.Status; namespace ZB.MOM.WW.OtOpcUa.Host.Grpc; /// -/// Node-side command ingress (ClusterClient→gRPC migration Phase 1). Central dials a driver node +/// Node-side command ingress (mesh gRPC command plane, Phase 1). Central dials a driver node /// and calls ; the envelope is decoded back into the same CLR record the -/// ClusterClient receptionist used to deliver and handed to this node's +/// node-local DistributedPubSub path delivers and handed to this node's /// NodeCommunicationActor, which re-emits it on the node's EventStream exactly as -/// before. Nothing downstream of that actor can tell which transport carried the command. +/// it would otherwise. Nothing downstream of that actor can tell which transport carried it. /// /// /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index 80f3048b..1d4762e7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -527,20 +527,19 @@ if (hasAdmin) // hosted service has built the system). builder.Services.TryAddSingleton>(sp => () => sp.GetRequiredService()); builder.Services.AddSingleton(); - // Per-cluster mesh PSKs (ClusterClient→gRPC). Admin-only, and singleton because its whole job is + // Per-cluster mesh PSKs (mesh gRPC command plane). Admin-only, and singleton because its whole job is // to hold a cache: central talks to N clusters and would otherwise re-read the secret store on // every inbound ack. A node needs nothing of this — it holds exactly its own MeshTransport:GrpcPsk // — so registering it fleet-wide would give a driver-only node a lookup it can never use. // Depends on ISecretResolver, which AddOtOpcUaSecrets registers unconditionally below (DI // registration order is irrelevant; resolution order is not). builder.Services.AddSingleton(); - // Central's outbound command dial (ClusterClient→gRPC Phase 1A). Admin-only for the same reason + // Central's outbound command dial (mesh gRPC command plane, Phase 1A). Admin-only for the same reason // as the provider above — a node never dials another node — and singleton because it owns a // GrpcChannel per node endpoint; per-call channels would leak sockets across a fleet-wide fan. // Registered in every mode: CentralCommunicationActor only reaches it under Mode=Grpc, and an // idle registration costs nothing, whereas gating it here would mean flipping the mode on a - // running fleet needs a central restart before any command lands — the same reasoning that - // registers the ClusterClient receptionist unconditionally. + // running fleet needs a central restart before any command lands. builder.Services.AddSingleton(); } @@ -564,7 +563,7 @@ builder.Services.AddOtOpcUaObservability(builder.Configuration); // Registered whenever either role is present so an admin-only node still serves config and a // driver-only node still gates sync + telemetry. // -// The two mesh-command interceptors (ClusterClient→gRPC migration) join on the same terms: +// The two mesh-command interceptors (mesh gRPC command plane) join on the same terms: // MeshCommandAuthInterceptor gates the node-side NodeCommandService (driver-role, one key — this // node's own), CentralCommandAuthInterceptor gates the central-side CentralCommandService // (admin-role, one key per cluster via IMeshPskProvider). Both are registered NOW and are pure @@ -745,7 +744,7 @@ if (hasAdmin && configServeGrpcPort > 0) { app.MapGrpcService(); - // Central command ingress (ClusterClient→gRPC migration): the node→central deployment ack. It + // Central command ingress (mesh gRPC command plane): the node→central deployment ack. It // rides the SAME listener rather than claiming a port of its own — a second cleartext h2c port // would buy nothing but another firewall rule, and both endpoints are admin-role and dialled by // the same driver nodes. Deliberately NOT gated on MeshTransport:Mode: mapping it under Dps @@ -759,7 +758,7 @@ if (hasAdmin && configServeGrpcPort > 0) // Driver-node live-telemetry stream (per-cluster mesh Phase 5). Central dials in; gated by // TelemetryStreamAuthInterceptor (fail-closed on empty Telemetry:ApiKey). // -// The node-side command endpoint (ClusterClient→gRPC migration Phase 1) rides the SAME driver-role +// The node-side command endpoint (mesh gRPC command plane, Phase 1) rides the SAME driver-role // h2c listener rather than claiming a fourth port: central already dials this node here, both // directions are per-cluster-keyed, and a separate port would be one more thing to open on every // site firewall for no isolation gain. Mapping it is what makes MeshCommandAuthInterceptor live — diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/GrpcApplyAckSender.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/GrpcApplyAckSender.cs index b47b2a96..56d9c7fe 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/GrpcApplyAckSender.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/GrpcApplyAckSender.cs @@ -12,14 +12,14 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// /// Posts this node's to central's CentralCommandService as a unary -/// gRPC call (ClusterClient→gRPC migration Task 1B.2), the MeshTransport:Mode = Grpc -/// replacement for the ClusterClient Send. +/// gRPC call (mesh gRPC command plane, Task 1B.2), the MeshTransport:Mode = Grpc +/// cross-mesh ack transport. /// /// /// /// One ordered pass, stopping at the first success. /// is an availability list, not a -/// fan-out list — the semantic of the ClusterClient.Send being replaced (Send, +/// fan-out list — the single-target semantic this leg owes (one ack per node per dispatch, /// deliberately not SendToAll). /// /// @@ -35,7 +35,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// depth and a bound on wasted work, not a correctness dependency. /// /// -/// No retry beyond that single pass, and no queue. The receptionist buffered nothing +/// No retry beyond that single pass, and no queue. The transport buffers nothing /// and retried nothing, and this is a like-for-like replacement, not an upgrade to delivery /// guarantees. The deploy path's own apply deadline stays the only recovery mechanism; a /// retry loop here would instead pile acks up behind an unreachable central and deliver a @@ -43,7 +43,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// /// /// All endpoints failing is a DROP — but a loud one. That is the same outcome the -/// ClusterClient path had when it could not reach central; what changes is that the operator +/// node has when it cannot reach central; what changes is that the operator /// now gets the node, the deployment, the correlation id and the status each endpoint /// answered with, which is what distinguishes "central is down" from "central rejected this /// node's key". diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/IApplyAckSender.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/IApplyAckSender.cs index 092d30ab..adeaf07c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/IApplyAckSender.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/IApplyAckSender.cs @@ -4,15 +4,15 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// /// Posts this node's deployment acknowledgment to central over the mesh_transport.v1 -/// command plane (ClusterClient→gRPC migration Phase 1B). The seam +/// command plane (mesh gRPC command plane, Phase 1B). The seam /// uses under MeshTransport:Mode = Grpc, in place of -/// the ClusterClient.Send it uses under ClusterClient. +/// the node-local DistributedPubSub publish it uses under Dps. /// /// /// /// One ack per node per dispatch. Whatever the implementation does about redundancy, it /// posts to one central endpoint at a time rather than fanning out — the semantic of the -/// ClusterClient.Send this replaces (Send, deliberately not SendToAll). +/// one-ack-per-node-per-dispatch semantic the deploy coordinator counts on. /// This is defence in depth, not a correctness dependency: the coordinator counts acks /// in a NodeId-keyed dictionary behind an expected-set membership guard /// (ConfigPublishCoordinator.HandleAck), so a duplicate from one node overwrites its own diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index 7d14508e..4b865dab 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -374,9 +374,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// Database context factory for configuration database access. /// The local cluster node identifier. /// Where ApplyAcks are sent. A direct coordinator handle in tests; under - /// MeshTransport:Mode = ClusterClient and = Grpc the node's - /// NodeCommunicationActor, which relays across the boundary over whichever transport it was - /// built with. Null publishes on the deployment-acks DPS topic. + /// MeshTransport:Mode = Grpc the node's NodeCommunicationActor, which relays the ack + /// across the mesh boundary. Null publishes on the deployment-acks DPS topic. /// Optional driver factory; defaults to null factory if not provided. /// Optional set of roles assigned to the local node. /// Optional actor reference for dependency multiplexing. @@ -459,9 +458,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// Database context factory for configuration database access. /// The local cluster node identifier. /// Where ApplyAcks are sent. A direct coordinator handle in tests; under - /// MeshTransport:Mode = ClusterClient and = Grpc the node's - /// NodeCommunicationActor, which relays across the boundary over whichever transport it was - /// built with. Null publishes on the deployment-acks DPS topic. + /// MeshTransport:Mode = Grpc the node's NodeCommunicationActor, which relays the ack + /// across the mesh boundary. Null publishes on the deployment-acks DPS topic. /// Optional driver factory; defaults to null factory if not provided. /// Optional set of roles assigned to the local node. /// Optional actor reference for dependency multiplexing. @@ -584,11 +582,11 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _mediator.Tell( new Subscribe(ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.RedundancyStateTopic, Self)); - // Per-cluster mesh Phase 2 dark switch. Under MeshTransport:Mode = ClusterClient these same - // commands arrive from NodeCommunicationActor on the node-local EventStream instead of the - // mesh-wide DPS topic. Subscribing to BOTH unconditionally is deliberate: exactly one of the - // two ever publishes, so the transport flag lives entirely on the publishing side and the - // switch can be flipped (or reverted) without touching any subscriber. + // Per-cluster mesh Phase 2. Under MeshTransport:Mode = Grpc these same commands arrive from + // NodeCommunicationActor on the node-local EventStream instead of the mesh-wide DPS topic. + // Subscribing to BOTH unconditionally is deliberate: exactly one of the two ever publishes, so + // the transport choice lives entirely on the publishing side and the mode can be flipped (or + // reverted) without touching any subscriber. Context.System.EventStream.Subscribe(Self, typeof(DispatchDeployment)); Context.System.EventStream.Subscribe(Self, typeof(RestartDriver)); Context.System.EventStream.Subscribe(Self, typeof(ReconnectDriver)); @@ -2492,11 +2490,11 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers var ack = new ApplyAck(deploymentId, _localNode, outcome, failureReason, correlation); if (_ackRouter is not null) { - // Either a direct coordinator handle (tests) or, under MeshTransport:Mode = ClusterClient - // and = Grpc alike, the NodeCommunicationActor that relays this across the boundary (it - // owns the choice of transport, not this actor). Renamed from _coordinatorOverride in - // per-cluster mesh Phase 2: under those two modes this ref is emphatically NOT a - // coordinator, and the old name would send the next reader looking for one. + // Either a direct coordinator handle (tests) or, under MeshTransport:Mode = Grpc, the + // NodeCommunicationActor that relays this across the mesh boundary (it owns the transport, + // not this actor). Renamed from _coordinatorOverride in per-cluster mesh Phase 2: under + // Grpc this ref is emphatically NOT a coordinator, and the old name would send the next + // reader looking for one. _ackRouter.Tell(ack); } else diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs index 6d18058c..e4a9b208 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs @@ -627,7 +627,7 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor // Per-cluster mesh Phase 2 dark switch. An AdminUI-originated ack/shelve arrives from // NodeCommunicationActor on the node-local EventStream under - // MeshTransport:Mode = ClusterClient; the DPS subscribe above still carries the node-LOCAL + // MeshTransport:Mode = Grpc; the DPS subscribe above still carries the node-LOCAL // publisher (OtOpcUaServerHostedService, the OPC UA Part 9 method calls), which never // crosses the boundary and stays on DPS in both modes. Subscribing to both is safe: exactly // one publisher feeds each path. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs index f93b6ebe..2291388f 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs @@ -64,10 +64,10 @@ public class MeshTransportOptionsValidatorTests [Fact] public void Unknown_mode_fails() { - var result = Validate(new MeshTransportOptions { Mode = "receptionist" }); + var result = Validate(new MeshTransportOptions { Mode = "not-a-transport" }); result.Failed.ShouldBeTrue(); - result.FailureMessage.ShouldContain("receptionist"); + result.FailureMessage.ShouldContain("not-a-transport"); } [Fact] diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs index 47975f89..f11faf74 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs @@ -94,7 +94,7 @@ public class SplitTopologyTransportValidatorTests // still a mode that delivers nothing across the boundary. The retired 'ClusterClient' value // lands here too. var result = Validate( - meshMode: "receptionist", + meshMode: "not-a-transport", telemetryMode: TelemetryOptions.ModeGrpc, telemetryDialMode: null, "driver", "cluster-SITE-A"); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportDtoMapperTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportDtoMapperTests.cs index 0177a85a..aa2adcee 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportDtoMapperTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportDtoMapperTests.cs @@ -12,7 +12,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; namespace ZB.MOM.WW.OtOpcUa.Commons.Tests; /// -/// Round-trip goldens for — the ClusterClient→gRPC command +/// Round-trip goldens for — the mesh gRPC command /// plane's only data contract. Every golden goes domain → proto → serialized bytes → proto → /// domain, because the failure class this suite exists to catch is not a mis-assigned property but a /// member that survives an in-memory copy and vanishes on the wire: proto3 explicit presence is a diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportProtoContractTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportProtoContractTests.cs index a7c33c08..fb5d7617 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportProtoContractTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/MeshTransportProtoContractTests.cs @@ -6,7 +6,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Protos.MeshTransport.V1; namespace ZB.MOM.WW.OtOpcUa.Commons.Tests; /// -/// Contract-lock for the ClusterClient→gRPC command envelope's oneof. If a fifth command is added +/// Contract-lock for the mesh command envelope's oneof. If a fifth command is added /// to mesh_transport.proto without adding a matching entry to /// , this test goes red — the single guard /// that keeps the wire contract and the handled-case set from silently drifting apart. A drift diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/ClusterNodeTransportPortsTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/ClusterNodeTransportPortsTests.cs index 8b72f86d..0e7df9cd 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/ClusterNodeTransportPortsTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/ClusterNodeTransportPortsTests.cs @@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests; /// /// Round-trips / through /// the real schema (per-cluster mesh Phase 1). These are central's dial targets: Phase 2 -/// builds its ClusterClient contact points from Host + AkkaPort, and Phase 5 dials +/// built its Akka contact points from Host + AkkaPort, and Phase 5 dials /// GrpcPort for the telemetry stream. Central cannot see a site node's appsettings once /// the meshes split, so the value has to live in a row central can read. /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/CentralCommandGrpcServiceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/CentralCommandGrpcServiceTests.cs index b2242985..5b3ccf99 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/CentralCommandGrpcServiceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/CentralCommandGrpcServiceTests.cs @@ -14,9 +14,9 @@ using ZB.MOM.WW.OtOpcUa.ControlPlane; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Grpc; /// -/// The central-side gRPC ingress for a node's deployment ack (ClusterClient→gRPC migration +/// The central-side gRPC ingress for a node's deployment ack (mesh gRPC command plane /// Phase 1B). It decodes the wire message and hands it to the registered -/// CentralCommunicationActor — the same actor the ClusterClient receptionist delivered to, +/// CentralCommunicationActor — the same actor the DistributedPubSub leg delivers to, /// whose forward-to-the-coordinator path is untouched. /// /// 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 283ad7f8..0020c7ff 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 @@ -471,7 +471,7 @@ public class CentralCommunicationActorTests : ControlPlaneActorTestBase { // The refresh is not decoration: operators add and retire nodes, and central must pick them // up without a restart. Under Grpc that is the ONLY thing keeping the dispatch set current — - // there is no receptionist to notice a new registration on its own. + // nothing on the node side announces itself; the DB row is the only source. // // Driven by telling the actor its own RefreshContacts tick rather than by shortening // ContactRefreshSeconds: a one-second timer would run for the whole fact, adding DB load to diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshGrpcCommandDialerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshGrpcCommandDialerTests.cs index f5fd9776..2d110eb6 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshGrpcCommandDialerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshGrpcCommandDialerTests.cs @@ -13,7 +13,7 @@ using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Communication; /// -/// ClusterClient→gRPC Phase 1A — central's outbound command dial. One node, one call, no retry. +/// mesh gRPC command plane, Phase 1A — central's outbound command dial. One node, one call, no retry. /// /// /// The facts that matter are the credential ones: the bearer must be the TARGET node's cluster's diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderTests.cs index 3360ab9f..6ccafb65 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderTests.cs @@ -11,7 +11,7 @@ using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Communication; /// -/// ClusterClient→gRPC Phase 0 — central's per-cluster PSK lookup. Central talks to N site +/// mesh gRPC command plane, Phase 0 — central's per-cluster PSK lookup. Central talks to N site /// clusters, each with its own key, so every outbound dial and every inbound ack verification /// goes through here. /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs index 9751788f..0e65d371 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs @@ -10,11 +10,12 @@ namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; /// /// /// -/// /user/central-communication and /user/node-communication are the wire -/// contract of the central↔node boundary: central sends to a path string, and the node -/// registers under one. Nothing else in the codebase asserts they agree. Renaming either -/// actor would compile, pass every unit test, deploy — and deliver nothing, with no error -/// on either side, because a ClusterClient send to an unregistered path is simply dropped. +/// /user/central-communication and /user/node-communication are where each +/// side's gRPC ingress expects to find its comm actor: the services resolve them by registry +/// key, and these constants are the names the actors are spawned under. Nothing else in the +/// codebase asserts they agree. Renaming either actor would compile, pass every unit test, +/// deploy — and refuse every inbound call as Unavailable ("the node is still +/// starting"), which reads as a boot-window race rather than a permanent misnaming. /// /// /// Booting the real host is the point. A test that spawned the actors itself would pin the @@ -30,8 +31,8 @@ public class MeshCommActorPathTests .Ask(new Identify(path), TimeSpan.FromSeconds(10)); identity.Subject.ShouldNotBeNull( - $"{path} is the ClusterClient wire contract for the central↔node boundary; nothing else " - + "pins it, and a mismatch delivers nothing without erroring on either side"); + $"{path} is where the central↔node boundary expects its comm actor; nothing else pins " + + "it, and a mismatch turns every inbound call into a misleading readiness rejection"); } [Fact] @@ -60,7 +61,7 @@ public class MeshCommActorPathTests // is left to live-gate step 8 (stop the OLDEST central; the survivor's comm actor must still // receive a node ack, which a singleton hosted on the stopped node cannot). // - // NOT MeshClusterClientBoundaryTests: that class proves a send only arrives if the target is - // registered with the receptionist, which is necessary but not discriminating — with one node + // NOT MeshGrpcBoundaryTests: that class proves a command only arrives if the target is + // registered under its registry key, which is necessary but not discriminating — with one node // per side, per-node and singleton registration look identical. } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcBoundaryTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcBoundaryTests.cs new file mode 100644 index 00000000..73564688 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshGrpcBoundaryTests.cs @@ -0,0 +1,422 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net; +using Akka.Actor; +using Akka.Cluster; +using Akka.Configuration; +using Akka.Hosting; +using Grpc.Core; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; +using ZB.MOM.WW.OtOpcUa.Host.Grpc; +using ZB.MOM.WW.OtOpcUa.Runtime; +using ZB.MOM.WW.OtOpcUa.Runtime.Communication; +using AkkaCluster = Akka.Cluster.Cluster; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// Drives the mesh command plane across two genuinely separate s over +/// a real gRPC wire — the only test in the repo where the central↔node boundary carries a +/// message off-process. +/// +/// +/// +/// Replaces MeshClusterClientBoundaryTests, which did the same job for the Akka +/// ClusterClient transport that this program deleted. The reason it exists is unchanged: +/// a fake dialer tests actor logic and cannot fail for any transport reason — not a +/// service that was never mapped, not an interceptor that refuses the call, not a +/// serialization break. Those are the failure modes this class exercises, end to end +/// through the REAL CentralCommunicationActor, MeshGrpcCommandDialer, +/// MeshCommandAuthInterceptor, NodeCommandGrpcService and +/// NodeCommunicationActor. +/// +/// +/// Three clusters, one ActorSystem name, joined ONLY by gRPC. The shared name is kept +/// from the predecessor so the shape is comparable; each system seeds itself and none lists +/// another, and +/// +/// asserts the separation outright. That assertion is the whole point of the program — the +/// cross-WAN Akka association is gone — so it is kept as a test rather than left to a live +/// gate: if the three systems ever gossiped into one mesh, every delivery below could be +/// plain local messaging and the class would be testing nothing. +/// +/// +/// Per-cluster keys, not one fleet key. Each site node's interceptor is configured +/// with its OWN MeshTransport:GrpcPsk and central's provider hands the dialer the key +/// for the cluster it is dialling. A fan that presented the wrong cluster's key would be +/// refused at the far side, so the happy path below also pins that the dialer selects by +/// ClusterId. +/// +/// +public sealed class MeshGrpcBoundaryTests : IAsyncLifetime +{ + private const string SharedSystemName = "otopcua"; + private const string SiteAKey = "psk-site-a"; + private const string SiteBKey = "psk-site-b"; + + private static readonly TimeSpan Arrival = TimeSpan.FromSeconds(30); + + /// + /// How long the falsifiability control waits before concluding nothing arrived. Deliberately + /// shorter than but far longer than a successful delivery takes in + /// practice, so a pass means "the boundary refused it", not "the clock ran out first". + /// + private static readonly TimeSpan Silence = TimeSpan.FromSeconds(10); + + private ActorSystem _central = null!; + private SiteNode _siteA = null!; + private SiteNode _siteB = null!; + + /// + public async ValueTask InitializeAsync() + { + _central = StartSelfSeededSystem(); + _siteA = await SiteNode.StartAsync("SITE-A", SiteAKey); + _siteB = await SiteNode.StartAsync("SITE-B", SiteBKey); + + await Task.WhenAll( + AwaitSingleMemberUpAsync(_central), + AwaitSingleMemberUpAsync(_siteA.System), + AwaitSingleMemberUpAsync(_siteB.System)); + } + + /// + public async ValueTask DisposeAsync() + { + await _siteA.DisposeAsync(); + await _siteB.DisposeAsync(); + await ShutdownAsync(_central); + } + + [Fact] + public void No_cross_mesh_Akka_association_forms_between_central_and_either_site() + { + // THE assertion of the ClusterClient→gRPC program, kept as a test rather than a live-gate + // note. Three systems sharing a name in one process are one gossip accident away from being + // one cluster, at which point the gRPC boundary is no longer the thing under test — and, + // more importantly, the cross-WAN Akka association this program removed would be back. + var central = AkkaCluster.Get(_central); + var siteA = AkkaCluster.Get(_siteA.System); + var siteB = AkkaCluster.Get(_siteB.System); + + central.State.Members.Count.ShouldBe(1, + "central must be its own single-member cluster; commands cross to a site over gRPC only"); + siteA.State.Members.Count.ShouldBe(1, "site-A must be its own single-member cluster"); + siteB.State.Members.Count.ShouldBe(1, "site-B must be its own single-member cluster"); + + // Stated separately from the counts: a count of 1 on each side would also hold in the instant + // before a join completed, whereas naming the addresses says no side ever learned another's. + foreach (var (name, cluster) in new[] + { + ("central", central), ("site-A", siteA), ("site-B", siteB), + }) + { + var others = new[] { central.SelfAddress, siteA.SelfAddress, siteB.SelfAddress } + .Where(a => a != cluster.SelfAddress); + foreach (var other in others) + { + cluster.State.Members.ShouldNotContain( + m => m.Address == other, + $"{name} must not have {other} as a cluster member — the cross-mesh Akka " + + "association is exactly what the gRPC command plane replaced"); + } + } + } + + [Fact] + public async Task One_dispatch_through_central_reaches_a_node_in_every_cluster_mesh() + { + // The direct counterpart of the deleted ClusterClient fan-out test, at the transport level and + // driving the REAL CentralCommunicationActor. Two genuinely separate site meshes, each with + // its own gRPC command endpoint and its own per-cluster key; one MeshCommand must reach BOTH. + var dbFactory = SeedTwoClusterDb(_siteA, _siteB); + var actor = _central.ActorOf(CentralCommunicationActor.Props( + dbFactory, + GrpcOptions(), + () => null, + () => Dialer(), + () => new FixedPskProvider())); + + var command = new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid()); + var deadline = DateTime.UtcNow + Arrival; + while (DateTime.UtcNow < deadline + && !(_siteA.Inbox.Task.IsCompleted && _siteB.Inbox.Task.IsCompleted)) + { + // Re-sent rather than sent once: the actor's first contact load is a piped DB read, and a + // command told before it lands is dropped as "no targets" by design. Re-sending weakens + // nothing — the wrong-key control below re-sends on the same schedule and must still + // receive nothing. + actor.Tell(new MeshCommand("deployments", command)); + await Task.Delay(250, TestContext.Current.CancellationToken); + } + + _siteA.Inbox.Task.IsCompleted.ShouldBeTrue( + "site-A's mesh must receive the dispatch on its own gRPC command endpoint"); + _siteB.Inbox.Task.IsCompleted.ShouldBeTrue( + "site-B's mesh must receive the SAME dispatch on its own endpoint, under its own " + + "per-cluster key — a fan that stopped at the first cluster would leave site-B silently " + + "on its old configuration while the deployment sealed green"); + + (await _siteA.Inbox.Task).ShouldBeOfType().DriverInstanceId.ShouldBe("driver-1"); + (await _siteB.Inbox.Task).ShouldBeOfType().DriverInstanceId.ShouldBe("driver-1"); + } + + [Fact] + public async Task Falsifiability_control_a_dial_carrying_the_wrong_cluster_key_delivers_nothing() + { + // Without this, the test above cannot distinguish "the gRPC boundary carried the command" + // from "something else in the process delivered it". Same wire, same service, same comm + // actor — and with a key site-A's interceptor does not accept, it must NOT arrive. + var envelope = MeshTransportDtoMapper.ToEnvelope( + new RestartDriver("C1", "driver-1", "alice", Guid.NewGuid())); + var dialer = Dialer(new FixedPskProvider("not-the-key")); + + var deadline = DateTime.UtcNow + Silence; + while (DateTime.UtcNow < deadline && !_siteA.Inbox.Task.IsCompleted) + { + try + { + await dialer.DispatchAsync( + _siteA.Endpoint, envelope, "SITE-A", TestContext.Current.CancellationToken); + } + catch (RpcException) + { + // Expected: the node's MeshCommandAuthInterceptor refuses the bearer. The assertion + // is what did NOT arrive, so the rejection itself is swallowed rather than asserted + // on here — MeshCommandAuthInterceptorTests owns the status-code contract. + } + + await Task.Delay(500, TestContext.Current.CancellationToken); + } + + _siteA.Inbox.Task.IsCompleted.ShouldBeFalse( + "delivery survived presenting the wrong per-cluster key, so the positive test above is " + + "not measuring the authenticated gRPC boundary at all"); + + // Same node, same dialer shape, right key: proves the silence above was the auth gate rather + // than a dead endpoint or a misaddressed call. + await Dialer().DispatchAsync( + _siteA.Endpoint, envelope, "SITE-A", TestContext.Current.CancellationToken); + await _siteA.Inbox.Task.WaitAsync(Arrival, TestContext.Current.CancellationToken); + } + + // ---- harness --------------------------------------------------------------------------------- + + private static MeshGrpcCommandDialer Dialer(IMeshPskProvider? psk = null) => + new(psk ?? new FixedPskProvider(), Options.Create(GrpcOptions()), + NullLogger.Instance); + + private static MeshTransportOptions GrpcOptions() => new() + { + Mode = MeshTransportOptions.ModeGrpc, + ContactRefreshSeconds = 1, + DispatchTimeoutSeconds = 10, + }; + + /// + /// Hands out the per-cluster key each site node's interceptor expects. Stands in for the real + /// MeshPskProvider, whose own subject (secret-store lookup, caching, the known-cluster + /// gate) is MeshPskProviderTests' and would add a KEK and a SQLite file to a class + /// whose subject is the wire. + /// + private sealed class FixedPskProvider : IMeshPskProvider + { + private readonly string? _override; + + public FixedPskProvider(string? overrideKey = null) => _override = overrideKey; + + public bool TryGetPsk(string clusterId, [NotNullWhen(true)] out string? psk) + { + psk = _override ?? KeyFor(clusterId); + return psk is not null; + } + + public bool Matches(string clusterId, string presented) => + string.Equals(presented, KeyFor(clusterId), StringComparison.Ordinal); + + public void SetKnownClusters(IReadOnlyCollection clusterIds) { } + + public void Invalidate(string clusterId) { } + + public void InvalidateAll() { } + + private static string? KeyFor(string clusterId) => clusterId switch + { + "SITE-A" => SiteAKey, + "SITE-B" => SiteBKey, + _ => null, + }; + } + + private static IDbContextFactory SeedTwoClusterDb(SiteNode a, SiteNode b) + { + var factory = new InMemoryConfigDbFactory(Guid.NewGuid().ToString("N")); + using var db = factory.CreateDbContext(); + db.ClusterNodes.Add(NodeRow("SITE-A", "node-a", a.Port)); + db.ClusterNodes.Add(NodeRow("SITE-B", "node-b", b.Port)); + db.SaveChanges(); + return factory; + } + + private static ClusterNode NodeRow(string clusterId, string nodeId, int grpcPort) => new() + { + NodeId = nodeId, + ClusterId = clusterId, + Host = "127.0.0.1", + AkkaPort = 4053, + GrpcPort = grpcPort, + ApplicationUri = $"urn:{nodeId}", + Enabled = true, + MaintenanceMode = false, + CreatedBy = "test", + }; + + private sealed class InMemoryConfigDbFactory(string dbName) + : IDbContextFactory + { + public OtOpcUaConfigDbContext CreateDbContext() + { + var opts = new DbContextOptionsBuilder() + .UseInMemoryDatabase(dbName) + .Options; + return new OtOpcUaConfigDbContext(opts); + } + } + + /// + /// One site: its own single-member cluster, its own real NodeCommunicationActor, and a + /// real h2c Kestrel listener serving the real NodeCommandGrpcService behind the real + /// MeshCommandAuthInterceptor — the production shape of a driver node's command port. + /// + private sealed class SiteNode : IAsyncDisposable + { + private WebApplication _server = null!; + + public ActorSystem System { get; private set; } = null!; + + public int Port { get; private set; } + + public string Endpoint => $"http://127.0.0.1:{Port}"; + + /// Completes with the first command re-emitted on this node's EventStream. + public TaskCompletionSource Inbox { get; } = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + public static async Task StartAsync(string clusterId, string psk) + { + var node = new SiteNode { System = StartSelfSeededSystem(), Port = GetFreePort() }; + + // The REAL comm actor, under the registry key the gRPC service resolves. Nothing else + // makes it reachable — there is no receptionist registration any more. + var comm = node.System.ActorOf( + NodeCommunicationActor.Props(), MeshPaths.NodeCommunicationName); + node.System.EventStream.Subscribe( + node.System.ActorOf(CaptureActor.Props(node.Inbox)), typeof(RestartDriver)); + + var registry = new ActorRegistry(); + registry.Register(comm); + + var builder = WebApplication.CreateBuilder(); + builder.Environment.ApplicationName = typeof(MeshGrpcBoundaryTests).Assembly.GetName().Name!; + builder.Logging.ClearProviders(); + builder.Services.AddSingleton(registry); + builder.Services.Configure(o => + { + o.Mode = MeshTransportOptions.ModeGrpc; + o.GrpcPsk = psk; + }); + builder.Services.AddGrpc(o => o.Interceptors.Add()); + builder.WebHost.ConfigureKestrel(k => + k.ListenAnyIP(node.Port, o => o.Protocols = HttpProtocols.Http2)); + + node._server = builder.Build(); + node._server.MapGrpcService(); + await node._server.StartAsync(TestContext.Current.CancellationToken); + + _ = clusterId; // Named for readability at the call site; the key IS the cluster boundary. + return node; + } + + public async ValueTask DisposeAsync() + { + await _server.StopAsync(TimeSpan.FromSeconds(10)); + await _server.DisposeAsync(); + await ShutdownAsync(System); + } + } + + /// Starts a single-member cluster on a dynamic remoting port. + /// + /// Port 0 + a post-start self-Join, rather than a seed-nodes list: the address is not known + /// until the transport binds, and hard-coding ports makes the test collide with itself under + /// parallel execution. + /// + /// The started system. + private static ActorSystem StartSelfSeededSystem() + { + var config = ConfigurationFactory.ParseString( + "akka.remote.dot-netty.tcp.hostname = \"127.0.0.1\"\n" + + "akka.remote.dot-netty.tcp.public-hostname = \"127.0.0.1\"\n" + + "akka.remote.dot-netty.tcp.port = 0\n" + + "akka.cluster.seed-nodes = []\n" + + "akka.loglevel = \"WARNING\"\n") + .WithFallback(HoconLoader.LoadBaseConfig()); + + var system = ActorSystem.Create(SharedSystemName, config); + var cluster = AkkaCluster.Get(system); + cluster.Join(cluster.SelfAddress); + return system; + } + + private static async Task AwaitSingleMemberUpAsync(ActorSystem system) + { + var cluster = AkkaCluster.Get(system); + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); + while (DateTime.UtcNow < deadline) + { + if (cluster.State.Members.Any(m => m.Status == MemberStatus.Up)) return; + await Task.Delay(100, TestContext.Current.CancellationToken); + } + + cluster.State.Members.Any(m => m.Status == MemberStatus.Up).ShouldBeTrue( + $"{cluster.SelfAddress} never reached Up as its own seed"); + } + + private static async Task ShutdownAsync(ActorSystem? system) + { + if (system is null) return; + await system.Terminate().WaitAsync(TimeSpan.FromSeconds(15)); + } + + private static int GetFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Completes an inbox with the first message it receives. + private sealed class CaptureActor : ReceiveActor + { + public static Props Props(TaskCompletionSource sink) => + Akka.Actor.Props.Create(() => new CaptureActor(sink)); + + public CaptureActor(TaskCompletionSource sink) => ReceiveAny(m => sink.TrySetResult(m)); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/CentralCommandAuthInterceptorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/CentralCommandAuthInterceptorTests.cs index 510a5dbc..e04252c9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/CentralCommandAuthInterceptorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/CentralCommandAuthInterceptorTests.cs @@ -9,7 +9,7 @@ using ZB.MOM.WW.OtOpcUa.Host.Configuration; namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration; /// -/// ClusterClient→gRPC Phase 0 — the central-side ack endpoint's inbound gate. Mapping +/// mesh gRPC command plane, Phase 0 — the central-side ack endpoint's inbound gate. Mapping /// CentralCommandService onto the pipeline is a Phase 1 task; this test covers only the /// interceptor's own authorization behavior. /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/MeshCommandAuthInterceptorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/MeshCommandAuthInterceptorTests.cs index b96e28b0..596d36df 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/MeshCommandAuthInterceptorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/MeshCommandAuthInterceptorTests.cs @@ -9,7 +9,7 @@ using ZB.MOM.WW.OtOpcUa.Host.Configuration; namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration; /// -/// ClusterClient→gRPC Phase 0 — the node-side command endpoint's inbound gate. Mapping +/// mesh gRPC command plane, Phase 0 — the node-side command endpoint's inbound gate. Mapping /// NodeCommandService onto the pipeline is a Phase 1 task; this test covers only the /// interceptor's own authorization behavior. /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Grpc/NodeCommandGrpcServiceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Grpc/NodeCommandGrpcServiceTests.cs index 0ccd6ea8..a6a15050 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Grpc/NodeCommandGrpcServiceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Grpc/NodeCommandGrpcServiceTests.cs @@ -16,10 +16,10 @@ using ZB.MOM.WW.OtOpcUa.Runtime; namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Grpc; /// -/// Verifies the node-side command ingress (ClusterClient→gRPC migration Phase 1): each of the four +/// Verifies the node-side command ingress (mesh gRPC command plane, Phase 1): each of the four /// central→node commands is decoded off its and handed to the -/// registered NodeCommunicationActor as the same CLR record the ClusterClient transport -/// delivered, an undecodable envelope is refused with , and +/// registered NodeCommunicationActor as the same CLR record every other transport +/// delivers, an undecodable envelope is refused with , and /// a call landing in the boot window before the actor is registered is refused with /// . /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/GrpcApplyAckSenderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/GrpcApplyAckSenderTests.cs index bd6603f8..a8820cf1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/GrpcApplyAckSenderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/GrpcApplyAckSenderTests.cs @@ -13,21 +13,21 @@ using ZB.MOM.WW.OtOpcUa.Runtime.Communication; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Communication; /// -/// The node-side gRPC ack sender (ClusterClient→gRPC migration Task 1B.2). The client is a fake, +/// The node-side gRPC ack sender (mesh gRPC command plane, Task 1B.2). The client is a fake, /// so what is under test is the sender's own logic: the ordered single-pass over /// CentralGrpcEndpoints, the per-call credentials, and the drop that used to be silent. /// /// /// /// The load-bearing fact is that ONE endpoint is dialled on success. This replaces an -/// Akka ClusterClient.Send, which is single-target at-most-once — not a +/// one ack per node per dispatch, single-target at-most-once — not a /// SendToAll. Both central nodes route an ack to the same /// ConfigPublishCoordinator singleton proxy, whose NodeId-keyed upsert absorbs a /// duplicate idempotently — so the single-endpoint rule is defence in depth and a bound /// on wasted dials, not what protects the seal. /// /// -/// The failure side matters just as much: the receptionist dropped an undeliverable ack in +/// The failure side matters just as much: an undeliverable ack is dropped in /// silence, and the whole point of the migration is that the same drop is now a Warning naming /// every endpoint and the status it answered with. /// From cd7135570cbc4647eb6e2ceb8e29db510a806846 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 16:14:25 -0400 Subject: [PATCH 08/11] =?UTF-8?q?refactor(mesh):=20collapse=20MeshPaths=20?= =?UTF-8?q?to=20local=20actor=20names=20=E2=80=94=20no=20longer=20a=20wire?= =?UTF-8?q?=20contract=20(review=20follow-up)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MeshPaths.CentralCommunication / .NodeCommunication (the "/user/..." full-path constants) had ZERO production consumers after the ClusterClient deletion: both gRPC services resolve their comm actor through ActorRegistry by typed key, never by ActorSelection over a path. Only the path-probe test still read them, while the class doc went on calling them "the wire contract of the central↔node boundary" — a claim that stopped being true the moment nothing addressed an actor across a process boundary by name. - The two full-path constants are deleted; the two Name constants remain. - MeshPaths' doc now says what it is (the local actor-naming convention the registry-keyed services are spawned under) and records what it stopped being, so the next reader does not re-derive a contract from the file's history. - MeshCommActorPathTests composes "/user/" + Name itself — the absolute path is that test's own addressing detail now, not something the product declares. Renamed Both_mesh_comm_actors_exist_at_their_contract_paths -> ..._are_spawned_under_their_declared_names, because what it still pins is that the two registration sites actually ran on a real booted node; a rename is now a compile error, so "contract" was the wrong word for what would break. - NodeCommunicationActor's summary no longer cross-references the deleted constant or calls the name an address. CentralCommunicationActor was checked and makes no such claim. --- .../Messages/Mesh/MeshPaths.cs | 44 ++++++++++------- .../Communication/NodeCommunicationActor.cs | 8 ++-- .../MeshCommActorPathTests.cs | 47 ++++++++++++------- 3 files changed, 62 insertions(+), 37 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs index 99337b79..1bf2ec4d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs @@ -1,33 +1,43 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; /// -/// Actor paths the two mesh comm actors register under. These strings are the wire -/// contract of the central↔node boundary. +/// The local actor names the two mesh comm actors are spawned under. Not a wire contract: +/// nothing off-process addresses either actor by name. /// /// /// -/// Declared once, here, rather than as literals on each side. The deployments / -/// deployment-acks topic names are declared twice — once in -/// ConfigPublishCoordinator and again in DriverHostActor — and agree only by -/// convention; renaming one would compile, deploy, and deliver nothing. A mesh path -/// has the same failure mode and no compiler to catch it, so the constant is shared. +/// This used to be a wire contract and no longer is. While the cross-mesh transport +/// was an Akka ClusterClient, central addressed a node by the literal path string +/// /user/node-communication and the far side resolved it — so the two processes had to +/// agree on a string neither could see the other spell. Both gRPC services now resolve +/// their comm actor through ActorRegistry (CentralCommunicationActorKey / +/// NodeCommunicationActorKey), which is type-checked, so the name below never crosses +/// a process boundary. The full /user/... path constants that carried the old contract +/// are deleted; what remains is a naming convention. /// /// -/// Nothing else pins these values, which is why MeshCommActorPathTests boots a real -/// host and Identify-probes both paths. +/// Still declared once, here, rather than as literals at each spawn site — a shared constant +/// costs nothing and keeps the name an operator sees in a log or an ActorSelection +/// attached to one declaration. The registry key, not this string, is what a mis-rename +/// would break, and that failure is a compile error rather than a silent drop. +/// +/// +/// MeshCommActorPathTests boots a real host and Identify-probes +/// /user/{name} on both nodes, which is what pins these values against the two +/// registration sites. /// /// public static class MeshPaths { - /// Central-side comm actor — where a driver node sends its deployment acks. - public const string CentralCommunication = "/user/central-communication"; - - /// Node-side comm actor — where central sends commands. - public const string NodeCommunication = "/user/node-communication"; - - /// Actor name of (the path's final segment). + /// + /// Actor name of the central-side comm actor — where an inbound deployment ack is handed + /// over, after CentralCommandService has resolved it by registry key. + /// public const string CentralCommunicationName = "central-communication"; - /// Actor name of (the path's final segment). + /// + /// Actor name of the node-side comm actor — where an inbound command is handed over, after + /// NodeCommandService has resolved it by registry key. + /// public const string NodeCommunicationName = "node-communication"; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs index e54da301..5d6bbeea 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs @@ -8,9 +8,11 @@ using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// -/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2), living at -/// — the path this node's NodeCommandService -/// gRPC ingress hands every decoded command to. +/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2). This node's +/// NodeCommandService gRPC ingress hands it every decoded command, resolving it from +/// ActorRegistry by NodeCommunicationActorKey — the actor's name +/// () is a local convention, not an address anything +/// off-process uses. /// /// /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs index 0e65d371..ac2bc21b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs @@ -6,45 +6,58 @@ using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; /// -/// Pins the two mesh comm-actor paths against a really booted host. +/// Pins that both mesh comm actors are actually spawned, under their declared names, on a +/// really booted host. /// /// /// -/// /user/central-communication and /user/node-communication are where each -/// side's gRPC ingress expects to find its comm actor: the services resolve them by registry -/// key, and these constants are the names the actors are spawned under. Nothing else in the -/// codebase asserts they agree. Renaming either actor would compile, pass every unit test, -/// deploy — and refuse every inbound call as Unavailable ("the node is still -/// starting"), which reads as a boot-window race rather than a permanent misnaming. +/// What this pins is existence, not a wire contract. Since the cross-mesh transport +/// became gRPC, both services resolve their comm actor through ActorRegistry rather +/// than by path, so a rename is a compile error rather than a silent drop and +/// is only a local naming convention. What is still unpinned by +/// anything else is whether the two registration sites +/// (WithOtOpcUaControlPlaneSingletons / WithOtOpcUaRuntimeActors) actually run +/// and spawn these actors on a real node — and if either did not, every inbound call would be +/// refused as Unavailable ("the node is still starting"), which reads as a boot-window +/// race rather than as an actor that will never appear. /// /// /// Booting the real host is the point. A test that spawned the actors itself would pin the -/// constant against itself and prove nothing about the wiring in -/// WithOtOpcUaControlPlaneSingletons / WithOtOpcUaRuntimeActors. +/// constant against itself and prove nothing about that wiring. /// /// public class MeshCommActorPathTests { - private static async Task AssertActorExistsAsync(ActorSystem system, string path) + /// + /// Builds the ActorSelection string for a top-level actor name. Composed here rather + /// than held as a constant: the full path is a local addressing detail of this test, not + /// something the product declares — see . + /// + /// The declared actor name. + /// The absolute user-guardian path. + private static string UserPath(string actorName) => "/user/" + actorName; + + private static async Task AssertActorExistsAsync(ActorSystem system, string actorName) { + var path = UserPath(actorName); var identity = await system.ActorSelection(path) .Ask(new Identify(path), TimeSpan.FromSeconds(10)); identity.Subject.ShouldNotBeNull( - $"{path} is where the central↔node boundary expects its comm actor; nothing else pins " - + "it, and a mismatch turns every inbound call into a misleading readiness rejection"); + $"no actor exists at {path}; the registration that spawns it did not run, and every " + + "inbound mesh call to this node would be refused as a misleading readiness rejection"); } [Fact] - public async Task Both_mesh_comm_actors_exist_at_their_contract_paths() + public async Task Both_mesh_comm_actors_are_spawned_under_their_declared_names() { await using var harness = await TwoNodeClusterHarness.StartAsync(); // Harness nodes carry admin+driver, so BOTH comm actors live on each of them. - await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.CentralCommunication); - await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.NodeCommunication); - await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.CentralCommunication); - await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.NodeCommunication); + await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.CentralCommunicationName); + await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.NodeCommunicationName); + await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.CentralCommunicationName); + await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.NodeCommunicationName); } // NOT TESTED HERE: that the comm actors are registered per node rather than as cluster From 406b1ae5ac38168e91c047ee337eb6ff4f2fc779 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 16:23:23 -0400 Subject: [PATCH 09/11] =?UTF-8?q?docs(mesh):=20sweep=20for=20ClusterClient?= =?UTF-8?q?=20deletion=20=E2=80=94=20Dps|Grpc=20only,=20CentralContactPoin?= =?UTF-8?q?ts=20retired?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 3.2 of the ClusterClient->gRPC migration. Sweeps the docs that still described the Akka ClusterClient transport as live, following its Phase 3 deletion (this branch): MeshTransport:Mode is now Dps (pair-local DPS) or Grpc (cross-mesh unary calls) only, ClusterClient fails validation loudly, CentralContactPoints is deleted (not superseded), and the split-topology validator requires exactly Grpc rather than ClusterClient. - docs/Configuration.md: rewrite the MeshTransport section — table, dark-switch/PSK-seeding prose, listener sharing (:4056 driver-role, :4055 admin-role), remove the "shipped dark, cutover pending" framing. - docs/Redundancy.md: replace the Dps/ClusterClient transport table with Dps/Grpc, fix the Phase 6 CentralCommunicationActor + validator bullets. - docs/Telemetry.md: fix stale ClusterClient cross-refs, add a note that the driver-role telemetry listener also hosts NodeCommandService. - docs/v2/config-db-schema.md: one-line "(retired: ...)" annotation on the historical AkkaPort/GrpcPort note — left as a historical record otherwise. - CLAUDE.md: rewrite "## Mesh command transport (MeshTransport)" for the post-deletion world (no ClusterClient/receptionist, per-node gRPC dial, fire-and-forget semantics unchanged) and fix two Phase 6 bullets. deferment.md has no mesh-transport mentions; left untouched. docs-only change; `dotnet build ZB.MOM.WW.OtOpcUa.slnx` stays green (0 errors) after the edit. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- CLAUDE.md | 106 ++++++++++++++++++++++-------------- docs/Configuration.md | 39 ++++++------- docs/Redundancy.md | 42 ++++++++------ docs/Telemetry.md | 18 ++++-- docs/v2/config-db-schema.md | 6 +- 5 files changed, 126 insertions(+), 85 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ad5c1400..7ca4005d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -371,8 +371,8 @@ Assert the **effective** cluster config off a running `ActorSystem` (as `SplitBr - **The redundancy singleton is cluster-scoped and spawned per driver node.** `RedundancyStateActor` is now a `cluster-{ClusterId}`-scoped singleton (falls back to plain `driver` for a legacy node), so each pair elects and publishes its **own** Primary on its own mesh's DPS. **Two or more Primaries exist fleet-wide, one per pair — by design, not a bug.** ⚠️ **Wiring landmine (fixed `3a4ed8dd`, caught by the live gate):** the singleton's cluster-role *scope* is computed at `AddAkka`-configurator time, so it must come from `AkkaClusterOptions` (config) — **never** by resolving `IClusterRoleInfo` from the SP there. `ClusterRoleInfo` depends on the `ActorSystem`, and that lambda runs *while the ActorSystem is being built*, so resolving it recurses into ActorSystem construction and **stack-overflows every node at boot** (compiles clean; the rehome tests passed a hand-built fake straight into the extension, mocking around the composition-root line). Keep `WithOtOpcUaClusterRedundancySingleton`/`BuildClusterRedundancySingletonOptions` taking `AkkaClusterOptions`. - `ClusterNodeAddressReconciler` is **own-cluster-scoped** — an admin node can only reconcile `ClusterNode` rows in its own cluster; it has no gossip visibility into another mesh. -- `CentralCommunicationActor` creates **one `ClusterClient` per application `Cluster`** and fans commands across them (closing the `TODO(Phase 6)` Phase 2 left behind), rather than one fleet-wide client. -- **`SplitTopologyTransportValidator` fails host start** on a node carrying a `cluster-{ClusterId}` role unless it also runs the split-safe transports: `MeshTransport:Mode=ClusterClient`, `Telemetry:Mode=Grpc` (driver), `TelemetryDial:Mode=Grpc` (admin). A DPS transport on a cluster-role node would try to gossip across a partition that no longer exists. +- `CentralCommunicationActor` dials each node directly over gRPC — one `NodeCommandService.Execute` call per enabled `ClusterNode` row carrying a `GrpcPort` — rather than fanning through a client/receptionist object. **Superseded by the [ClusterClient→gRPC migration](docs/plans/2026-08-06-clusterclient-to-grpc-design.md):** the Akka `ClusterClient` this bullet originally described (one per application `Cluster`, closing the `TODO(Phase 6)` Phase 2 left behind) is deleted; the per-node gRPC dial is its replacement, not an evolution of it. +- **`SplitTopologyTransportValidator` fails host start** on a node carrying a `cluster-{ClusterId}` role unless it also runs the split-safe transports: `MeshTransport:Mode=Grpc`, `Telemetry:Mode=Grpc` (driver), `TelemetryDial:Mode=Grpc` (admin). A DPS transport on a cluster-role node would try to gossip across a partition that no longer exists. - **The compiled transport-mode defaults were deliberately NOT flipped** — `MeshTransport:Mode`/`Telemetry:Mode`/`TelemetryDial:Mode` all still default to `Dps`; a blast-radius assessment showed flipping the default breaks every integration test and appsettings profile that leaves the key unset. The validator + explicit per-deployment config is the guardrail, not a default flip. - **Secrets replication is pair-local** — each pair replicates its own Akka DPS secrets within itself; there is no fleet-wide/cross-cluster sync after the split (sites have no SQL, so no SQL-hub, consistent with Phase 4). @@ -380,51 +380,74 @@ See `docs/Redundancy.md` §"Per-cluster meshes (Phase 6)", `docs/Configuration.m ## Mesh command transport (`MeshTransport`) -Per-cluster mesh **Phase 2** added a second transport for the three central→node command channels -(`deployments`, `driver-control`, `alarm-commands`) and the `deployment-acks` reply, selected by -`MeshTransport:Mode`: `Dps` (the DistributedPubSub path this repo has always used, **the default**) -or `ClusterClient`, which does **not** require central and the node to share a gossip ring — the -prerequisite for splitting the fleet into one mesh per application `Cluster`. Both `CentralCommunicationActor` -(`/user/central-communication`) and `NodeCommunicationActor` (`/user/node-communication`) are spawned -and registered with the `ClusterClientReceptionist` in **both** modes, so flipping the flag is a config -change, not a redeploy. Things worth knowing before touching it: +**The Akka `ClusterClient` transport described in this section's earlier history is DELETED** +(branch `feat/grpc-phase3-deletion`, [ClusterClient→gRPC migration](docs/plans/2026-08-06-clusterclient-to-grpc-design.md), +Phase 3) — code, the `akka.cluster.client` HOCON block, the receptionist extension, +`MeshTransport:CentralContactPoints`, and the `ClusterClient` mode value itself, which now **fails +validation loudly** at boot (`MeshTransportOptionsValidator` names the key and the allowed values) +rather than silently falling back to `Dps`. What remains is exactly two transports, selected by +`MeshTransport:Mode`: -- **`SendToAll`, never `Send`.** Today's DPS publish reaches every `DriverHostActor`; there is no - ClusterId or node filter on the node side at all (scoping happens later, inside the artifact via - `DeploymentArtifact.ResolveClusterScope`). `ClusterClient.Send` delivers to exactly **one** registered - actor — it would deploy to a single node while every other node silently kept its old configuration, - and the deployment could still seal green. -- **RESOLVED by Phase 6 — one `ClusterClient` per application `Cluster`, not one fleet-wide.** When - this was written the fleet was still one mesh, so a receptionist served the whole cluster and - `SendToAll` reached every registered node-comm actor regardless of which node's address was dialled; - the `TODO(Phase 6)` in `CentralCommunicationActor.RebuildClient` asked for one client per application - `Cluster` instead. Phase 6 split the fleet into one mesh per `Cluster` and shipped exactly that — see - the [Redundancy](#redundancy) section's Phase 6 paragraph above. The `SendToAll`-within-a-mesh - behaviour below is otherwise unchanged; a `MaintenanceMode` node still isn't excluded from delivery - within its own mesh. -- **Inbound commands re-emit on the node's `EventStream`, not on their DPS topic** — DPS is mesh-wide and - central `SendToAll`s to every node, so a DPS re-publish would deliver N copies to every subscriber. - Node-side subscribers (`DriverHostActor`, `ScriptedAlarmHostActor`) accept both. -- **Comm actors are per-node, NOT singletons.** A client rotates across contact points and must find a live - comm actor at whichever node answers; `akka.cluster.client.receptionist.role` is empty for the same reason. -- **`buffer-size = 0` is a behaviour decision, not a tuning knob.** Akka's default buffers 1000 and replays - on reconnect — silently, on the client's own timer, at an unbounded remove from the dispatch. Do not raise - it to quiet a flaky test. **But do not over-claim what it buys:** the Phase 2 live gate observed a node - applying a deployment **44 s after it was sealed `TimedOut`** anyway, via `DriverHostActor.Bootstrap()` - replaying the orphan `Applying` row on restart. Zero buffering removes the silent replay, not every replay. +- **`Dps`** (default) — DistributedPubSub, the path this repo has always used. **Pair-local only**: + it works only when central and the node share a gossip ring, which after Phase 6 means within one + 2-node mesh. +- **`Grpc`** — unary gRPC calls on the `mesh_transport.v1` contract. **Required** on any node + carrying a `cluster-{ClusterId}` role; `SplitTopologyTransportValidator` refuses to boot such a + node on anything else. -Contact points are node **addresses only** (`akka.tcp://@:`); `/system/receptionist` is -appended at construction and `MeshTransportOptionsValidator` refuses to start a node whose contact carries an -actor-path suffix. `redundancy-state` stays on DPS in both modes (bidirectional, and built from -`Cluster.State`). See `docs/Configuration.md` §`MeshTransport`, `docs/Redundancy.md` §"Command transport", -and `docs/plans/2026-07-22-mesh-phase2-clusterclient-transport.md`. +Central→node commands (`deployments`, `driver-control`, `alarm-commands`) ride `NodeCommandService.Execute`, +served on each node's driver-role telemetry h2c listener (`Telemetry:GrpcListenPort`, the docker-dev +rig's `:4056` — it shares that listener with `TelemetryStreamGrpcService` rather than claiming a port +of its own). `CentralCommunicationActor` dials it **directly, one call per node** — the target set is +every enabled, non-maintenance `ClusterNode` row carrying a `GrpcPort`, dialled at `Host`+`GrpcPort` +and refreshed every `MeshTransport:ContactRefreshSeconds`. There is no client/receptionist object and +no rotation: each command goes to the specific node dialled, which is what makes per-node delivery +provable — the old `ClusterClient.SendToAll`/`Send` distinction this section used to explain no longer +applies to either direction. + +Node→central deploy acks ride `CentralCommandService.ReportApply`, served on central's admin-role +config-serve h2c listener (`ConfigServe:GrpcListenPort`, the rig's `:4055` — again sharing rather than +claiming a new port). `GrpcApplyAckSender` walks `MeshTransport:CentralGrpcEndpoints` in order and +stops at the first success. + +Auth is a **per-cluster PSK bearer**, both directions: `MeshTransport:GrpcPsk` (node-side, one key per +application `Cluster`) and `MeshTransport:ClusterPsks` (central-side map, consulted before the Secrets +store). `MeshCommandAuthInterceptor` (node) and `CentralCommandAuthInterceptor` (central) fail closed +on every unresolvable case — constant-time compare, no key material logged. Production keys are +`${secret:OT-MESH-PSK-}`; the docker-dev rig supplies three literal dev keys via env +(`dev-mesh-psk-MAIN`/`-SITE-A`/`-SITE-B`). + +Things that carried over unchanged from the `ClusterClient` era, because they were never +`ClusterClient`-specific to begin with: + +- **Fire-and-forget, no retry, on both transports.** A failed gRPC dispatch is logged (naming the node + and the gRPC status) and dropped in the dispatch second. No queue, no ordering guarantee across + nodes — the deploy coordinator's apply deadline and a node's boot-time replay are the only recovery. + The old `buffer-size = 0` decision this section used to describe is gone with the HOCON block that + set it; the gRPC transport has nothing to buffer in the first place — a dial either lands or doesn't. +- **Inbound commands re-emit on the node's `EventStream`, not back onto DPS.** DPS is mesh-wide, so a + DPS re-publish would deliver N copies to every subscriber. Node-side subscribers (`DriverHostActor`, + `ScriptedAlarmHostActor`) accept a command from either transport identically. +- **Do not over-claim what fire-and-forget buys.** The Phase 2 live gate observed a node applying a + deployment **44 s after it was sealed `TimedOut`** anyway, via `DriverHostActor.Bootstrap()` + replaying the orphan `Applying` row on restart. That backstop is unaffected by the ClusterClient + deletion — it was always about the deploy path, not the transport. + +`redundancy-state` stays on DPS in both modes (bidirectional, and built from `Cluster.State`, which +makes it genuinely mesh-bound). See `docs/Configuration.md` §`MeshTransport`, `docs/Redundancy.md` +§"Command transport", and the migration's [design doc](docs/plans/2026-08-06-clusterclient-to-grpc-design.md) / +[plan](docs/plans/2026-08-06-clusterclient-to-grpc.md) / [tracking doc](docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md) / +[live gate](docs/plans/2026-08-06-clusterclient-to-grpc-live-gate.md) for the phase-by-phase history — +those are historical records, not updated further by this sweep. `docs/plans/2026-07-22-mesh-phase2-clusterclient-transport.md` +is the original Phase 2 design doc and is likewise left as a historical record of the transport this +section now describes as deleted. **Phase 1 landed 2026-07-22 and changed a deploy-path behaviour.** `ConfigPublishCoordinator` sources its expected-ack set from **enabled `ClusterNode` rows**, not from `Akka.Cluster.State.Members` filtered by the `driver` role — central must be able to name a deployment's nodes without sharing a gossip ring with them. Consequences worth knowing before touching deploys: - **A configured node that is switched off now FAILS the deployment** at the apply deadline instead of letting it seal green without that node. The maintenance hatch is the **new `ClusterNode.MaintenanceMode` column**, NOT `Enabled = 0`: the live gate showed `Enabled = 0` is rejected outright by `DraftValidator.ValidateClusterTopology` (`ClusterEnabledNodeCountMismatch` — enabled count must equal `ServerCluster.NodeCount`), so on a 2-node pair — every cluster in the target topology — it can never be the hatch. `Enabled` = part of the declared topology; `MaintenanceMode` = do not expect it now. The deadline log names the silent nodes and points at the right flag. - **Every `ClusterNode` row is now *defined* to be a driver node.** The DB has no per-node role column and deliberately did not gain one (it would drift from `Cluster:Roles`). Giving an admin-only node a `ClusterNode` row makes every deploy time out. - **There is no such thing as a cluster-scoped deployment.** `Deployment` has no `ClusterId`, `ConfigComposer` always snapshots the whole DB, and `DeploymentArtifact.ResolveClusterScope` is *node-side* self-scoping of a fleet-wide artifact. The phase-1 plan asked for cluster-scope filtering of the ack set; it was dropped as describing a feature that does not exist. -- `ClusterNode` gained `AkkaPort` (NOT NULL, 4053) + `GrpcPort` (nullable) as **central's dial targets** for Phases 2/5. They duplicate the node's own `Cluster:Port` / `Cluster:PublicHostname` with no schema-level enforcement, so `ClusterNodeAddressReconcilerActor` (admin singleton) logs an Error on drift. **Phase 2 must revisit it** — once the meshes split, an admin node cannot see site members and every site row would report `EnabledRowNotInCluster` forever. +- `ClusterNode` gained `AkkaPort` (NOT NULL, 4053) + `GrpcPort` (nullable) as **central's dial targets** for Phases 2/5. They duplicate the node's own `Cluster:Port` / `Cluster:PublicHostname` with no schema-level enforcement, so `ClusterNodeAddressReconcilerActor` (admin singleton) logs an Error on drift. **Phase 2 must revisit it** — once the meshes split, an admin node cannot see site members and every site row would report `EnabledRowNotInCluster` forever. **Resolved, and not the way this bullet originally expected:** the [ClusterClient→gRPC migration](docs/plans/2026-08-06-clusterclient-to-grpc-design.md) deleted the Akka `ClusterClient` transport that would have dialled `Host`+`AkkaPort`; the Phase 2 command dial now uses `Host`+`GrpcPort`, the same column Phase 5 telemetry already dialled — `AkkaPort` is Akka remoting only again. ## Config source (`ConfigSource` / `ConfigServe`) @@ -488,8 +511,9 @@ touching it: `Cluster.State`, drives ServiceLevel + the Primary gate — genuinely mesh-bound by design, not an observability panel); `fleet-status` is central-internal (`FleetStatusBroadcaster` builds it from the admin node's own membership events, `Fleet.razor` polls the Config DB and ignores the feed — - never a node→central stream); `deployment-acks` already rides the Phase 2 `ClusterClient` - transport as a command-plane reply, not telemetry. + never a node→central stream); `deployment-acks` already rides the Phase 2 gRPC command transport + (`MeshTransport:Mode=Grpc`, `CentralCommandService.ReportApply`) as a command-plane reply, not + telemetry. - **The node ALWAYS hosts and ALWAYS double-publishes, in both modes.** Every publish seam emits into a node-local `ITelemetryLocalHub` **and** still publishes DPS, unconditionally; the gRPC server binds whenever `Telemetry:GrpcListenPort > 0`, independent of `Telemetry:Mode`. Only diff --git a/docs/Configuration.md b/docs/Configuration.md index a4a11183..66e59fd0 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -142,7 +142,7 @@ The checked-in `appsettings*.json` files are deliberately thin: they carry only - **`SplitTopologyTransportValidator`** (`src/Core/ZB.MOM.WW.OtOpcUa.Cluster/`, wired at `ValidateOnStart` alongside the other `Cluster`-family validators) fails host start on any node carrying a `cluster-{ClusterId}` role unless it is also configured for the split-safe transports: - `MeshTransport:Mode = ClusterClient`, `Telemetry:Mode = Grpc` on a driver node, `TelemetryDial:Mode = + `MeshTransport:Mode = Grpc`, `Telemetry:Mode = Grpc` on a driver node, `TelemetryDial:Mode = Grpc` on an admin node. A DPS-mode transport on a cluster-role node would try to gossip across a partition boundary that no longer exists, so the validator fails closed rather than let commands or telemetry silently vanish. A legacy node with no `cluster-{ClusterId}` role is exempt and may stay on @@ -155,45 +155,38 @@ The checked-in `appsettings*.json` files are deliberately thin: they carry only actually carries a `cluster-{ClusterId}` role must set the split-safe modes explicitly. - See [`Redundancy.md` § Per-cluster meshes (Phase 6)](Redundancy.md#per-cluster-meshes-phase-6) for the full runtime picture (one Primary per pair, the cluster-scoped `RedundancyStateActor` singleton, - `ClusterNodeAddressReconciler` own-cluster-scoping, and central's one-`ClusterClient`-per-`Cluster` - fan-out). + `ClusterNodeAddressReconciler` own-cluster-scoping, and central's per-node gRPC command dial — + one `NodeCommandService.Execute` call per enabled `ClusterNode` row carrying a `GrpcPort`). ### `MeshTransport` (central↔node command transport) -- **Purpose:** selects how central's three command channels (`deployments`, `driver-control`, `alarm-commands`) and the node's `deployment-acks` reply reach the other side — over the mesh-wide DistributedPubSub, or over an Akka `ClusterClient` that does **not** require central and the node to share a gossip ring. The second option is what [per-cluster mesh](plans/2026-07-21-per-cluster-mesh-design.md) Phase 6 needs once the meshes split. +- **Purpose:** selects how central's three command channels (`deployments`, `driver-control`, `alarm-commands`) and the node's `deployment-acks` reply reach the other side — over the mesh-wide DistributedPubSub (pair-local only), or over unary gRPC calls that do **not** require central and the node to share a gossip ring. The second option is what [per-cluster mesh](plans/2026-07-21-per-cluster-mesh-design.md) Phase 6 needs, now that the meshes are split. The Akka `ClusterClient` transport that used to be the second option here is **deleted** ([ClusterClient→gRPC migration](plans/2026-08-06-clusterclient-to-grpc-design.md), Phase 3) — `Grpc` is its replacement, not a third option alongside it. - **Options class:** `MeshTransportOptions` (`SectionName = "MeshTransport"`) — `src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptions.cs`. Bound and validated by `AddOtOpcUaCluster(config)`; `MeshTransportOptionsValidator` runs at `ValidateOnStart`. | Key | Type | Default | Meaning | |---|---|---|---| -| `Mode` | string | `Dps` | `Dps` = DistributedPubSub (the transport this repo has always used). `ClusterClient` = the mesh boundary. `Grpc` = the `mesh_transport.v1` gRPC contract — **shipped dark, cutover pending** (see below). Any other value **refuses to start**. | -| `CentralContactPoints` | string[] | `[]` | Node-side only: central node **addresses** (`akka.tcp://@:`). Required under `ClusterClient`, ignored under `Dps`/`Grpc`. | -| `ContactRefreshSeconds` | int | `60` | Central-side only: how often central rebuilds its ClusterClient contact set from the `ClusterNode` rows. | +| `Mode` | string | `Dps` | `Dps` = DistributedPubSub, pair-local only — cannot cross an application-`Cluster` boundary. `Grpc` = the `mesh_transport.v1` gRPC contract (below), required on any node carrying a `cluster-{ClusterId}` role. Any other value **refuses to start**, including the retired `ClusterClient` value — a node still configured for it is refused at boot rather than silently demoted to `Dps`, which on a split-topology node would deliver nothing at all. | +| `ContactRefreshSeconds` | int | `60` | Central-side only: how often central rebuilds its node dispatch set — the gRPC dial targets and the known-cluster PSK gate — from the `ClusterNode` rows. Runs in every mode; under `Dps` it still keeps the inbound-ack endpoint's known-cluster gate current even though nothing dials out. | -**Addresses only — no actor-path suffix.** `/system/receptionist` is appended at construction time; a contact carrying `/user/` or `/system/` is rejected at startup, because a doubled suffix produces a path that resolves to nothing and drops every command in silence. - -**It is a dark switch, and both sides are always wired.** The comm actors are spawned and registered with the `ClusterClientReceptionist` on every node in *both* modes, so flipping `Mode` is a config change rather than a redeploy. Under `Dps` the registration is simply idle. - -**Central's contact set comes from the enabled, non-maintenance `ClusterNode` rows** — the same source Phase 1 gave the deploy ack set. A node in `MaintenanceMode` is not dialled, but note that **the contact set does not scope delivery**: a receptionist serves its whole cluster, so `SendToAll` still reaches every registered node exactly as today's DPS broadcast does. The filter is about which receptionists are worth dialling, not about who receives. - -**Buffering is off by design** (`akka.cluster.client.buffer-size = 0`, set in the shipped `akka.conf`). If no receptionist is known, the command is dropped rather than queued. Akka's default of 1000 would replay a `DispatchDeployment` on reconnect — silently, on the client's own timer, at an unbounded remove from the dispatch. Do not "fix" it back to quiet a flaky test. - -> **What this does *not* buy you.** It does not guarantee a node never applies a deployment the DB records as `TimedOut`. A node that missed a dispatch keeps the orphan `Applying` row the coordinator pre-created, and `DriverHostActor.Bootstrap()` replays that row on the node's next restart — live-observed applying a deployment 44 s after it was sealed `TimedOut` (see the [Phase 2 live gate](plans/2026-07-22-mesh-phase2-live-gate.md), finding B). The boot replay is the better-behaved of the two — deliberate, Warning-logged, and bounded to a restart — but it is still a replay. `buffer-size = 0` removes the *silent, timer-driven* one only. - -On the docker-dev rig every node carries these keys already; flip the whole rig with `OTOPCUA_MESH_MODE=ClusterClient` at `docker compose up`. - -**`Grpc` mode — shipped dark, cutover pending** ([ClusterClient→gRPC migration design](plans/2026-08-06-clusterclient-to-grpc-design.md)). A third `Mode` value replaces `ClusterClient` with unary gRPC calls on the `mesh_transport.v1` contract: central dials each node's `NodeCommandService`, a node posts its deployment ack to central's `CentralCommandService`. Unlike `ClusterClient`, the command plane is authenticated end to end (`GrpcPsk` below) rather than relying on the mesh boundary alone. As of Phase 0 only the contract, DTO mapper, options and the two auth interceptors exist — the compiled default and the docker-dev rig both stay on `ClusterClient`, and setting `Mode=Grpc` today validates the keys below but nothing yet dials or serves over the contract. +**`Grpc` mode: central dials each node, a node posts its ack to central.** `CentralCommunicationActor` fans one `NodeCommandService.Execute` call per node — the target set is every enabled, non-maintenance `ClusterNode` row that carries a `GrpcPort` (dialled at `Host`+`GrpcPort`), rebuilt every `ContactRefreshSeconds`. There is no receptionist and no single fleet-wide (or per-cluster) client object: it is a plain per-node dial. A node posts its `ApplyAck` with `GrpcApplyAckSender`, which walks `CentralGrpcEndpoints` in order and stops at the first success — an availability list, not a fan-out list, so central normally sees one ack per node per dispatch (the walk as a whole is at-least-once on a slow-then-retried endpoint; `ConfigPublishCoordinator.HandleAck` absorbs a duplicate idempotently). Both directions are **fire-and-forget, with no retry**: a failed dispatch is logged (naming the node and the gRPC status) and dropped in the dispatch second — the deploy path's 2-minute apply deadline and a node's boot-time replay (`DriverHostActor.Bootstrap()` re-applying an orphan `Applying` row) are the only recovery mechanisms, exactly as they were under `ClusterClient`. | Key | Type | Default | Meaning | |---|---|---|---| | `GrpcPsk` | string? | `null` (unset) | Node-side only, required under `Grpc` on a driver-role node: this node's pre-shared key for its own `Cluster`, presented as the bearer on outbound acks and required on inbound `NodeCommandService` calls. Production form is `${secret:OT-MESH-PSK-}`; supply a literal only on a rig, via `MeshTransport__GrpcPsk` — never commit one. | -| `CentralGrpcEndpoints` | string[] | `[]` | Node-side only, required under `Grpc` on a driver-role node: full gRPC base URLs (`scheme://host:port`, e.g. `http://central-1:4055` for h2c) of the central pair's `CentralCommandService`, tried in order, first success wins. The `Grpc` counterpart of `CentralContactPoints`, and replaces it — a node on `Grpc` never reads the Akka contact points. | +| `CentralGrpcEndpoints` | string[] | `[]` | Node-side only, required under `Grpc` on a driver-role node: full gRPC base URLs (`scheme://host:port`, e.g. `http://central-1:4055` for h2c) of the central pair's `CentralCommandService`, tried in order, first success wins. There is no Akka-side counterpart any more — the old `MeshTransport:CentralContactPoints` (the `ClusterClient` contact-point list) is deleted along with `ClusterClient` itself, not merely superseded. | | `DispatchTimeoutSeconds` | int | `10` | Central-side per-node deadline for one `NodeCommandService.Execute` call. Must be positive under `Grpc`; validated on driver-role nodes (covers the central pair, which is always fused). | | `AckTimeoutSeconds` | int | `10` | Node-side per-endpoint deadline for one `CentralCommandService.ReportApply` call. Must be positive under `Grpc`. Applied per endpoint walked in `CentralGrpcEndpoints`, not per ack, so a dead first endpoint costs this deadline before the second is tried. | | `ClusterPsks` | `Dictionary` | `{}` (case-insensitive keys) | **Central-side only** override map, keyed by `Cluster` id: `IMeshPskProvider` consults this map *before* the Secrets store when central builds a channel to (or verifies an ack from) a given cluster. Production leaves this empty and resolves `OT-MESH-PSK-{clusterId}` from Secrets instead; the map exists so the docker-dev rig and integration tests can supply literals via the environment (`MeshTransport__ClusterPsks__SITE-A`) without standing up a KEK. | +**Auth is per-cluster PSK bearer, both directions, fail-closed.** `GrpcPsk` (node-side) and `ClusterPsks` (central-side) are two ends of the same key, one per application `Cluster` — not a fleet-wide shared bearer, because this channel carries `RestartDriver`, `AlarmCommand` and deployment dispatch, and a single fleet key would let one compromised site node command every other site or forge acks fleet-wide. Both `MeshCommandAuthInterceptor` (node-side, gates `NodeCommandService`) and `CentralCommandAuthInterceptor` (central-side, gates `CentralCommandService`) reject every call when no key is configured for the relevant cluster, compare in constant time, and never log key material. **PSK seeding:** production form is a secret reference, `${secret:OT-MESH-PSK-}` (e.g. `${secret:OT-MESH-PSK-SITE-A}`), resolved by the Secrets store so it works on a driver-only node with no ConfigDb; the docker-dev rig instead supplies three literal dev keys via environment — `dev-mesh-psk-MAIN`, `dev-mesh-psk-SITE-A`, `dev-mesh-psk-SITE-B` — one per cluster, node-side via `MeshTransport__GrpcPsk` and central-side via `MeshTransport__ClusterPsks__`. Never commit a literal key. + **`ClusterPsks` / `OT-MESH-PSK-` and `SecretName`'s allow-list.** `SecretName` (`ZB.MOM.WW.Secrets.Abstractions`) lowercases the secret name and restricts it to `[a-z0-9._/-]` — a `Cluster` id that carries any character outside that set cannot be expressed as `OT-MESH-PSK-` at all, so `MeshPskProvider.TryGetPsk` fails closed for that cluster on every call that reaches the store (`SecretName`'s constructor throws, `MeshPskProvider.ResolveFromStore` catches it, logs a warning, and returns no key) — `MeshTransport:ClusterPsks` is the only path that still works for such an id. The rig's three ids (`MAIN`, `SITE-A`, `SITE-B`) are all within the allow-list and resolve normally either way. -The `Grpc` mode's node-side comm actors and central-side services are not yet wired to `Mode` — Phase 0 lands only the contract, mapper, options and the two auth interceptors (`MeshCommandAuthInterceptor` node-side, `CentralCommandAuthInterceptor` central-side), both dark. See the [design doc](plans/2026-08-06-clusterclient-to-grpc-design.md) and [tracking doc](plans/2026-08-06-clusterclient-to-grpc-tracking.md) for the remaining phases. +**Both command-plane gRPC services ride an existing listener — neither claims a port of its own.** `NodeCommandService` is mapped alongside `TelemetryStreamGrpcService` on the driver-role node's `Telemetry:GrpcListenPort` listener (the docker-dev rig's `:4056`) whenever that port is bound, deliberately NOT gated on `MeshTransport:Mode` — mapping it under `Dps` costs nothing (nothing dials it), whereas gating it would mean a running fleet needs a central restart before any node could ack after flipping the mode. `CentralCommandService` (the node→central ack ingress) is mapped alongside `DeploymentArtifactService` on the admin-role node's `ConfigServe:GrpcListenPort` listener (the rig's `:4055`), for the same reason — see [`docs/Telemetry.md`](Telemetry.md) for the shared driver-role listener and [§ `ConfigSource`/`ConfigServe`](#configsource--configserve-config-fetch-and-cache) for the shared admin-role one. + +On the docker-dev rig every node carries these keys already; the rig's compose default is `Grpc` (`OTOPCUA_MESH_MODE:-Grpc`, cutover commit `caf2eec6`) — the compiled class default in `MeshTransportOptions` stays `Dps`, so an unconfigured deployment outside the rig still needs `Mode=Grpc` set explicitly. Every rig node carries a `cluster-{ClusterId}` role (Phase 6), so `OTOPCUA_MESH_MODE=Dps` is not a usable override on this rig as shipped — `SplitTopologyTransportValidator` refuses to boot any of them on `Dps`. + +See the [ClusterClient→gRPC migration design](plans/2026-08-06-clusterclient-to-grpc-design.md), [implementation plan](plans/2026-08-06-clusterclient-to-grpc.md), [tracking doc](plans/2026-08-06-clusterclient-to-grpc-tracking.md) and [live gate](plans/2026-08-06-clusterclient-to-grpc-live-gate.md) for the migration's phase-by-phase history — those are historical records of the cutover and are not updated further by this sweep. ### `ConfigSource` / `ConfigServe` (config fetch-and-cache) @@ -284,7 +277,7 @@ non-empty `ApiKey`** — fail-closed, same as `ConfigServe`/`LocalDb:Replication `redundancy-state` (pair-local, built from `Cluster.State`, stays on DPS in both modes — see [`Redundancy.md`](Redundancy.md#command-transport-centralnode)), `fleet-status` (central-internal; `Fleet.razor` polls the Config DB, not any stream), and `deployment-acks` (already rides the Phase 2 -`ClusterClient` transport as a command-plane reply). See +gRPC command transport, `CentralCommandService.ReportApply`, as a command-plane reply). See [`docs/Telemetry.md`](Telemetry.md) for the full architecture, the reconnect/generation-stamped dialer story, and the rationale for the node-hosts/central-dials inversion. diff --git a/docs/Redundancy.md b/docs/Redundancy.md index e05d97a6..0312c30e 100644 --- a/docs/Redundancy.md +++ b/docs/Redundancy.md @@ -139,13 +139,16 @@ Consequences: - **`ClusterNodeAddressReconciler` is own-cluster-scoped** — an admin node can only reconcile `ClusterNode` rows belonging to its own cluster; it has no gossip visibility into another mesh to reconcile against. -- **`CentralCommunicationActor` creates one `ClusterClient` per application `Cluster`** and fans - commands across them, rather than one fleet-wide client. Central reaches a site mesh over - `ClusterClient` (commands), gRPC (telemetry — see `docs/Telemetry.md`), and `ConfigServe` (config - fetch — see [`ConfigSource`/`ConfigServe`](../CLAUDE.md#config-source-configsource--configserve)), - never over gossip. +- **`CentralCommunicationActor` dials each node directly over gRPC** — one `NodeCommandService.Execute` + call per enabled, non-maintenance `ClusterNode` row carrying a `GrpcPort` — rather than fanning + through any client/receptionist object (the Akka `ClusterClient` transport this used to run on is + **deleted**; see [ClusterClient→gRPC migration](plans/2026-08-06-clusterclient-to-grpc-design.md)). + Central reaches a site mesh over gRPC for commands (`MeshTransport`), telemetry (see + `docs/Telemetry.md`), and config fetch (`ConfigServe` — see + [`ConfigSource`/`ConfigServe`](../CLAUDE.md#config-source-configsource--configserve)), never over + gossip. - **`SplitTopologyTransportValidator` fails host start** on a node carrying a `cluster-{ClusterId}` - role unless it is also configured for the split-safe transports: `MeshTransport:Mode = ClusterClient`, + role unless it is also configured for the split-safe transports: `MeshTransport:Mode = Grpc`, `Telemetry:Mode = Grpc` (driver nodes), `TelemetryDial:Mode = Grpc` (admin nodes). A DPS-mode transport on a cluster-role node would silently try to gossip across a partition that no longer exists, so the validator fails closed instead. Legacy/non-cluster-role nodes are exempt. @@ -538,27 +541,34 @@ See [ScriptedAlarms.md](ScriptedAlarms.md) and [AlarmTracking.md](AlarmTracking. Everything above rides one Akka mesh: central publishes on DistributedPubSub, every node subscribes, and the two ends must be members of the same cluster. Per-cluster mesh **Phase 2** +(cross-mesh cutover: [ClusterClient→gRPC migration](plans/2026-08-06-clusterclient-to-grpc-design.md)) adds a second transport under `MeshTransport:Mode` (see [Configuration.md § `MeshTransport`](Configuration.md#meshtransport-centralnode-command-transport)) that does not need shared membership — the prerequisite for splitting the fleet into one mesh per -application `Cluster`. +application `Cluster`. The Akka `ClusterClient` transport that originally filled this second slot is +**deleted**; `Grpc` is its replacement, not an alternative alongside it. -| | `Dps` (default) | `ClusterClient` | +| | `Dps` (default, pair-local) | `Grpc` (cross-mesh) | |---|---|---| -| Central → node commands | `Publish` on `deployments` / `driver-control` / `alarm-commands` | `ClusterClient.SendToAll` to `/user/node-communication`, re-emitted on the node's **local** `EventStream` | -| Node → central acks | `Publish` on `deployment-acks` | `ClusterClient.Send` to `/user/central-communication`, forwarded to the deploy-coordinator singleton | +| Central → node commands | `Publish` on `deployments` / `driver-control` / `alarm-commands` | `NodeCommandService.Execute`, one unary call per node dialled directly from `ClusterNode.Host`+`GrpcPort`; decoded and handed to the node's `NodeCommunicationActor`, which re-emits it on the node's **local** `EventStream` exactly as the DPS path does | +| Node → central acks | `Publish` on `deployment-acks` | `CentralCommandService.ReportApply`, walking `MeshTransport:CentralGrpcEndpoints` in order and stopping at the first success, forwarded to the deploy-coordinator singleton | | Requires shared cluster membership | **yes** | no | +| Auth | none (mesh membership is the boundary) | per-cluster PSK bearer, both directions, fail-closed | Two properties are load-bearing and easy to break: -- **Both comm actors are registered per node, NOT as cluster singletons.** A `ClusterClient` rotates - across its contact points and must find a live comm actor at whichever node answers. As a - singleton the boundary would be reachable through exactly one node, and rotation would fail - silently against the others — acks landing or not depending on where the client happened to - settle. `akka.cluster.client.receptionist.role` is deliberately empty for the same reason. +- **The gRPC dialer fans node by node, not through any shared client or receptionist object.** There + is no rotation and no "whichever node answers" — each command is dialled at the specific node's own + endpoint, so a node left out of the `ClusterNode` dispatch set (no `GrpcPort`, disabled, or in + `MaintenanceMode`) silently keeps its old configuration while the deployment still seals green + around it, the same failure shape `ClusterClient`'s `SendToAll` had. - **Inbound commands go to the node-local `EventStream`, never back onto a DPS topic.** DPS is - mesh-wide and central `SendToAll`s to *every* node, so a DPS re-publish would deliver N copies of + mesh-wide and central fans out to *every* node, so a DPS re-publish would deliver N copies of every command to every subscriber. +- **Both transports are fire-and-forget, with no retry or queue.** A failed gRPC dispatch is logged + (naming the node and the gRPC status) and dropped in the dispatch second — the deploy path's + 2-minute apply deadline and a node's boot-time replay (`DriverHostActor.Bootstrap()` re-applying an + orphan `Applying` row) are the only recovery mechanisms, unchanged from the DPS/`ClusterClient` era. Phase 2 changes no redundancy semantics: the Primary gate, ServiceLevel, and the singleton layout are untouched, and `redundancy-state` itself stays on DPS (it is bidirectional and built from diff --git a/docs/Telemetry.md b/docs/Telemetry.md index 1b12a764..0d90aebe 100644 --- a/docs/Telemetry.md +++ b/docs/Telemetry.md @@ -39,10 +39,20 @@ mirrors the inversion ScadaBridge already uses for the same problem (its `SiteSt A fused `admin,driver` node both hosts (as driver) and dials (as admin) — it dials itself plus its redundant pair peer, the same way central dials site nodes. +**This same driver-role listener also hosts the mesh gRPC command-plane ingress.** Whenever +`Telemetry:GrpcListenPort > 0`, the node maps both `TelemetryStreamGrpcService` (this stream) and +`NodeCommandService` (central→node commands under `MeshTransport:Mode=Grpc` — see +[`docs/Configuration.md` § `MeshTransport`](Configuration.md#meshtransport-centralnode-command-transport)) +on the one port. They share it rather than each claiming its own, since both are dialled by the same +central admin nodes and a second cleartext h2c port would buy nothing but another firewall rule. +`NodeCommandService` is gated by its own `MeshCommandAuthInterceptor`/`GrpcPsk` bearer, independent +of this stream's `Telemetry:ApiKey`. + Central discovers driver-node telemetry endpoints from the `ClusterNode` table (`Host` + `GrpcPort`, the latter added in Phase 1 for exactly this purpose) — the same DB-sourced discovery -Phase 1's ack set and Phase 2's `ClusterClient` contact set already use. No shared gossip -membership is required to find or dial a node. +Phase 1's ack set and Phase 2's gRPC command dial already use (see +[`docs/Configuration.md` § `MeshTransport`](Configuration.md#meshtransport-centralnode-command-transport)). +No shared gossip membership is required to find or dial a node. ## The dark switch @@ -139,8 +149,8 @@ each for a distinct, settled reason: nothing here for a per-node gRPC stream to carry. Revisit in Phase 6, once central genuinely loses gossip visibility of site nodes and needs another way to know a pair's membership. - **`deployment-acks`** — already migrated, but onto a different transport: it rides the Phase 2 - `ClusterClient` transport (`MeshTransport:Mode=ClusterClient`) as a command-plane reply, not an - observability broadcast. It was never a Phase 5 candidate. + gRPC command transport (`MeshTransport:Mode=Grpc`, `CentralCommandService.ReportApply`) as a + command-plane reply, not an observability broadcast. It was never a Phase 5 candidate. See [`docs/Redundancy.md`](Redundancy.md#command-transport-centralnode) for how `redundancy-state` and the command transports fit together. diff --git a/docs/v2/config-db-schema.md b/docs/v2/config-db-schema.md index 01bfde17..c4739540 100644 --- a/docs/v2/config-db-schema.md +++ b/docs/v2/config-db-schema.md @@ -177,7 +177,11 @@ from `Cluster:Port` in its own appsettings; these columns duplicate that value f cannot see the node's config. Today central shares a gossip ring with every node and does not need them — [per-cluster mesh Phase 2](../plans/2026-07-21-per-cluster-mesh-design.md) splits the fleet into one mesh per `Cluster`, after which `Host` + `AkkaPort` is how central builds its ClusterClient -contact points, and `GrpcPort` is the Phase 5 telemetry stream. +contact points (**retired**: the Akka `ClusterClient` transport was deleted by the +[ClusterClient→gRPC migration](../plans/2026-08-06-clusterclient-to-grpc-design.md) — central now +dials `NodeCommandService` at `Host`+`GrpcPort` instead, and `AkkaPort` is Akka remoting only), and +`GrpcPort` is the Phase 5 telemetry stream **and**, since that migration, the Phase 2 gRPC command +dial target too — see [`Configuration.md` § `MeshTransport`](../Configuration.md#meshtransport-centralnode-command-transport). `AkkaPort` is `NOT NULL DEFAULT 4053` because every node listens on a remoting port — `0` is never a truthful value, and rows predating the column must migrate to something real. `GrpcPort` is nullable From 8e25ed55f6a83d8364d43dd1d73625324e637f17 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 16:30:47 -0400 Subject: [PATCH 10/11] docs: fix two stale mesh-program claims found in review (Phase 5 gate passed; Phase 6 ended fleet-wide gossip) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the ClusterClient-deletion docs sweep (406b1ae5) flagged two pre-existing staleness items in the swept files, unrelated to ClusterClient but caught while touching the same passages: - docs/Telemetry.md: the Phase 5 telemetry section's Status line still said "code-complete ... live gate has not yet run" — the gate PASSED 2026-07-23 (merge 35552a96), already recorded in CLAUDE.md. Updated to match. - docs/v2/config-db-schema.md: the AkkaPort/GrpcPort note asserted in the present tense that "central shares a gossip ring with every node," which Phase 6 (2026-07-24) ended — central now runs one 2-node Akka mesh per application Cluster and reaches a site pair only over gRPC. Reworded the premise to past tense so it no longer contradicts the ClusterClient-deleted parenthetical sitting right after it. docs-only; dotnet build ZB.MOM.WW.OtOpcUa.slnx stays green (0 errors). Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- docs/Telemetry.md | 4 ++-- docs/v2/config-db-schema.md | 18 +++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/Telemetry.md b/docs/Telemetry.md index 0d90aebe..c7ce24f9 100644 --- a/docs/Telemetry.md +++ b/docs/Telemetry.md @@ -259,7 +259,7 @@ otherwise), and `Grpc` mode on either side requires a non-empty `ApiKey` — bot naturally node-sourced and fleet-wide-fanned, so putting central in the client role — dialing out to each node by its `ClusterNode`-recorded address — is what lets telemetry survive the mesh split cleanly: central never needs cluster membership with a site's mesh to keep watching it. -- **Status:** code-complete on `feat/mesh-phase5`; the live gate (flip the docker-dev rig to `Grpc`, +- **Status:** merged to `master` (`35552a96`); the live gate (flip the docker-dev rig to `Grpc`, confirm all panels stay green with the DPS telemetry bridges unspawned, kill-and-reconnect a site - node) has not yet run. See `docs/plans/2026-07-22-per-cluster-mesh-program.md` § Phase 5 and + node) PASSED 2026-07-23. See `docs/plans/2026-07-22-per-cluster-mesh-program.md` § Phase 5 and `docs/plans/2026-07-23-mesh-phase5-grpc-telemetry-stream.md` for the implementation plan. diff --git a/docs/v2/config-db-schema.md b/docs/v2/config-db-schema.md index c4739540..736117a6 100644 --- a/docs/v2/config-db-schema.md +++ b/docs/v2/config-db-schema.md @@ -174,14 +174,18 @@ CREATE UNIQUE INDEX UX_ClusterNode_Primary_Per_Cluster These are **the addresses central dials**, not the node's own binding configuration. The node binds from `Cluster:Port` in its own appsettings; these columns duplicate that value for a reader that -cannot see the node's config. Today central shares a gossip ring with every node and does not need -them — [per-cluster mesh Phase 2](../plans/2026-07-21-per-cluster-mesh-design.md) splits the fleet -into one mesh per `Cluster`, after which `Host` + `AkkaPort` is how central builds its ClusterClient -contact points (**retired**: the Akka `ClusterClient` transport was deleted by the +cannot see the node's config. When this column was added central still shared one gossip ring with +every node and did not strictly need them; that premise ended at +[per-cluster mesh Phase 6](../plans/2026-07-24-mesh-phase6-partition-and-colocation.md) +(2026-07-24), which split the fleet into one 2-node Akka mesh per application `Cluster` — central no +longer shares gossip with a site pair at all, and reaches it only over gRPC (and, before Phase 6 +finished landing, briefly over the now-deleted Akka `ClusterClient`). `Host` + `AkkaPort` was how +central would have built `ClusterClient` contact points; that transport was deleted by the [ClusterClient→gRPC migration](../plans/2026-08-06-clusterclient-to-grpc-design.md) — central now -dials `NodeCommandService` at `Host`+`GrpcPort` instead, and `AkkaPort` is Akka remoting only), and -`GrpcPort` is the Phase 5 telemetry stream **and**, since that migration, the Phase 2 gRPC command -dial target too — see [`Configuration.md` § `MeshTransport`](../Configuration.md#meshtransport-centralnode-command-transport). +dials `NodeCommandService` at `Host`+`GrpcPort` instead, and `AkkaPort` is Akka remoting only (still +needed pair-locally, within a `Cluster`'s own 2-node mesh). `GrpcPort` is the Phase 5 telemetry +stream target **and**, since that migration, the Phase 2 gRPC command dial target too — see +[`Configuration.md` § `MeshTransport`](../Configuration.md#meshtransport-centralnode-command-transport). `AkkaPort` is `NOT NULL DEFAULT 4053` because every node listens on a remoting port — `0` is never a truthful value, and rows predating the column must migrate to something real. `GrpcPort` is nullable From f8a43dfe96b04a1cadb2213e3a320560dd8ef666 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 16:56:17 -0400 Subject: [PATCH 11/11] =?UTF-8?q?docs(plans):=20Phase=203=20COMPLETE=20at?= =?UTF-8?q?=20DoD=20=E2=80=94=20deletion=20review-approved,=20grep-gates?= =?UTF-8?q?=20clean,=20full=20suite=209326/0=20green=20on=20branch=20tip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 3.1: 7 ordered deletion steps + MeshPaths review fix, spec+code reviews closed. Task 3.2: docs sweep + accuracy review, scadaproj index pushed on its main. DoD full-suite: run 3 exit 0 (50 suites, 9326 passed); runs 1-2 each hit one pre-existing unrelated timing flake (3 distinct tests, all untouched by the branch, 9/9 green in isolation). Also records the exit-code-capture trap found in run 1 and its retroactive caveat on Phase 2's check-8 wording. Merge to master is user-owned — STOPPED at the Phase 3 DoD. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1 --- .../2026-08-06-clusterclient-to-grpc-tracking.md | 2 +- .../2026-08-06-clusterclient-to-grpc.md.tasks.json | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md b/docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md index 30d37b03..e6c555ce 100644 --- a/docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md +++ b/docs/plans/2026-08-06-clusterclient-to-grpc-tracking.md @@ -26,7 +26,7 @@ posture). | 1A | Central→node command plane (`NodeCommandService` on the Telemetry listener; `MeshGrpcCommandDialer`; third mode branch in `CentralCommunicationActor`) — **worktree A** | `feat/grpc-node-command` | **COMPLETE @ `e54883a8` — gate re-run PASSED 2026-08-06.** Run 1 @ `fbf7b708` FAILED on defect 4 (no admin node could boot, mode-independent); fixed by `c0475f2d` + `e54883a8` and re-gated. 1A.1 ✅ `64774652` + 1A.2 ✅ `6a862011` (+fixes `48dc2120`/`86633f14`) all reviewed spec ✅ + code ✅. **MERGED to `master`** @ `b360239e` (2026-08-06) | `b360239e` | **PASS 8/8** (check 3's AdminUI half closed by composition — the control is structurally unreachable, see carry-forwards) | | 1B | Node→central ack plane (`CentralCommandService` on the ConfigServe listener; `GrpcApplyAckSender`; third branch in `NodeCommunicationActor`) — **worktree B**, rebased on merged 1A | `feat/grpc-central-ack` | **MERGED to `master`** @ `7203d135` (2026-08-06, branch tip `cf229a95` incl. the phases 2-4 plan update); 1B.1 ✅ + 1B.2 ✅ (Opus: deadline Critical found+fixed) + gate **PASS 7/7** incl. the 4b hang drill (~10 s failover = exactly one AckTimeoutSeconds — live proof of the fix); at-least-once duplicate absorbed idempotently exactly as traced | `7203d135` | **PASS 7/7** | | 2 | **COMPLETE — gate PASS 8/8 + baseline + PSK negatives** (2026-08-06, all on `master`, all review chains closed). Task 2.0 ✅ (`a94eb8a3`+`d7c9ead6`, small chain: one Important fixed — non-vacuous once-per-incarnation pin — then APPROVED; healthy racing boot = ZERO Warn/Error, proven live on both centrals 4/4 boots). Register-item-3 role-scoping pin ✅ (`2e06dfe5`+`027f0403`, review found+fixed a Critical phantom-block false pass in the scan — braceless `if (hasAdmin)` at Program.cs:146). Baseline REPRODUCED pre-flip (`1bb3a937`): cross-mesh `:4053` open unauthenticated; the site-b drop is actively suppressed into SAMPLED deadLetters INF (456/467 unlogged) with the deploy deadline as sole witness. Cutover `caf2eec6` (rig default → `Grpc`, committed). Gate: deploy e2e all-gRPC sealed 6/6; command matrix live incl. real driver restart; kill-site-a-1 → per-node WRN naming node+status, exactly 1 attempt, honest deadline, boot-replay-only recovery; kill-central-1 → ack ordered-walk failover logged per node, reconvergence 6/6; site-b-pair drill → BOTH failures logged in the dispatch second (the baseline contrast); clusterclient/receptionist lines = 0 on all six through the whole gate; 135-min soak 14/14 cycles, denied=0/unauth=0, review items explained (historian-sidecar-absent DBG noise; 14×2 known per-deploy authoring WRNs); **full suite exit 0 on the cutover SHA, run against the live rig** | `caf2eec6` (cutover) + gate records `1bb3a937`/`bcc91268`/`eaf14e01`+final | **PASS 8/8** | -| 3 | Delete ClusterClient (code/HOCON/config/tests); SplitTopology requires `Grpc`; grep-gates; docs sweep incl. scadaproj index | `feat/grpc-phase3-deletion` | NOT STARTED | — | — | +| 3 | Delete ClusterClient (code/HOCON/config/tests); SplitTopology requires `Grpc`; grep-gates; docs sweep incl. scadaproj index | `feat/grpc-phase3-deletion` | **COMPLETE @ DoD — awaiting the user's merge decision** (2026-08-06, branch tip `8e25ed55`, 10 commits off `master @ 55104526`, tree clean). Task 3.1 ✅ high-risk chain: 7 ordered steps `0db40a5e`/`ab5b958d`/`5d7ecdf7`/`d67a7e5d`/`85ca86e9`/`af3b0c31`/`4d2dfb5b`; spec review ✅ (all KEEPs verified: `LoadContactsFromDb`, DPS+frame-size canary, `deployment-acks` DPS ingress, Kestrel block, one-ctor interceptors, no retries, proto untouched; `MeshGrpcBoundaryTests`' no-association assert verified falsifiable); code review ✅ Approved, one Important fixed (`cd713557` — `MeshPaths` full-path constants were dead-in-production + doc claimed a wire-contract role they no longer have; collapsed to `*Name` constants). Retired-mode fail-closed: `Mode=ClusterClient` now FAILS validation naming `MeshTransport:Mode` + allowed values (pinned); `SplitTopologyTransportValidator` requires exactly `Grpc`. Task 3.2 ✅ docs sweep `406b1ae5` + review-fix `8e25ed55` (Configuration/Redundancy/Telemetry/config-db-schema/CLAUDE.md; deferment.md had no mention); docs-accuracy review ✅ Approved — every config key/port/behavior claim verified against source, two pre-existing Minor stalenesses fixed (Telemetry.md Phase 5 gate status; config-db-schema Phase 6 gossip premise). scadaproj index updated on its `main` and PUSHED (`0e416ca`, merge-state-accurate: Phase 3 recorded as pending merge). Grep-gates: `CentralContactPoints` in src/tests/docker-dev = 0; `clusterclient\|receptionist` = historical/fail-closed/retirement mentions only (classified by both reviewers). **DoD full-suite: run 3 = `dotnet test` exit 0, 50/50 suites, 9,326 passed / 0 failed / 57 skipped, on `8e25ed55`.** Runs 1–2 each had ONE unrelated single-test failure (three distinct tests across the two runs: `RouteNativeAlarmAck_on_non_primary_is_dropped`, `Rediscovery_flag_persists_on_subsequent_health_publishes`, `LoggerLifecycleTests.ConfigureLogging_DisposesPreviousLogger…`) — all in files the 55-file branch diff never touches, all 3/3 green in isolation (9/9 total): pre-existing under-load timing flakes, not Phase 3 regressions. ⚠️ Process find: run 1's "exit 0" captured `tail`'s exit code, not `dotnet test`'s (`cmd \| tail; $?` trap) — fixed for runs 2–3; **Phase 2's check-8 "full suite exit 0" used the same pipeline shape and its log did not survive, so that exit-code claim is unverifiable in hindsight** (its per-suite pass lines were observed at the time; Phase 4's terminal run on the merge SHA, with correct capture, supersedes it) | — (merge pending) | n/a (Phase 4 is the terminal live gate) | | 4 | Terminal live gate on the post-deletion image (8 checks, provenance-guarded) | (on `master`) | NOT STARTED | — | PENDING | **Execution rules recap:** phase order 0 → (1A ∥ 1B) → 2 → 3 → 4; 1A∥1B only in separate git diff --git a/docs/plans/2026-08-06-clusterclient-to-grpc.md.tasks.json b/docs/plans/2026-08-06-clusterclient-to-grpc.md.tasks.json index 63008cbf..3df23897 100644 --- a/docs/plans/2026-08-06-clusterclient-to-grpc.md.tasks.json +++ b/docs/plans/2026-08-06-clusterclient-to-grpc.md.tasks.json @@ -211,10 +211,13 @@ "branch": "feat/grpc-phase3-deletion", "classification": "high-risk", "parallelizableWith": [], - "status": "pending", + "status": "completed", "blockedBy": [ 12 - ] + ], + "commit": "0db40a5e..4d2dfb5b (7 steps) + review fix cd713557", + "review": "spec ✅ (all KEEPs verified, boundary assert falsifiable) + code ✅ Approved (1 Important fixed: dead MeshPaths full-path constants collapsed)", + "note": "Grep-gates clean: CentralContactPoints=0; clusterclient|receptionist = historical/fail-closed mentions only. MeshClusterClientBoundaryTests deleted in step 1 (compile ripple), replacement MeshGrpcBoundaryTests landed step 7 as planned; MeshRouterDispatchTests needed no change (mode-agnostic); MeshCommActorPathTests reshaped not retired." }, { "id": 14, @@ -223,10 +226,12 @@ "branch": "feat/grpc-phase3-deletion", "classification": "small", "parallelizableWith": [], - "status": "pending", + "status": "completed", "blockedBy": [ 13 - ] + ], + "commit": "406b1ae5 + review fixes 8e25ed55; scadaproj index 0e416ca on scadaproj main, pushed", + "review": "docs-accuracy ✅ Approved (every key/port/behavior/merge-state claim verified against source; 2 pre-existing Minor stalenesses fixed)" }, { "id": 15,