diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f47cc3..a02cd63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +## Unreleased — a template may exclude a named volume from every backup (R-49, operator ruling D21 = B, 2026-10-22) + +**MinAgent: 0.158.1** (unchanged). Committed, not released (night brief 2026-10-22: no controller release tonight). + +- **`.felhom.yml` `backup.volumes`** (`appbackup.VolumeSpec`): the compose file's top-level named volumes with a class; + only `excluded` means anything. An excluded volume is never dumped (`Manager.dumpableVolumes`, wired through the stack + adapter's `GetExcludedDockerVolumes`), so it is in no recovery unit — not on the drive's copy, the second drive or + off-site; its last tar leaves the unit through `DumpAppVolumes`' stale-tar clean-up. For a pure cache the app + rebuilds itself — Immich's machine-learning model cache, 62 % of its off-site copy (R-49). A restore finds no tar for + it and leaves the volume as the app makes it (Immich downloads its models again). +- **Guards:** the list is NOT read by `ValidateBackupSpec` (that rejection is whole-block — a wrong volume entry must + never cost the app its bind classifications); `appbackup.ExcludedDockerVolumes` ignores, with a logged reason, an + entry that is not `excluded`, a name the compose does not declare, and — all at once — exclusions that would cover + EVERY named volume (a unit with no volume dump is the R-403 hollow shape). An older controller ignores the key + (non-strict YAML everywhere). `deepCopyStack` copies the new list (R-591's alias guard caught it). +- Tests `TestR49_ExcludedDockerVolumes`, `TestR49_AVolumeEntryNeverInvalidatesTheBlock` (appbackup), + `TestR49_AnExcludedVolumeLeavesTheUnit` (the whole dump run through the F7 tar seam — no Docker; red-proved: with the + old volume list the cache is dumped again), `TestR49_NoExclusionProviderDumpsEverything`. + ## v0.339.0 (2026-10-22) — a rebuilt box never overwrites the household's mail list (R-1199 reopened); the host-swap sentence only when swap is really short (R-1216, D28 = A) **MinAgent: 0.158.1** (unchanged). Needs hub ≥ 0.165.0 for the read (an older hub: the box pushes nothing at start) — diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 64ae1ff..f3cc685 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -3123,6 +3123,16 @@ func (a *stackAdapter) GetDockerVolumes(name string) []string { return backup.ResolveDockerVolumeNames(s.ComposePath) } +// GetExcludedDockerVolumes (R-49, D21 = B): the named volumes the template excludes from every backup; an ignored +// template entry is logged, never fatal (backup/volume_exclusion.go). +func (a *stackAdapter) GetExcludedDockerVolumes(name string) []string { + ex, ignored := a.mgr.ExcludedDockerVolumes(name) + for _, why := range ignored { + log.Printf("[WARN] [backup] %s: template volume entry ignored — %s", name, why) + } + return ex +} + // GetImportRoot delegates to the stack manager's canonical drop-zone root (R-75). Both the backup // and the export provider need it: ${IMPORT_PATH} binds live on the SYSTEM drive and can NOT be // resolved from an app's HDD_PATH. diff --git a/controller/internal/appbackup/classify.go b/controller/internal/appbackup/classify.go index f9a9a25..9894f26 100644 --- a/controller/internal/appbackup/classify.go +++ b/controller/internal/appbackup/classify.go @@ -51,6 +51,19 @@ type BackupSpec struct { // WHOLE-BLOCK, so a stale `userdata: import/` would discard the app's OTHER classifications // (e.g. an hdd appdata path classed mandatory) and silently degrade it to legacy. Import []BindSpec `yaml:"import,omitempty" json:"import,omitempty"` + // Volumes classifies the compose file's top-level NAMED volumes (R-49, operator ruling D21 = B, 2026-10-22). + // Only `excluded` means anything: the volume is never dumped, so it is in no recovery unit — not on the + // drive's copy, the second drive or off-site. For a pure cache the app rebuilds by itself (Immich's model + // cache). Deliberately NOT checked by ValidateBackupSpec: a wrong entry must never cost the app its bind + // classifications (that rejection is whole-block) — ExcludedDockerVolumes ignores it with a reason instead. + Volumes []VolumeSpec `yaml:"volumes,omitempty" json:"volumes,omitempty"` +} + +// VolumeSpec is one classified named volume: Name is the compose `volumes:` key (not the project-prefixed +// Docker name). +type VolumeSpec struct { + Name string `yaml:"name" json:"name"` + Class BindClass `yaml:"class" json:"class"` } // BindSpec is one classified entry in a BackupSpec. diff --git a/controller/internal/appbackup/volume_exclusion.go b/controller/internal/appbackup/volume_exclusion.go new file mode 100644 index 0000000..7a1e69f --- /dev/null +++ b/controller/internal/appbackup/volume_exclusion.go @@ -0,0 +1,40 @@ +package appbackup + +import ( + "fmt" + "path/filepath" +) + +// ExcludedDockerVolumes (R-49, operator ruling D21 = B, 2026-10-22) returns the project-prefixed Docker names of the +// named volumes the template's backup block marks `excluded`, and a sentence per entry it ignored. An entry is ignored +// when its class is not `excluded`, when its name is not a top-level named volume of the compose file, or — for every +// entry at once — when the exclusions would cover EVERY named volume: a unit with no volume dump at all is the shape +// the hollow-unit guard (R-403, r403_hollow.go) alarms on, and an app whose every volume is a cache needs no volume +// leg anyway, so that is a template mistake, never a silent hollow unit. Pinned by TestR49_*. +func ExcludedDockerVolumes(spec *BackupSpec, composePath string) (excluded []string, ignored []string) { + if spec == nil || len(spec.Volumes) == 0 { + return nil, nil + } + declared := map[string]bool{} + vols := ParseComposeNamedVolumes(composePath) + for _, v := range vols { + declared[v.Name] = true + } + project := filepath.Base(filepath.Dir(composePath)) + seen := map[string]bool{} + for _, v := range spec.Volumes { + switch { + case v.Class != ClassExcluded: + ignored = append(ignored, fmt.Sprintf("volume %q: class %q — only \"excluded\" is meaningful for a named volume", v.Name, v.Class)) + case !declared[v.Name]: + ignored = append(ignored, fmt.Sprintf("volume %q is not a named volume of the compose file", v.Name)) + case !seen[v.Name]: + seen[v.Name] = true + excluded = append(excluded, project+"_"+v.Name) + } + } + if len(excluded) > 0 && len(excluded) == len(vols) { + return nil, append(ignored, fmt.Sprintf("the backup block excludes every named volume (%d) — ignored: a unit with no volume dump reads as hollow (R-403)", len(vols))) + } + return excluded, ignored +} diff --git a/controller/internal/appbackup/volume_exclusion_test.go b/controller/internal/appbackup/volume_exclusion_test.go new file mode 100644 index 0000000..0349c26 --- /dev/null +++ b/controller/internal/appbackup/volume_exclusion_test.go @@ -0,0 +1,46 @@ +package appbackup + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +// R-49 (D21 = B): the template's `backup.volumes` list — only `excluded`, only declared named volumes, never all of them. +func TestR49_ExcludedDockerVolumes(t *testing.T) { + dir := filepath.Join(t.TempDir(), "immich") + os.MkdirAll(dir, 0o755) + compose := filepath.Join(dir, "docker-compose.yml") + os.WriteFile(compose, []byte("services:\n a:\n image: x\n volumes:\n - immich_ml_cache:/cache\n - immich_postgres_data:/db\nvolumes:\n immich_ml_cache:\n immich_postgres_data:\n"), 0o644) + spec := &BackupSpec{Volumes: []VolumeSpec{ + {Name: "immich_ml_cache", Class: ClassExcluded}, + {Name: "no_such_volume", Class: ClassExcluded}, + {Name: "immich_postgres_data", Class: ClassMandatory}, + }} + ex, ignored := ExcludedDockerVolumes(spec, compose) + if !reflect.DeepEqual(ex, []string{"immich_immich_ml_cache"}) { + t.Fatalf("excluded = %v, want [immich_immich_ml_cache] (project-prefixed like ResolveDockerVolumeNames)", ex) + } + if len(ignored) != 2 { + t.Fatalf("ignored = %v, want the unknown name and the non-excluded class", ignored) + } + all := &BackupSpec{Volumes: []VolumeSpec{{Name: "immich_ml_cache", Class: ClassExcluded}, {Name: "immich_postgres_data", Class: ClassExcluded}}} + if ex, ig := ExcludedDockerVolumes(all, compose); ex != nil || len(ig) == 0 { + t.Fatalf("excluding EVERY named volume must be refused (a hollow unit), got %v / %v", ex, ig) + } + if ex, _ := ExcludedDockerVolumes(nil, compose); ex != nil { + t.Fatal("no backup block → nothing excluded") + } +} + +// The new key never costs an app its bind classifications: a block whose only fault is a bad volume entry still +// validates (ValidateBackupSpec does not read Volumes). +func TestR49_AVolumeEntryNeverInvalidatesTheBlock(t *testing.T) { + spec := &BackupSpec{HDD: []BindSpec{{Path: "appdata/immich", Class: ClassMandatory}}, + Volumes: []VolumeSpec{{Name: "nonsense", Class: "bogus"}}} + binds := []ComposeBind{{Root: RootHDD, RelPath: "appdata/immich"}} + if err := ValidateBackupSpec(spec, binds); err != nil { + t.Fatalf("a bad volume entry discarded the block: %v", err) + } +} diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index d137ee3..fd7add6 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -856,7 +856,7 @@ func (m *Manager) DumpAppVolumes(stackName string) error { return nil } - volumes := m.stackProvider.GetDockerVolumes(stackName) + volumes := m.dumpableVolumes(stackName) if len(volumes) == 0 { return nil } @@ -929,6 +929,8 @@ func (m *Manager) DumpAppVolumes(stackName string) error { continue } if !activeVols[name] && strings.HasSuffix(name, ".tar") { + // R-49 (D21 = B): this is also how a volume the template now EXCLUDES leaves the unit — its last + // tar would otherwise ride every later copy (Immich's 824 MB model cache, off-site each night). os.Remove(filepath.Join(dumpDir, name)) if m.isDebug() { m.logger.Printf("[DEBUG] [backup] Removed stale volume dump: %s/%s", stackName, name) diff --git a/controller/internal/backup/volume_exclusion.go b/controller/internal/backup/volume_exclusion.go new file mode 100644 index 0000000..e99beb3 --- /dev/null +++ b/controller/internal/backup/volume_exclusion.go @@ -0,0 +1,37 @@ +package backup + +// R-49, operator ruling D21 = B (2026-10-22): a template may mark a named volume `excluded` (a pure cache the app +// rebuilds by itself — Immich's machine-learning model cache, 62 % of its off-site copy). Such a volume is never +// dumped, so it is in no recovery unit: not on the drive's copy, not on the second drive, not off-site. Its last tar +// is removed by DumpAppVolumes' stale-tar clean-up. A restore then finds no tar for it and leaves the volume as the +// app makes it (Immich downloads its models again). The rule and its guards: appbackup.ExcludedDockerVolumes. + +// volumeExclusionProvider is implemented by the production stack adapter (cmd/controller); test fakes need not. +type volumeExclusionProvider interface { + GetExcludedDockerVolumes(name string) []string +} + +// dumpableVolumes is the stack's named Docker volumes minus the ones its template excludes. +func (m *Manager) dumpableVolumes(stackName string) []string { + all := m.stackProvider.GetDockerVolumes(stackName) + ex, ok := m.stackProvider.(volumeExclusionProvider) + if !ok || len(all) == 0 { + return all + } + skip := map[string]bool{} + for _, v := range ex.GetExcludedDockerVolumes(stackName) { + skip[v] = true + } + if len(skip) == 0 { + return all + } + out := make([]string, 0, len(all)) + for _, v := range all { + if skip[v] { + m.logger.Printf("[INFO] [backup] Volume %s/%s is excluded by the template (a cache the app rebuilds) — not dumped (R-49)", stackName, v) + continue + } + out = append(out, v) + } + return out +} diff --git a/controller/internal/backup/volume_exclusion_test.go b/controller/internal/backup/volume_exclusion_test.go new file mode 100644 index 0000000..776c2ac --- /dev/null +++ b/controller/internal/backup/volume_exclusion_test.go @@ -0,0 +1,66 @@ +package backup + +import ( + "io" + "log" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// exclFake is volDumpFakeProvider plus the production adapter's exclusion answer. +type exclFake struct { + volDumpFakeProvider + excluded []string +} + +func (f *exclFake) GetExcludedDockerVolumes(string) []string { return f.excluded } + +// R-49 (D21 = B) — the consequence across a whole dump run, through the F7 tar seam (no Docker): the excluded cache +// volume is not dumped, the other volume is, and the cache's OLD tar (from before the template marked it) is gone +// from the unit, so no later copy carries it. +// RED-PROOF: with DumpAppVolumes listing GetDockerVolumes again (no exclusion), the cache is dumped and its old tar +// replaced, not removed — seen failing 2026-10-22. +func TestR49_AnExcludedVolumeLeavesTheUnit(t *testing.T) { + drive := t.TempDir() + cfg := &config.Config{} + cfg.Paths.SystemDataPath = drive + fake := &exclFake{volDumpFakeProvider: volDumpFakeProvider{ + stacks: []StackSummary{{Name: "immich"}}, + volumes: map[string][]string{"immich": {"immich_immich_ml_cache", "immich_immich_postgres_data"}}, + }, excluded: []string{"immich_immich_ml_cache"}} + var tarred []string + m := &Manager{cfg: cfg, logger: log.New(io.Discard, "", 0), systemDataPath: drive, stackProvider: fake, + tarVolume: func(vol, dir string) ([]byte, error) { + tarred = append(tarred, vol) + return nil, os.WriteFile(filepath.Join(dir, vol+".tar.tmp"), []byte("TAR "+vol), 0o644) + }} + dumpDir := AppVolumeDumpPath(m.namespaceRoot(drive), "immich") + os.MkdirAll(dumpDir, 0o755) + old := filepath.Join(dumpDir, "immich_immich_ml_cache.tar") + os.WriteFile(old, []byte("824 MB OF MODELS"), 0o644) + + if err := m.DumpAppVolumes("immich"); err != nil { + t.Fatalf("DumpAppVolumes: %v", err) + } + if len(tarred) != 1 || tarred[0] != "immich_immich_postgres_data" { + t.Fatalf("dumped %v, want only the database volume", tarred) + } + if _, err := os.Stat(old); !os.IsNotExist(err) { + t.Fatal("the excluded cache's old tar is still in the unit — every later copy would carry it") + } + if _, err := os.Stat(filepath.Join(dumpDir, "immich_immich_postgres_data.tar")); err != nil { + t.Fatalf("the database volume's tar is missing: %v", err) + } +} + +// A provider without the exclusion answer (every test fake, an older wiring) dumps every volume, as before. +func TestR49_NoExclusionProviderDumpsEverything(t *testing.T) { + m := &Manager{logger: log.New(io.Discard, "", 0), stackProvider: &volDumpFakeProvider{ + volumes: map[string][]string{"a": {"a_x", "a_y"}}}} + if got := m.dumpableVolumes("a"); len(got) != 2 { + t.Fatalf("dumpable = %v, want both", got) + } +} diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index a177858..f16198f 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -1313,6 +1313,7 @@ func deepCopyStack(s *Stack) Stack { b.Userdata = append([]appbackup.BindSpec(nil), b.Userdata...) b.HDD = append([]appbackup.BindSpec(nil), b.HDD...) b.Import = append([]appbackup.BindSpec(nil), b.Import...) + b.Volumes = append([]appbackup.VolumeSpec(nil), b.Volumes...) cp.Meta.Backup = &b } diff --git a/controller/internal/stacks/metadata.go b/controller/internal/stacks/metadata.go index 089475a..e4beabd 100644 --- a/controller/internal/stacks/metadata.go +++ b/controller/internal/stacks/metadata.go @@ -597,6 +597,17 @@ func (m *Manager) ClassifiedBinds(name string) ([]appbackup.ClassifiedBind, bool return appbackup.ClassifyBinds(meta.Backup, binds) } +// ExcludedDockerVolumes returns the Docker names of the stack's named volumes its template excludes from every backup +// (R-49, D21 = B; appbackup.ExcludedDockerVolumes), and a sentence per template entry that was ignored. +func (m *Manager) ExcludedDockerVolumes(name string) ([]string, []string) { + stack, ok := m.GetStack(name) + if !ok || name == SambaStackName { + return nil, nil + } + meta := LoadMetadata(filepath.Dir(stack.ComposePath)) + return appbackup.ExcludedDockerVolumes(meta.Backup, stack.ComposePath) +} + // HasDeployField reports whether the app declares a deploy field for the given env var. // // R-351: the caller that needs this is the restore prefill, and the question it is really asking is