fix(opcua): hold last-known-good when a deploy's artifact cannot be read (#485)
A transient ConfigDb error while loading a deployment's artifact emptied the served address space. LoadArtifact caught the exception, logged "rebuild becomes no-op" and returned zero bytes; those parsed to an EMPTY composition, which the planner diffed against the live one as a PureRemove and the applier faithfully executed — 16 nodes gone, while the log claimed nothing happened. An artifact we could not obtain is not an empty configuration. Nothing legitimate produces a zero-length blob (an operator who really deletes everything still deploys a JSON document with empty arrays), so "no bytes" can only mean "no answer". HandleRebuild now abandons the rebuild on one, leaving both the materialised nodes and _lastApplied intact so the retry diffs against what is actually being served. The loader's own log line is now true. The two cases are logged differently: warn when there is a live address space being protected, debug when nothing has been deployed to this node yet. Covered by a RED-first actor test (verified failing: the load error tore down eq-2's subtree), plus a positive control proving a READABLE artifact that genuinely drops an equipment still removes it — the guard suppresses teardown only when the read failed. Found on the LocalDb Phase 1 follow-up live gate, which induced it by flapping SQL; that gate's log is the production evidence for the seam. Closes #485 Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
@@ -279,6 +279,85 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(naAfterFirst);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Issue #485 — a transient ConfigDb error while loading a deploy's artifact must NOT empty the
|
||||
/// served address space. The loader's own log already claims "rebuild becomes no-op", but an
|
||||
/// unreadable artifact used to become an EMPTY composition, which the planner then diffed against
|
||||
/// the live one as a PureRemove and tore every node down. A blip on the way to the database is not
|
||||
/// evidence that the operator deployed an empty configuration: the last-known-good address space
|
||||
/// must stand until a real artifact says otherwise.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_keeps_the_last_known_good_address_space_when_the_artifact_load_fails()
|
||||
{
|
||||
var db = new FlakyDbFactory(NewInMemoryDbFactory());
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var dep1 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"), ("eq-2", "Pump-2"));
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier));
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1)));
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2));
|
||||
var callsAfterFirst = sink.Calls.Count;
|
||||
|
||||
// The next deploy arrives while the ConfigDb is briefly unreachable.
|
||||
db.Fail = true;
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(Guid.NewGuid())));
|
||||
Thread.Sleep(200);
|
||||
|
||||
sink.Calls.ShouldNotContain(c => c.StartsWith("RE:")); // no equipment subtree torn down
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
sink.Calls.Count.ShouldBe(callsAfterFirst); // the failed load touched nothing at all
|
||||
|
||||
// …and the actor still remembers WHAT is applied: once the database is back, re-applying the same
|
||||
// composition diffs to an empty plan. A clobbered _lastApplied would re-materialise everything.
|
||||
db.Fail = false;
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1)));
|
||||
Thread.Sleep(200);
|
||||
sink.Calls.Count.ShouldBe(callsAfterFirst);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Positive control for <see cref="Rebuild_keeps_the_last_known_good_address_space_when_the_artifact_load_fails"/>:
|
||||
/// the guard suppresses teardown ONLY when the artifact could not be read. A real deploy that
|
||||
/// genuinely drops an equipment still tears its subtree down, so the removal path is not simply dead.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_still_removes_equipment_a_readable_artifact_genuinely_dropped()
|
||||
{
|
||||
var db = new FlakyDbFactory(NewInMemoryDbFactory());
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var dep1 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"), ("eq-2", "Pump-2"));
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier));
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1)));
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2));
|
||||
|
||||
// eq-2 really is gone from the next artifact — that IS a configuration change, so it must apply.
|
||||
var dep2 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"));
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2)));
|
||||
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("RE:eq-2"), duration: TimeSpan.FromSeconds(2));
|
||||
}
|
||||
|
||||
/// <summary>An <see cref="IDbContextFactory{TContext}"/> whose <c>CreateDbContext</c> can be made to
|
||||
/// throw the way a real transient ConfigDb outage does, so the artifact load fails exactly where issue
|
||||
/// #485 observed it failing.</summary>
|
||||
private sealed class FlakyDbFactory(IDbContextFactory<OtOpcUaConfigDbContext> inner)
|
||||
: IDbContextFactory<OtOpcUaConfigDbContext>
|
||||
{
|
||||
/// <summary>Gets or sets a value indicating whether the next context creation should throw.</summary>
|
||||
public bool Fail { get; set; }
|
||||
|
||||
/// <summary>Creates a context, or throws when <see cref="Fail"/> is set.</summary>
|
||||
/// <returns>A new context.</returns>
|
||||
public OtOpcUaConfigDbContext CreateDbContext() => Fail
|
||||
? throw new InvalidOperationException(
|
||||
"An error occurred using the connection to database 'OtOpcUa' on server 'sql,1433'.")
|
||||
: inner.CreateDbContext();
|
||||
}
|
||||
|
||||
/// <summary>Seal a deployment carrying the given (equipmentId, name) rows under a shared line, returning
|
||||
/// the new DeploymentId so a test can target THAT artifact. Renaming an equipment across two such
|
||||
/// deployments produces a ChangedEquipment delta (Rebuild), while adding a row is a PureAdd.</summary>
|
||||
|
||||
Reference in New Issue
Block a user