v0.330.0: „Végleges leszerelés — mégis" asks the agent's disk list first and starts again the apps it stopped when the agent refuses (R-1144)

Red-proofed twice. MinAgent 0.158.1.
2026-10-18 13:06:09 +02:00
parent 03ab121b25
commit 6c6bd87664
9 changed files with 253 additions and 37 deletions
@@ -0,0 +1,101 @@
+package web
+
+import (
+	"context"
+	"net/http"
+	"reflect"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/agentapi"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/util"
+)
+
+// fakeDecomStacks is the stack-manager seam of „Végleges leszerelés — mégis": it records stops and starts and never
+// reaches docker.
+type fakeDecomStacks struct {
+	list          []stacks.Stack
+	hdd           map[string]string
+	stops, starts []string
+}
+
+func (f *fakeDecomStacks) GetStacks() []stacks.Stack { return f.list }
+func (f *fakeDecomStacks) LoadAppConfigByName(n string) *stacks.AppConfig {
+	return &stacks.AppConfig{Deployed: true, Env: map[string]string{"HDD_PATH": f.hdd[n]}}
+}
+func (f *fakeDecomStacks) StopStack(n string) error  { f.stops = append(f.stops, n); return nil }
+func (f *fakeDecomStacks) StartStack(n string) error { f.starts = append(f.starts, n); return nil }
+
+func r1144Setup(t *testing.T) (*Server, *fakeDecomStacks) {
+	t.Helper()
+	s := testServer(t)
+	_ = s.settings.AddStoragePath(settings.StoragePath{Path: "/mnt/old", Schedulable: true, IsDefault: true})
+	_ = s.settings.AddStoragePath(settings.StoragePath{Path: "/mnt/new", Schedulable: true})
+	st := func(n string, state stacks.ContainerState) stacks.Stack {
+		return stacks.Stack{Name: n, Deployed: true, State: state, Meta: stacks.Metadata{DisplayName: strings.ToUpper(n)}}
+	}
+	f := &fakeDecomStacks{
+		list: []stacks.Stack{st("immich", stacks.StateRunning), st("jellyfin", stacks.StateUnhealthy), st("komga", stacks.StateStopped), st("other", stacks.StateRunning)},
+		hdd:  map[string]string{"immich": "/mnt/old", "jellyfin": "/mnt/old", "komga": "/mnt/old", "other": "/mnt/new"},
+	}
+	return s, f
+}
+
+// R-1144: the agent refuses AFTER the apps were stopped (403 here; its disk list gave no verdict) → exactly the apps
+// that were running before the press run again (immich, jellyfin — not komga, which was stopped, and not other, on
+// another drive), the drive is not marked, and the household reads both sentences.
+// RED-PROOF: without the restart leg in decommissionAnyway, starts = [] and this test fails.
+func TestR1144_RefusalAfterStopStartsTheRunningAppsAgain(t *testing.T) {
+	for _, code := range []int{http.StatusForbidden, http.StatusConflict} {
+		s, f := r1144Setup(t)
+		agent := &mockAgent{decommissionErr: &agentapi.DecommissionRefusedError{Status: code, Reason: "no"}}
+		stopped, all, err := s.decommissionAnyway(context.Background(), agent, f, "/mnt/old")
+		if err == nil || !driveUntouched(err) {
+			t.Fatalf("HTTP %d: err = %v, want a policy refusal", code, err)
+		}
+		if !reflect.DeepEqual(f.stops, []string{"immich", "jellyfin", "komga"}) {
+			t.Fatalf("stops = %v", f.stops)
+		}
+		if !reflect.DeepEqual(f.starts, []string{"immich", "jellyfin"}) || !all {
+			t.Fatalf("HTTP %d: starts = %v (all=%v), want exactly the apps that were running", code, f.starts, all)
+		}
+		if len(stopped) != 3 || s.settings.IsDecommissioned("/mnt/old") {
+			t.Fatalf("stopped=%v marked=%v", stopped, s.settings.IsDecommissioned("/mnt/old"))
+		}
+		for lang, frag := range map[string]string{"hu": "újra futnak", "en": "running again"} {
+			if !strings.Contains(util.Text(lang, "disk.decommission.apps_restarted"), frag) {
+				t.Errorf("[%s] the restart sentence lacks %q", lang, frag)
+			}
+		}
+	}
+}
+
+// When the agent's own disk list already says no (a protected role, or the whole-guest backup target), NOTHING is
+// stopped: the refusal comes before the first StopStack.
+func TestR1144_AgentVerdictBeforeAnythingIsStopped(t *testing.T) {
+	for name, d := range map[string]agentapi.DiskInfo{
+		"protected role": {MountPath: "/mnt/old", Role: "backup"},
+		"backup target":  {MountPath: "/mnt/old", Role: "user-data", BackupTarget: true},
+	} {
+		s, f := r1144Setup(t)
+		agent := &mockAgent{disks: agentapi.DisksResponse{Disks: []agentapi.DiskInfo{d}}}
+		_, _, err := s.decommissionAnyway(context.Background(), agent, f, "/mnt/old")
+		if !isDecommissionPolicyRefusal(err) || len(f.stops) != 0 || len(agent.decommissionCalls) != 0 {
+			t.Fatalf("%s: err=%v stops=%v agent calls=%v — want a refusal before anything is stopped", name, err, f.stops, agent.decommissionCalls)
+		}
+	}
+}
+
+// The agent says yes (200) → retired as before: marked, apps stay stopped, nothing restarted.
+func TestR1144_AcceptedRetirementKeepsTheAppsStopped(t *testing.T) {
+	s, f := r1144Setup(t)
+	agent := &mockAgent{disks: agentapi.DisksResponse{Disks: []agentapi.DiskInfo{{MountPath: "/mnt/old", Role: "user-data"}}}}
+	if _, all, err := s.decommissionAnyway(context.Background(), agent, f, "/mnt/old"); err != nil || !all {
+		t.Fatalf("err=%v all=%v", err, all)
+	}
+	if len(f.starts) != 0 || !s.settings.IsDecommissioned("/mnt/old") {
+		t.Fatalf("starts=%v marked=%v — a retired drive's apps must stay stopped", f.starts, s.settings.IsDecommissioned("/mnt/old"))
+	}
+}
@@ -547,28 +547,28 @@ func (s *Server) handleStorageDecommission(w http.ResponseWriter, r *http.Reques
 			writeDiskJSON(w, http.StatusBadRequest, false, s.msg(r, "disk.err.name_mismatch"), nil)
 			return
 		}
