d32d89c340
Five drivers ignored the config handed to them on reinitialize, serving the
options their constructor captured — and the deployment sealed green anyway.
Fixed on both halves, as chosen.
Seam (load-bearing): DriverSpawnPlanner routes a changed DriverConfig to
ToStop + ToSpawn instead of an in-place delta, making the factory the single
parse authority. 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 correct about the resilience pipeline and
wrong about the driver. The accepted price is a reconnect per config edit.
Per-driver, and this split was not anticipated:
- 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 and FOCAS. Each builds more
than options from config (Sql's dialect + resolved connection string,
FOCAS's client-factory backend, both injected at construction), so adopting
new options alone would run a NEW tag set against an OLD connection. Half a
re-parse is worse than none. SqlDriver's doc-comment asserted the opposite
premise — "config parsing belongs to the factory, which builds a fresh
instance" — which was false when written and is true only now.
Two green seals removed from ApplyChildDelta: it overwrote the cached Spec
synchronously BEFORE the child dequeued the message, so the host believed the
new config was live and the next reconcile computed no delta, sealing the
drift permanently; and it Tell'd with no Receive<ApplyResult> registered, so a
failed reinit — including Galaxy's deliberate NotSupportedException —
dead-lettered.
Exposed (not created) by the change: a factory throw is a CONFIG error, and
SpawnChild catches it and silently substitutes a stub. Only a brand-new driver
could reach that before; an ordinary config edit can now. Raised Warning ->
Error with an actionable message. It still does not fail the deployment —
doing so would let one malformed driver block a fleet deploy, so that is a
follow-up rather than a drive-by.
Tests: every pre-existing reinit test in these suites passes "{}", the exact
input a guarded re-parser treats as "keep constructor options" — blind to this
defect by construction. New tests pass CHANGED json; the Modbus one was
verified falsifiable by deleting the re-parse (goes red). Two tests asserted
the old behaviour and were rewritten, including the positive control in
DriverHostActorUnreadableArtifactTests, whose observable moved from
UnsubscribeAsync to ShutdownAsync now that teardown happens by stopping the
child rather than emptying its desired set.
Remaining full-suite failures are pre-existing and environmental, verified
identical on the pre-change tree: Host.IntegrationTests (3) and
Driver.AbLegacy.IntegrationTests (4, docker fixture-gated).
95 lines
5.2 KiB
C#
95 lines
5.2 KiB
C#
namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
|
|
|
|
/// <summary>
|
|
/// Pure diff between the currently-running driver children (keyed by
|
|
/// <c>DriverInstance.DriverInstanceId</c>) and the target spec list from a freshly-applied
|
|
/// deployment artifact. The DriverHostActor consumes the three lists and calls
|
|
/// spawn / ApplyDelta / stop on its child actors accordingly.
|
|
/// </summary>
|
|
/// <param name="ToSpawn">Specs with no current child — create a new actor.</param>
|
|
/// <param name="ToApplyDelta">
|
|
/// In-place config deltas. <b>Always empty since #516</b> — a DriverConfig change is now a
|
|
/// stop + respawn so the factory is the single parse authority. Retained because
|
|
/// <c>DriverInstanceActor</c> still handles <c>ApplyDelta</c> on its own config-adoption path
|
|
/// (a config arriving mid-connect), and removing the list would hide that seam.
|
|
/// </param>
|
|
/// <param name="ToStop">DriverInstanceIds currently running but missing from the new artifact, or now disabled.</param>
|
|
public sealed record DriverSpawnPlan(
|
|
IReadOnlyList<DriverInstanceSpec> ToSpawn,
|
|
IReadOnlyList<DriverInstanceSpec> ToApplyDelta,
|
|
IReadOnlyList<string> ToStop);
|
|
|
|
public static class DriverSpawnPlanner
|
|
{
|
|
/// <summary>
|
|
/// Compute the spawn/delta/stop sets. Disabled entries in <paramref name="target"/> are
|
|
/// treated as "not desired here": if a child exists for the id it goes into ToStop,
|
|
/// otherwise the row is dropped entirely (no spawn for a disabled driver).
|
|
/// </summary>
|
|
/// <param name="current">The currently running driver children keyed by ID.</param>
|
|
/// <param name="target">The target driver instances from the deployment artifact.</param>
|
|
/// <returns>The computed <see cref="DriverSpawnPlan"/> with spawn, apply-delta, and stop sets.</returns>
|
|
public static DriverSpawnPlan Compute(
|
|
IReadOnlyDictionary<string, DriverChildSnapshot> current,
|
|
IReadOnlyList<DriverInstanceSpec> target)
|
|
{
|
|
var toSpawn = new List<DriverInstanceSpec>();
|
|
var toDelta = new List<DriverInstanceSpec>();
|
|
var toStop = new List<string>();
|
|
|
|
var targetById = new Dictionary<string, DriverInstanceSpec>(StringComparer.Ordinal);
|
|
foreach (var spec in target) targetById[spec.DriverInstanceId] = spec;
|
|
|
|
foreach (var (id, snap) in current)
|
|
{
|
|
if (!targetById.TryGetValue(id, out var spec) || !spec.Enabled)
|
|
{
|
|
toStop.Add(id);
|
|
continue;
|
|
}
|
|
// 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.LastConfigJson, spec.DriverConfig, StringComparison.Ordinal))
|
|
{
|
|
toStop.Add(id);
|
|
toSpawn.Add(spec);
|
|
continue;
|
|
}
|
|
}
|
|
|
|
foreach (var (id, spec) in targetById)
|
|
{
|
|
if (!spec.Enabled) continue;
|
|
if (current.ContainsKey(id)) continue;
|
|
toSpawn.Add(spec);
|
|
}
|
|
|
|
return new DriverSpawnPlan(toSpawn, toDelta, toStop);
|
|
}
|
|
}
|
|
|
|
/// <summary>Snapshot of one running driver child as the host sees it. Used as the diff input.</summary>
|
|
/// <param name="DriverType">The driver type name the child was spawned for.</param>
|
|
/// <param name="LastConfigJson">The <c>DriverConfig</c> JSON the child currently serves.</param>
|
|
/// <param name="ResilienceConfig">The per-instance <c>ResilienceConfig</c> JSON the child's invoker was built with (null = tier defaults).</param>
|
|
public sealed record DriverChildSnapshot(string DriverType, string LastConfigJson, string? ResilienceConfig = null);
|