v0.234.0: seed installed_images at startup, so the label appears on an app nobody touched

The operator looked at demo-felhom the morning after v0.233.0 and found OpenGist - up 15 hours, running exactly the catalog pin - showing no badge at all. v0.233.0 wrote the record only from the four bring-up paths, so an app nobody restarts carried no record indefinitely. On a quiet box that is every app, which is the box we most want to see. The known limitation WAS the feature not working. BackfillInstalledImages runs once at startup, beside BackfillDesiredState and before the boot reconciler. It READS containers: starts nothing, restarts nothing, writes no compose file. It never overwrites an existing record. And it REFUSES to seed a partial observation, which is why this is not a three-line loop: the badge reads a service-count mismatch as BEHIND, so seeding a degraded app from what is visible would render 'Frissites elerheto' over an app that is perfectly current. The bring-up paths may write a partial because they follow a successful up -d where a gap is real news; a backfill meets any state. Same data, two writers, two admission rules - deliberately. Also fixes a calendar bomb of mine: the render test hardcoded catalog_since and the string '46 napja', but the render path reads time.Now(), so it was green on the day it was written and red the next morning. Now derived. Filed as R-457 with six other candidate files named as unchecked, not accused. +5 tests (1724 -> 1729), 28 packages green. Red-proof of the partial guard run and reverted; the wiring and its ORDER pinned by an AST walk.
2026-09-03 11:56:43 +02:00
parent 32da46cd64
commit 38d28b5b62
8 changed files with 402 additions and 5 deletions
@@ -436,3 +436,107 @@ func summariseInstalled(m map[string]InstalledImage) string {
 	}
 	return strings.Join(parts, ", ")
 }