-		// Stop the apps that live on this drive — but KEEP their HDD_PATH so the dashboard can name the
-		// drive in the "missing storage" indicator until the customer re-enrolls or migrates.
-		var stopped []string
-		for _, st := range s.stackMgr.GetStacks() {
-			if !st.Deployed {
-				continue
-			}
-			if cfg := s.stackMgr.LoadAppConfigByName(st.Name); cfg != nil && cfg.Env["HDD_PATH"] == req.Where {
-				if err := s.stackMgr.StopStack(st.Name); err != nil {
-					s.logger.Printf("[WARN] [web] decommission: stop %s failed: %v", st.Name, err)
-				}
-				stopped = append(stopped, st.Meta.DisplayName)
-			}
+		agent, aerr := s.agentClient()
+		if aerr != nil {
+			writeDiskJSON(w, http.StatusBadGateway, false, s.errText(r, aerr), nil)
+			return
 		}
-		if err := s.finalizeDecommission(r.Context(), req.Where, ""); err != nil {
-			// The M1 policy refusal is a 409 (CONFLICT with current state), not a gateway
-			// failure — and 409 passes the CF edge with its JSON body intact.
+		stopped, restartedAll, err := s.decommissionAnyway(r.Context(), agent, s.stackMgr, req.Where)
+		if err != nil {
+			// The policy refusals (M1, the agent's 403/409) are 409s (CONFLICT with current state), not gateway
+			// failures — and a 409 passes the CF edge with its JSON body intact.
 			status := http.StatusBadGateway
 			if isDecommissionPolicyRefusal(err) {
 				status = http.StatusConflict
 			}
-			writeDiskJSON(w, status, false, s.errText(r, err), nil)
+			msg := s.errText(r, err)
+			if len(stopped) > 0 && driveUntouched(err) {
+				key := "disk.decommission.apps_restarted"
+				if !restartedAll {
+					key = "disk.decommission.apps_not_all_restarted"
+				}
+				msg += " " + s.msg(r, key)
+			}
+			writeDiskJSON(w, status, false, msg, map[string]any{"stopped_apps": stopped, "restarted": restartedAll})
 			return
 		}
 		writeDiskJSON(w, http.StatusOK, true, "", map[string]any{"decommissioned": true, "where": req.Where, "stopped_apps": stopped})
@@ -590,6 +590,99 @@ var (
 	errDecommissionBackupTarget = util.MsgError("err.web.decommission_backup_target")
 )

+// errDecommissionNotDone marks a finalize failure in which the agent did NOT decommission the drive (its call failed):
+// like a policy refusal, the drive and the registry are as they were.
+var errDecommissionNotDone = errors.New("the drive was not decommissioned")
+
+// driveUntouched: the finalize failed BEFORE the drive was let go — a policy refusal or a failed agent call — so the
+// drive still serves and the apps stopped for it can run again. NOT true when the agent let go and only the registry
+// mark failed: then the drive is gone and its apps must stay stopped.
+func driveUntouched(err error) bool {
+	return isDecommissionPolicyRefusal(err) || errors.Is(err, errDecommissionNotDone)
+}
+
+// decomStacks is the slice of the stack manager „Végleges leszerelés — mégis" uses (a seam: tests never reach docker).
+type decomStacks interface {
+	GetStacks() []stacks.Stack
+	LoadAppConfigByName(name string) *stacks.AppConfig
+	StopStack(name string) error
+	StartStack(name string) error
+}
+
+// wasUp: the app was running (in any of its running shapes) before the press.
+func wasUp(st stacks.ContainerState) bool {
+	switch st {
+	case stacks.StateRunning, stacks.StateStarting, stacks.StateUnhealthy, stacks.StateDegraded, stacks.StateRestarting:
+		return true
+	}
+	return false
+}
+
+// decommissionAnyway is mode „anyway": ask the agent's verdict BEFORE stopping anything where the agent already says it
+// (its own disk list: a role other than user-data → 403, the whole-guest backup target → 409), then stop the drive's
+// apps, then finalize. R-1144 (2026-10-18): when the finalize leaves the drive untouched (a policy refusal, a failed
+// agent call), EXACTLY the apps that were running before the press are started again. Returns the stopped apps'
+// display names and whether every app that was running is running again. Pinned by TestR1144_*.
+func (s *Server) decommissionAnyway(ctx context.Context, agent diskAgent, sm decomStacks, where string) (stopped []string, restartedAll bool, err error) {
+	if perr := s.decommissionPrecheck(ctx, agent, where); perr != nil {
+		return nil, true, perr
+	}
+	// Stop the apps that live on this drive — but KEEP their HDD_PATH so the dashboard can name the drive in the
+	// "missing storage" indicator until the customer re-enrolls or migrates.
+	var wereUp []string
+	for _, st := range sm.GetStacks() {
+		if !st.Deployed {
+			continue
+		}
+		if cfg := sm.LoadAppConfigByName(st.Name); cfg != nil && cfg.Env["HDD_PATH"] == where {
+			up := wasUp(st.State)
+			if serr := sm.StopStack(st.Name); serr != nil {
+				s.logger.Printf("[WARN] [web] decommission: stop %s failed: %v", st.Name, serr)
+			}
+			if up {
+				wereUp = append(wereUp, st.Name)
+			}
+			stopped = append(stopped, st.Meta.DisplayName)
+		}
+	}
+	err = s.finalizeDecommissionWith(ctx, agent, where, "")
+	if err == nil || !driveUntouched(err) {
+		return stopped, err == nil, err
+	}
+	restartedAll = true
+	for _, name := range wereUp {
+		if serr := sm.StartStack(name); serr != nil {
+			restartedAll = false
+			s.logger.Printf("[ERROR] [web] decommission of %s refused, and %s did not start again: %v", where, name, serr)
+		}
+	}
+	s.logger.Printf("[INFO] [web] decommission of %s did not happen — %d app(s) that were running started again (all=%v)", where, len(wereUp), restartedAll)
+	return stopped, restartedAll, err
+}
+
+// decommissionPrecheck answers the agent's two policy refusals from its own disk list, before anything is stopped.
+// The agent stays the authority (finalize asks it again); a list it cannot give, or a drive not in it, is no verdict.
+func (s *Server) decommissionPrecheck(ctx context.Context, agent diskAgent, where string) error {
+	resp, err := agent.Disks(ctx)
+	if err != nil {
+		return nil
+	}
+	raw := agentWhere(where)
+	for _, d := range resp.Disks {
+		if d.MountPath != raw && d.GuestPath != where {
+			continue
+		}
+		if d.BackupTarget {
+			return errDecommissionBackupTarget
+		}
+		if d.Role != "" && d.Role != "user-data" {
+			return errDecommissionProtected
+		}
+		return nil
+	}
+	return nil
+}
+
 // isDecommissionPolicyRefusal: a refusal the household can act on (409 with its JSON), not a gateway failure.
 func isDecommissionPolicyRefusal(err error) bool {
 	return errors.Is(err, errLastUsableDrive) || errors.Is(err, errDecommissionProtected) ||
@@ -632,7 +725,7 @@ func (s *Server) finalizeDecommissionWith(ctx context.Context, agent diskAgent,
 			}
 			return errDecommissionProtected
 		}
-		return util.MsgError("err.web.a_meghajto_leszerelese_sikertelen", err)
+		return util.MsgErrorf(errDecommissionNotDone, "err.web.a_meghajto_leszerelese_sikertelen", err)
 	}
 	if err := s.settings.SetDecommissioned(where, migratedTo); err != nil {
 		// The agent has let go of the drive; only the dashboard's record failed. Said loudly — the next