fix(R-1298): held-app reports only for installed apps (a restore hold always)
Red-proved. Commit only.
This commit is contained in:
1 parent
dfd0f8ec53
commit
b156e4526a
4 files changed
+75
-3
No files matched your search
@@ -34,6 +34,11 @@ hub answers 400 to both and nothing else changes.
|
|||||||
runs `psql -v ON_ERROR_STOP=1`. Tests `internal/appexport/r1187_abdfg_test.go` (6), red-proved with the old behaviour
|
runs `psql -v ON_ERROR_STOP=1`. Tests `internal/appexport/r1187_abdfg_test.go` (6), red-proved with the old behaviour
|
||||||
behind the new function shapes (link packing error, a 10-s hang on the FIFO, link not restored, import not refused,
|
behind the new function shapes (link packing error, a 10-s hang on the FIFO, link not restored, import not refused,
|
||||||
bad paths accepted, postgis missed).
|
bad paths accepted, postgis missed).
|
||||||
|
- **R-1298:** the hold report skips an install's hold (`unhealthy_stop`, `update_failed`, `putback_pending`) of an app that
|
||||||
|
is not installed (`holdreport.Reporter.SetInstalled`: the app's `app.yaml` exists — an unreadable one still counts,
|
||||||
|
R-1220) and reports it cleared if it was reported; a restore hold is always reported (R-379). 9202's four orphan holds
|
||||||
|
from a hand removal would otherwise have mailed weekly. Test `TestR1298_OnlyHoldsOfInstalledAppsAreReported`,
|
||||||
|
red-proved; the wiring test pins the `SetInstalled` line.
|
||||||
- `settings.RestoreHold.Why()` / `WireReason()`: the hold sentence moved out of `cmd/controller` (`holdWhy` calls it), so
|
- `settings.RestoreHold.Why()` / `WireReason()`: the hold sentence moved out of `cmd/controller` (`holdWhy` calls it), so
|
||||||
`--restore-holds` and the hub's list say the same thing.
|
`--restore-holds` and the hub's list say the same thing.
|
||||||
- Tests `internal/holdreport/holdreport_test.go` (8): the report field, the daily repeat, cleared once, cleared while
|
- Tests `internal/holdreport/holdreport_test.go` (8): the report field, the daily repeat, cleared once, cleared while
|
||||||
|
|||||||
@@ -1068,6 +1068,12 @@ func main() {
|
|||||||
// when one is gone, every hold again once a day and after each start. One observer of the hold store, so every
|
// when one is gone, every hold again once a day and after each start. One observer of the hold store, so every
|
||||||
// writer is covered. Pinned by internal/holdreport.
|
// writer is covered. Pinned by internal/holdreport.
|
||||||
holdReporter := holdreport.New(sett, notifier.PushEvent, logger)
|
holdReporter := holdreport.New(sett, notifier.PushEvent, logger)
|
||||||
|
// R-1298: „installed" = the app's app.yaml file exists (a product removal deletes it; an unreadable one still counts,
|
||||||
|
// the R-1220 rule). An install's hold of an app without one is not reported.
|
||||||
|
holdReporter.SetInstalled(func(stack string) bool {
|
||||||
|
_, err := os.Stat(filepath.Join(cfg.Paths.StacksDir, stack, "app.yaml"))
|
||||||
|
return err == nil
|
||||||
|
})
|
||||||
sched.Every("hold-report", time.Minute, func(ctx context.Context) error {
|
sched.Every("hold-report", time.Minute, func(ctx context.Context) error {
|
||||||
holdReporter.Tick(time.Now())
|
holdReporter.Tick(time.Now())
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -60,6 +60,9 @@ type Reporter struct {
|
|||||||
push Push
|
push Push
|
||||||
logger *log.Logger
|
logger *log.Logger
|
||||||
lastFull time.Time
|
lastFull time.Time
|
||||||
|
// installed (R-1298) — nil = every app counts as installed.
|
||||||
|
installed func(stack string) bool
|
||||||
|
skipLogged map[string]bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// New builds a reporter. The first Tick re-sends every hold (a controller start).
|
// New builds a reporter. The first Tick re-sends every hold (a controller start).
|
||||||
@@ -69,13 +72,42 @@ func New(store Store, push Push, logger *log.Logger) *Reporter {
|
|||||||
|
|
||||||
func key(h settings.RestoreHold) string { return h.WireReason() + "|" + h.At }
|
func key(h settings.RestoreHold) string { return h.WireReason() + "|" + h.At }
|
||||||
|
|
||||||
|
// SetInstalled wires the „is this app installed?" check (R-1298). nil = every app counts as installed.
|
||||||
|
func (r *Reporter) SetInstalled(f func(stack string) bool) { r.installed = f }
|
||||||
|
|
||||||
|
// installHold is a hold that belongs to an app's INSTALL (R-704): a removal ends it. A restore hold (restore_failed,
|
||||||
|
// restore_mixed) does not — a person clears it, so it stays reported whatever the app's state (R-379).
|
||||||
|
func installHold(h settings.RestoreHold) bool {
|
||||||
|
switch h.Reason {
|
||||||
|
case settings.HoldReasonUnhealthyStop, settings.HoldReasonUpdateFailed, settings.HoldReasonPutBackPending:
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// Tick sends what changed since the last tick, and every hold again when a day has passed.
|
// Tick sends what changed since the last tick, and every hold again when a day has passed.
|
||||||
func (r *Reporter) Tick(now time.Time) {
|
func (r *Reporter) Tick(now time.Time) {
|
||||||
if r.store == nil || r.push == nil {
|
if r.store == nil || r.push == nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
holds := r.store.ListRestoreHolds()
|
all := r.store.ListRestoreHolds()
|
||||||
sort.Slice(holds, func(i, j int) bool { return holds[i].Stack < holds[j].Stack })
|
sort.Slice(all, func(i, j int) bool { return all[i].Stack < all[j].Stack })
|
||||||
|
// R-1298: an install's hold of an app that is not installed any more (removed outside the product) is not a held
|
||||||
|
// app — it is skipped, and reported cleared if it was reported before. Logged once per stack per process.
|
||||||
|
holds := all[:0:0]
|
||||||
|
for _, h := range all {
|
||||||
|
if r.installed != nil && installHold(h) && !r.installed(h.Stack) {
|
||||||
|
if r.skipLogged == nil {
|
||||||
|
r.skipLogged = map[string]bool{}
|
||||||
|
}
|
||||||
|
if !r.skipLogged[h.Stack] {
|
||||||
|
r.skipLogged[h.Stack] = true
|
||||||
|
r.logf("[INFO] [holdreport] %s: a %s hold of an app that is not installed — not reported (R-1298)", h.Stack, h.WireReason())
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
holds = append(holds, h)
|
||||||
|
}
|
||||||
prev := r.store.GetReportedHolds()
|
prev := r.store.GetReportedHolds()
|
||||||
full := r.lastFull.IsZero() || now.Sub(r.lastFull) >= ResendEvery
|
full := r.lastFull.IsZero() || now.Sub(r.lastFull) >= ResendEvery
|
||||||
|
|
||||||
|
|||||||
@@ -155,7 +155,7 @@ func TestD53_TheReporterIsWiredInMain(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
src := strings.Join(live, "\n")
|
src := strings.Join(live, "\n")
|
||||||
for _, want := range []string{"holdreport.New(sett, notifier.PushEvent, logger)", `sched.Every("hold-report"`, "holdReporter.Tick("} {
|
for _, want := range []string{"holdreport.New(sett, notifier.PushEvent, logger)", `sched.Every("hold-report"`, "holdReporter.Tick(", "holdReporter.SetInstalled("} {
|
||||||
if !strings.Contains(src, want) {
|
if !strings.Contains(src, want) {
|
||||||
t.Errorf("main.go lacks %q — the hold report is not wired", want)
|
t.Errorf("main.go lacks %q — the hold report is not wired", want)
|
||||||
}
|
}
|
||||||
@@ -180,3 +180,32 @@ func TestD53_TheReportedRecordSurvivesAReload(t *testing.T) {
|
|||||||
t.Fatalf("after a reload the record reads %q", got)
|
t.Fatalf("after a reload the record reads %q", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// R-1298: an install's hold (crash-loop stop, failed update, waiting put-back) of an app that is NOT installed any more is
|
||||||
|
// not reported — on 9202 four such holds from a hand removal would have mailed the operator weekly about apps that do not
|
||||||
|
// exist. A restore hold stays reported whatever the app's state (R-379: a person clears it). A hold reported earlier and
|
||||||
|
// now skipped is reported cleared. COMPANION RED-PROOF: without the installed check all three holds are reported.
|
||||||
|
func TestR1298_OnlyHoldsOfInstalledAppsAreReported(t *testing.T) {
|
||||||
|
st := &fakeStore{holds: map[string]settings.RestoreHold{
|
||||||
|
"gone": {Stack: "gone", At: "2026-09-24T12:16:24Z", Reason: settings.HoldReasonUnhealthyStop},
|
||||||
|
"gonedb": {Stack: "gonedb", At: "2026-09-24T12:16:24Z", Reason: settings.HoldReasonRestoreFailed},
|
||||||
|
"present": {Stack: "present", At: "2026-10-26T17:37:52Z", Reason: settings.HoldReasonUpdateFailed},
|
||||||
|
}, reported: map[string]string{"gone": "unhealthy_stop|2026-09-24T12:16:24Z"}}
|
||||||
|
var out []sent
|
||||||
|
r := New(st, rec(&out), nil)
|
||||||
|
r.SetInstalled(func(stack string) bool { return stack == "present" })
|
||||||
|
r.Tick(t0)
|
||||||
|
got := map[string]string{}
|
||||||
|
for _, s := range out {
|
||||||
|
got[s.d.StackName] = s.typ
|
||||||
|
}
|
||||||
|
if got["gone"] != EventCleared {
|
||||||
|
t.Errorf("the hold of an app that is not installed must be reported cleared, got %q", got["gone"])
|
||||||
|
}
|
||||||
|
if got["gonedb"] != EventHeld {
|
||||||
|
t.Errorf("a restore hold must stay reported even when the app is gone, got %q", got["gonedb"])
|
||||||
|
}
|
||||||
|
if got["present"] != EventHeld {
|
||||||
|
t.Errorf("an installed app's hold must be reported, got %q", got["present"])
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user