fix(GWC-37): grade the alarm subtag watch-list instead of latching it silently
The subtag fallback's coverage was fixed at SubscribeAlarms time and never re-examined. AlarmWatchListResolver deliberately swallows a Galaxy Repository outage and returns whatever configuration contributed, so a discovery failure at startup left the monitor subscribed with an empty watch-list for the rest of its life. In Auto mode the alarmmgr primary keeps working, so nothing looks wrong until the failover that needs the standby finds it covering zero alarms — with a single LogWarning emitted possibly weeks earlier as the only evidence. ResolveAsync now returns an AlarmWatchListResolution (targets + whether discovery was attempted and whether it failed, with the failure message), which is what the caller needed to tell "discovery produced nothing" from "nothing was configured". GatewayAlarmMonitor grades that resolution against the effective mode and folds the verdict into AlarmProviderStatus, so it reaches the feed, the dashboard badge and every late-joining subscriber: - zero targets while the mode can need them (Auto/ForceSubtag) -> degraded with SubtagWatchListEmpty; discovery enabled but failing adds SubtagDiscoveryFailed and the message. ForceAlarmManager builds no subtag consumer and is not graded. - degraded is now the OR of "subtag is serving" and "the watch-list covers nothing", so the Auto case where alarmmgr is healthy and the standby is empty is visible. DashboardAlarmProviderStatus renders that combination as the amber "Alarm Manager (no subtag standby)" badge rather than the failover label, which would have named the wrong live provider. - on the failover edge (ApplyProviderModeChangeAsync, toMode == Subtag) the monitor re-resolves BEFORE publishing the status and reports any difference as SubtagWatchListStale with both counts. The re-resolution is deliberately NOT re-sent. The worker rejects a second SubscribeAlarms while subscribed (AlarmCommandHandler.Subscribe), and the only expressible replacement — UnsubscribeAlarms then SubscribeAlarms — rebuilds FailoverAlarmConsumer, which restarts on the primary that just failed and discards the warm standby state, so it would flap the feed rather than repair it. The finding's other alternative (treat empty coverage as a lifecycle failure) was rejected too: it would crash-loop the monitor every 5 s on a persistent GR outage and take the working alarmmgr feed down with it. A monitor restart re-resolves and re-sends, and the status now says so. Docs: docs/AlarmClientDiscovery.md gains the grading rules and corrects two stale claims (the resolver method name, and "re-runs discovery on its reconcile cadence and pushes an updated watch-list", which never happened); gateway.md gains the standby-coverage visibility paragraph; docs/GatewayConfiguration.md records the consequence on the UseGalaxyRepository and Mode rows. Tests: SubtagCoverage_WhenDiscoveryFailsAtSubscribe_ProviderStatusIsDegradedWith- Reason and SubtagCoverage_WhenDiscoveryRecoversByFailover_ReResolvesAndReportsDrift drive the fake-worker monitor; two resolver cases pin the new provenance fields. The pre-existing dashboard case for degraded+ALARMMGR now asserts the new label.
This commit is contained in:
Joseph Doherty
@@ -16,6 +16,21 @@ public sealed class AlarmWatchListResolverTests
|
||||
private static AlarmWatchListResolver CreateResolver(IGalaxyRepository repository) =>
|
||||
new(repository, NullLogger<AlarmWatchListResolver>.Instance);
|
||||
|
||||
/// <summary>
|
||||
/// Resolves and projects to the target list. These cases assert composition rules, which
|
||||
/// live in <see cref="AlarmWatchListResolution.Targets"/>; the discovery-provenance fields
|
||||
/// the resolution also carries (GWC-37) are asserted by their own cases below.
|
||||
/// </summary>
|
||||
/// <param name="resolver">The resolver under test.</param>
|
||||
/// <param name="options">Alarm options to resolve against.</param>
|
||||
/// <param name="cancellationToken">Token to cancel the resolution.</param>
|
||||
/// <returns>The resolved targets.</returns>
|
||||
private static async Task<IReadOnlyList<AlarmSubtagTarget>> ResolveTargetsAsync(
|
||||
AlarmWatchListResolver resolver,
|
||||
AlarmsOptions options,
|
||||
CancellationToken cancellationToken = default) =>
|
||||
(await resolver.ResolveAsync(options, cancellationToken)).Targets;
|
||||
|
||||
private static AlarmsOptions Options(
|
||||
bool useGalaxyRepository = true,
|
||||
string area = "",
|
||||
@@ -58,7 +73,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
include: ["pump01.fault", "Valve03.Position.Lo"],
|
||||
exclude: ["Tank02.Level.HiHi"]));
|
||||
|
||||
@@ -81,7 +96,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
subtags: new AlarmSubtagNameOptions
|
||||
{
|
||||
Active = "InAlarm",
|
||||
@@ -110,7 +125,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
// Default Priority is "Priority"; force it empty alongside empty AckComment.
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
subtags: new AlarmSubtagNameOptions { Priority = string.Empty, AckComment = string.Empty }));
|
||||
|
||||
AlarmSubtagTarget target = Assert.Single(result);
|
||||
@@ -140,7 +155,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
// Config area "DEV" must be ignored for a GR row that has a discovered area.
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver,
|
||||
Options(area: "DEV", defaultArea: "DEV"));
|
||||
|
||||
AlarmSubtagTarget target = Assert.Single(result);
|
||||
@@ -161,7 +176,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
// No discovery area and no default area.
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options());
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options());
|
||||
|
||||
AlarmSubtagTarget target = Assert.Single(result);
|
||||
Assert.Equal("Galaxy!Tank01.Level.HiHi", target.AlarmFullReference);
|
||||
@@ -178,7 +193,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
area: "Site_A",
|
||||
include: ["Tank01.Level.HiHi"]));
|
||||
|
||||
@@ -196,7 +211,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
area: "",
|
||||
defaultArea: "Plant",
|
||||
include: ["Tank01.Level.HiHi"]));
|
||||
@@ -217,7 +232,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
useGalaxyRepository: false,
|
||||
include: ["Tank01.Level.HiHi"]));
|
||||
|
||||
@@ -235,7 +250,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
include: ["Tank01.Level.HiHi"]));
|
||||
|
||||
// Did not throw; discovery set empty, include retained.
|
||||
@@ -243,6 +258,51 @@ public sealed class AlarmWatchListResolverTests
|
||||
Assert.Equal("Galaxy!Tank01.Level.HiHi", target.AlarmFullReference);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// GWC-37: swallowing the discovery failure is right for the resolver but leaves the
|
||||
/// caller unable to tell "discovery produced nothing" from "nothing was configured", which
|
||||
/// is why the empty watch-list could sit unnoticed for a monitor's whole life. The
|
||||
/// resolution therefore reports whether discovery was attempted, whether it failed, and
|
||||
/// the failure message the provider status quotes.
|
||||
/// </summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task ResolveAsync_RepositoryThrows_ReportsDiscoveryFailureOnTheResolution()
|
||||
{
|
||||
ThrowingGalaxyRepository repo = new(new InvalidOperationException("SQL unavailable"));
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
AlarmWatchListResolution resolution = await resolver.ResolveAsync(Options());
|
||||
|
||||
Assert.Empty(resolution.Targets);
|
||||
Assert.True(resolution.DiscoveryAttempted);
|
||||
Assert.True(resolution.DiscoveryFailed);
|
||||
Assert.Equal("SQL unavailable", resolution.DiscoveryFailureMessage);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The complement of the case above: discovery disabled is not discovery failed. An empty
|
||||
/// watch-list here means the deployment did not ask for subtag coverage, which the monitor
|
||||
/// must not report as a degraded standby (GWC-37).
|
||||
/// </summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task ResolveAsync_RepositoryDisabled_ReportsDiscoveryNotAttempted()
|
||||
{
|
||||
StubGalaxyRepository repo = new([]);
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
AlarmWatchListResolution resolution = await resolver.ResolveAsync(
|
||||
Options(useGalaxyRepository: false));
|
||||
|
||||
Assert.Empty(resolution.Targets);
|
||||
Assert.False(resolution.DiscoveryAttempted);
|
||||
Assert.False(resolution.DiscoveryFailed);
|
||||
Assert.Null(resolution.DiscoveryFailureMessage);
|
||||
}
|
||||
|
||||
/// <summary>Verifies the source object reference is derived from a config include entry, using the portion before the first dot when present.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
@@ -252,7 +312,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
include: ["Tank01.Level.HiHi", "StandaloneTag"]));
|
||||
|
||||
Assert.Equal("Tank01", result[0].SourceObjectReference);
|
||||
@@ -273,7 +333,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
useGalaxyRepository: false,
|
||||
include: ["Tank01.Level.HiHi"],
|
||||
exclude: ["Tank01.Level.HiHi"]));
|
||||
@@ -299,7 +359,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
useGalaxyRepository: true,
|
||||
exclude: ["Tank02.Level.HiHi"]));
|
||||
|
||||
@@ -324,7 +384,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
// The exclude array contains a whitespace-only string — should be a no-op.
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
useGalaxyRepository: true,
|
||||
exclude: [" "]));
|
||||
|
||||
@@ -351,7 +411,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
AlarmWatchListResolver resolver = CreateResolver(repo);
|
||||
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(() =>
|
||||
resolver.ResolveAsync(
|
||||
ResolveTargetsAsync(resolver,
|
||||
Options(include: ["Tank01.Level.HiHi"]),
|
||||
cts.Token));
|
||||
}
|
||||
@@ -375,7 +435,7 @@ public sealed class AlarmWatchListResolverTests
|
||||
|
||||
// Pump01.Fault is an explicit include AND an exclude (case-insensitive). It must
|
||||
// be dropped; the GR row Tank01 survives.
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await resolver.ResolveAsync(Options(
|
||||
IReadOnlyList<AlarmSubtagTarget> result = await ResolveTargetsAsync(resolver, Options(
|
||||
useGalaxyRepository: true,
|
||||
include: ["Pump01.Fault"],
|
||||
exclude: ["pump01.fault"]));
|
||||
|
||||
Reference in New Issue
Block a user