v0.351.0 (unreleased): old -rc controller images and old infra images are deleted (decision 56 amended, decision 285)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

controller/internal/stacks/infra_image_retention_test.go

@@ -0,0 +1,128 @@
+package stacks
+
+import (
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/dockerexec"
+)
+
+// Decision 285 (2026-11-09): the pinned infrastructure images keep the pin, every newer version, the previous pin and
+// every image a container uses; older ones go. The fixture is demo-hp's guest as measured on 2026-11-09 (before the
+// hand clean-up): traefik v3.6.7/v3.7.13/v3.7.16 beside the pinned v3.7.17, FileBrowser 1.3.3/1.5.6 beside 1.5.8.
+// Docker is the imageDocker seam (fakeImages) — nothing here reaches a daemon.
+
+var infraPins = []string{"traefik:v3.7.17", "gtstef/filebrowser:1.5.8-stable", "gitea.dooplex.hu/admin/felhom-samba:1.2.0"}
+
+func infraImages() *fakeImages {
+	return &fakeImages{
+		imgs: []localImage{
+			{ID: "sha256:T3717", Repo: "traefik", Tag: "v3.7.17", Size: "195MB"},
+			{ID: "sha256:T3716", Repo: "traefik", Tag: "v3.7.16", Size: "194MB"},
+			{ID: "sha256:T3713", Repo: "traefik", Tag: "v3.7.13", Size: "195MB"},
+			{ID: "sha256:T367", Repo: "traefik", Tag: "v3.6.7", Size: "186MB"},
+			{ID: "sha256:TNONE", Repo: "traefik", Tag: "<none>", Size: "190MB"},
+			{ID: "sha256:F158", Repo: "gtstef/filebrowser", Tag: "1.5.8-stable", Size: "196MB"},
+			{ID: "sha256:F156", Repo: "gtstef/filebrowser", Tag: "1.5.6-stable", Size: "195MB"},
+			{ID: "sha256:F133", Repo: "gtstef/filebrowser", Tag: "1.3.3-stable", Size: "215MB"},
+			// Samba is pinned but not enabled on this box (no container): the pin itself must stay.
+			{ID: "sha256:S120", Repo: "gitea.dooplex.hu/admin/felhom-samba", Tag: "1.2.0", Size: "107MB"},
+			{ID: "sha256:S110", Repo: "gitea.dooplex.hu/admin/felhom-samba", Tag: "1.1.0", Size: "103MB"},
+			{ID: "sha256:S100", Repo: "gitea.dooplex.hu/admin/felhom-samba", Tag: "1.0.0", Size: "101MB"},
+			// not an infra repository: never touched by this pass
+			{ID: "sha256:A1", Repo: "acme/web", Tag: "1", Size: "100MB"},
+			{ID: "sha256:ALP", Repo: "alpine", Tag: "latest", Size: "8MB"},
+		},
+		containers: map[string]string{"c-traefik": "sha256:T3717", "c-fb": "sha256:F158"},
+	}
+}
+
+// COMPANION RED-PROOF: drop the `prev` case from the keep switch → "the previous pin traefik:v3.7.16 was deleted";
+// drop the `v >= pinned` case → "the pinned Samba image (no container) was deleted".
+func TestInfraRetention_KeepsPinAndPreviousDeletesOlder(t *testing.T) {
+	m := retentionManager(t)
+	f := infraImages()
+	withFakeImages(t, f)
+	got, err := m.RetainInfraImages(infraPins)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := rmiSet(f)
+	for _, keep := range []string{"traefik:v3.7.17", "traefik:v3.7.16", "gtstef/filebrowser:1.5.8-stable", "gtstef/filebrowser:1.5.6-stable"} {
+		if strings.Contains(s, ","+keep+",") {
+			t.Fatalf("the previous pin %s was deleted: %v", keep, f.rmi)
+		}
+	}
+	if strings.Contains(s, "felhom-samba:1.2.0") || strings.Contains(s, "felhom-samba:1.1.0") {
+		t.Fatalf("the pinned Samba image (no container) or its previous pin was deleted: %v", f.rmi)
+	}
+	for _, want := range []string{"traefik:v3.7.13", "traefik:v3.6.7", "sha256:TNONE", "gtstef/filebrowser:1.3.3-stable", "gitea.dooplex.hu/admin/felhom-samba:1.0.0"} {
+		if !strings.Contains(s, ","+want+",") {
+			t.Fatalf("%s (older than the previous pin / untagged) was not deleted: rmi=%v", want, f.rmi)
+		}
+	}
+	if strings.Contains(s, "acme/web") || strings.Contains(s, "sha256:A1") || strings.Contains(s, "alpine") {
+		t.Fatalf("an image outside the pinned repositories was touched: %v", f.rmi)
+	}
+	if len(got) != 5 {
+		t.Fatalf("deleted %d, want 5: %v", len(got), got)
+	}
+}
+
+// A box rolled back to an older controller pins an OLDER traefik: the newer one stays (the way forward), and a
+// container that still runs an old image keeps it.
+// COMPANION RED-PROOF: drop the `used[id]` check → "an image a container uses was deleted".
+func TestInfraRetention_NewerPinAndInUseKept(t *testing.T) {
+	m := retentionManager(t)
+	f := infraImages()
+	f.containers["c-old-fb"] = "sha256:F133"
+	withFakeImages(t, f)
+	if _, err := m.RetainInfraImages([]string{"traefik:v3.7.16", "gtstef/filebrowser:1.5.8-stable"}); err != nil {
+		t.Fatal(err)
+	}
+	s := rmiSet(f)
+	if strings.Contains(s, "traefik:v3.7.17") {
+		t.Fatalf("a version above the pin was deleted: %v", f.rmi)
+	}
+	if strings.Contains(s, "traefik:v3.7.13") || !strings.Contains(s, "traefik:v3.6.7") {
+		t.Fatalf("pinned v3.7.16: want v3.7.13 kept (previous) and v3.6.7 deleted: %v", f.rmi)
+	}
+	if strings.Contains(s, "1.3.3-stable") {
+		t.Fatalf("an image a container uses was deleted: %v", f.rmi)
+	}
+}
+
+// While an image-pulling compose command runs (R-863), or the controller swaps itself, NOTHING is deleted.
+// COMPANION RED-PROOF: drop the TryImageCleanup check → "deleted while image work was in flight".
+func TestInfraRetention_NothingWhileImageWorkOrSwap(t *testing.T) {
+	m := retentionManager(t)
+	f := infraImages()
+	withFakeImages(t, f)
+	end := dockerexec.HoldImageWork()
+	_, err := m.RetainInfraImages(infraPins)
+	end()
+	if err != errImageRetentionBusy || len(f.rmi) != 0 {
+		t.Fatalf("deleted while image work was in flight: err=%v rmi=%v", err, f.rmi)
+	}
+	m.SetSelfUpdatingCheck(func() bool { return true })
+	_, err = m.RetainInfraImages(infraPins)
+	m.SetSelfUpdatingCheck(func() bool { return false })
+	if err != errImageRetentionBusy || len(f.rmi) != 0 {
+		t.Fatalf("deleted while the controller swaps itself: err=%v rmi=%v", err, f.rmi)
+	}
+}
+
+// A pin that is not a version, and a tag that is not a version, are left alone.
+func TestInfraRetention_NonVersionsLeftAlone(t *testing.T) {
+	m := retentionManager(t)
+	f := infraImages()
+	f.imgs = append(f.imgs, localImage{ID: "sha256:TLATEST", Repo: "traefik", Tag: "latest", Size: "190MB"})
+	withFakeImages(t, f)
+	if _, err := m.RetainInfraImages([]string{"traefik:v3.7.17", "gtstef/filebrowser:latest"}); err != nil {
+		t.Fatal(err)
+	}
+	s := rmiSet(f)
+	if strings.Contains(s, "traefik:latest") || strings.Contains(s, "filebrowser") {
+		t.Fatalf("a non-version tag or a repository with a non-version pin was touched: %v", f.rmi)
+	}
+}