+
+// BackfillInstalledImages records what every deployed app is ALREADY running, for apps that have no
+// record yet. Call ONCE at startup, after ScanStacks and after the recoveries.
+//
+// ── WHY THIS EXISTS AT ALL ───────────────────────────────────────────────────────────────────
+//
+// v0.233.0 wrote the record only from the four bring-up paths, so an app nobody restarts carried no
+// record — and no badge — INDEFINITELY. On a quiet box that is every app, which is the box we most
+// want to be able to see. The operator found it on demo-felhom the day after the release: OpenGist,
+// up 15 hours, running exactly what the catalog pins, and showing nothing at all.
+//
+// Reading a container is a pure OBSERVATION: it starts nothing, restarts nothing, upgrades nothing
+// and writes no compose file. That is what makes a backfill safe here and is why it is the same
+// shape as BackfillDesiredState — with one deliberate difference, below.
+//
+// ── NEVER OVERWRITES AN EXISTING RECORD ──────────────────────────────────────────────────────
+//
+// Apps with a record are skipped entirely. The bring-up paths own updates; this only seeds absences.
+//
+// ── AND IT REFUSES TO SEED A PARTIAL OBSERVATION ─────────────────────────────────────────────
+//
+// THE TRAP, and it is the whole reason this is not a three-line loop: web.compareInstalledToTemplate
+// reads a service-count mismatch as BEHIND. A crash-looping or degraded app can have fewer live
+// containers than the template has services, so seeding what we can see would render
+// "Frissítés elérhető" over an app that is perfectly current — a confident WRONG answer to the
+// customer, which is worse than the silence it replaces.
+//
+// The bring-up paths do not have this problem: they run immediately after a SUCCESSFUL `compose up
+// -d`, where a missing container is real news and is already logged as a WARN. A backfill runs over
+// whatever state a box happens to be in at boot, so it must be stricter. An app it cannot observe
+// COMPLETELY is left with no record — unknown, which renders nothing, which is the honest answer.
+func (m *Manager) BackfillInstalledImages() int {
+	backfilled, skippedHaveRecord, skippedIncomplete := 0, 0, 0
+
+	for _, s := range m.GetStacks() {
+		if !s.Deployed || s.Protected || s.Deploying {
+			continue
+		}
+		if s.AppConfig != nil && len(s.AppConfig.InstalledImages) > 0 {
+			skippedHaveRecord++
+			continue
+		}
+		stackDir := filepath.Dir(s.ComposePath)
+
+		tpl, err := ParseComposeImages(s.ComposePath)
+		if err != nil || len(tpl) == 0 {
+			// Cannot tell what a complete observation would even BE. Leave it unknown.
+			skippedIncomplete++
+			continue
+		}
+		observed, err := m.observeInstalledImages(stackDir, m.stackEnv(stackDir))
+		if err != nil {
+			m.logger.Printf("[WARN] [stacks] installed-images backfill: %s: %v", s.Name, err)
+			skippedIncomplete++
+			continue
+		}
+		if !observationCoversTemplate(observed, tpl) {
+			// Stopped, degraded, crash-looping, or mid-anything. See the comment above: a partial
+			// seed would render as "behind" on an app that is current.
+			skippedIncomplete++
+			continue
+		}
+
+		cfg := LoadAppConfig(stackDir)
+		if cfg == nil {
+			skippedIncomplete++
+			continue
+		}
+		cfg.InstalledImages = observed
+		meta := LoadMetadata(stackDir)
+		if err := SaveAppConfig(stackDir, cfg, m.encKey, SensitiveEnvVars(&meta)); err != nil {
+			m.logger.Printf("[ERROR] [stacks] installed-images backfill: %s: %v", s.Name, err)
+			continue
+		}
+		m.mu.Lock()
+		if st, ok := m.stacks[s.Name]; ok && st.AppConfig != nil {
+			st.AppConfig.InstalledImages = observed
+		}
+		m.mu.Unlock()
+		backfilled++
+		m.logger.Printf("[INFO] [stacks] installed-images backfill: %s recorded %d service(s) (%s)",
+			s.Name, len(observed), summariseInstalled(observed))
+	}
+
+	// A POSITIVE OBSERVABLE EITHER WAY (standing rule 3): "0 backfilled" and "the backfill never ran"
+	// must not look the same in a log.
+	m.logger.Printf("[INFO] [stacks] installed-images backfill: %d app(s) recorded, %d already had a record, %d left unrecorded (could not be observed completely — unknown, which renders nothing)",
+		backfilled, skippedHaveRecord, skippedIncomplete)
+	return backfilled
+}
+
+// observationCoversTemplate reports whether EVERY compose service the template declares was observed.
+// Extra observed services are fine (a stray container is not a missing one); a missing one is not.
+func observationCoversTemplate(observed map[string]InstalledImage, tpl map[string]string) bool {
+	if len(observed) == 0 {
+		return false
+	}
+	for svc := range tpl {
+		if _, ok := observed[svc]; !ok {
+			return false
+		}
+	}
+	return true
+}
@@ -515,3 +515,189 @@ volumes:
 		t.Error("an unreadable file must be an ERROR — cannot-tell must never read as no-images")
 	}
 }
