Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 856a127cd6 |
@@ -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)
|
## 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
|
**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
|
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).
|
// 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,
|
// 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
|
// restart re-arms the repair, which is correct — a restart is exactly when a box should re-check
|
||||||
// everything it depends on.
|
// everything it depends on.
|
||||||
type storeGrantRepairer struct {
|
type storeGrantRepairer struct {
|
||||||
run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error)
|
run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error)
|
||||||
log *slog.Logger
|
log *slog.Logger
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
last map[string]time.Time // target id → last ATTEMPT (success or failure)
|
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.
|
// 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()
|
defer cancel()
|
||||||
privs, err := px.Permissions(pctx, "/storage/"+targetID)
|
privs, err := px.Permissions(pctx, "/storage/"+targetID)
|
||||||
s = storeGrantVerdict(targetID, critical, privs, err)
|
s = storeGrantVerdict(targetID, critical, privs, err)
|
||||||
if err != nil || s.Status != capability.StatusDegraded {
|
if err != nil {
|
||||||
return s
|
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 ──────────
|
// ── 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
|
// 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
|
// 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).
|
// 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)
|
s = storeGrantRepairedVerdict(targetID, critical)
|
||||||
repairLogger(repair).Error("store-grant: GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED — investigate the loss (R-190)",
|
repairLogger(repair).Error("store-grant: GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED — investigate the loss (R-190)",
|
||||||
"target", targetID, "privilege", storeGrantRequiredPriv,
|
"target", targetID, "privilege", storeGrantRequiredPriv,
|
||||||
@@ -600,6 +650,22 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string,
|
|||||||
return s
|
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
|
// 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).
|
// 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")
|
"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