R-521: an app a lost drive stopped no longer mails app_start_failed one by one

One unplugged drive sent storage_disconnected plus an app_start_failed per app on it (five operator mails). The dead-app job now hands the notifier the apps of a disconnected drive (its recorded StoppedStacks plus apps whose HDD_PATH is that drive) as not down; the storage event already names them. The dashboard's dead list is unchanged. The hub-side cooldown (F6/F7) and a household mail for a lost drive are not in this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-10-05 21:52:26 +02:00
parent f885100d29
commit 800b32ceec
2 changed files with 154 additions and 1 deletions
@@ -930,7 +930,10 @@ func main() {
 		}
 		dead, states := scanDeployedAppRunStates(stackMgr, quiesceLoop, appStopGuard, backupMgr)
 		alertMgr.SetDeadAppAlerts(dead)
-		notifier.NotifyAppStartFailures(states)
+		// R-521: an app a lost drive stopped is already named in that drive's storage_disconnected
+		// event; its own app_start_failed adds nothing (one unplug mailed the operator five times). The
+		// dashboard's dead-app list (`dead`) is NOT masked — only the per-app mail.
+		notifier.NotifyAppStartFailures(maskDriveLost(states, driveLostApps(sett, stackMgr)))
 		// R-514: a worker OOM-killed inside a running container leaves the app „Fut". Surface it.
 		ooms, oerr := stackMgr.ScanOOMKilled()
 		if oerr != nil {
@@ -2678,6 +2681,62 @@ type updateHeldLister interface {
 	UpdateHeldStacks() map[string]bool
 }

+// driveLostApps (R-521) is the set of apps stopped because their drive is flagged disconnected: the
+// stacks the disconnect recorded (StoppedStacks) plus any deployed app whose app.yaml HDD_PATH is that
+// drive. Nil (no allocation, no app.yaml reads) when no drive is disconnected — the common case.
+func driveLostApps(sett *settings.Settings, mgr hddPathReader) map[string]bool {
+	if sett == nil {
+		return nil
+	}
+	lost := map[string]bool{}
+	var out map[string]bool
+	for _, sp := range sett.GetStoragePaths() {
+		if !sp.Disconnected {
+			continue
+		}
+		lost[sp.Path] = true
+		if out == nil {
+			out = map[string]bool{}
+		}
+		for _, n := range sp.StoppedStacks {
+			out[n] = true
+		}
+	}
+	if len(lost) == 0 || mgr == nil {
+		return out
+	}
+	for _, st := range mgr.GetStacks() {
+		if !st.Deployed {
+			continue
+		}
+		if cfg := mgr.LoadAppConfigByName(st.Name); cfg != nil && lost[cfg.Env["HDD_PATH"]] {
+			out[st.Name] = true
+		}
+	}
+	return out
+}
+
+// hddPathReader is the slice of *stacks.Manager driveLostApps reads (a seam for the test).
+type hddPathReader interface {
+	GetStacks() []stacks.Stack
+	LoadAppConfigByName(name string) *stacks.AppConfig
+}
+
+// maskDriveLost reports the drive-lost apps as not down to the NOTIFIER only (see R-521 at the call).
+func maskDriveLost(states []notify.AppRunState, lost map[string]bool) []notify.AppRunState {
+	if len(lost) == 0 {
+		return states
+	}
+	out := make([]notify.AppRunState, len(states))
+	copy(out, states)
+	for i := range out {
+		if out[i].Down && lost[out[i].Name] {
+			out[i].Down = false
+		}
+	}
+	return out
+}
+
 // updateHeldSet is nil-safe over a nil interface (a box with backup disabled has no holds).
 func updateHeldSet(l updateHeldLister) map[string]bool {
 	if l == nil {
@@ -0,0 +1,94 @@
+package main
+
+import (
+	"go/ast"
+	"io"
+	"log"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
+)
+
+type fakeHDD struct {
+	sts []stacks.Stack
+	hdd map[string]string
+}
+
+func (f fakeHDD) GetStacks() []stacks.Stack { return f.sts }
+func (f fakeHDD) LoadAppConfigByName(n string) *stacks.AppConfig {
+	return &stacks.AppConfig{Env: map[string]string{"HDD_PATH": f.hdd[n]}}
+}
+
+// R-521: one unplugged drive mailed the operator five times — storage_disconnected plus an
+// app_start_failed per app on it. The consequence asserted: the NOTIFIER is told the drive's apps are
+// not down (no per-app mail), while the dashboard's dead list still carries them, and an app on
+// another drive that is genuinely down still alarms.
+func TestR521_DriveLostAppsDoNotMailOneByOne(t *testing.T) {
+	sett, err := settings.Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, p := range []string{"/mnt/felhom-drives/usb", "/mnt/felhom-drives/other"} {
+		if err := sett.AddStoragePath(settings.StoragePath{Path: p, Label: p, Schedulable: true}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := sett.SetDisconnected("/mnt/felhom-drives/usb", true, []string{"immich"}); err != nil {
+		t.Fatal(err)
+	}
+	sts := []stacks.Stack{
+		stack("immich", stacks.StateExited, true, false),    // recorded by the disconnect
+		stack("paperless", stacks.StateExited, true, false), // on the lost drive by HDD_PATH
+		stack("romm", stacks.StateExited, true, false),      // another drive, genuinely broken
+	}
+	for i := range sts {
+		sts[i].AppConfig = &stacks.AppConfig{DesiredState: stacks.DesiredStateRunning}
+	}
+	mgr := fakeHDD{sts: sts, hdd: map[string]string{"immich": "/mnt/felhom-drives/usb", "paperless": "/mnt/felhom-drives/usb", "romm": "/mnt/felhom-drives/other"}}
+
+	dead, states := classifyRunStates(sts, nil, nil, time.Now())
+	notified := downByName(maskDriveLost(states, driveLostApps(sett, mgr)))
+	if notified["immich"] || notified["paperless"] {
+		t.Fatalf("R-521: an app stopped by the lost drive is still reported down to the notifier: %v", notified)
+	}
+	if !notified["romm"] {
+		t.Fatal("a genuinely broken app on another drive stopped alarming — over-correction")
+	}
+	if dn := deadNames(dead); !dn["immich"] || !dn["paperless"] {
+		t.Fatalf("the dashboard's dead list must still carry the drive's apps: %v", dn)
+	}
+	// Reconnected: nothing is masked any more.
+	if err := sett.SetDisconnected("/mnt/felhom-drives/usb", false, nil); err != nil {
+		t.Fatal(err)
+	}
+	if got := driveLostApps(sett, mgr); len(got) != 0 {
+		t.Fatalf("with no drive disconnected nothing may be masked, got %v", got)
+	}
+}
+
+// The wiring: the dead-app job hands the notifier the masked states.
+func TestR521_DeadAppJobMasksDriveLost(t *testing.T) {
+	found := false
+	ast.Inspect(mainBody(t), func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel.Name != "NotifyAppStartFailures" || len(call.Args) != 1 {
+			return true
+		}
+		if inner, ok := call.Args[0].(*ast.CallExpr); ok {
+			if id, ok := inner.Fun.(*ast.Ident); ok && id.Name == "maskDriveLost" {
+				found = true
+			}
+		}
+		return true
+	})
+	if !found {
+		t.Fatal("R-521: main calls NotifyAppStartFailures without maskDriveLost")
+	}
+}