controller v0.94.0: pull-based config-refresh (re-pull + self-restart on config_version change)

PushResponse.ConfigVersion from the report ACK; ConfigRefresher reconciles vs. the last-applied version (settings.applied_config_version) and on a change calls bootstrap.RefreshConfig (re-pull controller.yaml + re-merge local_api) then GracefulSelfRestart. First-run records baseline (no restart); unchanged = no-op (no storm); failed pull keeps config + retries. Companion to hub v0.26.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxLA1mZurFq9kt8hneFeCs
2026-06-30 21:49:47 +02:00
parent 464b14f029
commit 419d3d0b4e
9 changed files with 432 additions and 0 deletions
@@ -0,0 +1,75 @@
+package report
+
+import "log"
+
+// ConfigRefresher reconciles the hub-advertised config_version (from the report ACK) against the
+// controller's last-applied version and, on a change, re-pulls controller.yaml and self-restarts —
+// the pull-based config-delivery path (the hub never connects into the box; this rides the existing
+// report cycle exactly like the Phase 2 version floor).
+//
+// All side effects are injected so the reconcile is unit-testable without a real hub / filesystem /
+// process exit:
+//   - Applied  reads the persisted last-applied config_version (0 = none recorded yet).
+//   - Record   persists a newly-applied config_version.
+//   - Refresh  re-pulls controller.yaml from the hub and writes it (re-merging local_api). It must
+//              NOT touch settings.json. A hub-unreachable / write failure returns an error.
+//   - Restart  triggers the graceful self-restart (process exit → Docker restart → fresh config).
+type ConfigRefresher struct {
+	Applied func() int
+	Record  func(int) error
+	Refresh func() error
+	Restart func()
+	Logger  *log.Logger
+}
+
+// Reconcile applies the config-refresh decision for one report ACK. Rules (acceptance B + safety §5):
+//   - ackVersion == 0           → no-op (hub didn't advertise; old hub / no config row).
+//   - no version recorded yet   → record the baseline WITHOUT restarting (first-ever ACK; the
+//     first-boot pull already fetched the current config).
+//   - ackVersion == applied     → no-op (no change; this is what prevents a restart storm — after a
+//     refresh, applied == ackVersion so the next report is a no-op).
+//   - ackVersion != applied     → Refresh; on success Record THEN Restart (record-before-restart so
+//     the post-restart process sees it applied); on a failed Refresh keep the current config, do NOT
+//     record, do NOT restart — retried on the next report cycle.
+func (cr *ConfigRefresher) Reconcile(ackVersion int) {
+	if ackVersion == 0 {
+		return // hub didn't advertise a config_version
+	}
+	applied := cr.Applied()
+	if applied == 0 {
+		// First-ever ACK carrying a config_version: record the baseline, do NOT restart (the box came
+		// up on the first-boot pull, which already has the current config). Mirrors the floor's
+		// first-run-records-baseline.
+		if err := cr.Record(ackVersion); err != nil {
+			cr.logf("[WARN] config-refresh: failed to record baseline config_version=%d: %v", ackVersion, err)
+			return
+		}
+		cr.logf("[INFO] config-refresh: baseline config_version=%d recorded (no restart)", ackVersion)
+		return
+	}
+	if ackVersion == applied {
+		return // no change
+	}
+	cr.logf("[INFO] config-refresh: hub config_version=%d != applied=%d — re-pulling controller.yaml", ackVersion, applied)
+	if err := cr.Refresh(); err != nil {
+		// Fail-safe: keep the current config, do NOT record, do NOT restart — retry next cycle.
+		cr.logf("[WARN] config-refresh: re-pull failed: %v — keeping current config, will retry next report", err)
+		return
+	}
+	// Record BEFORE restarting so the freshly-started process sees the version as applied and does not
+	// loop. (The restart is delayed, so the record persists first.)
+	if err := cr.Record(ackVersion); err != nil {
+		cr.logf("[WARN] config-refresh: applied config but failed to record version=%d: %v — skipping restart to avoid a loop", ackVersion, err)
+		return
+	}
+	cr.logf("[INFO] config-refresh: applied config_version=%d — self-restarting to load it", ackVersion)
+	if cr.Restart != nil {
+		cr.Restart()
+	}
+}
+
+func (cr *ConfigRefresher) logf(format string, args ...interface{}) {
+	if cr.Logger != nil {
+		cr.Logger.Printf(format, args...)
+	}
+}
@@ -0,0 +1,129 @@
+package report
+
+import (
+	"errors"
+	"io"
+	"log"
+	"testing"
+)
+
+// recorder collects the side effects a Reconcile would cause, so each case asserts exactly what
+// happened (pull/record/restart) without a real hub, filesystem, or process exit.
+type recorder struct {
+	applied    int
+	recorded   []int
+	refreshes  int
+	refreshErr error
+	restarts   int
+	recordErr  error
+}
+
+func (r *recorder) refresher() *ConfigRefresher {
+	return &ConfigRefresher{
+		Applied: func() int { return r.applied },
+		Record: func(v int) error {
+			if r.recordErr != nil {
+				return r.recordErr
+			}
+			r.recorded = append(r.recorded, v)
+			r.applied = v
+			return nil
+		},
+		Refresh: func() error {
+			r.refreshes++
+			return r.refreshErr
+		},
+		Restart: func() { r.restarts++ },
+		Logger:  log.New(io.Discard, "", 0),
+	}
+}
+
+// A version change re-pulls, records the new version, then restarts (record BEFORE restart).
+func TestReconcile_VersionChange_RefreshRecordRestart(t *testing.T) {
+	r := &recorder{applied: 1}
+	r.refresher().Reconcile(2)
+
+	if r.refreshes != 1 {
+		t.Errorf("refreshes = %d, want 1", r.refreshes)
+	}
+	if len(r.recorded) != 1 || r.recorded[0] != 2 {
+		t.Errorf("recorded = %v, want [2]", r.recorded)
+	}
+	if r.restarts != 1 {
+		t.Errorf("restarts = %d, want 1", r.restarts)
+	}
+}
+
+// RED-PROOF for the no-restart-storm guard: when the ACK version == the applied version, Reconcile
+// must do nothing — no refresh, no restart. (Drop the `ackVersion == applied` guard in Reconcile and
+// this test fails: it would refresh + restart on every report.)
+func TestReconcile_SameVersion_NoOp(t *testing.T) {
+	r := &recorder{applied: 5}
+	r.refresher().Reconcile(5)
+
+	if r.refreshes != 0 {
+		t.Errorf("refreshes = %d, want 0 (unchanged version must not re-pull)", r.refreshes)
+	}
+	if r.restarts != 0 {
+		t.Errorf("restarts = %d, want 0 (unchanged version must NOT restart — restart storm)", r.restarts)
+	}
+	if len(r.recorded) != 0 {
+		t.Errorf("recorded = %v, want [] (nothing to record)", r.recorded)
+	}
+}
+
+// First-ever ACK (nothing recorded yet): record the baseline WITHOUT restarting or re-pulling — the
+// box already came up on the first-boot pull.
+func TestReconcile_FirstRun_RecordsBaselineNoRestart(t *testing.T) {
+	r := &recorder{applied: 0}
+	r.refresher().Reconcile(3)
+
+	if r.refreshes != 0 {
+		t.Errorf("refreshes = %d, want 0 (baseline must not re-pull)", r.refreshes)
+	}
+	if r.restarts != 0 {
+		t.Errorf("restarts = %d, want 0 (baseline must not restart)", r.restarts)
+	}
+	if len(r.recorded) != 1 || r.recorded[0] != 3 {
+		t.Errorf("recorded = %v, want [3] (baseline recorded)", r.recorded)
+	}
+}
+
+// A failed pull keeps the current config: do NOT record, do NOT restart (retried next cycle).
+func TestReconcile_FailedPull_NoRecordNoRestart(t *testing.T) {
+	r := &recorder{applied: 1, refreshErr: errors.New("hub unreachable")}
+	r.refresher().Reconcile(2)
+
+	if r.refreshes != 1 {
+		t.Errorf("refreshes = %d, want 1 (attempted)", r.refreshes)
+	}
+	if r.restarts != 0 {
+		t.Errorf("restarts = %d, want 0 (failed pull must not restart)", r.restarts)
+	}
+	if len(r.recorded) != 0 {
+		t.Errorf("recorded = %v, want [] (failed pull must not record — version stays so it retries)", r.recorded)
+	}
+}
+
+// ackVersion == 0 (old hub / report-only customer) is a no-op.
+func TestReconcile_ZeroVersion_NoOp(t *testing.T) {
+	r := &recorder{applied: 4}
+	r.refresher().Reconcile(0)
+	if r.refreshes != 0 || r.restarts != 0 || len(r.recorded) != 0 {
+		t.Errorf("zero version should be a no-op; got refreshes=%d restarts=%d recorded=%v", r.refreshes, r.restarts, r.recorded)
+	}
+}
+
+// If recording the applied version fails after a successful pull, skip the restart (avoid a loop:
+// a restart without a recorded version would re-pull + restart forever).
+func TestReconcile_RecordFails_SkipsRestart(t *testing.T) {
+	r := &recorder{applied: 1, recordErr: errors.New("disk full")}
+	r.refresher().Reconcile(2)
+
+	if r.refreshes != 1 {
+		t.Errorf("refreshes = %d, want 1", r.refreshes)
+	}
+	if r.restarts != 0 {
+		t.Errorf("restarts = %d, want 0 (must not restart if the version couldn't be recorded)", r.restarts)
+	}
+}
@@ -32,6 +32,11 @@ type PushResponse struct {
 	// "update to latest" button — never the auto-target).
 	MinControllerVersion string `json:"min_controller_version"`
 	LatestVersion        string `json:"latest_version"`
+	// ConfigVersion is the hub's per-customer config counter (v0.26.0). On a change vs. the
+	// last-applied version, the controller re-pulls controller.yaml + self-restarts (pull-based config
+	// delivery — the hub never connects into the box). 0 = the hub didn't advertise it (old hub, or a
+	// report-only customer with no config row) → the controller does nothing.
+	ConfigVersion int `json:"config_version"`
 }

 // Pusher sends reports to the central hub.