amadan
appconfig: pin the unit-path removal, the no-spurious-restart claim and a sharper Get-failure case (plan 4, task 11 review follow-up)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 file changed,
+167
−10
internal/appconfig/instances_test.go+167 −10
diff --git a/internal/appconfig/instances_test.go b/internal/appconfig/instances_test.go| index eeca7a231..4d87cafb1 100644 |
| --- a/internal/appconfig/instances_test.go |
| +++ b/internal/appconfig/instances_test.go |
| @@ -558,19 +558,50 @@ func TestSyncCostsOneListingPerAppAndOneGetPerOverlay(t *testing.T) { |
| // dedup for a failed LISTING must not swallow the second route's Get |
| // failure — that would leave one of two stale credential files invisible, |
| // which is exactly what this layer exists to surface. |
| +// |
| +// book converges with a valid overlay first, so the case below covers the |
| +// sharper claim too: an already-delivered route's file survives untouched |
| +// when its overlay later becomes unreadable, not just that a route that |
| +// never had a file stays fileless. |
| func TestSyncReportsEachRoutesOverlayGetFailureSeparately(t *testing.T) { |
| ctx, s, reg, envDir, priv := syncInstanceKit(t) |
| if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes"}); err != nil { |
| t.Fatalf("SetEnv: %v", err) |
| } |
| + // Only book is on the box for the baseline pass, so cart genuinely never |
| + // has a file at any point — registering it before the baseline would |
| + // let it converge on the shared APP_WIDE bundle too, which would blur |
| + // the two cases this test is pinning apart. |
| + unitRoute(t, reg, "book.example.com", "titogo", "bab", "") |
| + |
| + if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com", |
| + map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil { |
| + t.Fatalf("ApplyInstance: %v", err) |
| + } |
| + if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil { |
| + t.Fatalf("Sync (baseline): %v", err) |
| + } |
| + delivered, err := os.ReadFile(filepath.Join(envDir, "book.example.com.env")) |
| + if err != nil { |
| + t.Fatalf("read book's file after the baseline pass: %v", err) |
| + } |
| + if !bytes.Contains(delivered, []byte("TENANT=book\n")) { |
| + t.Fatalf("the baseline overlay did not land: %q", delivered) |
| + } |
| + unitRoute(t, reg, "cart.example.com", "titogo", "bab", "") |
| + |
| // Schema 2 is refused by getIfExistsAtETag ("newer than this binary |
| // understands"), which is a Get-time failure specific to the object |
| // read, not a listing failure — instanceOverlays only reads keys, never |
| - // their bodies, so the listing itself succeeds for both hosts. |
| + // their bodies, so the listing itself succeeds for both hosts. Deleting |
| + // book's valid overlay first, then putting the corrupt one raw (Put with |
| + // ifMatch "" is create-only), is what turns an already-delivered route's |
| + // overlay unreadable, mid-flight. |
| + if err := DeleteInstanceConfig(ctx, s, "bab", "titogo", "book.example.com"); err != nil { |
| + t.Fatalf("DeleteInstanceConfig: %v", err) |
| + } |
| putInstanceRaw(t, ctx, s, "bab", "titogo", "book.example.com", &Manifest{Schema: 2, App: "titogo"}) |
| putInstanceRaw(t, ctx, s, "bab", "titogo", "cart.example.com", &Manifest{Schema: 2, App: "titogo"}) |
| - unitRoute(t, reg, "book.example.com", "titogo", "bab", "") |
| - unitRoute(t, reg, "cart.example.com", "titogo", "bab", "") |
| out := captureStdout(t, func() { |
| if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil { |
| @@ -585,13 +616,18 @@ func TestSyncReportsEachRoutesOverlayGetFailureSeparately(t *testing.T) { |
| if n := strings.Count(out, "instance config unresolved"); n != 2 { |
| t.Fatalf("instance config unresolved lines = %d, want 2 (one per corrupt overlay), got:\n%s", n, out) |
| } |
| - // Neither route had a file before this pass, and a Get failure must |
| - // leave that exactly as it is: no env file materializes from a resolved |
| - // group merge that an unresolved overlay was supposed to sit on top of. |
| - for _, host := range []string{"book.example.com", "cart.example.com"} { |
| - if _, err := os.Stat(filepath.Join(envDir, host+".env")); !os.IsNotExist(err) { |
| - t.Fatalf("%s.env exists after an unresolved overlay: %v", host, err) |
| - } |
| + // cart never had a file before this pass, and a Get failure must leave |
| + // that exactly as it is: no env file materializes from a resolved group |
| + // merge that an unresolved overlay was supposed to sit on top of. |
| + if _, err := os.Stat(filepath.Join(envDir, "cart.example.com.env")); !os.IsNotExist(err) { |
| + t.Fatalf("cart.example.com.env exists after an unresolved overlay: %v", err) |
| + } |
| + // book already had a delivered file, and its Get failure must leave that |
| + // file exactly as it was: an overlay going unreadable is not licence to |
| + // touch a route that already converged on it. |
| + cur, err := os.ReadFile(filepath.Join(envDir, "book.example.com.env")) |
| + if err != nil || !bytes.Equal(cur, delivered) { |
| + t.Fatalf("book's already-delivered file changed after its overlay became unreadable: %q, want %q (err %v)", cur, delivered, err) |
| } |
| } |
| @@ -1179,3 +1215,124 @@ func TestOnePassOverEveryInstanceLayerShape(t *testing.T) { |
| t.Fatalf("desk was disturbed by a sibling app's delete: %q", got) |
| } |
| } |
| + |
| +// The unit-path half of the removal claim. TestOnePassOverEveryInstanceLayerShape |
| +// pins the exec sweep's removal convergence; nothing else in the package |
| +// runs Sync (root's unit path) after a delete, so without this the claim |
| +// that a route "goes back to what the bundles alone say" once its overlay |
| +// is removed is only ever true for SyncExec. |
| +func TestSyncConvergesBackToTheBundleAfterAnOverlayIsDeleted(t *testing.T) { |
| + ctx, s, reg, envDir, priv := syncInstanceKit(t) |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes", "TENANT": "none"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com", |
| + map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil { |
| + t.Fatalf("ApplyInstance: %v", err) |
| + } |
| + unitRoute(t, reg, "book.example.com", "titogo", "bab", "") |
| + |
| + if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + before, err := os.ReadFile(filepath.Join(envDir, "book.example.com.env")) |
| + if err != nil { |
| + t.Fatalf("read env file before the delete: %v", err) |
| + } |
| + if !bytes.Contains(before, []byte("TENANT=book\n")) { |
| + t.Fatalf("the overlay did not land before the delete: %q", before) |
| + } |
| + |
| + if err := DeleteInstanceConfig(ctx, s, "bab", "titogo", "book.example.com"); err != nil { |
| + t.Fatalf("DeleteInstanceConfig: %v", err) |
| + } |
| + if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, nil); err != nil { |
| + t.Fatalf("Sync (after delete): %v", err) |
| + } |
| + after, err := os.ReadFile(filepath.Join(envDir, "book.example.com.env")) |
| + if err != nil { |
| + t.Fatalf("read env file after the delete: %v", err) |
| + } |
| + if bytes.Contains(after, []byte("TENANT=book\n")) || !bytes.Contains(after, []byte("TENANT=none\n")) { |
| + t.Fatalf("the route did not converge back to the bundle alone after the overlay was deleted: %q", after) |
| + } |
| +} |
| + |
| +// Every other test in this file passes a no-op restart callback, so a |
| +// regression that made resolving one route's overlay spuriously mark a |
| +// different route as changed would still pass the whole suite. This one |
| +// records which hosts restart is actually called for. The first pass |
| +// converges both routes from nothing and restarts both once, which is |
| +// ordinary first-convergence behaviour and not the claim under test; that |
| +// recording is discarded before the pass that matters, where only book's |
| +// overlay changes and cart's rendered bytes do not move at all. |
| +func TestSyncDoesNotRestartAnUnchangedSiblingWhileAnOverlayIsResolved(t *testing.T) { |
| + ctx, s, reg, envDir, priv := syncInstanceKit(t) |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes", "TENANT": "none"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + unitRoute(t, reg, "book.example.com", "titogo", "bab", "") |
| + unitRoute(t, reg, "cart.example.com", "titogo", "bab", "") |
| + |
| + var restarted []string |
| + record := func(host string) error { |
| + restarted = append(restarted, host) |
| + return nil |
| + } |
| + if err := Sync(ctx, reg, s, priv, envDir, record, nil); err != nil { |
| + t.Fatalf("Sync (baseline): %v", err) |
| + } |
| + restarted = nil |
| + |
| + if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com", |
| + map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil { |
| + t.Fatalf("ApplyInstance: %v", err) |
| + } |
| + if err := Sync(ctx, reg, s, priv, envDir, record, nil); err != nil { |
| + t.Fatalf("Sync (overlay pass): %v", err) |
| + } |
| + if len(restarted) != 1 || restarted[0] != "book.example.com" { |
| + t.Fatalf("restarted = %v, want exactly [book.example.com]: cart has no overlay and its rendered file did not change, so it must not restart", restarted) |
| + } |
| +} |
| + |
| +// The exec mirror of the test above, over bounce rather than restart. |
| +func TestSyncExecDoesNotBounceAnUnchangedSiblingWhileAnOverlayIsResolved(t *testing.T) { |
| + ctx, s, reg, dd, privB64, _ := execKit(t) |
| + priv, err := ParsePrivateKey(privB64, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes", "TENANT": "none"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + for _, host := range []string{"book.example.com", "cart.example.com"} { |
| + if err := reg.Upsert(registry.Route{ |
| + Host: host, Kind: "instance", App: "titogo", Account: "bab", Status: "ready", |
| + Socket: "/run/carlos/" + host + ".sock", |
| + }); err != nil { |
| + t.Fatalf("Upsert %s: %v", host, err) |
| + } |
| + } |
| + |
| + var bounced []string |
| + record := func(host string) error { |
| + bounced = append(bounced, host) |
| + return nil |
| + } |
| + if err := SyncExec(ctx, reg, s, priv, dd, record, "", nil); err != nil { |
| + t.Fatalf("SyncExec (baseline): %v", err) |
| + } |
| + bounced = nil |
| + |
| + if _, _, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com", |
| + map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil); err != nil { |
| + t.Fatalf("ApplyInstance: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, priv, dd, record, "", nil); err != nil { |
| + t.Fatalf("SyncExec (overlay pass): %v", err) |
| + } |
| + if len(bounced) != 1 || bounced[0] != "book.example.com" { |
| + t.Fatalf("bounced = %v, want exactly [book.example.com]: cart has no overlay and its rendered file did not change, so it must not bounce", bounced) |
| + } |
| +} |