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 1728ff5d..6a7ec7b1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/CentralCommandGrpcService.cs @@ -97,11 +97,17 @@ public sealed class CentralCommandGrpcService : GeneratedServiceBase if (!_registry.TryGet(out var actor)) { + // Peer and correlation id are carried here for the same reason the decode-failure warning + // above carries them: these two rejections are the only ways an ack fails to reach the + // coordinator, and an operator has to correlate either against the node's own log. The + // decode has already succeeded on this path, so the DECODED correlation id is used — + // unlike above, where the raw wire string is the only value guaranteed readable. _log.LogWarning( - "Received an ApplyAck for {DeploymentId} from {NodeId} before the central " - + "communication actor was registered on this node; refusing with Unavailable so the " - + "caller retries rather than recording a failure against a node that applied", - ack.DeploymentId, ack.NodeId); + "Received an ApplyAck for {DeploymentId} (correlation '{CorrelationId}') from {NodeId} " + + "at {Peer} before the central communication actor was registered on this node; " + + "refusing with Unavailable so the caller retries rather than recording a failure " + + "against a node that applied", + ack.DeploymentId, ack.CorrelationId, ack.NodeId, context.Peer); throw new RpcException(new Status(StatusCode.Unavailable, "central command actor not ready")); } 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 e3eab46b..294897b2 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs @@ -454,6 +454,29 @@ public class MeshTransportOptionsValidatorTests "driver", "cluster-SITE-A").Succeeded.ShouldBeTrue(); } + [Fact] + public void Grpc_fused_admin_and_driver_node_must_satisfy_BOTH_surfaces() + { + // The central pair is fused (admin,driver,cluster-MAIN), so it is the one node shape that owes + // both halves at once — and the two blocks are independent `if`s, not an `if`/`else if`. Were + // they chained, a fused node would be graded on the driver surface only and its missing + // ConfigServe listener would pass validation, leaving central with no place for any node's ack + // to land. Asserted from BOTH sides so a future chaining in either order is caught. + var missingNodeSide = new MeshTransportOptions { Mode = MeshTransportOptions.ModeGrpc }; + var withPort = Validate(missingNodeSide, ConfigServePort("4055"), "driver", "admin", "cluster-MAIN"); + withPort.Failed.ShouldBeTrue(); + withPort.FailureMessage.ShouldContain("MeshTransport:GrpcPsk"); + withPort.FailureMessage.ShouldContain("MeshTransport:CentralGrpcEndpoints"); + + var missingAdminSide = Validate(WellFormedGrpc(), "driver", "admin", "cluster-MAIN"); + missingAdminSide.Failed.ShouldBeTrue(); + missingAdminSide.FailureMessage.ShouldContain("ConfigServe:GrpcListenPort"); + + // And the fully-configured fused node passes — the rule is "both", not "neither". + Validate(WellFormedGrpc(), ConfigServePort("4055"), "driver", "admin", "cluster-MAIN") + .Succeeded.ShouldBeTrue(); + } + [Fact] public void ClusterPsks_defaults_to_an_empty_case_insensitive_map() { 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 8fd3682c..b2242985 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 @@ -182,6 +182,26 @@ public sealed class CentralCommandGrpcServiceTests : TestKit .ShouldBeTrue("the rejection log must carry the raw correlation id"); } + [Fact] + public async Task A_not_ready_rejection_logs_the_peer_and_the_correlation_id() + { + // Diagnostic symmetry with the decode-failure warning above. The two rejections are the only + // two ways an ack fails to reach the coordinator, and an operator reading the central log has + // to correlate BOTH against the node's own log — which needs the correlation id — and against + // which node was calling, which needs the peer. Decoding has already succeeded on this path, + // so the decoded correlation id costs nothing to include. + var logger = new CapturingLogger(); + var request = WellFormed(); + + await Should.ThrowAsync(() => + new CentralCommandGrpcService(new ActorRegistry(), logger) + .ReportApply(request, new FakeServerCallContext())); + + var warning = logger.Entries.Single(e => e.Level == LogLevel.Warning); + warning.Message.ShouldContain("ipv4:127.0.0.1:0"); + warning.Message.ShouldContain(request.CorrelationId); + } + /// Test logger that captures all log entries. /// The category type. private sealed class CapturingLogger : ILogger