diff --git a/deferment.md b/deferment.md index b2801ebc..4c29ad33 100644 --- a/deferment.md +++ b/deferment.md @@ -77,7 +77,10 @@ Every gap below is in the **authoring/dispatch layer, not driver runtime code**. - `TagConfigEditorMap.cs:25-28` + `TagConfigValidator.cs:27-28` — "`DriverTypeNames.Sql` deliberately absent until Task 11"; the constant exists (`DriverTypeNames.cs:57`) and the values are identical, so behaviour is correct. - `RawDriverTypeDialog.razor:2-4, 59-60` — "its factory lands in Wave C". - `CsvColumnMap.cs:322-324` — hardcodes `CalculationDriverType = "Calculation"` "Not yet a `DriverTypeNames` constant"; it has been one since `DriverTypeNames.cs:54`. -- `SqlDriver.cs:218-220` — justifies not re-parsing config on the premise "the factory builds a fresh instance", **disproved** by `DriverInstanceActor.cs:316` + `DriverHostActor.cs:2565`. Should be corrected alongside #516. +- ~~`SqlDriver.cs:218-220` — justifies not re-parsing config on the premise "the factory builds a fresh + instance", **disproved** by `DriverInstanceActor.cs:316` + `DriverHostActor.cs:2565`.~~ ✅ **Fixed** — + the comment now says the premise was false when written and is true *now* because a config change + respawns. See §9. --- @@ -350,7 +353,8 @@ appear in §7.1 as well. 2. ~~**#518 + #507 together** — neither fix is observable alone.~~ ✅ **Done.** The framing was right that they had to be handled together, but wrong about the resolution: **#507 was not revivable**, so it was deleted rather than fixed. See §9. -3. **#516** — silent config discard on 5 drivers; then re-scope #489, which it partly subsumes. +3. ~~**#516** — silent config discard on 5 drivers; then re-scope #489, which it partly subsumes.~~ + ✅ **Done.** See §9. #489 is still open and should now be re-scoped. 4. **G-4** — extend the existing picker-parity test to `DriverConfigModal` + `DeviceModal`; it would have caught G-1 and G-2 for free, and this class has now recurred twice. 5. **Bookkeeping sweep** — mark the 8 stale `.tasks.json` files complete so the 19-task AdminUI plan @@ -369,7 +373,7 @@ the work, so this register never disagrees with the tree. |---|---|---| | 1. ACL enforcement decision | ✅ **Done** — decided *make non-enforcement explicit*; Gitea **#520** tracks the wire-up | `d1e88dc4` | | 2. #518 + #507 | ✅ **Done** — `IRediscoverable` consumed as a re-browse prompt; #507's injection path **deleted**; `IHostConnectivityProbe` half split to Gitea **#521** | `09a401b8`, `97c9f4b4` | -| 3. #516 config discard | ⏳ Not started — decided *both* (per-driver re-parse **and** seam respawn) | — | +| 3. #516 config discard | ✅ **Done** — seam respawn + re-parse on 3 drivers; Sql/FOCAS deliberately respawn-only | `2dc19f30` | | 4. G-4 dispatch-map parity | ⏳ Not started | — | | 5. Bookkeeping sweep | ⏳ Not started | — | | 6. Tier truth | ⏳ Not started — decided *document the truth, defer the code decision* | — | @@ -461,6 +465,53 @@ Two traps worth keeping: `DriverHostStatus` table was re-created deliberately in the v3 initial migration, so deleting on inference would be wrong. Split to Gitea **#521** with both options costed. +### 2026-07-27 — §8.3 #516 driver config edits silently discarded + +Decision: **both** — per-driver re-parse *and* the seam respawn. Reading the drivers split the +"per-driver" half in two, which the register did not anticipate: + +- **Re-parse in place** (`Modbus`, `AbLegacy`, `OpcUaClient`) — `ParseOptions` extracted from each + factory, called from `InitializeAsync` behind a `HasConfigBody` guard so `"{}"` still keeps the + constructor options. +- **Respawn-only, deliberately NOT re-parsed** (`Sql`, `FOCAS`) — each builds **more than options** + from config: Sql's `ISqlDialect` + resolved connection string, FOCAS's client-factory backend, both + injected at construction. Adopting new options alone would run a **new tag set against an old + connection**. Half a re-parse is worse than none. Their doc-comments now say so. + +**The seam respawn is the load-bearing half.** `DriverSpawnPlanner` now routes a changed +`DriverConfig` to `ToStop` + `ToSpawn`, making the factory the single parse authority. +`ToApplyDelta` is consequently always empty from the reconcile path. This **reverses the deliberate +decision documented at `DriverSpawnPlan.cs:49-50`** ("a pure DriverConfig change stays an in-place +delta — no reconnect"). That reasoning was right about resilience and wrong about the driver; the +accepted price is a reconnect on every config edit. + +Two seals removed from `ApplyChildDelta`: it overwrote the cached `Spec` **synchronously, before the +child had dequeued the message**, so the host immediately believed the new config was live and the +next reconcile computed no delta — sealing the drift permanently; and it `Tell`d with no +`Receive` registered, so a failed reinit (including Galaxy's deliberate +`NotSupportedException`) dead-lettered. + +**A new visibility gap this change exposed, not created:** a factory throw is a *config* error +(`TryCreate` is pure parsing; device I/O happens later), and `SpawnChild` catches it and silently +substitutes a stub. Previously only a brand-new driver could hit it; now an ordinary config edit can. +Raised from `Warning` to `Error` with an actionable message. It still does **not** fail the +deployment — making it do so would let one malformed driver block a fleet deploy, so that is a +deliberate follow-up rather than a drive-by change. + +**Tests.** Every pre-existing reinit test in the five suites passes `"{}"` — precisely the input a +guarded re-parser treats as "keep the constructor options", so they were blind to this defect *by +construction*. The new tests pass **changed** JSON. The Modbus one was verified falsifiable by +deleting the re-parse line (goes red). Two existing tests asserted the old behaviour and were +rewritten: `DriverSpawnPlannerTests` (two cases), and +`DriverHostActorUnreadableArtifactTests.Dropping_a_drivers_last_tag_does_clear_its_subscription` — +a **positive control** for a sibling absence assertion, whose observable moved from `UnsubscribeAsync` +to `ShutdownAsync` because the teardown now happens by stopping the child rather than emptying its +desired set. Same event, different route; the control still calibrates the settle window. + +Full-solution run: only pre-existing environment failures remain — `Host.IntegrationTests` (3, +verified identical on the pre-change tree) and `Driver.AbLegacy.IntegrationTests` (4, docker +fixture-gated, and notably these **fail rather than skip**, unlike every other fixture-gated suite). + --- *Generated 2026-07-27 against `master` @ `90bdaa44` by five parallel source-verification agents. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 94add6bd..faadaafc 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -13,7 +13,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, IDisposable, IAsyncDisposable { - private readonly AbLegacyDriverOptions _options; + /// Mutable so can adopt a re-parsed config on reinitialize + /// (#516). Only ever written on the init path, before any reader/session uses it. + private AbLegacyDriverOptions _options; private readonly string _driverInstanceId; private readonly IAbLegacyTagFactory _tagFactory; private readonly ILogger _logger; @@ -94,12 +96,28 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover /// public string DriverType => "AbLegacy"; + /// True when the supplied DriverConfig JSON carries a real body. The bootstrapper always + /// passes a populated document; some unit tests pass "{}" or an empty string to exercise + /// lifecycle shape without a config — those keep the constructor-supplied options. + private static bool HasConfigBody(string? driverConfigJson) + { + if (string.IsNullOrWhiteSpace(driverConfigJson)) return false; + var trimmed = driverConfigJson.Trim(); + return trimmed is not "{}" and not "[]"; + } + /// public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { _health = new DriverHealth(DriverState.Initializing, null, null); try { + // #516: re-parse the supplied DriverConfig so a config change delivered through the IDriver + // contract is honoured. Without this the driver keeps serving the options it was CONSTRUCTED + // with, so an operator's edit is discarded while the deployment still seals green. + if (HasConfigBody(driverConfigJson)) + _options = AbLegacyDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + foreach (var device in _options.Devices) { var addr = AbLegacyHostAddress.TryParse(device.HostAddress) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs index fdd3dd57..cf55b578 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriverFactoryExtensions.cs @@ -49,6 +49,23 @@ public static class AbLegacyDriverFactoryExtensions /// Optional logger factory for the driver instance. /// A configured instance. internal static AbLegacyDriver CreateInstance(string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) + { + return new AbLegacyDriver( + ParseOptions(driverInstanceId, driverConfigJson), driverInstanceId, + tagFactory: null, + logger: loggerFactory?.CreateLogger()); + } + + /// + /// Parses an AB Legacy DriverConfig JSON document into typed options. Extracted so + /// can re-parse a CHANGED config on reinitialize + /// (Gitea #516) instead of serving the options it was constructed with — which silently discarded + /// every operator edit while the deployment still sealed green. + /// + /// The unique driver instance identifier. + /// The driver configuration as a JSON string. + /// The parsed . + internal static AbLegacyDriverOptions ParseOptions(string driverInstanceId, string driverConfigJson) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); @@ -82,10 +99,7 @@ public static class AbLegacyDriverFactoryExtensions Timeout = PositiveTimeoutOrDefault(dto.TimeoutMs, 2_000), }; - return new AbLegacyDriver( - options, driverInstanceId, - tagFactory: null, - logger: loggerFactory?.CreateLogger()); + return options; } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 1c3b654c..0d834c59 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -100,7 +100,19 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// public string DriverType => "FOCAS"; - /// + /// + /// Opens the configured CNC handles and builds the authored tag table. + /// is deliberately not re-parsed here, unlike + /// Modbus / AbLegacy / OpcUaClient (Gitea #516). This driver builds more than options from config — + /// its IFocasClientFactory is selected from the Backend key and injected at + /// construction — so adopting a new FocasDriverOptions alone would run a NEW device/tag set + /// against the OLD backend. Half a re-parse is worse than none. + /// A config change is covered by the stop + respawn in DriverSpawnPlanner, which + /// rebuilds options and backend together through the factory. + /// + /// The driver configuration JSON (see remarks — not re-parsed). + /// Cancellation token for the operation. + /// A completed task. public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { Volatile.Write(ref _health, new DriverHealth(DriverState.Initializing, null, null)); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 12854f7c..3d972795 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -23,7 +23,9 @@ public sealed class ModbusDriver { // ---- instance fields (grouped at top for auditability) ---- - private readonly ModbusDriverOptions _options; + /// Mutable so can adopt a re-parsed config on reinitialize + /// (#516). Only ever written on the init path, before any reader/transport uses it. + private ModbusDriverOptions _options; private readonly Func _transportFactory; private readonly string _driverInstanceId; private readonly ILogger _logger; @@ -188,12 +190,29 @@ public sealed class ModbusDriver /// public string DriverType => "Modbus"; + /// True when the supplied DriverConfig JSON carries a real body. The bootstrapper always + /// passes a populated document; some unit tests pass "{}" or an empty string to exercise + /// lifecycle shape without a config — those keep the constructor-supplied options. + private static bool HasConfigBody(string? driverConfigJson) + { + if (string.IsNullOrWhiteSpace(driverConfigJson)) return false; + var trimmed = driverConfigJson.Trim(); + return trimmed is not "{}" and not "[]"; + } + /// public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { WriteHealth(new DriverHealth(DriverState.Initializing, null, null)); try { + // #516: re-parse the supplied DriverConfig so a config change delivered through the IDriver + // contract is honoured. Without this the driver keeps serving the options it was CONSTRUCTED + // with, so an operator's edit is discarded while the deployment still seals green. An empty / + // placeholder document (the "{}" some unit tests pass) keeps the constructor-supplied options. + if (HasConfigBody(driverConfigJson)) + _options = ModbusDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + _transport = _transportFactory(_options); await _transport.ConnectAsync(cancellationToken).ConfigureAwait(false); // Build the RawPath → definition table from the authored raw tags. Each entry's TagConfig diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs index 93fdb4a7..ba587ce6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs @@ -44,6 +44,23 @@ public static class ModbusDriverFactoryExtensions /// Optional logger factory for creating loggers per driver instance. /// The constructed instance. public static ModbusDriver CreateInstance(string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) + { + return new ModbusDriver( + ParseOptions(driverInstanceId, driverConfigJson), driverInstanceId, + transportFactory: null, + logger: loggerFactory?.CreateLogger()); + } + + /// + /// Parses a Modbus DriverConfig JSON document into typed options. Extracted from + /// so + /// can re-parse a CHANGED config on reinitialize (Gitea #516) rather than serving the options it was + /// constructed with — which silently discarded every edit while the deployment still sealed green. + /// + /// The unique identifier for the driver instance. + /// The JSON configuration string for the driver. + /// The parsed . + internal static ModbusDriverOptions ParseOptions(string driverInstanceId, string driverConfigJson) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); @@ -106,10 +123,7 @@ public static class ModbusDriverFactoryExtensions }, }; - return new ModbusDriver( - options, driverInstanceId, - transportFactory: null, - logger: loggerFactory?.CreateLogger()); + return options; } private static T ParseEnum(string? raw, string? tagName, string driverInstanceId, string field) where T : struct, Enum diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index d9ac8555..26dece00 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -54,7 +54,9 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit _secretResolver = secretResolver ?? NullSecretResolver.Instance; } - private readonly OpcUaClientDriverOptions _options; + /// Mutable so can adopt a re-parsed config on reinitialize + /// (#516). Only ever written on the init path, before any reader/session uses it. + private OpcUaClientDriverOptions _options; private readonly ISecretResolver _secretResolver; private readonly string _driverInstanceId; // ---- IAlarmSource state ---- @@ -156,12 +158,29 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// public string DriverType => "OpcUaClient"; + /// True when the supplied DriverConfig JSON carries a real body. The bootstrapper always + /// passes a populated document; some unit tests pass "{}" or an empty string to exercise + /// lifecycle shape without a config — those keep the constructor-supplied options. + private static bool HasConfigBody(string? driverConfigJson) + { + if (string.IsNullOrWhiteSpace(driverConfigJson)) return false; + var trimmed = driverConfigJson.Trim(); + return trimmed is not "{}" and not "[]"; + } + /// public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { _health = new DriverHealth(DriverState.Initializing, null, null); try { + // #516: re-parse the supplied DriverConfig so a config change delivered through the IDriver + // contract is honoured. Without this the driver keeps serving the options it was CONSTRUCTED + // with — the endpoint, security policy and tag set were all frozen at construction, and only + // secret rotation was picked up. + if (HasConfigBody(driverConfigJson)) + _options = OpcUaClientDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + // Enforce the Equipment-vs-SystemPlatform choice at startup per driver-specs.md // §8 "Namespace Assignment" — a misconfigured remote fails draft validation here, // not as a runtime surprise. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs index 9da3a22f..dfd53972 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs @@ -63,16 +63,31 @@ public static class OpcUaClientDriverFactoryExtensions public static OpcUaClientDriver CreateInstance( string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory = null, ISecretResolver? secretResolver = null) + { + return new OpcUaClientDriver( + ParseOptions(driverInstanceId, driverConfigJson), driverInstanceId, + loggerFactory?.CreateLogger(), + secretResolver ?? NullSecretResolver.Instance); + } + + /// + /// Parses an OpcUaClient DriverConfig JSON document into typed options. Extracted so + /// can re-parse a CHANGED config on reinitialize + /// (Gitea #516) instead of serving the options it was constructed with. Note the driver's own + /// doc-comment previously claimed "resolving on every InitializeAsync picks up rotations" — that was + /// true of SECRET rotation only; the endpoint, security policy and tag set were all frozen at + /// construction. + /// + /// The unique driver instance identifier. + /// The driver configuration as a JSON string. + /// The parsed . + internal static OpcUaClientDriverOptions ParseOptions(string driverInstanceId, string driverConfigJson) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); - var options = JsonSerializer.Deserialize(driverConfigJson, JsonOptions) + return JsonSerializer.Deserialize(driverConfigJson, JsonOptions) ?? throw new InvalidOperationException( $"OpcUaClient driver config for '{driverInstanceId}' deserialised to null"); - - return new OpcUaClientDriver( - options, driverInstanceId, loggerFactory?.CreateLogger(), - secretResolver ?? NullSecretResolver.Instance); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index e99c3355..6af0b649 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -216,8 +216,16 @@ public sealed class SqlDriver /// /// Builds the authored RawPath table and proves the database is reachable. /// is not re-parsed here: the driver serves the - /// typed it was constructed with, exactly as ModbusDriver does - /// (config parsing belongs to the factory, which builds a fresh instance). + /// typed it was constructed with. That premise — "config parsing + /// belongs to the factory, which builds a fresh instance" — was false when written + /// (Gitea #516): the host reinitialized the EXISTING child in place, so every config edit was + /// silently discarded and the deployment still sealed green. It is true now, because + /// DriverSpawnPlanner routes a changed DriverConfig through a stop + respawn. + /// This driver deliberately does not also re-parse in place, unlike Modbus / AbLegacy / + /// OpcUaClient. It builds more than options from config — the and the + /// resolved connection string are both injected at construction — so adopting a new + /// alone would run a NEW tag set against the OLD connection. Half a + /// re-parse is worse than none; the respawn rebuilds all three together. /// The table is built first because it is pure and cannot fail; the liveness check is the only /// I/O, and on failure this method records and rethrows — /// DriverInstanceActor reads a throw as InitializeFailed and lands in Reconnecting with its 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 cb2152ee..82d5267a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -244,6 +244,11 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// value maps so stale condition state never leaks across redeploys. private readonly NativeAlarmProjector _nativeAlarmProjector = new(); + /// In-flight sends, keyed by correlation, so + /// can advance the cached spec only once the child confirms it adopted + /// the config (#516). Bounded by the number of driver children with a delta in flight. + private readonly Dictionary _pendingDelta = new(); + /// The composition from the most-recent apply (set at the END of /// ). Null until the first apply. private AddressSpaceComposition? _lastComposition; @@ -868,6 +873,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleDeltaApplied); + Receive(HandleApplyResult); Receive(HandleRestartDriver); Receive(HandleReconnectDriver); Receive(HandleRouteNodeWrite); @@ -901,6 +907,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleDeltaApplied); + Receive(HandleApplyResult); Receive(HandleRestartDriver); Receive(HandleReconnectDriver); Receive(HandleRouteNodeWrite); @@ -1353,6 +1360,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers // A late DeltaApplied (an apply completed just before the DB went Stale) — re-register the driver's // mux adapter anyway; it simply re-reads the driver's current refs (harmless, no DB access). Receive(HandleDeltaApplied); + Receive(HandleApplyResult); Receive(_ => { /* PubSub ack */ }); Timers.StartPeriodicTimer("retry-db", RetryConfigDbConnection.Instance, ReconnectInterval); } @@ -2007,7 +2015,15 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers try { driver = _driverFactory.TryCreate(spec.DriverType, spec.DriverInstanceId, spec.DriverConfig); } catch (Exception ex) { - _log.Warning(ex, "DriverHost {Node}: factory for {Type} threw on {Id}; stubbing", + // A factory throw is a CONFIG error, not a connectivity one — TryCreate is pure parsing; + // device I/O happens later in InitializeAsync. Logged at Error because the node silently + // degrades to a stub that answers nothing, and since #516 routed DriverConfig changes + // through a respawn this path is now reachable by an ordinary operator edit rather than + // only by a brand-new driver. It still does NOT fail the deployment — making it do so is + // a deliberate follow-up, since it would let one malformed driver block a fleet deploy. + _log.Error(ex, + "DriverHost {Node}: factory for {Type} REJECTED the config for {Id} — the driver is " + + "stubbed and will serve nothing until the config is fixed and redeployed", _localNode, spec.DriverType, spec.DriverInstanceId); } if (driver is null) @@ -2087,15 +2103,54 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers Context.Stop(adapter); } + /// + /// Sends an in-place config delta to a running child. + /// Unreachable from the reconcile path since #516 — a DriverConfig change is now a + /// stop + respawn (), so ToApplyDelta is always empty. + /// Kept because the seam is still exercised by DriverInstanceActor's own mid-connect config + /// adoption. + /// Two seals were removed here. This used to overwrite the cached + /// Spec SYNCHRONOUSLY, before the child had even dequeued the message — so the host + /// immediately believed the new config was live, the NEXT reconcile computed no delta against it, + /// and any drift was sealed permanently. It also Telld with no Receive<ApplyResult> + /// handler registered, so a FAILED reinit — including Galaxy's deliberate + /// NotSupportedException — dead-lettered and was never surfaced. + /// private void ApplyChildDelta(DriverInstanceSpec spec) { if (!_children.TryGetValue(spec.DriverInstanceId, out var entry)) return; - entry.Actor.Tell(new DriverInstanceActor.ApplyDelta(spec.DriverConfig, CorrelationId.NewId())); - // Store the full new spec — a delta can change Name, Enabled, ClusterId, etc. in addition to config. - _children[spec.DriverInstanceId] = entry with { Spec = spec }; + var correlation = CorrelationId.NewId(); + _pendingDelta[correlation] = spec; + entry.Actor.Tell(new DriverInstanceActor.ApplyDelta(spec.DriverConfig, correlation), Self); _log.Debug("DriverHost {Node}: ApplyDelta queued for {Id}", _localNode, spec.DriverInstanceId); } + /// + /// A child's reply to . On success the cached Spec is advanced — + /// only now, when the child has actually adopted the config, so a failed reinit leaves the host + /// believing the OLD config is live and the next reconcile re-attempts the change instead of + /// silently treating the drift as applied. A failure is logged at Error rather than swallowed. + /// + private void HandleApplyResult(DriverInstanceActor.ApplyResult msg) + { + if (msg.Success) + { + if (_pendingDelta.Remove(msg.Correlation, out var spec) + && _children.TryGetValue(spec.DriverInstanceId, out var entry)) + { + // A delta can change Name, Enabled, ClusterId etc. alongside the config. + _children[spec.DriverInstanceId] = entry with { Spec = spec }; + } + return; + } + + _pendingDelta.Remove(msg.Correlation, out var failed); + _log.Error( + "DriverHost {Node}: driver {Id} REJECTED an in-place config change ({Reason}) — the host keeps " + + "the previous config so the next reconcile re-attempts it", + _localNode, failed?.DriverInstanceId ?? "", msg.Reason ?? "no reason given"); + } + /// /// A driver child finished applying an in-place (its /// completed). Re-register THAT driver's dependency-consumer diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverSpawnPlan.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverSpawnPlan.cs index 5399691f..37ba98ab 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverSpawnPlan.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverSpawnPlan.cs @@ -7,7 +7,12 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; /// spawn / ApplyDelta / stop on its child actors accordingly. /// /// Specs with no current child — create a new actor. -/// Specs whose child exists but config JSON or type differs. +/// +/// In-place config deltas. Always empty since #516 — a DriverConfig change is now a +/// stop + respawn so the factory is the single parse authority. Retained because +/// DriverInstanceActor still handles ApplyDelta on its own config-adoption path +/// (a config arriving mid-connect), and removing the list would hide that seam. +/// /// DriverInstanceIds currently running but missing from the new artifact, or now disabled. public sealed record DriverSpawnPlan( IReadOnlyList ToSpawn, @@ -42,23 +47,33 @@ public static class DriverSpawnPlanner toStop.Add(id); continue; } - // A driver TYPE change can't be reinitialized in-place (factory-bound) — stop + respawn. - // A RESILIENCE-CONFIG change likewise forces a respawn: the CapabilityInvoker (and its - // resolved options) is bound to the child actor at spawn time, so the only way a changed - // ResilienceConfig takes effect is to rebuild the child. The factory invalidates the stale - // cached pipelines on the respawn's Create call. (A pure DriverConfig change stays an - // in-place delta — no reconnect — because it doesn't touch the resilience pipeline.) + // ANY of the three config surfaces changing forces a stop + respawn, so the FACTORY is the + // single parse authority for everything a driver is built from. + // + // • DriverType — factory-bound, can't be reinitialized in place. + // • ResilienceConfig — the CapabilityInvoker (and its resolved options) binds to the child + // actor at spawn time; the factory invalidates the stale cached pipelines on respawn. + // • DriverConfig — was an in-place ApplyDelta until #516. It is now a respawn. + // + // #516: the in-place delta silently DISCARDED the edit on five drivers (Modbus, FOCAS, + // OpcUaClient, AbLegacy, Sql), whose InitializeAsync served the options captured by their + // constructor and never looked at the driverConfigJson they were handed. The deployment still + // sealed green. Those five now re-parse (belt), and this respawn is the braces: several + // drivers build MORE than options from config — Sql derives its dialect + connection string + // and FOCAS its client-factory backend, both injected at construction — so an in-place + // re-parse of options ALONE would apply a new tag set against an old connection. Only a + // respawn rebuilds all of it. + // + // The accepted cost is a reconnect on every DriverConfig edit, replacing the prior + // no-reconnect in-place path. That is deliberate: a correct reconnect beats a silent no-op. if (!string.Equals(snap.DriverType, spec.DriverType, StringComparison.Ordinal) - || !string.Equals(snap.ResilienceConfig, spec.ResilienceConfig, StringComparison.Ordinal)) + || !string.Equals(snap.ResilienceConfig, spec.ResilienceConfig, StringComparison.Ordinal) + || !string.Equals(snap.LastConfigJson, spec.DriverConfig, StringComparison.Ordinal)) { toStop.Add(id); toSpawn.Add(spec); continue; } - if (!string.Equals(snap.LastConfigJson, spec.DriverConfig, StringComparison.Ordinal)) - { - toDelta.Add(spec); - } } foreach (var (id, spec) in targetById) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs new file mode 100644 index 00000000..4aaa3feb --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReinitConfigAdoptionTests.cs @@ -0,0 +1,49 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// +/// Gitea #516 — AbLegacyDriver.InitializeAsync used to serve the options its CONSTRUCTOR +/// captured and never look at the driverConfigJson it was handed, so an operator's config edit +/// was silently discarded while the deployment still sealed green. +/// Every pre-existing reinit test in this suite passes "{}" — the exact input the guarded +/// re-parser treats as "keep the constructor options" — so they were blind to the defect. +/// +[Trait("Category", "Unit")] +public sealed class AbLegacyReinitConfigAdoptionTests +{ + /// + /// The driver validates every device's HostAddress during init. Reinitializing with a config + /// whose device address is structurally invalid must therefore THROW — a driver still serving the + /// constructor's valid device list would validate that instead and succeed, which is precisely how + /// the discarded edit stayed invisible. + /// + [Fact] + public async Task Reinitialize_adopts_a_changed_device_list() + { + var driver = AbLegacyDriverFactoryExtensions.CreateInstance( + "ab1", + """{"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"Slc500"}]}""", + loggerFactory: null); + + await Should.ThrowAsync( + () => driver.ReinitializeAsync( + """{"devices":[{"hostAddress":"not-a-valid-ab-address","plcFamily":"Slc500"}]}""", + CancellationToken.None), + "AbLegacyDriver kept its constructor-supplied device list after a reinitialize with a changed config (#516)"); + } + + /// An empty/placeholder document must still keep the constructor options, so the many + /// lifecycle tests that pass "{}" keep meaning what they meant. + [Fact] + public async Task Reinitialize_with_an_empty_document_keeps_the_constructor_options() + { + var driver = AbLegacyDriverFactoryExtensions.CreateInstance( + "ab1", + """{"devices":[{"hostAddress":"ab://10.0.0.1:44818/1,0","plcFamily":"Slc500"}]}""", + loggerFactory: null); + + await Should.NotThrowAsync(() => driver.ReinitializeAsync("{}", CancellationToken.None)); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusReinitConfigAdoptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusReinitConfigAdoptionTests.cs new file mode 100644 index 00000000..98481200 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusReinitConfigAdoptionTests.cs @@ -0,0 +1,72 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// Gitea #516 — used to serve the options its CONSTRUCTOR +/// captured and never look at the driverConfigJson it was handed, so an operator's config edit +/// was silently discarded while the deployment still sealed green. +/// This test passes a CHANGED config on purpose. Every pre-existing reinit test in this +/// suite passes "{}" — exactly the input the guarded re-parser treats as "keep the constructor +/// options" — so those tests were blind to the defect by construction. +/// +[Trait("Category", "Unit")] +public sealed class ModbusReinitConfigAdoptionTests +{ + /// The transport factory receives the options the driver actually decided to use, so it is a + /// direct read of which config won — no log-string matching. + [Fact] + public async Task Reinitialize_with_a_changed_host_rebuilds_the_transport_against_the_new_host() + { + var seen = new List(); + var options = ModbusDriverFactoryExtensions.ParseOptions("m1", """{"host":"10.0.0.1","port":502}"""); + var driver = new ModbusDriver(options, "m1", opts => + { + seen.Add($"{opts.Host}:{opts.Port}"); + return new NeverConnectingTransport(); + }); + + try { await driver.InitializeAsync("""{"host":"10.0.0.1","port":502}""", CancellationToken.None); } + catch { /* connect failure is expected and irrelevant */ } + + try { await driver.ReinitializeAsync("""{"host":"10.0.0.99","port":1502}""", CancellationToken.None); } + catch { /* ditto */ } + + seen.Count.ShouldBeGreaterThanOrEqualTo(2); + seen[^1].ShouldBe("10.0.0.99:1502", + "ModbusDriver kept its constructor-supplied host after a reinitialize with a changed config (#516)"); + } + + /// A placeholder / empty document must still keep the constructor options — the guard exists so + /// the many lifecycle tests that pass "{}" keep meaning what they meant. + [Fact] + public async Task Reinitialize_with_an_empty_document_keeps_the_constructor_options() + { + var seen = new List(); + var options = ModbusDriverFactoryExtensions.ParseOptions("m1", """{"host":"10.0.0.1","port":502}"""); + var driver = new ModbusDriver(options, "m1", opts => + { + seen.Add($"{opts.Host}:{opts.Port}"); + return new NeverConnectingTransport(); + }); + + try { await driver.ReinitializeAsync("{}", CancellationToken.None); } + catch { /* connect failure is expected */ } + + seen.ShouldAllBe(s => s == "10.0.0.1:502"); + } + + private sealed class NeverConnectingTransport : IModbusTransport + { + public bool IsConnected => false; + public Task ConnectAsync(CancellationToken cancellationToken) + => throw new IOException("test transport never connects"); + public Task DisconnectAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken cancellationToken) + => throw new IOException("test transport never connects"); + public void Dispose() { } + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientReinitConfigAdoptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientReinitConfigAdoptionTests.cs new file mode 100644 index 00000000..5299fdb5 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientReinitConfigAdoptionTests.cs @@ -0,0 +1,50 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// +/// Gitea #516 — OpcUaClientDriver.InitializeAsync used to serve the options its CONSTRUCTOR +/// captured. Its own comment claimed "resolving on every InitializeAsync … picks up rotations", which +/// was true of SECRET rotation only: the endpoint, the security policy and the tag set were all frozen +/// at construction, so an operator's edit was discarded while the deployment still sealed green. +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientReinitConfigAdoptionTests +{ + /// + /// ValidateNamespaceKind runs at the top of init and rejects TargetNamespaceKind=Equipment + /// with an empty UNS mapping table. Reinitializing INTO that shape must therefore throw — a driver + /// still serving the constructor's valid options would validate those and succeed, which is exactly + /// how the discarded edit stayed invisible. + /// + [Fact] + public async Task Reinitialize_adopts_a_changed_config() + { + var driver = OpcUaClientDriverFactoryExtensions.CreateInstance( + "opc1", + """{"endpointUrl":"opc.tcp://10.0.0.1:4840","targetNamespaceKind":"Equipment","unsMappingTable":{"a":"b"}}"""); + + await Should.ThrowAsync( + () => driver.ReinitializeAsync( + """{"endpointUrl":"opc.tcp://10.0.0.1:4840","targetNamespaceKind":"Equipment","unsMappingTable":{}}""", + CancellationToken.None), + "OpcUaClientDriver kept its constructor-supplied options after a reinitialize with a changed config (#516)"); + } + + /// An empty/placeholder document must still keep the constructor options. + [Fact] + public async Task Reinitialize_with_an_empty_document_keeps_the_constructor_options() + { + var driver = OpcUaClientDriverFactoryExtensions.CreateInstance( + "opc1", + """{"endpointUrl":"opc.tcp://10.0.0.1:4840","targetNamespaceKind":"Equipment","unsMappingTable":{"a":"b"}}"""); + + // "{}" keeps the (valid) constructor options, so ValidateNamespaceKind passes and the driver gets as + // far as the connect — which fails against an unreachable endpoint. Anything BUT the validation + // throw proves the constructor options survived. + var ex = await Record.ExceptionAsync(() => driver.ReinitializeAsync("{}", CancellationToken.None)); + (ex is InvalidOperationException ioe && ioe.Message.Contains("UnsMappingTable", StringComparison.OrdinalIgnoreCase)) + .ShouldBeFalse("an empty document must keep the constructor-supplied options, not re-validate an empty config"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorUnreadableArtifactTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorUnreadableArtifactTests.cs index 1036d4a3..a3f5f1bc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorUnreadableArtifactTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorUnreadableArtifactTests.cs @@ -78,10 +78,17 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas factory.LastSubscribedRefs.ShouldNotBeNull()!.ShouldContain(SpeedRef); } - /// Positive control for the subscription half: a READABLE artifact that keeps the driver but - /// drops its last tag genuinely does clear the live subscription — the child receives an empty desired - /// set and unsubscribes. This proves both that the unsubscribe is observable through this factory and - /// that it lands well within , so the absence asserted above is real. + /// + /// Positive control for the subscription half: a READABLE artifact that drops the driver's last tag + /// genuinely does tear the live subscription down, and does so well within + /// — without this, the absence asserted above would pass on a race. + /// The observable changed with #516. Dropping a tag changes the driver's + /// DriverConfig, which used to be an in-place delta: the child received an empty desired set + /// and called UnsubscribeAsync. A config change is now a stop + respawn, so the old child is + /// STOPPED (taking its subscription with it via ShutdownAsync) and a fresh one spawns with + /// nothing to subscribe to. The teardown is therefore observed as a shutdown, not an unsubscribe — + /// the same event, reached by a different route. + /// [Fact] public void Dropping_a_drivers_last_tag_does_clear_its_subscription() { @@ -90,13 +97,13 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas var (actor, coordinator) = SpawnHostAndApply(db, SeedV3Deployment(db, RevA), factory); AwaitAssert(() => factory.LastSubscribedRefs.ShouldNotBeNull()!.ShouldContain(SpeedRef), duration: Timeout); - factory.UnsubscribeCount.ShouldBe(0); + factory.ShutdownCount.ShouldBe(0); actor.Tell(new DispatchDeployment(SeedTaglessDriverDeployment(db, RevB), RevB, CorrelationId.NewId())); coordinator.ExpectMsg(Timeout); - AwaitAssert(() => factory.UnsubscribeCount.ShouldBe(1), duration: SettleWindow); - AskDiagnostics(actor).Drivers.Select(d => d.Name).ShouldContain("drv-1"); // the driver itself stayed + AwaitAssert(() => factory.ShutdownCount.ShouldBeGreaterThanOrEqualTo(1), duration: SettleWindow); + AskDiagnostics(actor).Drivers.Select(d => d.Name).ShouldContain("drv-1"); // respawned under the same id } /// Positive control: the guard keys on "the read gave us nothing", not on "the new config has @@ -318,6 +325,9 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas /// Number of UnsubscribeAsync calls — non-zero means a live handle was torn down. public int UnsubscribeCount => Volatile.Read(ref _driver.UnsubscribeCount); + /// Number of ShutdownAsync calls — non-zero means a driver child was stopped. + public int ShutdownCount => Volatile.Read(ref _driver.ShutdownCount); + /// public IDriver? TryCreate(string driverType, string driverInstanceId, string driverConfigJson) => string.Equals(driverType, supportedType, StringComparison.Ordinal) ? _driver : null; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverSpawnPlannerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverSpawnPlannerTests.cs index f9e18d4a..541d5a1f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverSpawnPlannerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverSpawnPlannerTests.cs @@ -40,9 +40,18 @@ public sealed class DriverSpawnPlannerTests plan.ToStop.ShouldBeEmpty(); } - /// Verifies that different configuration is routed to ApplyDelta. + /// + /// A changed DriverConfig forces a stop + respawn (#516), NOT an in-place delta. + /// This test previously asserted the opposite. The in-place path silently discarded the edit + /// on five drivers whose InitializeAsync served constructor-captured options and never read + /// the driverConfigJson they were handed — and the deployment still sealed green. Routing + /// through the factory makes it the single parse authority, which matters most for the drivers that + /// build MORE than options from config (Sql's dialect + connection string, FOCAS's client-factory + /// backend) where re-parsing options alone would apply a new tag set against an old connection. + /// The accepted cost is a reconnect on every config edit. + /// [Fact] - public void Different_config_routes_to_ApplyDelta() + public void Different_config_routes_to_stop_and_respawn() { var current = new Dictionary { @@ -52,9 +61,11 @@ public sealed class DriverSpawnPlannerTests var plan = DriverSpawnPlanner.Compute(current, target); - plan.ToApplyDelta.Single().DriverInstanceId.ShouldBe("a"); - plan.ToSpawn.ShouldBeEmpty(); - plan.ToStop.ShouldBeEmpty(); + plan.ToStop.Single().ShouldBe("a"); + plan.ToSpawn.Single().DriverInstanceId.ShouldBe("a"); + plan.ToSpawn.Single().DriverConfig.ShouldBe("{\"host\":\"new\"}"); + // The whole point: nothing is left on the in-place path that could discard the edit. + plan.ToApplyDelta.ShouldBeEmpty(); } /// Verifies that removed drivers are routed to ToStop. @@ -166,11 +177,14 @@ public sealed class DriverSpawnPlannerTests } /// - /// A pure DriverConfig change with an UNCHANGED ResilienceConfig stays an in-place delta (no - /// reconnect) — the resilience pipeline is untouched, so there's no reason to respawn. + /// A pure DriverConfig change respawns even when the ResilienceConfig is UNCHANGED. + /// This asserted the opposite until #516. The reasoning then was "the resilience pipeline is + /// untouched, so there's no reason to respawn" — correct about resilience, wrong about the driver: + /// five drivers ignored the config the in-place delta handed them, and the deployment sealed green + /// anyway. The reconnect is the accepted price of the edit actually taking effect. /// [Fact] - public void DriverConfig_change_with_unchanged_ResilienceConfig_stays_ApplyDelta() + public void DriverConfig_change_respawns_even_when_ResilienceConfig_is_unchanged() { var current = new Dictionary { @@ -180,8 +194,8 @@ public sealed class DriverSpawnPlannerTests var plan = DriverSpawnPlanner.Compute(current, target); - plan.ToApplyDelta.Single().DriverInstanceId.ShouldBe("a"); - plan.ToSpawn.ShouldBeEmpty(); - plan.ToStop.ShouldBeEmpty(); + plan.ToStop.Single().ShouldBe("a"); + plan.ToSpawn.Single().DriverInstanceId.ShouldBe("a"); + plan.ToApplyDelta.ShouldBeEmpty(); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs index 2ab4213d..b9780be4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs @@ -50,9 +50,17 @@ internal class StubDriver : IDriver return Task.CompletedTask; } + /// Number of calls — a child actor stopping raises it, so a test + /// can observe a driver child being torn down (e.g. by the #516 config-change respawn). + public int ShutdownCount; + /// Shuts down the driver. /// Cancellation token for the operation. - public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task ShutdownAsync(CancellationToken cancellationToken) + { + Interlocked.Increment(ref ShutdownCount); + return Task.CompletedTask; + } /// Gets the health status of the driver. public DriverHealth GetHealth() => new(DriverState.Healthy, DateTime.UtcNow, null); /// Gets the memory footprint of the driver.