amadan
zdd-readiness: final review round 1, a failed wake is proven by the layout its launch used
Finding 6 (Astra, Medium): hostCgroupIsTheWakes read the row's layout at cleanup time, so a flat failed wake whose layout read fenced or conflicting by then released its lease without proving carlos-exec@<host> empty. The wake now records the layout read immediately before its launch (wokenInstance.launchLayout, kept on unprovenWake for the sweep's retries). The host cgroup proof is skipped only for a unit-backed row or a launch that was nested and still reads nested; an unknown, fenced or conflicting layout, or a recorded and derived layout that disagree, requires the proof.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 files changed,
+118
−24
internal/activator/activator.go+13 −0internal/activator/finalreview1_failedwake_test.go+67 −0internal/activator/maintenance.go+3 −2internal/activator/opset.go+34 −21internal/activator/opset_test.go+1 −1
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index 99a59f6c6..dffc3f03b 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -1645,6 +1645,13 @@ type wokenInstance struct { |
| // kill was not proven (genStartUnproven). cleanupFailedWake keeps the |
| // lease until the sweep proves it, whatever a.gens says. |
| genUnproven bool |
| + // launchLayout is the layout the row read immediately before this |
| + // wake's launch (final review round 1, finding 6, Astra): the failed |
| + // wake's cleanup decides from it, not from the layout at cleanup time, |
| + // whether the host cgroup must be proven empty (hostCgroupIsTheWakes). |
| + // "" when the backend cannot say (no Classifier), which requires the |
| + // proof. |
| + launchLayout Layout |
| // spawnTook is how long Backend.Start took: exec to the socket accepting, |
| // which is the instance's OWN boot and nothing of the platform's. wakeTook |
| // is the whole wake around it, filled in by wake() rather than here |
| @@ -1877,6 +1884,12 @@ func (a *Service) doWake(ctx context.Context, e *actEntry) (*wokenInstance, erro |
| } |
| w.stopOutcome.uncertain = true |
| spawnStart := a.now() |
| + // The layout this launch uses, read on the row it launches, right |
| + // before it (finding 6): a layout that moves before a failed wake's |
| + // cleanup must not move where its descendants are looked for. |
| + if c, ok := a.backend.(Classifier); ok { |
| + w.launchLayout, _ = c.Layout(rt) |
| + } |
| if e.worker { |
| // Mint this wake's token here — under the lease, after the restore, |
| // immediately before the spawn (design §7.2's order). A mint failure |
diff --git a/internal/activator/finalreview1_failedwake_test.go b/internal/activator/finalreview1_failedwake_test.go| new file mode 100644 |
| index 000000000..1318f3f3d |
| --- /dev/null |
| +++ b/internal/activator/finalreview1_failedwake_test.go |
| @@ -0,0 +1,67 @@ |
| +package activator |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "sync/atomic" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// Final review round 1, finding 6 (Astra): a failed wake's cleanup decides |
| +// whether to prove the host cgroup from the layout its launch used, not |
| +// from the layout the row reads at cleanup. A flat launch whose layout reads |
| +// fenced or conflicting by the time the cleanup runs is still proven, and so |
| +// is a nested launch whose layout no longer reads nested (unknown, fenced, |
| +// conflicting or flat): only a launch that was nested, and still reads |
| +// nested, keeps its own proof (ruling c). Here the host cgroup stays |
| +// populated, so a proven wake is retained and its lease kept. |
| +func TestAFailedWakeIsProvenByTheLayoutItsLaunchUsed(t *testing.T) { |
| + for _, row := range []struct { |
| + name string |
| + launch, cleanup Layout |
| + retained, killed bool |
| + }{ |
| + {"flat, fenced at cleanup", LayoutFlat, LayoutFenced, true, true}, |
| + {"flat, conflicting at cleanup", LayoutFlat, LayoutConflict, true, true}, |
| + {"nested, fenced at cleanup", LayoutNested, LayoutFenced, true, true}, |
| + {"nested, flat at cleanup", LayoutNested, LayoutFlat, true, true}, |
| + {"nested, still nested", LayoutNested, LayoutNested, false, false}, |
| + } { |
| + t.Run(row.name, func(t *testing.T) { |
| + a, b, _, _ := testActivator(t, time.Hour) |
| + a.gens = false |
| + var launched atomic.Bool |
| + b.layoutFn = func(registry.Route) (Layout, string) { |
| + if launched.Load() { |
| + return row.cleanup, "changed after the launch" |
| + } |
| + return row.launch, "" |
| + } |
| + leader := &sigtermIgnorer{} |
| + leader.exited.Store(true) |
| + kb := &killGenBackend{fakeBackend: b, err: errNoGenContainment} |
| + ub := &unprovenStartBackend{killGenBackend: kb, fail: func(uint64) (Proc, error) { |
| + launched.Store(true) // the layout changes once the launch has run |
| + return leader, errors.New("health: no answer within 30s") |
| + }} |
| + hb := &uncontainedStartBackend{unprovenStartBackend: ub, populated: true} |
| + a.backend = hb |
| + rt := testRoute("f6-layout.example.com") |
| + if _, err := a.beginRequest(context.Background(), rt); err == nil { |
| + t.Fatal("the wake should have failed") |
| + } |
| + a.mu.Lock() |
| + _, retained := a.unprovenWakes[rt.Host] |
| + a.mu.Unlock() |
| + if retained != row.retained { |
| + t.Fatalf("retained = %v, want %v", retained, row.retained) |
| + } |
| + if asked := hb.hostKillCount() > 0; asked != row.killed { |
| + t.Fatalf("KillHost asked = %v, want %v", asked, row.killed) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/activator/maintenance.go b/internal/activator/maintenance.go| index 72834a246..4ddfbb552 100644 |
| --- a/internal/activator/maintenance.go |
| +++ b/internal/activator/maintenance.go |
| @@ -280,7 +280,7 @@ func (a *Service) cleanupFailedWake(ctx context.Context, rt registry.Route, w *w |
| err := w.proc.Stop(stopCtx) |
| w.stopOutcome.mainStop, w.stopOutcome.mainExited = err == nil, w.proc.Exited() |
| } |
| - if (a.gens || w.genUnproven || w.proc != nil) && !a.failedWakeStopped(rt, w.gen, w.proc) { |
| + if (a.gens || w.genUnproven || w.proc != nil) && !a.failedWakeStopped(rt, w.gen, w.proc, w.launchLayout) { |
| // Its leader did not exit (ErrStillRunning, a failed kill) or its |
| // child is not proven empty. |
| w.stopOutcome.mainExited = false |
| @@ -293,7 +293,8 @@ func (a *Service) cleanupFailedWake(ctx context.Context, rt registry.Route, w *w |
| // A worker's staged identity moves with it (final review F12): the |
| // failed wake's entry is about to go, and the generations file must |
| // keep naming the build its retained process may still run. |
| - a.unprovenWakes[rt.Host] = &unprovenWake{rt: rt, gen: w.gen, proc: w.proc, lease: lease, rep: w.rep, build: w.spawnIdentity} |
| + a.unprovenWakes[rt.Host] = &unprovenWake{rt: rt, gen: w.gen, proc: w.proc, lease: lease, rep: w.rep, build: w.spawnIdentity, |
| + layout: w.launchLayout} |
| a.mu.Unlock() |
| return |
| } |
diff --git a/internal/activator/opset.go b/internal/activator/opset.go| index d8681180d..e482d35d4 100644 |
| --- a/internal/activator/opset.go |
| +++ b/internal/activator/opset.go |
| @@ -326,8 +326,8 @@ func (a *Service) genChildProof(rt registry.Route, m *genMember) (proven, uncont |
| // exit is all there is, as before. Keeping the lease there would refuse |
| // the host on this box until the agent restarts while proving nothing. |
| // Any other failure keeps the wake retained, and the sweep asks again. |
| -func (a *Service) uncontainedHostEmpty(rt registry.Route, g Gen) bool { |
| - if !a.hostCgroupIsTheWakes(rt, g) { |
| +func (a *Service) uncontainedHostEmpty(rt registry.Route, g Gen, launched Layout) bool { |
| + if !a.hostCgroupIsTheWakes(rt, g, launched) { |
| return true // nested or unit-backed: its own proof already ran |
| } |
| hk, ok := a.backend.(hostKiller) |
| @@ -348,31 +348,39 @@ func (a *Service) uncontainedHostEmpty(rt registry.Route, g Gen) bool { |
| return false |
| } |
| -// hostCgroupIsTheWakes reports whether a failed wake of rt ran in |
| -// carlos-exec@<host>: an exec child, that is, a flat or foreign row. A |
| -// unit-backed row runs under systemd (SplitBackend's unit side), and a |
| -// nested row runs as a carlos-instance unit in its host slice; both have |
| -// their own proof. A worker spawns into carlos-exec@<host> whatever its |
| -// layout says (StartWorker), and a flag-off worker's wake is the only one |
| -// with no number (an instance wake is always numbered, nextGen), so a |
| -// number of 0 counts as an exec child. A backend that cannot say its |
| -// layout (a test double) spawns exec children. |
| -func (a *Service) hostCgroupIsTheWakes(rt registry.Route, g Gen) bool { |
| +// hostCgroupIsTheWakes reports whether a failed wake of rt may have run in |
| +// carlos-exec@<host>, so that cgroup must be proven empty: an exec child, |
| +// that is, a flat or foreign row. A unit-backed row runs under systemd |
| +// (SplitBackend's unit side), and a nested row runs as a carlos-instance |
| +// unit in its host slice; both have their own proof. A worker spawns into |
| +// carlos-exec@<host> whatever its layout says (StartWorker), and a |
| +// flag-off worker's wake is the only one with no number (an instance wake |
| +// is always numbered, nextGen), so a number of 0 counts as an exec child. |
| +// rt is the row the wake launched against, so its backing is the launch's. |
| +// |
| +// The layout is the one the launch used (launched), not the one the row |
| +// reads now (final review round 1, finding 6, Astra): a flat wake whose |
| +// layout reads fenced or conflicting by cleanup time still ran in |
| +// carlos-exec@<host>. Only a launch that was nested skips the proof, and |
| +// only while the row still reads nested: a launch layout that is unknown |
| +// (a backend that cannot say, "" here), fenced or conflicting, or a |
| +// recorded and a derived layout that disagree, requires it. |
| +func (a *Service) hostCgroupIsTheWakes(rt registry.Route, g Gen, launched Layout) bool { |
| if rt.Backing == registry.BackingUnit { |
| return false |
| } |
| if g.N == 0 || g.Worker { |
| return true |
| } |
| - c, ok := a.backend.(Classifier) |
| - if !ok { |
| + if launched != LayoutNested { |
| return true |
| } |
| - switch l, _ := c.Layout(rt); l { |
| - case LayoutFlat, LayoutForeign: |
| + c, ok := a.backend.(Classifier) |
| + if !ok { |
| return true |
| } |
| - return false |
| + l, _ := c.Layout(rt) |
| + return l != LayoutNested |
| } |
| // dropMember stops m and takes it out of e's set once its termination is |
| @@ -619,6 +627,10 @@ type unprovenWake struct { |
| // the generations file keeps listing it until the proof holds, since |
| // the retained process may still run those bytes. |
| build release.StagedIdentity |
| + // layout is the layout the failed wake's launch used |
| + // (wokenInstance.launchLayout), which every retry of the proof decides |
| + // by (finding 6). |
| + layout Layout |
| } |
| // errWakeUnproven refuses a wake while the host's last failed wake is not |
| @@ -645,8 +657,9 @@ func genStartUnproven(err error) bool { |
| // Either way, a generation with no child of its own (flag off) is proven |
| // only once its host cgroup is empty too (uncontainedHostEmpty; Task 15, |
| // Astra IMPL-3 F6). Its leader's exit, all genTerminated can see there, |
| -// leaves whatever the leader forked running in carlos-exec@<host>. |
| -func (a *Service) failedWakeStopped(rt registry.Route, g Gen, p Proc) bool { |
| +// leaves whatever the leader forked running in carlos-exec@<host>. launched |
| +// is the layout the wake's launch used (wokenInstance.launchLayout). |
| +func (a *Service) failedWakeStopped(rt registry.Route, g Gen, p Proc, launched Layout) bool { |
| if p == nil && g.N == 0 { |
| log.Printf("activator: %s: failed wake left no process handle and no generation number; it cannot be proven stopped, so its lease stays held", rt.Host) |
| return false |
| @@ -661,7 +674,7 @@ func (a *Service) failedWakeStopped(rt registry.Route, g Gen, p Proc) bool { |
| return false |
| } |
| if uncontained { |
| - return a.uncontainedHostEmpty(rt, g) |
| + return a.uncontainedHostEmpty(rt, g, launched) |
| } |
| return true |
| } |
| @@ -698,7 +711,7 @@ func (a *Service) reapUnprovenWake(host string, u *unprovenWake) { |
| log.Printf("activator: %s: retry stop of failed wake's generation %d: %v", host, u.gen.N, err) |
| } |
| } |
| - if !a.failedWakeStopped(u.rt, u.gen, u.proc) { |
| + if !a.failedWakeStopped(u.rt, u.gen, u.proc, u.layout) { |
| a.mu.Lock() |
| u.busy = false |
| a.mu.Unlock() |
diff --git a/internal/activator/opset_test.go b/internal/activator/opset_test.go| index 7346c49bf..8018503f9 100644 |
| --- a/internal/activator/opset_test.go |
| +++ b/internal/activator/opset_test.go |
| @@ -1257,7 +1257,7 @@ func TestFailedWakeStoppedProvesTheHostCgroupWithoutContainment(t *testing.T) { |
| } else { |
| a.backend = ub |
| } |
| - if got := a.failedWakeStopped(testRoute("f6.example.com"), row.gen, exited); got != row.want { |
| + if got := a.failedWakeStopped(testRoute("f6.example.com"), row.gen, exited, ""); got != row.want { |
| t.Fatalf("failedWakeStopped = %v, want %v", got, row.want) |
| } |
| if asked := hb.hostKillCount() > 0; asked != row.wantHost { |