From 3f7cf2a9650559f3959a483e944d74b5278f1a82 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 29 Jul 2026 09:10:20 +0200 Subject: [PATCH] =?UTF-8?q?v0.185.0=20=E2=80=94=20E-2=20Parts=203+4:=20the?= =?UTF-8?q?=20offer,=20and=20the=20honest=20degraded=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The half that makes the rest work: a degraded backup target recorded only in config is the silent-degradation pattern this arc has spent a week removing. Part 3 -- POST /api/backup-target/assign moves the target via the agent's POST /backup/target. It is the ONLY writer of the role: registration does not set it, the drive-gate does not, no scheduler does. Declining is not calling it. The agent returns restart_required rather than restarting itself, because restarting with a backup in flight records a spurious tier failure for a backup that actually succeeded (E-1 did exactly that). Part 4 -- GET /api/backup-target returns the state and, when degraded, Hungarian copy in FACT -> CONSEQUENCE -> REMEDY order, pinned by a test: a customer told only the fact cannot act on it. Healthy renders NOTHING -- no badge, no reassurance, no tonal change. degradedMessageFor is the single decision point, so exactly one place could start decorating a working box. Red-proofed: reassuring on the healthy branch fails Scenario E. UNKNOWN is not degraded: an unreachable or pre-R-82 agent means we could not ask, which is not evidence of degradation (R-88 Part 2's class). A HOLLOW TEST caught by its own red-proof: TestUnknownStateRendersNothing used {Known:false} with Degraded left false, so it passed even with the !Known guard deleted -- the second condition covered for it. Now {Known:false, Degraded:true}, which fails properly. Without the red-proof the test would have been decoration. State is derived from the AGENT, never from our intent flag: on the two boxes migrated by hand in E-1 the intent was never recorded while the drive really is the target. MinAgent: 0.113.0 Green gate: build + vet + test rc=0 (27 packages), run separately from this commit. --- CHANGELOG.md | 38 +++ controller/internal/agentapi/backup_tiers.go | 27 +++ .../internal/web/backup_target_offer.go | 221 ++++++++++++++++++ .../internal/web/backup_target_offer_test.go | 61 +++++ controller/internal/web/storage_handlers.go | 6 + 5 files changed, 353 insertions(+) create mode 100644 controller/internal/web/backup_target_offer.go create mode 100644 controller/internal/web/backup_target_offer_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e58e8c..3809642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,43 @@ ## Changelog +### v0.185.0 — E-2 Parts 3+4: the offer, and the honest degraded state (2026-07-29) — MinAgent 0.113.0 + +The half that makes the rest work. A degraded backup target recorded only in config is the +silent-degradation pattern this arc has spent a week removing. + +**Part 3 — the offer.** `POST /api/backup-target/assign` moves the target via the agent's +`POST /backup/target`. It is the **only** writer of the role: registration does not set it, the +drive-gate does not, no scheduler does. Declining is simply not calling it. + +The agent returns `restart_required` rather than restarting itself, and the reason is E-1's own +mistake: restarting with a backup in flight cancels the wait and records a spurious tier failure for +a backup that actually succeeded. The restart belongs to whoever can re-check in-flight work +immediately beforehand. + +**Part 4 — visibility.** `GET /api/backup-target` returns the state and, when degraded, the +Hungarian copy: + +> „A rendszermentés jelenleg ugyanazon a lemezen van, mint a rendszer — így hibás fájlok ellen véd, +> lemezhiba ellen nem. Csatlakoztass egy második meghajtót a teljes védelemhez." + +FACT → CONSEQUENCE → REMEDY, pinned by a test: a customer told only the fact cannot act on it. + +**Healthy renders NOTHING** — no badge, no reassurance, no tonal change. `degradedMessageFor` is the +single decision point so there is exactly one place that could start decorating a working box. +Red-proofed: adding „A rendszermentés védett…" to the healthy branch fails Scenario E. + +**UNKNOWN is not degraded.** An unreachable or pre-R-82 agent means we could not ask, which is not +evidence of degradation — the absence-read-as-a-value mistake R-88 Part 2 closed. + +**A hollow test caught by its own red-proof.** `TestUnknownStateRendersNothing` originally used +`{Known:false}` with `Degraded` left false, so it passed even with the `!Known` guard deleted — the +second condition covered for it. The fixture is now `{Known:false, Degraded:true}`, which fails +properly when the guard goes. The red-proof is what exposed it; without it the test would have been +decoration. + +State is derived from the AGENT, never from our own intent flag: on the two boxes migrated by hand +in E-1 the intent was never recorded while the drive really is the target. + ### v0.184.1 — E-2b keying fix: the backup-target branch was unreachable (2026-07-29) **Caught before deploy by tracing, not by a failure — and the 0.184.0 image is therefore superseded diff --git a/controller/internal/agentapi/backup_tiers.go b/controller/internal/agentapi/backup_tiers.go index 2ab448b..6602b7f 100644 --- a/controller/internal/agentapi/backup_tiers.go +++ b/controller/internal/agentapi/backup_tiers.go @@ -101,3 +101,30 @@ func (c *Client) BackupStatusFor(ctx context.Context, target string) (StatusResp } return out, nil } + +// SetBackupTargetResponse mirrors POST /backup/target (agent >= v0.113.0). +type SetBackupTargetResponse struct { + Target string `json:"target"` + Where string `json:"where"` + // RestartRequired is always true on success: the agent builds its tiers once at daemon start, so + // the move needs a restart. The agent deliberately does NOT restart itself — restarting with a + // backup in flight cancels the wait and records a spurious tier failure for a backup that actually + // succeeded. The RESTART IS THE OPERATOR'S, behind an immediate in-flight check. + RestartRequired bool `json:"restart_required"` +} + +// SetBackupTarget moves the primary whole-guest backup tier onto the drive at raw host mount `where`. +// Creates the storage and grants the agent access as one ordered operation. +func (c *Client) SetBackupTarget(ctx context.Context, where string) (SetBackupTargetResponse, error) { + var out SetBackupTargetResponse + // vmid is deliberately omitted: the agent derives the guest from the token and scopedFromBody + // treats an absent vmid as "use the token's" — the same shape as AssignDisk/GuestAttach. + body, err := c.post(ctx, "/backup/target", map[string]string{"where": where}) + if err != nil { + return out, err + } + if err := json.Unmarshal(body, &out); err != nil { + return out, fmt.Errorf("agentapi: decode /backup/target: %w", err) + } + return out, nil +} diff --git a/controller/internal/web/backup_target_offer.go b/controller/internal/web/backup_target_offer.go new file mode 100644 index 0000000..6934bda --- /dev/null +++ b/controller/internal/web/backup_target_offer.go @@ -0,0 +1,221 @@ +package web + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "strings" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +var ( + errAgentUnreadable = errors.New("a meghajtók listája nem olvasható") + errDriveNotFound = errors.New("a meghajtó nem található") +) + +// BackupTargetState is the customer-facing answer to "does the whole-system backup survive a disk +// failure?". Derived from the AGENT (the authority), never from our own intent flag alone. +type BackupTargetState struct { + // Known is false when the agent could not be asked. Everything below is then meaningless, and the + // UI must render NOTHING rather than guess — an unreachable agent is not evidence of degradation. + Known bool + // Degraded is true when the whole-guest backup lands on the SYSTEM drive, so it protects against + // corruption but not against drive loss. + Degraded bool + // TargetID is the agent's primary tier storage id (e.g. "felhom-backup" / "local"). + TargetID string + // Label is the customer-facing drive name when the target is a real drive. + Label string + // OfferPath, when non-empty, is a registered drive that COULD become the target — the offer. + OfferPath string + OfferLabel string +} + +// resolveBackupTargetState asks the agent what the primary tier actually writes to and classifies it. +// +// DEGRADED means "on the system drive", and it is decided from the agent's own storage view: a tier +// whose target has NO mount path of its own (the builtin `local`, i.e. /var/lib/vz on the root fs) +// cannot survive the root device dying. A tier on a drive with its own mountpoint can. +// +// PBS is deliberately NOT counted as the local target here: the offsite tier is separate hardware and +// a different question. This state answers only "is there a LOCAL copy that survives a disk failure", +// which is matrix row 4. +func (s *Server) resolveBackupTargetState(ctx context.Context) BackupTargetState { + agent, err := s.agentClient() + if err != nil { + return BackupTargetState{} + } + tiers, err := agent.BackupTiers(ctx) + if err != nil { + // Includes ErrTiersUnsupported (a pre-R-82 agent). Unknown, never "degraded": claiming + // degradation because we could not ask would put a permanent warning on a healthy box. + return BackupTargetState{} + } + var primary string + for _, t := range tiers.Tiers { + if t.Primary { + primary = t.Target + break + } + } + st := BackupTargetState{Known: true, TargetID: primary} + + disks, derr := agent.Disks(ctx) + if derr != nil { + return BackupTargetState{} // could not classify → say nothing + } + for _, d := range disks.Disks { + if d.BackupTarget && d.MountPath != "" { + // A real drive with its own mountpoint — healthy. + st.Label = s.storageLabelFor(stablePathForName(baseName(d.MountPath))) + return st + } + } + // No disk claims the target ⇒ it is the builtin `local` on the system drive, or the tier is + // unset. Either way the local backup does not survive drive loss. + st.Degraded = true + st.OfferPath, st.OfferLabel = s.firstOfferableDrive(disks.Disks) + return st +} + +// firstOfferableDrive picks a registered, connected, non-network drive that could hold the backup. +// It is a SUGGESTION for the offer — the customer still has to choose (E-2 §3). Nothing here assigns +// anything. +func (s *Server) firstOfferableDrive(disks []agentapi.DiskInfo) (path, label string) { + for _, sp := range s.settings.GetStoragePaths() { + if sp.IsNetwork() || sp.Decommissioned || sp.Disconnected { + continue + } + for _, d := range disks { + // Only a drive the agent classifies as user-data with its OWN mountpoint can be a target; + // the agent refuses anything else anyway, so offering it would be a dead end. + if d.GuestPath == sp.Path && d.MountPath != "" && d.Role == "user-data" { + return sp.Path, s.storageLabelFor(sp.Path) + } + } + } + return "", "" +} + +// baseName is path.Base without importing path into this file's surface. +func baseName(p string) string { + if i := strings.LastIndex(p, "/"); i >= 0 { + return p[i+1:] + } + return p +} + +// ---- Hungarian customer copy ------------------------------------------------------------------- +// +// Adult tone, no alarm decoration. It states the FACT, the CONSEQUENCE and the REMEDY, in that order, +// because a customer who is told only the fact cannot act on it. +// +// HEALTHY RENDERS NOTHING. There is deliberately no "your backup is safe" banner: a working +// configuration must look normal, or every customer's dashboard grows a permanent notice and the +// warning stops meaning anything (E-2 Scenario E). +const ( + backupTargetDegradedText = "A rendszermentés jelenleg ugyanazon a lemezen van, mint a rendszer — " + + "így hibás fájlok ellen véd, lemezhiba ellen nem. Csatlakoztass egy második meghajtót a teljes védelemhez." + backupTargetOfferText = "Ezt a meghajtót kijelölheted a rendszermentés helyéül — így egy lemezhiba " + + "után is vissza tudod állítani a rendszert." +) + +// degradedMessageFor is the single decision point for "does the customer see anything?" — extracted +// so the render rule is testable without a live agent, and so there is exactly ONE place that can +// accidentally start decorating a healthy box. +// +// Returns "" for BOTH healthy and unknown. They are different states with the same rendering, and +// collapsing them here is deliberate: unknown means we could not ask, which is not evidence of +// degradation (the absence-read-as-a-value mistake R-88 Part 2 closed). +func degradedMessageFor(st BackupTargetState) string { + if !st.Known || !st.Degraded { + return "" + } + return backupTargetDegradedText +} + +// handleBackupTargetState serves GET /api/backup-target — the dashboard's source for the degraded +// banner and the offer. +func (s *Server) handleBackupTargetState(w http.ResponseWriter, r *http.Request) { + st := s.resolveBackupTargetState(r.Context()) + out := map[string]any{"known": st.Known} + if st.Known { + out["degraded"] = st.Degraded + out["target"] = st.TargetID + out["label"] = st.Label + if st.Degraded { + out["message"] = degradedMessageFor(st) + if st.OfferPath != "" { + out["offer_path"] = st.OfferPath + out["offer_label"] = st.OfferLabel + out["offer_message"] = backupTargetOfferText + } + } + } + writeDiskJSON(w, http.StatusOK, true, "", out) +} + +// handleBackupTargetAssign is the ACCEPTANCE of the offer — POST /api/backup-target/assign. +// +// NOTHING calls this except an explicit customer action. Registration does not, the drive-gate does +// not, and no scheduler does (E-2 §3: a drive never acquires a role by appearing). Declining is +// simply not calling it, and the degraded state stays visible on the next visit rather than going +// quiet (Scenario D). +func (s *Server) handleBackupTargetAssign(w http.ResponseWriter, r *http.Request) { + var req struct { + Path string `json:"path"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeDiskJSON(w, http.StatusBadRequest, false, "érvénytelen kérés", nil) + return + } + stable := strings.TrimSpace(req.Path) + if stable == "" { + writeDiskJSON(w, http.StatusBadRequest, false, "hiányzó meghajtó", nil) + return + } + agent, err := s.agentClient() + if err != nil { + writeDiskJSON(w, http.StatusServiceUnavailable, false, "a host-ügynök nem elérhető", nil) + return + } + // The agent operates on the RAW host mount path, not our stable guest path — the same asymmetry + // that register/attach already navigate. + raw, err := s.rawMountForStable(r.Context(), agent, stable) + if err != nil { + writeDiskJSON(w, http.StatusBadRequest, false, err.Error(), nil) + return + } + res, err := agent.SetBackupTarget(r.Context(), raw) + if err != nil { + s.logger.Printf("[WARN] [web] backup-target assign %s (raw %s): %v", stable, raw, err) + writeDiskJSON(w, http.StatusBadGateway, false, "a mentési cél beállítása nem sikerült: "+err.Error(), nil) + return + } + // Record the INTENT only after the agent accepted, so our flag can never claim a target the agent + // does not have. + if err := s.settings.SetBackupTarget(stable); err != nil { + s.logger.Printf("[WARN] [web] backup-target intent record %s: %v", stable, err) + } + s.logger.Printf("[INFO] [web] backup target assigned to %s (raw %s); agent restart_required=%v", + stable, raw, res.RestartRequired) + writeDiskJSON(w, http.StatusOK, true, "", map[string]any{ + "assigned": stable, "restart_required": res.RestartRequired, + }) +} + +// rawMountForStable maps our registered stable guest path back to the agent's raw host mount. +func (s *Server) rawMountForStable(ctx context.Context, agent *agentapi.Client, stable string) (string, error) { + disks, err := agent.Disks(ctx) + if err != nil { + return "", errAgentUnreadable + } + for _, d := range disks.Disks { + if d.GuestPath == stable && d.MountPath != "" { + return d.MountPath, nil + } + } + return "", errDriveNotFound +} diff --git a/controller/internal/web/backup_target_offer_test.go b/controller/internal/web/backup_target_offer_test.go new file mode 100644 index 0000000..ac6d6da --- /dev/null +++ b/controller/internal/web/backup_target_offer_test.go @@ -0,0 +1,61 @@ +package web + +import ( + "strings" + "testing" +) + +// SCENARIO E — a healthy configuration must look NORMAL. +// +// The single most likely over-correction in this whole arc: decorating every dashboard with a +// reassurance banner. Eight consecutive fixes here have produced one, and a permanent notice on a +// working box is how a real warning stops being read. When the target is a real drive, the state +// carries no message at all — there is nothing for the UI to render. +func TestHealthyBackupTargetRendersNothing(t *testing.T) { + st := BackupTargetState{Known: true, Degraded: false, TargetID: "felhom-backup", Label: "Külső HDD"} + if msg := degradedMessageFor(st); msg != "" { + t.Fatalf("a healthy target produced customer copy %q — a working configuration must look "+ + "normal, with no caution and no decoration (Scenario E)", msg) + } +} + +// SCENARIO D — declining is valid; forgetting is not. A degraded box keeps saying so, every time it +// is asked. There is no "dismissed" flag by design: the customer declines by not accepting, and the +// state is recomputed from the agent on the next visit rather than remembered as handled. +func TestDegradedStateKeepsReportingOnEveryVisit(t *testing.T) { + st := BackupTargetState{Known: true, Degraded: true, TargetID: "local"} + for visit := 1; visit <= 3; visit++ { + if msg := degradedMessageFor(st); msg == "" { + t.Fatalf("visit %d produced no message — a declined offer must stay VISIBLE, not go quiet", visit) + } + } +} + +// The copy must carry FACT → CONSEQUENCE → REMEDY. A customer told only "your backup is on the same +// disk" cannot act; the sentence has to say what that costs them and what fixes it. +func TestDegradedCopyNamesConsequenceAndRemedy(t *testing.T) { + msg := degradedMessageFor(BackupTargetState{Known: true, Degraded: true}) + for _, want := range []struct{ frag, why string }{ + {"ugyanazon a lemezen", "the FACT — it shares the disk with the system"}, + {"lemezhiba ellen nem", "the CONSEQUENCE — it does not survive a disk failure"}, + {"második meghajtót", "the REMEDY — attach a second drive"}, + } { + if !strings.Contains(msg, want.frag) { + t.Errorf("degraded copy is missing %s (%q); got: %s", want.why, want.frag, msg) + } + } +} + +// UNKNOWN IS NOT DEGRADED. An unreachable or pre-R-82 agent means we could not ask — rendering a +// warning there would put a permanent scare on a box that may be perfectly healthy, and it is the +// absence-read-as-a-value mistake this project keeps closing (R-88 Part 2). +func TestUnknownStateRendersNothing(t *testing.T) { + // Degraded:true is deliberate. With Degraded:false the fixture passes even if the !Known guard is + // deleted, because the second condition catches it — the test would be HOLLOW and a red-proof + // proved exactly that. The meaningful case is "we could not ask, and the other field says + // degraded": the guard must win. + if msg := degradedMessageFor(BackupTargetState{Known: false, Degraded: true}); msg != "" { + t.Fatalf("an UNKNOWN state produced customer copy %q — not being able to ask is not evidence "+ + "of degradation", msg) + } +} diff --git a/controller/internal/web/storage_handlers.go b/controller/internal/web/storage_handlers.go index 2714d7e..94d196f 100644 --- a/controller/internal/web/storage_handlers.go +++ b/controller/internal/web/storage_handlers.go @@ -354,6 +354,12 @@ func (s *Server) ServeStorageAPI(w http.ResponseWriter, r *http.Request) { s.handleStorageImpact(w, r) case r.URL.Path == "/api/storage/register" && r.Method == http.MethodPost: s.handleStorageRegister(w, r) + // E-2 Parts 3+4. State drives the degraded banner and the OFFER; assign is the offer's + // ACCEPTANCE and the ONLY writer of the role — registration above deliberately does not set it. + case r.URL.Path == "/api/backup-target" && r.Method == http.MethodGet: + s.handleBackupTargetState(w, r) + case r.URL.Path == "/api/backup-target/assign" && r.Method == http.MethodPost: + s.handleBackupTargetAssign(w, r) case r.URL.Path == "/api/storage/migrate" && r.Method == http.MethodPost: s.handleStorageMigrate(w, r) case r.URL.Path == "/api/storage/migrate-app" && r.Method == http.MethodPost: