diff --git a/controller/internal/web/netstorage_handlers.go b/controller/internal/web/netstorage_handlers.go index fa72158..a36ab01 100644 --- a/controller/internal/web/netstorage_handlers.go +++ b/controller/internal/web/netstorage_handlers.go @@ -3,6 +3,7 @@ package web import ( "context" "encoding/json" + "fmt" "net/http" "sort" "strings" @@ -298,7 +299,19 @@ func (s *Server) handleNetStorageRemove(w http.ResponseWriter, r *http.Request) writeDiskJSON(w, http.StatusBadRequest, false, "érvénytelen név", nil) return } - agent, err := s.agentClient() + where := settings.NetworkMountRoot + "/" + name + // C6B-F2 guard (v0.130.0): refuse the removal while a DEPLOYED app's HDD_PATH lives on the + // share. Removing the share under a bound app strands the app's storage AND orphans the host + // automount (the agent's stop steps are tolerated best-effort, so a busy mount gets its unit + // files deleted anyway → an unreapable autofs mount until host reboot — the C6B-F2 orphan). + if apps := s.deployedAppsOnPath(where); len(apps) > 0 { + s.logger.Printf("[WARN] [web] netstorage remove %q refused: deployed app(s) on the share: %s", name, strings.Join(apps, ", ")) + writeDiskJSON(w, http.StatusConflict, false, fmt.Sprintf( + "A tároló nem távolítható el, amíg alkalmazás használja: %s. Előbb távolítsa el vagy költöztesse át az alkalmazást.", + strings.Join(apps, ", ")), nil) + return + } + agent, err := s.netAgentForAdd() if err != nil { writeDiskJSON(w, http.StatusServiceUnavailable, false, err.Error(), nil) return @@ -308,7 +321,6 @@ func (s *Server) handleNetStorageRemove(w http.ResponseWriter, r *http.Request) writeDiskJSON(w, http.StatusBadGateway, false, err.Error(), nil) return } - where := settings.NetworkMountRoot + "/" + name if err := s.settings.RemoveStoragePath(where); err != nil { s.logger.Printf("[WARN] [web] netstorage deregister %q: %v", where, err) } @@ -316,6 +328,36 @@ func (s *Server) handleNetStorageRemove(w http.ResponseWriter, r *http.Request) writeDiskJSON(w, http.StatusOK, true, "", map[string]any{"removed": true, "name": name}) } +// deployedAppsOnPath returns the display names of DEPLOYED stacks whose HDD_PATH is base itself +// or a subpath of it (apps on a share store /). Nil-safe on stackMgr. +func (s *Server) deployedAppsOnPath(base string) []string { + if s.stackMgr == nil || base == "" { + return nil + } + var apps []string + for _, st := range s.stackMgr.GetStacks() { + if !st.Deployed { + continue + } + cfg := s.stackMgr.LoadAppConfigByName(st.Name) + if cfg == nil { + continue + } + hdd := cfg.Env["HDD_PATH"] + if hdd == "" { + continue + } + if hdd == base || strings.HasPrefix(hdd, base+"/") { + name := st.Meta.DisplayName + if name == "" { + name = st.Name + } + apps = append(apps, name) + } + } + return apps +} + // listNetStorage resolves the agent's live share list (test seam first, then the shared client). func (s *Server) listNetStorage(ctx context.Context) ([]agentapi.NetworkMountStatus, error) { if s.netListFn != nil { diff --git a/controller/internal/web/netstorage_job.go b/controller/internal/web/netstorage_job.go index 2997951..06bb470 100644 --- a/controller/internal/web/netstorage_job.go +++ b/controller/internal/web/netstorage_job.go @@ -113,7 +113,9 @@ func (s *netAddState) snapshot() *netAddJob { return &cp } -// netAgentForAdd resolves the orchestrator's agent surface (test seam first, then the shared client). +// netAgentForAdd resolves the net-storage agent surface (test seam first, then the shared +// client). Despite the name it serves the whole share lifecycle — the remove handler resolves +// through it too (v0.130.0). func (s *Server) netAgentForAdd() (netAgent, error) { if s.netAgentFn != nil { return s.netAgentFn() diff --git a/controller/internal/web/netstorage_remove_guard_test.go b/controller/internal/web/netstorage_remove_guard_test.go new file mode 100644 index 0000000..6c5d0c8 --- /dev/null +++ b/controller/internal/web/netstorage_remove_guard_test.go @@ -0,0 +1,128 @@ +package web + +import ( + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// C6B-F2 guard (v0.130.0, scenario F): removing a network share while a DEPLOYED app's HDD_PATH +// lives on it is REFUSED up front — the agent's unit teardown never starts, so the busy-mount → +// tolerated-stop → unit-files-deleted-anyway → orphaned-autofs chain (the observed C6B-F2) cannot +// be triggered through the product flow. RED-PROOF: drop the deployedAppsOnPath guard from +// handleNetStorageRemove → the 409/zero-agent-call/still-registered assertions fail (the pre-fix +// behavior: removed:true while sonarr ran on the share). + +// testRemoveGuardServer builds a Server with a REAL stacks.Manager over a temp stacks dir holding +// one app (deployed flag per arg) whose HDD_PATH sits ON the share, plus the share registered. +func testRemoveGuardServer(t *testing.T, deployed bool) (*Server, *fakeNetAgent, string) { + t.Helper() + lg := log.New(io.Discard, "", 0) + dir := t.TempDir() + cfg := &config.Config{} + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Paths.DataDir = filepath.Join(dir, "data") + cfg.Stacks.ComposeCommand = "docker compose" + + share := settings.NetworkMountRoot + "/campaign6" + stackDir := filepath.Join(cfg.Paths.StacksDir, "sonarr") + if err := os.MkdirAll(stackDir, 0o755); err != nil { + t.Fatal(err) + } + os.WriteFile(filepath.Join(stackDir, ".felhom.yml"), []byte("display_name: Sonarr\n"), 0o644) + os.WriteFile(filepath.Join(stackDir, "docker-compose.yml"), []byte("services: {}\n"), 0o644) + appYAML := "deployed: false\n" + if deployed { + // the C6B live shape: HDD_PATH is a SUBPATH of the share root (/) + appYAML = "deployed: true\nenv:\n HDD_PATH: " + share + "/sonarr\n" + } + os.WriteFile(filepath.Join(stackDir, "app.yaml"), []byte(appYAML), 0o644) + + sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg) + if err != nil { + t.Fatal(err) + } + if err := sett.AddStoragePath(settings.StoragePath{ + Path: share, Label: "Kampány 6 teszt", Schedulable: true, Kind: settings.StorageKindNetwork, + }); err != nil { + t.Fatal(err) + } + + mgr, err := stacks.NewManager(cfg, lg) + if err != nil { + t.Fatal(err) + } + _ = mgr.ScanStacks() // container-status refresh may fail on docker-less hosts — discovery is enough + if _, ok := mgr.GetStack("sonarr"); !ok { + t.Fatal("sonarr not discovered by ScanStacks") + } + + agent := &fakeNetAgent{} + s := &Server{cfg: cfg, settings: sett, stackMgr: mgr, logger: lg} + s.netAgentFn = func() (netAgent, error) { return agent, nil } + return s, agent, share +} + +func postNetRemove(t *testing.T, s *Server, name string) *httptest.ResponseRecorder { + t.Helper() + r := httptest.NewRequest(http.MethodPost, "/api/storage/netstorage/remove", + strings.NewReader(`{"name":"`+name+`"}`)) + w := httptest.NewRecorder() + s.handleNetStorageRemove(w, r) + return w +} + +func TestNetStorageRemove_RefusedWhileAppDeployedOnShare(t *testing.T) { + s, agent, share := testRemoveGuardServer(t, true) + + w := postNetRemove(t, s, "campaign6") + if w.Code != http.StatusConflict { + t.Fatalf("remove with a deployed app on the share: got %d want 409 (%s)", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "Sonarr") { + t.Errorf("the refusal must NAME the blocking app, got: %s", w.Body.String()) + } + if !strings.Contains(w.Body.String(), "nem távolítható el") { + t.Errorf("expected the Hungarian refusal, got: %s", w.Body.String()) + } + // The agent teardown must NEVER have started — that is what orphans the automount. + if got := agent.removed(); len(got) != 0 { + t.Fatalf("agent RemoveNetStorage ran despite the refusal: %v", got) + } + // The share stays registered (nothing half-removed). + found := false + for _, sp := range s.settings.GetStoragePaths() { + if sp.Path == share { + found = true + } + } + if !found { + t.Fatal("the share was deregistered despite the refusal") + } +} + +func TestNetStorageRemove_ProceedsWithoutDeployedApps(t *testing.T) { + s, agent, share := testRemoveGuardServer(t, false) + + w := postNetRemove(t, s, "campaign6") + if w.Code != http.StatusOK { + t.Fatalf("remove with no deployed apps: got %d want 200 (%s)", w.Code, w.Body.String()) + } + if got := agent.removed(); len(got) != 1 || got[0] != "campaign6" { + t.Fatalf("agent RemoveNetStorage calls = %v, want [campaign6]", got) + } + for _, sp := range s.settings.GetStoragePaths() { + if sp.Path == share { + t.Fatal("the share must be deregistered after a successful remove") + } + } +}