amadan
zdd-6 final fix wave 1: W6 a policy-only bundle restarts nothing
Config sync leaves a route's config file absent when the bundle renders nothing and the route has no instance overlay: no file, no restart, no delivered revision, no ack, which is the state of every never-configured app. An existing file converges on its bytes as before, and the held render is unchanged. Astra plan 6 R1 #1, plan Q3.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+139
−6
internal/appconfig/renderheld.go+1 −1internal/appconfig/swap.go+4 −3internal/appconfig/swap_sync_test.go+116 −0internal/appconfig/syncexec.go+18 −2
diff --git a/internal/appconfig/renderheld.go b/internal/appconfig/renderheld.go| index 1258805b0..719dc674a 100644 |
| --- a/internal/appconfig/renderheld.go |
| +++ b/internal/appconfig/renderheld.go |
| @@ -87,7 +87,7 @@ func renderHeld(ctx context.Context, s release.Store, priv *ecdh.PrivateKey, dat |
| if w == nil { |
| return HeldRender{}, hostwrite.ErrUnavailable |
| } |
| - r, err := renderExecConfig(ctx, w, rt, m, priv, dataDir, "", v.Overlay) |
| + r, err := renderExecConfig(ctx, w, rt, m, priv, dataDir, "", v.Overlay, false) |
| if err != nil { |
| return HeldRender{}, err |
| } |
diff --git a/internal/appconfig/swap.go b/internal/appconfig/swap.go| index 1d2e1bed2..25f08166c 100644 |
| --- a/internal/appconfig/swap.go |
| +++ b/internal/appconfig/swap.go |
| @@ -51,9 +51,10 @@ func GetSwapPolicy(ctx context.Context, s release.Store, acct, app string) (Swap |
| // Defaults are stored as ABSENT keys, so an app set back to overlap/1 has the |
| // bytes it had before. A call that changes nothing writes nothing and touches |
| // nothing: setting the defaults on an app with no config.json never creates |
| -// one, because boxes read an existing empty bundle differently from an absent |
| -// one (SyncExec writes a file for the first and removes it for the second), |
| -// and that difference would read as a config change on every box. |
| +// one. A first non-default policy does create one, holding no vars, and |
| +// config sync leaves an instance whose config file is absent as it is for |
| +// such a bundle (renderExecConfig's keepAbsent), so that first write |
| +// restarts nothing (Astra plan 6 R1 #1). |
| func SetSwapPolicy(ctx context.Context, s release.Store, acct, app, swap string, concurrency int) (claimResult0 SwapPolicy, claimResult1 error) { |
| claimResult1 = release.WithAppMutation(ctx, s, acct, app, "appconfig.SetSwapPolicy", func(claimContext context.Context) error { |
| claimResult0, claimResult1 = claimRawSetSwapPolicy(claimContext, s, acct, app, swap, concurrency) |
diff --git a/internal/appconfig/swap_sync_test.go b/internal/appconfig/swap_sync_test.go| new file mode 100644 |
| index 000000000..dd5690894 |
| --- /dev/null |
| +++ b/internal/appconfig/swap_sync_test.go |
| @@ -0,0 +1,116 @@ |
| +package appconfig |
| + |
| +import ( |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// Astra plan 6 R1 #1 (plan Q3): the first non-default swap policy on an app |
| +// with no config.json creates the bundle, which renders no vars. Config |
| +// sync must not turn that into a new, empty config file and a restart of a |
| +// running instance: the instance's file is absent before and after, which |
| +// is the state of every app never configured. No file, no restart, no |
| +// delivered revision for the release scheduler, no ack. The first real var |
| +// then converges as it always did: written, restarted, delivered, acked. |
| +func TestAPolicyOnlyBundleNeitherWritesTheFileNorRestarts(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "inst.example.com", Kind: "instance", App: "eleven", Account: "bab", |
| + Status: "ready", Socket: "/run/carlos/inst.sock", |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + var bounced, delivered []string |
| + var acks []ConfigAck |
| + cb := Callbacks{ |
| + Restart: func(h string) error { bounced = append(bounced, h); return nil }, |
| + Ack: func(a ConfigAck) { acks = append(acks, a) }, |
| + Delivered: func(h, rev string) { delivered = append(delivered, h+" "+rev) }, |
| + } |
| + pass := func() { |
| + t.Helper() |
| + bounced, delivered, acks = nil, nil, nil |
| + if err := SyncExecWithCallbacks(ctx, reg, s, pk, dd, "", cb); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + } |
| + path := filepath.Join(dd, "inst.example.com.config.env") |
| + |
| + pass() // no config.json: nothing to do |
| + if _, err := SetSwapPolicy(ctx, s, "bab", "eleven", SwapHold, 3); err != nil { |
| + t.Fatalf("SetSwapPolicy: %v", err) |
| + } |
| + if m, err := Get(ctx, s, "bab", "eleven"); err != nil || m == nil { |
| + t.Fatalf("the policy write did not create the bundle: %v, %v", m, err) |
| + } |
| + pass() |
| + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { |
| + t.Errorf("a policy-only bundle created the config file: %v", err) |
| + } |
| + if len(bounced)+len(delivered)+len(acks) != 0 { |
| + t.Fatalf("a policy-only bundle restarted %v, delivered %v, acked %d", bounced, delivered, len(acks)) |
| + } |
| + pass() // and it stays quiet |
| + if len(bounced)+len(delivered)+len(acks) != 0 { |
| + t.Fatalf("a second pass restarted %v, delivered %v, acked %d", bounced, delivered, len(acks)) |
| + } |
| + |
| + if err := SetEnv(ctx, s, "bab", "eleven", map[string]string{"LOG_LEVEL": "debug"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + pass() |
| + if got, err := os.ReadFile(path); err != nil || string(got) != "LOG_LEVEL=debug\n" { |
| + t.Fatalf("config env = %q, %v", got, err) |
| + } |
| + if len(bounced) != 1 || len(delivered) != 1 || len(acks) != 1 { |
| + t.Fatalf("the first real var restarted %v, delivered %v, acked %d: want one of each", bounced, delivered, len(acks)) |
| + } |
| +} |
| + |
| +// The skip is for an ABSENT file only. A file that exists keeps converging |
| +// on its bytes: an empty file an older agent wrote stays (no restart), and |
| +// a file with vars whose bundle now renders none is emptied and restarted, |
| +// as before. |
| +func TestAnEmptyRenderStillConvergesAnExistingFile(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, h := range []string{"empty.example.com", "full.example.com"} { |
| + if err := reg.Upsert(registry.Route{Host: h, Kind: "instance", App: "eleven", Account: "bab", |
| + Status: "ready", Socket: "/run/carlos/" + h + ".sock"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if err := os.WriteFile(filepath.Join(dd, "empty.example.com.config.env"), nil, 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(filepath.Join(dd, "full.example.com.config.env"), []byte("LOG_LEVEL=debug\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := SetSwapPolicy(ctx, s, "bab", "eleven", SwapHold, 0); err != nil { |
| + t.Fatal(err) |
| + } |
| + var bounced []string |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(h string) error { bounced = append(bounced, h); return nil }, "", nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got, err := os.ReadFile(filepath.Join(dd, "full.example.com.config.env")); err != nil || len(got) != 0 { |
| + t.Fatalf("full file = %q, %v: want it emptied", got, err) |
| + } |
| + if _, err := os.Stat(filepath.Join(dd, "empty.example.com.config.env")); err != nil { |
| + t.Fatalf("the existing empty file went away: %v", err) |
| + } |
| + if len(bounced) != 1 || bounced[0] != "full.example.com" { |
| + t.Fatalf("bounced %v, want only full.example.com", bounced) |
| + } |
| +} |
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go| index cca52b4ea..183bcf214 100644 |
| --- a/internal/appconfig/syncexec.go |
| +++ b/internal/appconfig/syncexec.go |
| @@ -1248,7 +1248,9 @@ func syncExecHostClaimed(ctx context.Context, work hostwrite.Work, rt registry.R |
| } |
| return nil |
| } |
| - r, err := renderExecConfig(ctx, work, rt, m, priv, dataDir, header, instETag) |
| + // keepAbsent: no instance overlay. A bundle that renders nothing for a |
| + // route whose file is absent leaves it absent (renderExecConfig). |
| + r, err := renderExecConfig(ctx, work, rt, m, priv, dataDir, header, instETag, instETag == "") |
| if err != nil { |
| return err |
| } |
| @@ -1324,7 +1326,18 @@ type execRender struct { |
| // reader, renderer and writer SyncExec does, rather than a second copy of |
| // them. The bare-hostname check stays with each caller, before any path is |
| // derived. |
| -func renderExecConfig(ctx context.Context, work hostwrite.Work, rt registry.Route, m *Manifest, priv *ecdh.PrivateKey, dataDir, header, instETag string) (execRender, error) { |
| +// |
| +// keepAbsent (config sync's pass, for a route with no instance overlay): |
| +// when the file is absent and the render is empty, nothing is written and |
| +// the render counts as up to date. An absent file and an empty one give a |
| +// spawn the same environment (the activator reads a missing config env as |
| +// none), and an absent file is what every never-configured app's instances |
| +// already have. Writing the empty file would be a config change for every |
| +// running instance: a restart, a delivered revision and an ack, for a |
| +// bundle that only gained settings boxes read from the bucket, such as the |
| +// swap policy (Astra plan 6 R1 #1, plan Q3). The held render (RenderHeld) |
| +// passes false: a move's ack hashes the file it wrote. |
| +func renderExecConfig(ctx context.Context, work hostwrite.Work, rt registry.Route, m *Manifest, priv *ecdh.PrivateKey, dataDir, header, instETag string, keepAbsent bool) (execRender, error) { |
| path := ConfigEnvPath(dataDir, rt) |
| // A nested row's current file lives on tenant-owned ground, so its read |
| // goes through instancefs the same way its write below does: a plain |
| @@ -1381,6 +1394,9 @@ func renderExecConfig(ctx context.Context, work hostwrite.Work, rt registry.Rout |
| if existed && bytes.Equal(cur, rendered) { |
| return out, nil // up to date — stay quiet on the periodic tick |
| } |
| + if keepAbsent && !existed && len(rendered) == 0 { |
| + return out, nil // absent, and nothing to put in it: as an unconfigured app's |
| + } |
| id := renderedIdentity(rendered, true, rt.Environment != "", instETag != "") |
| out.id = id |
| // A nested row's config file lives inside its own instance directory, |