Mark failed PBS poll as failure and lock down with regression tests

The PBS poller's version-failure exit at monitor_pbs_pmg.go did not
set pollErr before returning, so even after fixing the defer-arg
capture in bf6261adc the deferred recordTaskResult still saw nil and
recorded the poll as a success. PMG's analogous path already sets
pollErr correctly. Mirror that here so the per-instance pollStatusMap,
the connections aggregator, and the circuit breaker all see PBS auth
and version failures as failures.

Add assertions to the existing PBS and PMG auth-failure tests that
the per-instance pollStatusMap entry has a zero LastSuccess and a
non-zero ConsecutiveFailures. The original tests covered downstream
state but not the recorder, which is why two distinct cases of this
class of bug went unnoticed.
This commit is contained in:
rcourtman
2026-05-08 19:53:03 +01:00
parent c0153f8d41
commit bd7d196c11
3 changed files with 31 additions and 0 deletions
@@ -64,6 +64,21 @@ func TestMonitor_PollPBSInstance_AuthFailure(t *testing.T) {
t.Error("Expected connection health to be false")
}
// Regression: pollStatusMap must record the failure. A defer-arg bug
// previously captured pollErr at register-time (always nil), so failed
// polls were recorded as success and the connections aggregator reported
// broken instances as healthy.
if status := m.pollStatusMap["pbs::pbs-auth-fail"]; status == nil {
t.Fatal("expected pollStatusMap entry for pbs::pbs-auth-fail, got nil")
} else {
if !status.LastSuccess.IsZero() {
t.Errorf("expected LastSuccess to remain zero on auth failure, got %v", status.LastSuccess)
}
if status.ConsecutiveFailures == 0 {
t.Error("expected ConsecutiveFailures > 0 after auth failure, got 0")
}
}
// We can't easily check authFailures map as it is private and no getter (except checking if it backs off?)
}
+1
View File
@@ -188,6 +188,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie
pbsInst.Status = "offline"
pbsInst.ConnectionHealth = "error"
monErr := errors.WrapConnectionError("get_pbs_version", instanceName, versionErr)
pollErr = monErr
log.Error().Err(monErr).Str("instance", instanceName).Msg("failed to connect to PBS")
m.setProviderConnectionHealth(InstanceTypePBS, instanceName, false)
+15
View File
@@ -249,4 +249,19 @@ func TestPollPMGInstanceRecordsAuthFailures(t *testing.T) {
if failures := mon.authFailures["pmg-failing"]; failures != 1 {
t.Fatalf("expected one auth failure tracked, got %d", failures)
}
// Regression: pollStatusMap must record the failure. A defer-arg bug
// previously captured pollErr at register-time (always nil), so failed
// polls were recorded as success and the connections aggregator reported
// broken instances as healthy.
status := mon.pollStatusMap["pmg::failing"]
if status == nil {
t.Fatal("expected pollStatusMap entry for pmg::failing, got nil")
}
if !status.LastSuccess.IsZero() {
t.Errorf("expected LastSuccess to remain zero on auth failure, got %v", status.LastSuccess)
}
if status.ConsecutiveFailures == 0 {
t.Error("expected ConsecutiveFailures > 0 after auth failure, got 0")
}
}