v0.124.1: the repair record must survive the probe that did NOT feed the hub (R-190)
gates / gates (push) Successful in 6s
gates / gates (push) Successful in 6s
v0.124.0's transition record never reached the hub, and only the live run showed it. The capability reported degraded for "one cycle" — the probe call that did the repair. But probeAll is invoked independently by the self-check log and by the collector building a host report. On the demo box the repairing call was the log's (09:39:34, journal shows the self-repair and degraded=1) and the report three seconds later found the grant present and sent ok. The agent's journal had the record; the hub had nothing. That is the silence R-190 is about, re-created inside its own mitigation, with every unit test green. Fixed with a latch on TIME, not call count: a confirmed repair reports for 20 minutes, which exceeds the 900s report interval, so at least one report must carry it. It clears on its own and is per tier. Two hollow tests caught and fixed on the way — one asserting a value it built itself, one asserting the latch helper rather than the path consuming it (its red-proof duly passed). The decisions now live in storeGrantHealthyVerdict and storeGrantRepairedVerdict and the tests call those.
This commit is contained in:
@@ -1,3 +1,26 @@
|
||||
## v0.124.1 — the repair record must survive the probe that did NOT feed the hub (2026-08-04, R-190)
|
||||
|
||||
**v0.124.0's transition record did not reach the hub, and the live run is what showed it.** The
|
||||
capability reported degraded for "one cycle" — meaning the probe call that performed the repair. But
|
||||
`probeAll` is invoked **independently** by the periodic self-check log and by the collector building a
|
||||
host-report. On the demo box the repairing call was the log's (`09:39:34`, journal shows
|
||||
`GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED` and `degraded=1`), and the host-report built three
|
||||
seconds later found the grant present and sent **`ok`**. The agent's journal had the record; the hub
|
||||
had nothing; the operator would have learned nothing.
|
||||
|
||||
That is the exact silence R-190 is about, re-created inside its own mitigation — and every unit test
|
||||
passed while it was true.
|
||||
|
||||
**The fix is a latch on TIME rather than on call count.** A confirmed repair is reported for
|
||||
`storeGrantRepairReportWindow` (20 minutes), which comfortably exceeds the 900 s host-report interval,
|
||||
so at least one report must carry the transition. It clears on its own — a permanently degraded
|
||||
capability would be its own false alarm — and it is per tier.
|
||||
|
||||
**Two hollow tests were caught and fixed on the way**, both the same shape this repo keeps finding: a
|
||||
test asserting a value it constructed itself, and a test asserting the latch HELPER rather than the
|
||||
path that consumes it — whose red-proof duly passed. The decisions now live in
|
||||
`storeGrantHealthyVerdict` and `storeGrantRepairedVerdict`, and the tests call those.
|
||||
|
||||
## v0.124.0 — a lost storage grant repairs itself, and says that it was lost (2026-08-04, R-190)
|
||||
|
||||
**R-190 is a grant that demonstrably worked at 04:44 on 2026-08-03 and was gone by 09:24** — with a
|
||||
|
||||
@@ -451,6 +451,21 @@ func storeGrantStatuses(ctx context.Context, px *proxmox.Client, cfg config.Conf
|
||||
return out
|
||||
}
|
||||
|
||||
// storeGrantRepairReportWindow is how long after a repair the capability keeps reporting the
|
||||
// transition. It MUST exceed the hub report interval, or the record never reaches the operator.
|
||||
//
|
||||
// FOUND BY THE LIVE RUN, NOT BY THE TESTS (2026-08-04). The first implementation reported degraded
|
||||
// for exactly "one cycle" — the probe call that did the repair. But `probeAll` is invoked
|
||||
// INDEPENDENTLY by the startup/periodic self-check log and by the collector building a host report,
|
||||
// so the repairing call was the LOG's, and the report built three seconds later found the grant
|
||||
// present and reported `ok`. The agent's journal had the record; the hub had nothing; the operator
|
||||
// would have learned nothing. That is precisely the silence R-190 is about, re-created inside its own
|
||||
// mitigation.
|
||||
//
|
||||
// A latch on TIME rather than on call count fixes it: 20 minutes comfortably exceeds the 900 s report
|
||||
// interval, so at least one host-report must carry the transition, and it still clears on its own.
|
||||
const storeGrantRepairReportWindow = 20 * time.Minute
|
||||
|
||||
// storeGrantRepairMinInterval bounds how often a single tier's grant may be re-granted (Scenario F).
|
||||
//
|
||||
// A storage can be unreadable for reasons an ACL cannot fix — the storage is gone, PVE is wedged,
|
||||
@@ -464,10 +479,37 @@ const storeGrantRepairMinInterval = time.Hour
|
||||
// restart re-arms the repair, which is correct — a restart is exactly when a box should re-check
|
||||
// everything it depends on.
|
||||
type storeGrantRepairer struct {
|
||||
run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error)
|
||||
log *slog.Logger
|
||||
mu sync.Mutex
|
||||
last map[string]time.Time // target id → last ATTEMPT (success or failure)
|
||||
run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error)
|
||||
log *slog.Logger
|
||||
mu sync.Mutex
|
||||
last map[string]time.Time // target id → last ATTEMPT (success or failure)
|
||||
repaired map[string]time.Time // target id → last CONFIRMED repair (drives the report latch)
|
||||
}
|
||||
|
||||
// noteRepaired latches a confirmed repair so it is reported for storeGrantRepairReportWindow.
|
||||
func (r *storeGrantRepairer) noteRepaired(target string, now time.Time) {
|
||||
if r == nil {
|
||||
return
|
||||
}
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
if r.repaired == nil {
|
||||
r.repaired = map[string]time.Time{}
|
||||
}
|
||||
r.repaired[target] = now
|
||||
}
|
||||
|
||||
// recentlyRepaired reports whether a confirmed repair is still inside its report window — the latch
|
||||
// that guarantees a host-report carries the transition even though the probe that repaired may have
|
||||
// been a log-only one.
|
||||
func (r *storeGrantRepairer) recentlyRepaired(target string, now time.Time) bool {
|
||||
if r == nil {
|
||||
return false
|
||||
}
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
t, ok := r.repaired[target]
|
||||
return ok && now.Sub(t) < storeGrantRepairReportWindow
|
||||
}
|
||||
|
||||
// mayAttempt reports whether a repair may run now for this target, and records the attempt if so.
|
||||
@@ -544,9 +586,16 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string,
|
||||
defer cancel()
|
||||
privs, err := px.Permissions(pctx, "/storage/"+targetID)
|
||||
s = storeGrantVerdict(targetID, critical, privs, err)
|
||||
if err != nil || s.Status != capability.StatusDegraded {
|
||||
if err != nil {
|
||||
return s
|
||||
}
|
||||
if s.Status != capability.StatusDegraded {
|
||||
// Healthy — but if this tier was repaired moments ago, keep REPORTING the transition until a
|
||||
// host-report has certainly carried it. Without this latch the repairing probe may be a
|
||||
// log-only one and the hub never learns anything happened (measured live, see the window's
|
||||
// comment).
|
||||
return storeGrantHealthyVerdict(targetID, critical, s, repair.recentlyRepaired(targetID, time.Now()))
|
||||
}
|
||||
|
||||
// ── R-190 mitigation: the grant is missing — repair it, and SAY that it was missing ──────────
|
||||
//
|
||||
@@ -593,6 +642,7 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string,
|
||||
// produces exactly one alert pair and the operator learns of it. NOTHING NEW WAS BUILT: no wire
|
||||
// change, no hub change, no new event type. The `Feature` text carries the explanation because
|
||||
// that is the field the hub puts in the operator's e-mail (the Reason does not travel).
|
||||
repair.noteRepaired(targetID, time.Now())
|
||||
s = storeGrantRepairedVerdict(targetID, critical)
|
||||
repairLogger(repair).Error("store-grant: GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED — investigate the loss (R-190)",
|
||||
"target", targetID, "privilege", storeGrantRequiredPriv,
|
||||
@@ -600,6 +650,22 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string,
|
||||
return s
|
||||
}
|
||||
|
||||
// storeGrantHealthyVerdict decides what a HEALTHY probe reports — which is not always "ok".
|
||||
//
|
||||
// Split out so the tests exercise this decision rather than a copy of it. An earlier version of this
|
||||
// guard lived inline and its red-proof PASSED, because the test asserted the latch helper instead of
|
||||
// the path that consumes it — the same hollow shape this file has now caught twice.
|
||||
//
|
||||
// If the tier was repaired inside the report window, the transition is reported even though the grant
|
||||
// is present: the probe that repaired may have been a log-only one, and without this the host-report
|
||||
// carries `ok` and the operator never learns the permission vanished (measured live 2026-08-04).
|
||||
func storeGrantHealthyVerdict(targetID string, critical bool, healthy capability.Status, repairedRecently bool) capability.Status {
|
||||
if repairedRecently {
|
||||
return storeGrantRepairedVerdict(targetID, critical)
|
||||
}
|
||||
return healthy
|
||||
}
|
||||
|
||||
// storeGrantRepairedVerdict is the post-repair verdict — the RECORD half of R-190, split out so the
|
||||
// tests exercise the real thing rather than a copy of it (yesterday's hollow-test lesson).
|
||||
//
|
||||
|
||||
@@ -353,3 +353,65 @@ func TestMainWiresTheGrantRepair(t *testing.T) {
|
||||
"leave it, which is v0.123.0's behaviour and not R-190's mitigation")
|
||||
}
|
||||
}
|
||||
|
||||
// The transition must survive a probe that is NOT the one feeding the hub.
|
||||
//
|
||||
// MEASURED LIVE 2026-08-04, and this test exists because the first implementation failed it in
|
||||
// production while every unit test passed: `probeAll` is called independently by the self-check LOG
|
||||
// and by the collector building a host-report. The repairing call was the log's; the report three
|
||||
// seconds later found the grant present and reported `ok`. The agent's journal had the record and the
|
||||
// hub had nothing — the exact silence R-190 is about, re-created inside its own mitigation.
|
||||
//
|
||||
// COMPANION RED-PROOF (observed): delete the `recentlyRepaired` branch from the healthy path →
|
||||
//
|
||||
// --- FAIL: TestGrantRepair_TransitionSurvivesALaterProbe
|
||||
// storegrant_test.go: a probe AFTER the repair must still report the transition; got "ok" —
|
||||
// the host-report would carry ok and the operator would never learn the grant vanished
|
||||
//
|
||||
// Restored.
|
||||
func TestGrantRepair_TransitionSurvivesALaterProbe(t *testing.T) {
|
||||
r := newRepairer(&fakeRepairRunner{})
|
||||
// Jittered, never landing on the window boundary.
|
||||
repairedAt := time.Date(2026, 8, 4, 9, 39, 34, 0, time.UTC)
|
||||
r.noteRepaired("felhom-backup", repairedAt)
|
||||
|
||||
// The DECISION a later probe makes — the production function, not the helper it calls. An
|
||||
// earlier draft asserted `recentlyRepaired` directly and its red-proof PASSED, because removing
|
||||
// the latch's USE left the helper untouched.
|
||||
healthy := probeWith(permGranted, "felhom-backup", true)
|
||||
if healthy.Status != capability.StatusOK {
|
||||
t.Fatalf("precondition: a granted tier is ok; got %q", healthy.Status)
|
||||
}
|
||||
got := storeGrantHealthyVerdict("felhom-backup", true,
|
||||
healthy, r.recentlyRepaired("felhom-backup", repairedAt.Add(3*time.Second)))
|
||||
if got.Status != capability.StatusDegraded {
|
||||
t.Fatalf("a probe AFTER the repair must still report the transition; got %q — the host-report "+
|
||||
"would carry ok and the operator would never learn the grant vanished", got.Status)
|
||||
}
|
||||
if !strings.Contains(got.Feature, "RESTORED") {
|
||||
t.Fatalf("the later probe must carry the explanation into the hub's e-mail; got: %s", got.Feature)
|
||||
}
|
||||
// Outside the window it reports plain ok again.
|
||||
late := storeGrantHealthyVerdict("felhom-backup", true,
|
||||
healthy, r.recentlyRepaired("felhom-backup", repairedAt.Add(storeGrantRepairReportWindow+time.Minute)))
|
||||
if late.Status != capability.StatusOK {
|
||||
t.Fatalf("outside the window a healthy tier reports ok; got %q — a permanent degraded state "+
|
||||
"would be its own false alarm", late.Status)
|
||||
}
|
||||
if !r.recentlyRepaired("felhom-backup", repairedAt.Add(14*time.Minute+37*time.Second)) {
|
||||
t.Fatal("the latch must outlast the 900s hub report interval, or the record never reaches the hub")
|
||||
}
|
||||
// ...and it clears on its own rather than latching a box degraded forever.
|
||||
if r.recentlyRepaired("felhom-backup", repairedAt.Add(storeGrantRepairReportWindow+time.Minute+7*time.Second)) {
|
||||
t.Fatal("the latch must clear — a permanent degraded state would be its own false alarm")
|
||||
}
|
||||
// It is per tier.
|
||||
if r.recentlyRepaired("felhom-pbs", repairedAt.Add(time.Second)) {
|
||||
t.Fatal("one tier's repair must not latch another tier's status")
|
||||
}
|
||||
// The window MUST exceed the report interval — the property, asserted rather than assumed.
|
||||
if storeGrantRepairReportWindow <= 15*time.Minute {
|
||||
t.Fatalf("the report window (%s) must exceed the 900s hub report interval, or a transition can "+
|
||||
"be missed entirely", storeGrantRepairReportWindow)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user