carlos / platform Public

zdd-readiness: Task 15 fix round 1, a flat health failure keeps its handle and every flag-off exec failed wake proves its host cgroup

Ruling a: startChildAt returns the stopped child with the health error, as startUnitAt does, so doWake's failed-wake cleanup runs. Ruling b: cleanupFailedWake proves a wake that left a process stopped whatever the switch, so a flag-off exec child's carlos-exec@<host> is proven empty (KillHost) before its lease goes, not only when the leader lives. Ruling c: the host proof is gated to exec children (flat, foreign, or a flag-off worker); nested and unit-backed rows are never KillHost'd; the stray-sidecar comment now says a flag-off stray is untracked. Ruling d: removeGenFiles returns its removal errors and the sweep logs a leftover it could not remove.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev 23d42f0f3e62748ca6a1d42a2e8861d48cc7c535 parent 7c0f254
6 files changed, +308 −39
  • internal/activator/backend_exec.go +7 −1
  • internal/activator/maintenance.go +15 −13
  • internal/activator/opset.go +47 −13
  • internal/activator/opset_test.go +180 −5
  • internal/instancectl/gen.go +13 −7
  • internal/instancectl/server_gen_test.go +46 −0
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 26f6a8808..debefccb7 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -1331,7 +1331,13 @@ func (b *ExecBackend) startChildAt(ctx context.Context, rt registry.Route, sock
return p, "", fmt.Errorf("%w; %w: kill generation %d after a failed health wait: %v", err, errGenUnproven, n, kerr)
}
}
- return nil, "", err
+ // The handle goes back with the health error, as startUnitAt's does
+ // (P2-13). The leader's exit is not the end of what it started: with
+ // no g<n>, a process it forked lives on in carlos-exec@<host>, and
+ // the caller's failed-wake cleanup proves that cgroup empty before
+ // the lease goes (Task 15 fix round 1, ruling a; Astra IMPL-3 F6).
+ // The leader is already stopped, so the cleanup's Stop is a no-op.
+ return p, "", err
}
b.normalizeSocketMode(rt, sock, p)
return p, spawnVer, nil
diff --git a/internal/activator/maintenance.go b/internal/activator/maintenance.go
index 3b6eb2e4a..72834a246 100644
--- a/internal/activator/maintenance.go
+++ b/internal/activator/maintenance.go
@@ -259,26 +259,28 @@ func maintenanceResponse(w http.ResponseWriter, r *http.Request) {
// §7.4) whenever that proof is asked for: with generations on, when the
// start's own error said its kill was unproven (w.genUnproven, which only
// GenContainment produces and which can be on with generations off), or,
-// whatever the switch, when the process did not stop (leaderLives below).
-// Unproven, ownership and the lease are kept in unprovenWakes, the sweep
-// retries the stop and the proof (reapUnprovenWakesLocked), and no new wake
-// of the host starts until it holds.
+// whatever the switch, when the wake left a process (below). Unproven,
+// ownership and the lease are kept in unprovenWakes, the sweep retries the
+// stop and the proof (reapUnprovenWakesLocked), and no new wake of the host
+// starts until it holds.
func (a *Service) cleanupFailedWake(ctx context.Context, rt registry.Route, w *wokenInstance, lease Lease) {
stopCtx, cancel := context.WithTimeout(context.Background(), actStopTimeout)
defer cancel()
- // leaderLives: the wake's process did not stop when asked (Stop answered
- // ErrStillRunning) or is still alive. Its lease is kept whatever the
- // switch says, with generations off too: a fail-closed exception to
- // dormancy (zero-downtime plan 3, task 2 fix), because giving the lease
- // back would let another box wake the host while this process still
- // writes. Every other flag-off failed wake is unchanged.
- leaderLives := false
+ // A wake that left a process is proven stopped before its lease goes,
+ // whatever the switch says: two fail-closed exceptions to dormancy.
+ // One is a leader that did not stop when asked (ErrStillRunning) or is
+ // still alive (zero-downtime plan 3, task 2 fix): giving the lease back
+ // would let another box wake the host while it still writes. The other
+ // is an exec child whose leader did stop: with the flag off it ran in
+ // carlos-exec@<host> with no g<n>, and what it forked may still run
+ // there, so that cgroup is proven empty too (failedWakeStopped's
+ // uncontainedHostEmpty; Task 15 fix round 1, ruling b; Astra IMPL-3
+ // F6). A nested or unit-backed row's own proof is unchanged.
if w.proc != nil {
err := w.proc.Stop(stopCtx)
w.stopOutcome.mainStop, w.stopOutcome.mainExited = err == nil, w.proc.Exited()
- leaderLives = errors.Is(err, ErrStillRunning) || !w.stopOutcome.mainExited
}
- if (a.gens || w.genUnproven || leaderLives) && !a.failedWakeStopped(rt, w.gen, w.proc) {
+ if (a.gens || w.genUnproven || w.proc != nil) && !a.failedWakeStopped(rt, w.gen, w.proc) {
// Its leader did not exit (ErrStillRunning, a failed kill) or its
// child is not proven empty.
w.stopOutcome.mainExited = false
diff --git a/internal/activator/opset.go b/internal/activator/opset.go
index 02fcb97aa..d8681180d 100644
--- a/internal/activator/opset.go
+++ b/internal/activator/opset.go
@@ -300,22 +300,26 @@ func (a *Service) genChildProof(rt registry.Route, m *genMember) (proven, uncont
return false, false
}
-// uncontainedHostEmpty proves a retained failed wake's host cgroup empty
-// when its generation had no child of its own (Task 15, Astra IMPL-3 F6,
-// flag off): KillHost kills whatever is left in carlos-exec@<host> and
-// waits it empty, as the worker proof does. An absent cgroup is empty.
+// uncontainedHostEmpty proves a failed wake's host cgroup empty when its
+// generation had no child of its own (Task 15, Astra IMPL-3 F6, flag off):
+// KillHost kills whatever is left in carlos-exec@<host> and waits it
+// empty, as the worker proof does. An absent cgroup is empty. Only an exec
+// child's wake ran there (hostCgroupIsTheWakes); a nested or unit-backed
+// row's proof is its own, and a cgroup that is not the row's is never
+// killed (fix round 1, ruling c).
//
// Killing is safe here because nothing of another entry can be in that
-// cgroup while the wake is retained. The host's next wake, request or
-// claim, is refused (errWakeUnproven) until this proof holds; the failed
-// wake's own entry is gone, and it never reached the legacy sidecar's
-// co-spawn, which comes after every failure that retains a wake; sidecar
-// and seed starts run only under the host's live entry; workers and
-// sidecar members are rows of their own, each with its own
+// cgroup at this point. The failed wake still holds the host's lease, and
+// while it is retained the host's next wake, request or claim, is refused
+// (errWakeUnproven); the failed wake never reached the legacy sidecar's
+// co-spawn, which comes after every failure that reaches this cleanup;
+// sidecar and seed starts run only under the host's live entry; workers
+// and sidecar members are rows of their own, each with its own
// carlos-exec@<host>. What can be there is the wake's own leftovers, or a
-// stray sidecar of an earlier entry whose stop was not proven, which is
-// meant to be gone too. The killed cgroup is recreated by the next start
-// into it (Task 19).
+// legacy sidecar of an earlier entry whose stop did not end it. With the
+// flag off such a sidecar is untracked (straySidecars is filled only with
+// generations on), and it is meant to be gone, so killing it is right. The
+// killed cgroup is recreated by the next start into it (Task 19).
//
// A box with no delegated cgroup (cgroup.ErrNoDelegation) ran the wake
// unaccounted, with nothing that could reach a descendant: the leader's
@@ -323,6 +327,9 @@ func (a *Service) genChildProof(rt registry.Route, m *genMember) (proven, uncont
// 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) {
+ return true // nested or unit-backed: its own proof already ran
+ }
hk, ok := a.backend.(hostKiller)
if !ok {
return true // a backend with no host cgroup: the leader's exit is all there is
@@ -341,6 +348,33 @@ 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 {
+ if rt.Backing == registry.BackingUnit {
+ return false
+ }
+ if g.N == 0 || g.Worker {
+ return true
+ }
+ c, ok := a.backend.(Classifier)
+ if !ok {
+ return true
+ }
+ switch l, _ := c.Layout(rt); l {
+ case LayoutFlat, LayoutForeign:
+ return true
+ }
+ return false
+}
+
// dropMember stops m and takes it out of e's set once its termination is
// proven. It reports whether termination was proven. An unproven member
// stays in the set, marked, and the sweep's reaper finishes it.
diff --git a/internal/activator/opset_test.go b/internal/activator/opset_test.go
index 7a2b76750..7346c49bf 100644
--- a/internal/activator/opset_test.go
+++ b/internal/activator/opset_test.go
@@ -7,6 +7,7 @@ import (
"log"
"os"
"os/exec"
+ "path/filepath"
"slices"
"strings"
"sync"
@@ -658,8 +659,11 @@ func (g *execGenBackend) firstGen() uint64 {
// the process goes back with an error wrapping errGenUnproven and the wake
// keeps its lease until the sweep proves the child empty. When it
// succeeds, nothing is left and the lease goes back at once. With
-// containment off nothing is placed, no kill is made, and the lease goes
-// back as before.
+// containment off nothing is placed and startChildAt makes no kill. Its
+// handle now comes back with the health error (Task 15 fix round 1, ruling
+// a), so the failed-wake cleanup's proof asks KillGen once, which answers
+// errNoGenContainment as ExecBackend's does with containment off; this
+// backend has no host kill, so the lease goes back.
func TestAPlacedSpawnWithAFailedHealthWaitProvesItsChild(t *testing.T) {
old := spawnHealthTimeout
spawnHealthTimeout = 300 * time.Millisecond
@@ -672,7 +676,7 @@ func TestAPlacedSpawnWithAFailedHealthWaitProvesItsChild(t *testing.T) {
}{
{"placed, kill fails", true, errors.New("g still holds a process"), true},
{"placed, kill proves the child empty", true, nil, false},
- {"containment off", false, nil, false},
+ {"containment off", false, errNoGenContainment, false},
}
for _, row := range rows {
t.Run(row.name, func(t *testing.T) {
@@ -701,8 +705,8 @@ func TestAPlacedSpawnWithAFailedHealthWaitProvesItsChild(t *testing.T) {
t.Fatalf("retained = %v, want %v (err %v)", kept, row.wantKept, err)
}
if !row.containment {
- if k := kb.killed(); len(k) != 0 {
- t.Fatalf("KillGen calls = %v with containment off, want none", k)
+ if k := kb.killed(); !slices.Equal(k, []uint64{n}) {
+ t.Fatalf("KillGen calls = %v with containment off, want only the cleanup's proof of generation %d", k, n)
}
if got := b.releaseCount(); got != 1 {
t.Fatalf("lease releases = %d, want 1", got)
@@ -1262,3 +1266,174 @@ func TestFailedWakeStoppedProvesTheHostCgroupWithoutContainment(t *testing.T) {
})
}
}
+
+// realFlatBackend is fakeBackend's lease, restore and replica with a real
+// flag-off ExecBackend's spawn (StartGen, startChildAt and its health
+// wait), its KillGen (errNoGenContainment) and its layout. Only KillHost
+// is a fake, since the test box has no delegated cgroup v2: it fails while
+// populated, as a host cgroup that still holds a process does.
+type realFlatBackend struct {
+ *fakeBackend
+ exec *ExecBackend
+ hmu sync.Mutex
+ populated bool
+ hostKills int
+}
+
+func (r *realFlatBackend) StartGen(ctx context.Context, rt registry.Route, n uint64, perGen bool) (Proc, Gen, error) {
+ return r.exec.StartGen(ctx, rt, n, perGen)
+}
+
+func (r *realFlatBackend) NextDueVia(ctx context.Context, rt registry.Route, _ string) (int64, error) {
+ return r.fakeBackend.NextDue(ctx, rt)
+}
+
+func (r *realFlatBackend) KillGen(ctx context.Context, rt registry.Route, n uint64) error {
+ return r.exec.KillGen(ctx, rt, n)
+}
+
+func (r *realFlatBackend) Layout(rt registry.Route) (Layout, string) { return r.exec.Layout(rt) }
+
+func (r *realFlatBackend) KillHost(context.Context, registry.Route) error {
+ r.hmu.Lock()
+ defer r.hmu.Unlock()
+ r.hostKills++
+ if r.populated {
+ return errors.New("carlos-exec@<host> still holds a process")
+ }
+ return nil
+}
+
+func (r *realFlatBackend) setPopulated(on bool) {
+ r.hmu.Lock()
+ r.populated = on
+ r.hmu.Unlock()
+}
+
+func (r *realFlatBackend) hostKillCount() int {
+ r.hmu.Lock()
+ defer r.hmu.Unlock()
+ return r.hostKills
+}
+
+var (
+ _ GenBackend = (*realFlatBackend)(nil)
+ _ genKiller = (*realFlatBackend)(nil)
+ _ hostKiller = (*realFlatBackend)(nil)
+ _ Classifier = (*realFlatBackend)(nil)
+)
+
+// Task 15 fix round 1 (rulings a and b), F6 in the shape it really takes:
+// a flat exec child whose health wait times out. startChildAt stopped the
+// leader and returned no handle, so doWake gave the lease back at once,
+// with nothing asked of carlos-exec@<host>, where whatever the app forked
+// still ran. The handle now comes back with the health error, and the
+// failed-wake cleanup proves the host cgroup empty before any release:
+// populated, the wake is retained with its lease; emptied, the sweep gives
+// the lease back.
+func TestARealFlatHealthFailedWakeProvesItsHostCgroup(t *testing.T) {
+ requireUnixSockets(t)
+ old := spawnHealthTimeout
+ spawnHealthTimeout = 500 * time.Millisecond
+ t.Cleanup(func() { spawnHealthTimeout = old })
+ a, b, _, _ := testActivator(t, time.Hour)
+ a.gens = false
+ exec := testExecBackend(t)
+ host := "unhealthy-f6.example.com"
+ stageServingLive(t, exec, host)
+ t.Setenv("ACTIVATOR_FAKE_APP_UNHEALTHY", "1")
+ rb := &realFlatBackend{fakeBackend: b, exec: exec, populated: true}
+ a.backend = rb
+ rt := registry.Route{Host: host, Kind: "instance", Socket: tmpSock(t), DBPath: filepath.Join(t.TempDir(), host+".db"), Hibernate: true}
+ if l, _ := exec.Layout(rt); l != LayoutFlat && l != LayoutForeign {
+ t.Fatalf("layout = %s, want an exec child's (flat or foreign)", l)
+ }
+ retained := func() bool {
+ a.mu.Lock()
+ defer a.mu.Unlock()
+ return a.unprovenWakes[host] != nil
+ }
+ settled := func() bool {
+ a.mu.Lock()
+ defer a.mu.Unlock()
+ u := a.unprovenWakes[host]
+ return u == nil || !u.busy
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ defer cancel()
+ if _, err := a.beginRequest(ctx, rt); err == nil {
+ t.Fatal("a never-healthy app woke")
+ }
+ if got := b.releaseCount(); got != 0 || !retained() {
+ t.Fatalf("lease releases = %d, retained %v after a health failure with carlos-exec@<host> populated; want 0 and retained", got, retained())
+ }
+ if rb.hostKillCount() == 0 {
+ t.Fatal("the failed wake's cleanup never asked KillHost")
+ }
+ a.sweepOnce()
+ waitFor(t, "the retained wake's retry", settled)
+ if got := b.releaseCount(); got != 0 || !retained() {
+ t.Fatalf("lease releases = %d after a sweep with the host cgroup still populated, want 0", got)
+ }
+ rb.setPopulated(false)
+ a.sweepOnce()
+ waitFor(t, "the lease release", func() bool { return b.releaseCount() == 1 })
+ waitFor(t, "the retained wake dropped", func() bool { return !retained() })
+}
+
+// Ruling b with childProc's semantics and no live leader: a flag-off failed
+// wake whose leader stopped (Stop nil, Exited true, as childProc always
+// answers) still proves the host cgroup before its lease goes. Before, the
+// proof ran only for a leader that lived, so this released at once.
+// Ruling c: a nested row and a unit-backed row have their own proof and are
+// never KillHost'd; their failed wake releases as before.
+func TestAFlagOffFailedWakeProvesTheHostCgroupOnlyForAnExecChild(t *testing.T) {
+ for _, row := range []struct {
+ name string
+ layout Layout
+ backing string
+ retained bool // populated host cgroup: the wake is kept
+ killHost bool
+ }{
+ {"flat", LayoutFlat, "", true, true},
+ {"foreign", LayoutForeign, "", true, true},
+ {"nested", LayoutNested, "", false, false},
+ {"unit-backed", LayoutFlat, registry.BackingUnit, false, false},
+ } {
+ t.Run(row.name, func(t *testing.T) {
+ a, b, _, _ := testActivator(t, time.Hour)
+ a.gens = false
+ b.layoutFn = func(registry.Route) (Layout, string) { return row.layout, "" }
+ leader := &sigtermIgnorer{}
+ leader.exited.Store(true)
+ kb := &killGenBackend{fakeBackend: b, err: errNoGenContainment}
+ ub := &unprovenStartBackend{killGenBackend: kb, fail: func(uint64) (Proc, error) {
+ return leader, errors.New("health: no answer within 30s")
+ }}
+ hb := &uncontainedStartBackend{unprovenStartBackend: ub, populated: true}
+ a.backend = hb
+ host := "exited-" + row.name + ".example.com"
+ rt := testRoute(host)
+ rt.Backing = row.backing
+ if _, err := a.beginRequest(context.Background(), rt); err == nil {
+ t.Fatal("the wake should have failed")
+ }
+ a.mu.Lock()
+ _, retained := a.unprovenWakes[host]
+ a.mu.Unlock()
+ if retained != row.retained {
+ t.Fatalf("retained = %v, want %v", retained, row.retained)
+ }
+ wantReleases := 1
+ if row.retained {
+ wantReleases = 0
+ }
+ if got := b.releaseCount(); got != wantReleases {
+ t.Fatalf("lease releases = %d, want %d", got, wantReleases)
+ }
+ if asked := hb.hostKillCount() > 0; asked != row.killHost {
+ t.Fatalf("KillHost asked = %v, want %v", asked, row.killHost)
+ }
+ })
+ }
+}
diff --git a/internal/instancectl/gen.go b/internal/instancectl/gen.go
index 6be63042a..0d60d21fa 100644
--- a/internal/instancectl/gen.go
+++ b/internal/instancectl/gen.go
@@ -79,22 +79,25 @@ func (s *Server) ensureGenExe(host string, n uint64) error {
// removeGenFiles removes a generation unit's drop-in and executable layout,
// by the unit's name as systemd reports it. Best-effort: a leftover is a
-// few bytes in /run, never a running unit.
-func (s *Server) removeGenFiles(unit string) {
+// few bytes in /run, never a running unit, so most callers drop the error.
+// The sweep logs it, so a leftover it cannot remove says why (Task 15).
+func (s *Server) removeGenFiles(unit string) error {
esc, n, ok := ParseGenUnitName(unit)
if !ok {
- return
+ return nil
}
+ var errs []error
if s.DropInDir != "" {
- _ = os.RemoveAll(filepath.Join(s.DropInDir, unit+".d"))
+ errs = append(errs, os.RemoveAll(filepath.Join(s.DropInDir, unit+".d")))
}
iid, ok := Unescape(esc)
if !ok || iid == "" || strings.ContainsAny(iid, "/\x00") || iid == "." || iid == ".." {
- return
+ return errors.Join(errs...)
}
if s.ExeRoot != "" {
- _ = os.RemoveAll(filepath.Join(s.ExeRoot, GenKeyOf(iid, n)))
+ errs = append(errs, os.RemoveAll(filepath.Join(s.ExeRoot, GenKeyOf(iid, n))))
}
+ return errors.Join(errs...)
}
// slicePopulated reports whether anything runs in host's slice. Absent is
@@ -347,7 +350,10 @@ func (s *Server) sweepGenLeftover(ctx context.Context, unit string) {
if err := WaitCgroupEmpty(wctx, s.CgroupRoot, "carlos-instances-"+esc+".slice", unit, time.Millisecond); err != nil {
return
}
- s.removeGenFiles(unit)
+ if err := s.removeGenFiles(unit); err != nil {
+ s.logf("instance-control: sweep: could not remove the files of %s: %v", unit, err)
+ return
+ }
s.logf("instance-control: sweep: removed the files of %s", unit)
}
diff --git a/internal/instancectl/server_gen_test.go b/internal/instancectl/server_gen_test.go
index 263b0cda6..86aefc0f4 100644
--- a/internal/instancectl/server_gen_test.go
+++ b/internal/instancectl/server_gen_test.go
@@ -4,9 +4,11 @@ import (
"context"
"encoding/json"
"errors"
+ "fmt"
"os"
"path/filepath"
"strings"
+ "sync"
"testing"
"time"
@@ -469,6 +471,50 @@ func TestSweepRemovesAnExeOnlyGenerationLeftover(t *testing.T) {
}
}
+// Task 15 fix round 1 (d): a leftover the sweep cannot remove is logged as
+// not removed, with the removal's error. removeGenFiles dropped every
+// error, so the sweep logged "removed the files of" over an exe directory
+// still on disk.
+func TestSweepLogsAnExeLeftoverItCouldNotRemove(t *testing.T) {
+ if os.Geteuid() == 0 {
+ t.Skip("root removes from a read-only directory")
+ }
+ var (
+ mu sync.Mutex
+ logged []string
+ )
+ ctl := genCtl()
+ exe := t.TempDir()
+ s, _ := serveOnSocket(t, ctl, func(s *Server) {
+ s.DropInDir, s.ExeRoot = t.TempDir(), exe
+ s.Logf = func(f string, a ...any) {
+ mu.Lock()
+ logged = append(logged, fmt.Sprintf(f, a...))
+ mu.Unlock()
+ }
+ })
+ if err := s.ensureGenExe(genHost, 7); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Chmod(exe, 0o555); err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { _ = os.Chmod(exe, 0o755) })
+ s.sweepGenLeftovers(context.Background(), nil)
+ if _, err := os.Stat(filepath.Join(exe, GenKey(genHost, 7))); err != nil {
+ t.Fatalf("the fixture's exe directory went after all: %v", err)
+ }
+ mu.Lock()
+ all := strings.Join(logged, "\n")
+ mu.Unlock()
+ if strings.Contains(all, "removed the files of "+genUnit) {
+ t.Errorf("the sweep logged a removal that failed:\n%s", all)
+ }
+ if !strings.Contains(all, "could not remove the files of "+genUnit) || !strings.Contains(all, "permission denied") {
+ t.Errorf("the sweep did not log the failed removal and its error:\n%s", all)
+ }
+}
+
// An exe-only leftover is kept while its unit is not proven gone (active,
// or its cgroup populated) and while a start of that unit is preparing it:
// the start lays the exe out before it writes the drop-in, so for a moment