+
+// --- GROUP G: the startup backfill (v0.234.0) ---
+//
+// v0.233.0 wrote the record only from the four bring-up paths, so an app nobody restarts showed no
+// badge indefinitely. Found live on demo-felhom the day after the release: OpenGist, up 15 hours,
+// running exactly what the catalog pins, and showing nothing.
+
+// newBackfillManager registers `names` as deployed stacks under one temp root.
+func newBackfillManager(t *testing.T, specs map[string]string) (*Manager, map[string]string) {
+	t.Helper()
+	root := t.TempDir()
+	cfg := &config.Config{}
+	cfg.Paths.StacksDir = root
+	m := &Manager{
+		cfg: cfg, logger: log.New(io.Discard, "", 0), composeCmd: "docker compose",
+		encKey: []byte("0123456789abcdef0123456789abcdef"),
+		stacks: map[string]*Stack{},
+	}
+	dirs := map[string]string{}
+	for name, compose := range specs {
+		dir := filepath.Join(root, name)
+		if err := os.MkdirAll(dir, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, "docker-compose.yml"), []byte(compose), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("deployed: true\nenv: {}\n"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		m.stacks[name] = &Stack{
+			Name: name, Deployed: true,
+			ComposePath: filepath.Join(dir, "docker-compose.yml"),
+			AppConfig:   LoadAppConfig(dir),
+		}
+		dirs[name] = dir
+	}
+	return m, dirs
+}
+
+// TestGroupG_BackfillSeedsAnUntouchedApp is the case the operator reported: a deployed app that has
+// simply been running, with no record and therefore no badge.
+func TestGroupG_BackfillSeedsAnUntouchedApp(t *testing.T) {
+	m, dirs := newBackfillManager(t, map[string]string{
+		"opengist": "services:\n  opengist:\n    image: ghcr.io/thomiceli/opengist:1.13\n",
+	})
+	m.installedExecFn = scriptedInstalledDocker(t,
+		`{"ID":"aaa111","Name":"opengist","Service":"opengist"}`,
+		[]fakeContainer{{id: "aaa111", ref: "ghcr.io/thomiceli/opengist:1.13", imageID: "sha256:og"}},
+		map[string]string{"sha256:og": "ghcr.io/thomiceli/opengist@sha256:seeded"})
+
+	if n := m.BackfillInstalledImages(); n != 1 {
+		t.Fatalf("backfilled %d, want 1", n)
+	}
+	got := readInstalled(t, dirs["opengist"]).InstalledImages
+	if len(got) != 1 || got["opengist"].Digest != "sha256:seeded" {
+		t.Fatalf("app.yaml holds %+v", got)
+	}
+	// And the in-memory view, so the badge does not wait for the next ScanStacks.
+	if s, _ := m.GetStack("opengist"); s.AppConfig.InstalledImages["opengist"].Digest != "sha256:seeded" {
+		t.Error("the in-memory AppConfig must be seeded too")
+	}
+}
+
+// TestGroupG_BackfillRefusesAPartialObservation is THE reason this is not a three-line loop.
+//
+// compareInstalledToTemplate reads a service-count mismatch as BEHIND. A degraded or crash-looping
+// app has fewer live containers than its template has services, so seeding what can be seen would
+// render „Frissítés elérhető" over an app that is perfectly current — a confident WRONG answer,
+// which is worse than the silence it replaces.
+//
+// COMPANION RED-PROOF (run 2026-09-03): delete the `observationCoversTemplate` guard from
+// BackfillInstalledImages. This test then fails with "backfilled 1, want 0" and the follow-up
+// assertion shows a 1-of-2 record on disk — the exact shape that renders a false "update available".
+// Reverted.
+func TestGroupG_BackfillRefusesAPartialObservation(t *testing.T) {
+	m, dirs := newBackfillManager(t, map[string]string{
+		"bookstack": threeServiceCompose,
+	})
+	cs, digs := threeContainers()
+	// Only TWO of the three services are observable — the `cache` container is gone.
+	m.installedExecFn = scriptedInstalledDocker(t,
+		`{"ID":"aaa111","Name":"bookstack","Service":"web"}
+{"ID":"bbb222","Name":"bookstack-db","Service":"db"}`, cs, digs)
+
+	if n := m.BackfillInstalledImages(); n != 0 {
+		t.Fatalf("backfilled %d, want 0 — a partial observation must NOT be seeded", n)
+	}
+	if got := readInstalled(t, dirs["bookstack"]).InstalledImages; len(got) != 0 {
+		t.Fatalf("app.yaml must carry NO record rather than a partial one, got %+v", got)
+	}
+}
+
+// TestGroupG_BackfillNeverOverwritesAnExistingRecord — the bring-up paths own updates; this only
+// seeds absences. Overwriting would let a boot re-stamp a record the lifecycle paths had just moved.
+func TestGroupG_BackfillNeverOverwritesAnExistingRecord(t *testing.T) {
+	m, dirs := newBackfillManager(t, map[string]string{
+		"app": "services:\n  web:\n    image: nginx:1.27\n",
+	})
+	existing := `deployed: true
+env: {}
+installed_images:
+    web:
+        ref: nginx:1.26
+        digest: sha256:original
+        at: "2026-08-01T00:00:00Z"
+`
+	if err := os.WriteFile(filepath.Join(dirs["app"], "app.yaml"), []byte(existing), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	m.stacks["app"].AppConfig = LoadAppConfig(dirs["app"])
+	m.installedExecFn = func(context.Context, string, []string, string, ...string) (string, error) {
+		t.Fatal("an app that already has a record must not even be OBSERVED")
+		return "", nil
+	}
+	if n := m.BackfillInstalledImages(); n != 0 {
+		t.Fatalf("backfilled %d, want 0", n)
+	}
+	if got := readInstalled(t, dirs["app"]).InstalledImages["web"]; got.Digest != "sha256:original" || got.At != "2026-08-01T00:00:00Z" {
+		t.Fatalf("the existing record was disturbed: %+v", got)
+	}
+}
+
+// TestGroupG_BackfillSkipsProtectedAndUndeployed — infra is not the customer's to update, and an
+// undeployed template has nothing running to read.
+func TestGroupG_BackfillSkipsProtectedAndUndeployed(t *testing.T) {
+	m, _ := newBackfillManager(t, map[string]string{
+		"traefik": "services:\n  traefik:\n    image: traefik:v3\n",
+		"unused":  "services:\n  web:\n    image: nginx:1.27\n",
+	})
+	m.stacks["traefik"].Protected = true
+	m.stacks["unused"].Deployed = false
+	m.installedExecFn = func(context.Context, string, []string, string, ...string) (string, error) {
+		t.Fatal("neither a protected nor an undeployed stack may be observed")
+		return "", nil
+	}
+	if n := m.BackfillInstalledImages(); n != 0 {
+		t.Fatalf("backfilled %d, want 0", n)
+	}
+}
+
+// TestGroupG_BackfillIsWiredAtStartup — the seam-discipline half. BackfillInstalledImages cannot be
+// driven from this package's tests through main(), so the call is proven by walking the AST of the
+// production entry point, NOT by a strings.Contains that a commented-out call would satisfy.
+//
+// It also asserts the ORDER against its sibling: both backfills run before the boot reconciler, so a
+// just-recovered app is observed in its settled state.
+func TestGroupG_BackfillIsWiredAtStartup(t *testing.T) {
+	src := filepath.Join("..", "..", "cmd", "controller", "main.go")
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, src, nil, 0)
+	if err != nil {
+		t.Skipf("cmd/controller is gitignored in some checkouts: %v", err)
+	}
+	var backfillPos, desiredPos, reconPos int
+	ast.Inspect(f, func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok {
+			return true
+		}
+		switch sel.Sel.Name {
+		case "BackfillInstalledImages":
+			backfillPos = fset.Position(call.Pos()).Line
+		case "BackfillDesiredState":
+			desiredPos = fset.Position(call.Pos()).Line
+		case "runBootReconcile":
+			if reconPos == 0 {
+				reconPos = fset.Position(call.Pos()).Line
+			}
+		}
+		return true
+	})
+	if backfillPos == 0 {
+		t.Fatal("BackfillInstalledImages is never called from cmd/controller — a backfill nothing invokes seeds nothing")
+	}
+	if desiredPos == 0 || backfillPos <= desiredPos {
+		t.Errorf("the installed-images backfill (line %d) must run after the desired-state one (line %d)", backfillPos, desiredPos)
+	}
+	if reconPos != 0 && backfillPos > reconPos {
+		t.Errorf("the backfill (line %d) must run BEFORE the boot reconciler (line %d)", backfillPos, reconPos)
+	}
+}