test(site-call-audit, deployment): cover error, retry and state-transition branches (ScadaBridge#73)
Backfills the branches arch-review 08 flagged as thin, chosen for what they decide rather than for count. Every new test was mutation-checked: the guarded line was broken and the test went red. SiteCallAudit (SiteCallAuditFailurePathTests, 17 cases, in-memory - the 18 MSSQL-backed actor tests skip without a database, so the ingest and read fault paths had no runnable coverage): - a failed page write holds the reconciliation cursor; the next tick re-pulls the same window and advances only after the write lands - the pinned latch publishes Pinned=true once across many pinned ticks and Pinned=false once when the site starts honouring after_id - a site-enumeration fault skips one tick without wedging the single-flight guard - an endless backlog drains at most 50 pages per tick, unpinned, and the next tick resumes from the advanced cursor - purge single-flight: ticks during a blocked purge are dropped - ingest fault replies Accepted=false and the next upsert is accepted - detail / KPI / per-site / per-node / query faults map to Success=false with the root-cause message and an empty payload; the actor keeps serving - query page size clamped to [1, 200]; an empty page carries no cursor - Discard NotParked / OperationFailed (site message verbatim) with no audit row; Discard before transport registration is SiteUnreachable - a throwing audit sink leaves an Applied relay Applied; a missing mirror row or unknown channel still audits the operator, falling back to ApiOutbound DeploymentManager (DeploymentServiceFailurePathTests, 11 cases): - a site that answers Failed commits a Failed record with the site's message (no timeout marker), leaves instance state and the deployed snapshot untouched, and audits Deploy (not DeployFailed) - a prior non-timeout Failed record skips the query-before-redeploy - a failing Failed-status write detaches pending changes and retries once (none when nothing was detached); the caller still gets the original error - Disable / Enable / Delete answered with Success=false leave the instance unchanged, audit the attempt, and surface the site's message or the default Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
+291
@@ -0,0 +1,291 @@
|
||||
using Akka.Actor;
|
||||
using Akka.TestKit.Xunit2;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using NSubstitute;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Lifecycle;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
|
||||
using ZB.MOM.WW.ScadaBridge.Communication;
|
||||
using ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #73: <see cref="DeploymentService"/> branches the existing suites leave untested —
|
||||
/// a site that answers but REFUSES (as opposed to timing out or throwing), the query-before-
|
||||
/// redeploy trigger for a non-timeout failure, and the detach-and-retry-once recovery of the
|
||||
/// Failed-status write. Each one decides an instance's or a deployment record's state.
|
||||
/// </summary>
|
||||
public class DeploymentServiceFailurePathTests : TestKit
|
||||
{
|
||||
private readonly IDeploymentManagerRepository _repo = Substitute.For<IDeploymentManagerRepository>();
|
||||
private readonly IFlatteningPipeline _pipeline = Substitute.For<IFlatteningPipeline>();
|
||||
private readonly IAuditService _audit = Substitute.For<IAuditService>();
|
||||
|
||||
/// <summary>
|
||||
/// Stand-in central communication actor: answers each site-bound message with whatever
|
||||
/// <c>reply</c> returns for it (null drops it), and counts every message it saw.
|
||||
/// </summary>
|
||||
private sealed class ScriptedSiteActor : ReceiveActor
|
||||
{
|
||||
/// <summary>Initializes the actor.</summary>
|
||||
/// <param name="reply">Maps a site-bound message to the site's answer.</param>
|
||||
/// <param name="seen">Sink for every message received.</param>
|
||||
public ScriptedSiteActor(Func<object, object?> reply, List<object> seen)
|
||||
{
|
||||
Receive<SiteEnvelope>(env =>
|
||||
{
|
||||
lock (seen) seen.Add(env.Message);
|
||||
var answer = reply(env.Message);
|
||||
if (answer is not null)
|
||||
Sender.Tell(answer);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private DeploymentService CreateService(IActorRef? commActor)
|
||||
{
|
||||
var comms = new CommunicationService(
|
||||
Options.Create(new CommunicationOptions
|
||||
{
|
||||
QueryTimeout = TimeSpan.FromSeconds(5),
|
||||
DeploymentTimeout = TimeSpan.FromSeconds(5),
|
||||
}),
|
||||
NullLogger<CommunicationService>.Instance);
|
||||
if (commActor is not null)
|
||||
comms.SetCommunicationActor(commActor);
|
||||
|
||||
var siteRepo = Substitute.For<ISiteRepository>();
|
||||
siteRepo.GetSiteByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
.Returns(ci => new Site($"Site {ci.ArgAt<int>(0)}", $"site-{ci.ArgAt<int>(0)}") { Id = ci.ArgAt<int>(0) });
|
||||
|
||||
return new DeploymentService(
|
||||
_repo, siteRepo, Substitute.For<ITemplateEngineRepository>(), _pipeline, comms,
|
||||
new OperationLockManager(), _audit,
|
||||
new DiffService(),
|
||||
new RevisionHashService(),
|
||||
new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
|
||||
Options.Create(new DeploymentManagerOptions
|
||||
{
|
||||
OperationLockTimeout = TimeSpan.FromSeconds(5),
|
||||
LifecycleCommandTimeout = TimeSpan.FromSeconds(5),
|
||||
}),
|
||||
Options.Create(new CommunicationOptions
|
||||
{
|
||||
CentralFetchBaseUrl = "https://central.test:9000",
|
||||
PendingDeploymentTtl = TimeSpan.FromMinutes(5),
|
||||
}),
|
||||
NullLogger<DeploymentService>.Instance);
|
||||
}
|
||||
|
||||
private (IActorRef Actor, List<object> Seen) SiteActor(Func<object, object?> reply)
|
||||
{
|
||||
var seen = new List<object>();
|
||||
return (Sys.ActorOf(Props.Create(() => new ScriptedSiteActor(reply, seen))), seen);
|
||||
}
|
||||
|
||||
private Instance ArrangeDeployable(int id, InstanceState state, string revisionHash = "sha256:new")
|
||||
{
|
||||
var instance = new Instance($"Inst{id}") { Id = id, SiteId = 1, State = state };
|
||||
_repo.GetInstanceByIdAsync(id, Arg.Any<CancellationToken>()).Returns(instance);
|
||||
_pipeline.FlattenAndValidateAsync(id, Arg.Any<CancellationToken>())
|
||||
.Returns(Result<FlatteningPipelineResult>.Success(new FlatteningPipelineResult(
|
||||
new FlattenedConfiguration { InstanceUniqueName = instance.UniqueName },
|
||||
revisionHash,
|
||||
ValidationResult.Success())));
|
||||
return instance;
|
||||
}
|
||||
|
||||
// ── Deploy: the site answers, but with Failed ──
|
||||
|
||||
/// <summary>
|
||||
/// A site that receives the deploy but reports Failed (e.g. its own compile step rejected the
|
||||
/// config) commits a Failed record carrying the SITE's message — not a timeout marker, so the
|
||||
/// next redeploy does not waste a site query — and leaves the instance's state and deployed
|
||||
/// snapshot exactly as they were: the site is not running the new config.
|
||||
/// </summary>
|
||||
/// <param name="initialState">The instance state before the deploy.</param>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Theory]
|
||||
[InlineData(InstanceState.NotDeployed)]
|
||||
[InlineData(InstanceState.Disabled)]
|
||||
public async Task Deploy_SiteReportsFailed_RecordFailedWithSiteError_InstanceAndSnapshotUntouched(InstanceState initialState)
|
||||
{
|
||||
var instance = ArrangeDeployable(10, initialState);
|
||||
DeploymentRecord? record = null;
|
||||
await _repo.AddDeploymentRecordAsync(Arg.Do<DeploymentRecord>(r => record = r), Arg.Any<CancellationToken>());
|
||||
var (actor, _) = SiteActor(msg => msg is RefreshDeploymentCommand d
|
||||
? new DeploymentStatusResponse(d.DeploymentId, d.InstanceUniqueName,
|
||||
DeploymentStatus.Failed, "Script 'OnStart' failed to compile at site", DateTimeOffset.UtcNow)
|
||||
: null);
|
||||
|
||||
var result = await CreateService(actor).DeployInstanceAsync(10, "admin");
|
||||
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.Equal("Deployment failed: Script 'OnStart' failed to compile at site", result.Error);
|
||||
Assert.NotNull(record);
|
||||
Assert.Equal(DeploymentStatus.Failed, record!.Status);
|
||||
Assert.Equal("Script 'OnStart' failed to compile at site", record.ErrorMessage);
|
||||
Assert.NotNull(record.CompletedAt);
|
||||
|
||||
Assert.Equal(initialState, instance.State);
|
||||
await _repo.DidNotReceive().UpdateInstanceAsync(Arg.Any<Instance>(), Arg.Any<CancellationToken>());
|
||||
await _repo.DidNotReceive().AddDeployedSnapshotAsync(Arg.Any<DeployedConfigSnapshot>(), Arg.Any<CancellationToken>());
|
||||
await _repo.DidNotReceive().UpdateDeployedSnapshotAsync(Arg.Any<DeployedConfigSnapshot>(), Arg.Any<CancellationToken>());
|
||||
// The outcome is audited as a Deploy with Failed status — not as a DeployFailed fault.
|
||||
await _audit.Received(1).LogAsync("admin", "Deploy", "Instance", "10", "Inst10",
|
||||
Arg.Any<object>(), Arg.Any<CancellationToken>());
|
||||
await _audit.DidNotReceive().LogAsync(Arg.Any<string>(), "DeployFailed", Arg.Any<string>(),
|
||||
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<object>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A prior deployment that failed for a NON-timeout reason (the site refused it) cannot have
|
||||
/// been applied, so the redeploy goes straight to the site without a state query. Only an
|
||||
/// InProgress or timeout-marked Failed record may hide an apply central never heard about.
|
||||
/// </summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task Redeploy_PriorFailedForNonTimeoutReason_SkipsSiteQuery()
|
||||
{
|
||||
ArrangeDeployable(11, InstanceState.Enabled);
|
||||
_repo.GetCurrentDeploymentStatusAsync(11, Arg.Any<CancellationToken>())
|
||||
.Returns(new DeploymentRecord("dep-prior", "admin")
|
||||
{
|
||||
InstanceId = 11,
|
||||
Status = DeploymentStatus.Failed,
|
||||
ErrorMessage = "Script 'OnStart' failed to compile at site",
|
||||
});
|
||||
var (actor, seen) = SiteActor(msg => msg switch
|
||||
{
|
||||
RefreshDeploymentCommand d => new DeploymentStatusResponse(
|
||||
d.DeploymentId, d.InstanceUniqueName, DeploymentStatus.Success, null, DateTimeOffset.UtcNow),
|
||||
DeploymentStateQueryRequest q => new DeploymentStateQueryResponse(
|
||||
q.CorrelationId, q.InstanceUniqueName, true, "dep-x", "sha256:new", DateTimeOffset.UtcNow),
|
||||
_ => null,
|
||||
});
|
||||
|
||||
var result = await CreateService(actor).DeployInstanceAsync(11, "admin");
|
||||
|
||||
Assert.True(result.IsSuccess, result.IsFailure ? result.Error : null);
|
||||
lock (seen)
|
||||
{
|
||||
Assert.DoesNotContain(seen, m => m is DeploymentStateQueryRequest);
|
||||
Assert.Single(seen, m => m is RefreshDeploymentCommand);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Failed-status write: detach and retry once ──
|
||||
|
||||
/// <summary>
|
||||
/// When the cleanup write of the Failed status itself fails, the pending changes are detached
|
||||
/// and the write is retried ONCE on the clean unit of work, so the record still reaches its
|
||||
/// terminal state; with nothing to detach there is nothing to retry. Either way the caller
|
||||
/// gets the ORIGINAL deployment error, never the cleanup error.
|
||||
/// </summary>
|
||||
/// <param name="detached">What <c>DetachPendingChanges</c> reports.</param>
|
||||
/// <param name="expectedFailedWrites">Failed-status write attempts expected.</param>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Theory]
|
||||
[InlineData(2, 2)]
|
||||
[InlineData(0, 1)]
|
||||
public async Task Deploy_FailedStatusWriteFails_DetachesAndRetriesOnce(int detached, int expectedFailedWrites)
|
||||
{
|
||||
ArrangeDeployable(12, InstanceState.NotDeployed);
|
||||
DeploymentRecord? record = null;
|
||||
await _repo.AddDeploymentRecordAsync(Arg.Do<DeploymentRecord>(r => record = r), Arg.Any<CancellationToken>());
|
||||
_repo.DetachPendingChanges().Returns(detached);
|
||||
|
||||
var failedWrites = 0;
|
||||
_repo.SaveChangesAsync(Arg.Any<CancellationToken>()).Returns(_ =>
|
||||
{
|
||||
if (record?.Status == DeploymentStatus.Failed && ++failedWrites == 1)
|
||||
throw new InvalidOperationException("poisoned unit of work");
|
||||
return Task.FromResult(1);
|
||||
});
|
||||
|
||||
// No communication actor: the send faults with a non-timeout error.
|
||||
var result = await CreateService(commActor: null).DeployInstanceAsync(12, "admin");
|
||||
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.DoesNotContain("poisoned", result.Error);
|
||||
Assert.Equal(DeploymentStatus.Failed, record!.Status);
|
||||
Assert.StartsWith("Deployment error:", record.ErrorMessage);
|
||||
Assert.Equal(expectedFailedWrites, failedWrites);
|
||||
_repo.Received(1).DetachPendingChanges();
|
||||
// The failure audit row is written even when the status write could not be.
|
||||
await _audit.Received(1).LogAsync("admin", "DeployFailed", "Instance", "12", "Inst12",
|
||||
Arg.Any<object>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
// ── Lifecycle commands: the site answers, but refuses ──
|
||||
|
||||
/// <summary>Lifecycle actions the theory below drives.</summary>
|
||||
public enum LifecycleAction
|
||||
{
|
||||
/// <summary>DisableInstanceAsync.</summary>
|
||||
Disable,
|
||||
|
||||
/// <summary>EnableInstanceAsync.</summary>
|
||||
Enable,
|
||||
|
||||
/// <summary>DeleteInstanceAsync.</summary>
|
||||
Delete,
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A lifecycle command the site answers with Success=false leaves the central instance
|
||||
/// exactly as it was — state unchanged, nothing deleted — still audits the attempt, and
|
||||
/// surfaces the site's message (or a default when the site gave none).
|
||||
/// </summary>
|
||||
/// <param name="action">The lifecycle action.</param>
|
||||
/// <param name="initialState">The instance state before the command.</param>
|
||||
/// <param name="siteError">The site's error message, or null.</param>
|
||||
/// <param name="expectedError">The error the caller must see.</param>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Theory]
|
||||
[InlineData(LifecycleAction.Disable, InstanceState.Enabled, "Instance actor busy", "Instance actor busy")]
|
||||
[InlineData(LifecycleAction.Disable, InstanceState.Enabled, null, "Disable failed.")]
|
||||
[InlineData(LifecycleAction.Enable, InstanceState.Disabled, "No stored config", "No stored config")]
|
||||
[InlineData(LifecycleAction.Enable, InstanceState.Disabled, null, "Enable failed.")]
|
||||
[InlineData(LifecycleAction.Delete, InstanceState.Enabled, "Delete refused", "Delete refused")]
|
||||
[InlineData(LifecycleAction.Delete, InstanceState.Disabled, null, "Delete failed. Site may be unreachable.")]
|
||||
public async Task Lifecycle_SiteRefuses_InstanceUnchanged_AttemptAudited(
|
||||
LifecycleAction action, InstanceState initialState, string? siteError, string expectedError)
|
||||
{
|
||||
var instance = new Instance("LcInst") { Id = 20, SiteId = 1, State = initialState };
|
||||
_repo.GetInstanceByIdAsync(20, Arg.Any<CancellationToken>()).Returns(instance);
|
||||
var (actor, _) = SiteActor(msg => msg switch
|
||||
{
|
||||
DisableInstanceCommand c => new InstanceLifecycleResponse(c.CommandId, c.InstanceUniqueName, false, siteError, DateTimeOffset.UtcNow),
|
||||
EnableInstanceCommand c => new InstanceLifecycleResponse(c.CommandId, c.InstanceUniqueName, false, siteError, DateTimeOffset.UtcNow),
|
||||
DeleteInstanceCommand c => new InstanceLifecycleResponse(c.CommandId, c.InstanceUniqueName, false, siteError, DateTimeOffset.UtcNow),
|
||||
_ => null,
|
||||
});
|
||||
var service = CreateService(actor);
|
||||
|
||||
var result = action switch
|
||||
{
|
||||
LifecycleAction.Disable => await service.DisableInstanceAsync(20, "admin"),
|
||||
LifecycleAction.Enable => await service.EnableInstanceAsync(20, "admin"),
|
||||
_ => await service.DeleteInstanceAsync(20, "admin"),
|
||||
};
|
||||
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.Equal(expectedError, result.Error);
|
||||
Assert.Equal(initialState, instance.State);
|
||||
await _repo.DidNotReceive().UpdateInstanceAsync(Arg.Any<Instance>(), Arg.Any<CancellationToken>());
|
||||
await _repo.DidNotReceive().DeleteInstanceAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
|
||||
await _repo.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>());
|
||||
await _audit.Received(1).LogAsync("admin", action.ToString(), "Instance", "20", "LcInst",
|
||||
Arg.Any<object>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user