diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ace9c4..db62354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,35 @@ ## Changelog +### v0.184.0 — E-2b + Part 5: the drive-absent alarm that was never wired (2026-07-29) — MinAgent 0.112.0 + +**`NotifyStorageDisconnected` and `NotifyStorageReconnected` were defined and called from NOWHERE.** +Registered in `allowedEventTypes`, in `DefaultEnabledEvents`, and given a Hungarian message on the +hub — and never invoked. A drive going absent produced apps stopped, a `[WARN]` log and a UI badge, +then **silence on every channel**. Verified against the gitignored-`cmd/` trap with a positive +control. Fifth instance of this class in the project, found by E-2's Phase 0 rather than by a +failure. + +A drive that is *only* a backup target has no apps to stop, so it was silent twice over. + +`ReconcileDriveGates` now calls both halves. When the absent drive is the **whole-guest backup +target** it raises the more specific `backup_target_absent` (error) instead — never both; two mails +for one event trains people to ignore the channel — and recovers as `backup_target_restored` (info, +the existing pairing-gated pattern; `severityNotifies` is NOT widened). The recovery must mirror the +alarm's choice or the operator cannot match them. + +**Which drive is the target comes from the AGENT** (`/disks` `backup_target`, agent ≥ 0.112.0), not +from our own `StoragePath.BackupTarget`: that field is customer INTENT, and on the two boxes migrated +by hand in E-1 the intent was never recorded while the drive really is the target. An older agent +omits the field → false → the generic disconnect alarm, never a wrong one. + +Before this, an absent backup target had **no prompt signal at all**: the tier stays DUE +(`targetStoragePresent` checks name presence, never reachability), so the only evidence was the +tier's own failure at its next due cycle — up to ~24 h on the daily local tier. The R-100 shape. + +**Tests** observe the WIRE, not a mock, because the failure class is "nothing arrives": a real +`Notifier` posts to an `httptest` hub and the test asserts the event type and severity that actually +went out. A typo in the type string is not cosmetic — the hub 400s it and the event vanishes. + ### UNRELEASED — E-2 Part 1: the backup-target role (foundation; NOT yet wired to a UI) **Status: foundation only. No version bump — nothing customer-visible changes yet.** The field is diff --git a/controller/internal/agentapi/client.go b/controller/internal/agentapi/client.go index ef0e36c..a006219 100644 --- a/controller/internal/agentapi/client.go +++ b/controller/internal/agentapi/client.go @@ -323,6 +323,12 @@ type DiskInfo struct { // opposed to merely present on the host (F9) — the signal whose absence let the HDD look available // when it wasn't attached. LEGACY (per-drive mp model); the intermediary model uses BoundUnderParent. GuestAttached bool `json:"guest_attached"` + // BackupTarget (E-2, agent >= v0.112.0) reports that this drive backs the PRIMARY whole-guest + // backup tier. The agent is the only component that can answer: our own + // settings.StoragePath.BackupTarget is customer INTENT, and on a box migrated by hand (E-1) that + // intent was never recorded while the drive really IS the target. Absent on an older agent → + // false, which degrades to the pre-E-2 behaviour (a generic disconnect alarm, never a wrong one). + BackupTarget bool `json:"backup_target,omitempty"` // GuestPath is the drive's STABLE in-guest path in the intermediary-mount model // (/mnt/felhom-drives/) — what the controller registers + repoints HDD_PATH to. Distinct from // MountPath (the raw /mnt/ host PVE mount the agent ops on). "" for non-user-data drives. diff --git a/controller/internal/notify/backup_target_notify_test.go b/controller/internal/notify/backup_target_notify_test.go new file mode 100644 index 0000000..d7895bc --- /dev/null +++ b/controller/internal/notify/backup_target_notify_test.go @@ -0,0 +1,103 @@ +package notify + +import ( + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// capturedEvent is one POST /api/v1/event body the hub would have received. +type capturedEvent struct { + EventType string `json:"event_type"` + Severity string `json:"severity"` + Message string `json:"message"` +} + +// notifierAgainstHub wires a REAL Notifier at an httptest hub and returns a channel of what actually +// went over the wire. The whole failure class E-2b addresses is "the method exists and nobody calls +// it / nothing arrives", so the test has to observe the WIRE, not a mock's call count. +func notifierAgainstHub(t *testing.T) (*Notifier, <-chan capturedEvent) { + t.Helper() + got := make(chan capturedEvent, 8) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + var ev capturedEvent + _ = json.Unmarshal(body, &ev) + got <- ev + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + sett, err := settings.Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatalf("settings: %v", err) + } + n := New(srv.URL, "test-key", "demo-hp", sett, log.New(os.Stderr, "", 0), false) + return n, got +} + +func awaitEvent(t *testing.T, ch <-chan capturedEvent) capturedEvent { + t.Helper() + select { + case ev := <-ch: + return ev + case <-time.After(3 * time.Second): + t.Fatal("no event reached the hub within 3s — the notification never went out") + return capturedEvent{} + } +} + +// E-2 Part 5 — the absent backup target must emit its OWN event type, at error severity. +// +// It has to be `backup_target_absent` specifically: the hub allowlists that string, and an event type +// the hub does not allowlist is answered 400 and VANISHES (R-97a). A typo here is not a cosmetic bug, +// it is total silence — the exact condition E-2 exists to end. +func TestBackupTargetAbsentEmitsItsOwnEventType(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyBackupTargetAbsent("Külső HDD 1TB", "/mnt/hdd_1") + ev := awaitEvent(t, got) + if ev.EventType != "backup_target_absent" { + t.Errorf("event_type = %q, want backup_target_absent (any other string is 400'd by the hub and lost)", ev.EventType) + } + if ev.Severity != "error" { + t.Errorf("severity = %q, want error — a lost whole-system backup is not a warning", ev.Severity) + } + if ev.Message == "" { + t.Error("message is empty — the operator mail would name no drive") + } +} + +// The recovery half. `info` severity is deliberate and load-bearing: the recovery-mail pairing is +// gated on it, and widening severityNotifies to carry recoveries at warning/error would page the +// operator for good news across every event family, not just this one. +func TestBackupTargetRestoredIsPairedAndInfo(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyBackupTargetRestored("Külső HDD 1TB", "/mnt/hdd_1") + ev := awaitEvent(t, got) + if ev.EventType != "backup_target_restored" { + t.Errorf("event_type = %q, want backup_target_restored", ev.EventType) + } + if ev.Severity != "info" { + t.Errorf("severity = %q, want info — recovery mails are pairing-gated on info", ev.Severity) + } +} + +// E-2b — the seam that was defined and never called. This pins that NotifyStorageDisconnected +// actually reaches the hub, which it never did before E-2: it was registered in allowedEventTypes, +// DefaultEnabledEvents and the hub's Hungarian customerMessages, and called from nowhere in the repo. +func TestStorageDisconnectedActuallyReachesTheHub(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyStorageDisconnected("Külső HDD 1TB", []string{"paperless"}) + ev := awaitEvent(t, got) + if ev.EventType != "storage_disconnected" { + t.Errorf("event_type = %q, want storage_disconnected", ev.EventType) + } +} diff --git a/controller/internal/notify/notifier.go b/controller/internal/notify/notifier.go index af27aee..a2aad0f 100644 --- a/controller/internal/notify/notifier.go +++ b/controller/internal/notify/notifier.go @@ -360,6 +360,27 @@ func (n *Notifier) NotifyStorageDisconnected(label string, stoppedApps []string) }) } +// NotifyBackupTargetAbsent (E-2) reports that the drive holding the WHOLE-GUEST backup is gone. +// +// Distinct from NotifyStorageDisconnected on purpose. That one means "a drive went away and some apps +// may have stopped"; this means "the thing that makes your backup survive a disk failure is gone" — +// a different customer action and a different operator urgency. Before E-2 this had NO prompt signal +// at all: the tier stays DUE (targetStoragePresent checks name presence, never reachability), so the +// only evidence was its own failure at the next due cycle, up to ~24 h away on the daily local tier. +func (n *Notifier) NotifyBackupTargetAbsent(label, target string) { + n.PushEvent("backup_target_absent", "error", + fmt.Sprintf("A rendszermentés meghajtója nem érhető el: %s (%s)", label, target), + StorageDetails{Label: label}) +} + +// NotifyBackupTargetRestored is the paired recovery. info severity — the existing recovery pattern; +// severityNotifies is deliberately NOT widened. +func (n *Notifier) NotifyBackupTargetRestored(label, target string) { + n.PushEvent("backup_target_restored", "info", + fmt.Sprintf("A rendszermentés meghajtója újra elérhető: %s (%s)", label, target), + StorageDetails{Label: label}) +} + // NotifyStorageReconnected sends a drive reconnection event. func (n *Notifier) NotifyStorageReconnected(label string) { n.PushEvent("storage_reconnected", "info", diff --git a/controller/internal/web/intermediary.go b/controller/internal/web/intermediary.go index 7d2f832..eca0bc0 100644 --- a/controller/internal/web/intermediary.go +++ b/controller/internal/web/intermediary.go @@ -286,6 +286,15 @@ func (s *Server) ReconcileDriveGates() { s.logger.Printf("[WARN] [gate] mark disconnected %s: %v", a.Path, err) } s.logger.Printf("[WARN] [gate] drive ABSENT %s — stopped+blocked %d app(s): %v", a.Path, len(stopped), stopped) + // E-2b: THE SEAM THAT WAS NEVER WIRED. NotifyStorageDisconnected existed, was registered in + // allowedEventTypes + DefaultEnabledEvents + the hub's Hungarian customerMessages — and was + // called from nowhere, so a drive going absent produced a log line and silence on every + // channel. A drive that is ONLY a backup target has no apps to stop, so it was silent twice + // over. Fifth instance of this class in the project; found by E-2's Phase 0. + // + // E-2 Part 5: when the absent drive is the BACKUP TARGET, that is the more specific and more + // urgent fact, so it gets its own event rather than being folded into the generic one. + s.notifyDriveAbsent(a.Path, stopped, driveTargetByPath(resp.Disks)) go s.SyncFileBrowserMounts() case a.Return: if a.Raw != "" { @@ -304,6 +313,9 @@ func (s *Server) ReconcileDriveGates() { s.logger.Printf("[WARN] [gate] clear disconnected %s: %v", a.Path, err) } s.logger.Printf("[INFO] [gate] drive RETURNED %s — re-attached + restarted gate-stopped apps", a.Path) + // E-2b: the recovery half. An operator told a drive vanished must be told it came back — + // otherwise the alarm is a dead end and the next one is trusted less. + s.notifyDriveReturned(a.Path, driveTargetByPath(resp.Disks)) go s.SyncFileBrowserMounts() } } @@ -582,3 +594,59 @@ func (s *Server) gateWhere(w http.ResponseWriter, r *http.Request) (string, bool } return where, true } + +// driveTargetByPath maps host mount path → the agent's backup-target flag. The agent is the authority +// (E-2): our own StoragePath.BackupTarget is customer intent, and on the two hand-migrated boxes that +// intent was never recorded while the drive really is the target. An older agent omits the field, so +// every entry is false and we degrade to the generic disconnect alarm — never a wrong one. +func driveTargetByPath(disks []agentapi.DiskInfo) map[string]bool { + out := make(map[string]bool, len(disks)) + for _, d := range disks { + if d.MountPath != "" { + out[d.MountPath] = d.BackupTarget + } + } + return out +} + +// storageLabelFor returns the customer-facing label for a registered path, falling back to the path +// itself. An alarm that names a device node the customer has never seen is not actionable. +func (s *Server) storageLabelFor(path string) string { + for _, sp := range s.settings.GetStoragePaths() { + if sp.Path == path && strings.TrimSpace(sp.Label) != "" { + return sp.Label + } + } + return path +} + +// notifyDriveAbsent raises the right alarm for a drive that vanished: the backup-target-specific one +// when it holds the whole-guest backup, the generic one otherwise. Never both — two emails for one +// event trains people to ignore the channel. +func (s *Server) notifyDriveAbsent(path string, stopped []string, isTarget map[string]bool) { + if s.notifier == nil { + return + } + label := s.storageLabelFor(path) + if isTarget[path] { + s.logger.Printf("[ERROR] [gate] the ABSENT drive %s is the WHOLE-GUEST BACKUP TARGET — the system backup cannot run until it returns", path) + s.notifier.NotifyBackupTargetAbsent(label, path) + return + } + s.notifier.NotifyStorageDisconnected(label, stopped) +} + +// notifyDriveReturned is the recovery counterpart, and it must mirror notifyDriveAbsent's choice or +// the pairing breaks: a target that alarmed as backup_target_absent has to recover as +// backup_target_restored, not as a generic reconnect the operator cannot match to the original. +func (s *Server) notifyDriveReturned(path string, isTarget map[string]bool) { + if s.notifier == nil { + return + } + label := s.storageLabelFor(path) + if isTarget[path] { + s.notifier.NotifyBackupTargetRestored(label, path) + return + } + s.notifier.NotifyStorageReconnected(label) +}