amadan

carlos / platform Public

zdd-5 final review w2: the exec gate is asked before each cgroup start (F1)

A flat staged spawn's exec gate now runs as the cgroup manager's
before-start hook (StartGated / StartChildGated): after the parent's
tier, the child cgroup and its limits are written, and again before the
PID-move retry that follows a failed clone3 start. A refusal starts
nothing, removes the empty cgroup dir as a failed start does, and comes
back as the gate's own error. The strict arm asks it after the strict
group is prepared. With no gate on ctx the manager gets no hook and every
spawn is as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude fe65e9b3fa6aa0c27d40b45270cc791144d2fa8d parent 6ae15fd
9 files changed, +379 −47
  • internal/activator/backend_exec.go +35 −21
  • internal/activator/containment.go +10 −4
  • internal/activator/exec_gate_test.go +128 −0
  • internal/activator/genbackend.go +17 −0
  • internal/activator/genbackend_test.go +1 −1
  • internal/cgroup/cgroup.go +66 −18
  • internal/cgroup/cgroup_linux_test.go +59 −0
  • internal/cgroup/gen.go +11 −3
  • internal/cgroup/gen_test.go +52 −0
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 21848458d..cedac13fa 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -217,9 +217,12 @@ type ExecBackend struct {
// cleanup path. Test-only: it makes interleavings deterministic.
strictHook func(point string)
// launchHook, when set, is called by a staged spawn once its
- // preparation is done, immediately before its exec gate (runExecGate).
- // Test-only: it lands a lease loss, a fence or a supersession after
- // the preparation and before the launch (final review F1).
+ // preparation is done, immediately before its exec gate (runExecGate):
+ // flat, inside the cgroup manager's before-start hook, so once per
+ // start attempt (a PID-move retry asks again); nested, before root's
+ // start call. Test-only: it lands a lease loss, a fence or a
+ // supersession after the preparation and before the launch (final
+ // review F1).
launchHook func(host string)
}
@@ -357,8 +360,17 @@ func (b *ExecBackend) tier(rt registry.Route) cgroup.Limit {
// for the host in between. With Strict nil (a box with no moves wiring)
// none of this runs and no lock is taken: the spawn is main's exactly.
func (b *ExecBackend) spawnInCgroup(rt registry.Route, build func() *exec.Cmd) (*exec.Cmd, func(), error) {
+ return b.spawnInCgroupGated(rt, build, nil)
+}
+
+// spawnInCgroupGated is spawnInCgroup with a before-start hook (final review
+// F1), asked immediately before each start attempt: the cgroup manager's
+// (StartGated), or, strict, after the strict group is prepared. A refusal
+// starts nothing and is cleaned up as a failed start is. A nil before is
+// spawnInCgroup exactly.
+func (b *ExecBackend) spawnInCgroupGated(rt registry.Route, build func() *exec.Cmd, before func() error) (*exec.Cmd, func(), error) {
if b.Strict == nil {
- cmd, err := b.Cgroups.Start(rt.Host, b.tier(rt), build)
+ cmd, err := b.Cgroups.StartGated(rt.Host, b.tier(rt), build, before)
return cmd, func() {}, err
}
strict := b.Strict(rt.Host)
@@ -377,7 +389,7 @@ func (b *ExecBackend) spawnInCgroup(rt registry.Route, build func() *exec.Cmd) (
return nil, nil, fmt.Errorf("%s: %w", rt.Host, cause)
}
if !strict {
- cmd, err := b.Cgroups.Start(rt.Host, b.tier(rt), build)
+ cmd, err := b.Cgroups.StartGated(rt.Host, b.tier(rt), build, before)
g.mu.Unlock()
return cmd, func() {}, err
}
@@ -391,6 +403,13 @@ func (b *ExecBackend) spawnInCgroup(rt registry.Route, build func() *exec.Cmd) (
}
o.sp = sp
cmd := build()
+ if before != nil {
+ if err := before(); err != nil {
+ // Nothing started: the strict group goes as after a failed start.
+ b.settleStrict(rt.Host, o)
+ return nil, nil, err
+ }
+ }
if err := sp.Start(cmd); err != nil {
b.settleStrict(rt.Host, o)
return nil, nil, fmt.Errorf("%s: %w: %v", rt.Host, ErrStrictRequired, err)
@@ -1445,26 +1464,21 @@ func (b *ExecBackend) spawnChildFrom(ctx context.Context, rt registry.Route, soc
if exe == "" {
spawnVer, _ = b.LiveVersion(host)
}
- // The caller's exec gate (final review F1), after every step above: the
- // token, the config read and the control socket's bind all take time,
- // and the lease, a fence or the attempt can change meanwhile. A refusal
- // starts nothing and unbinds the listener, as a failed spawn does; no
- // child cgroup has been placed yet. With no gate on ctx (every live-link
- // spawn) this does nothing.
- if b.launchHook != nil {
- b.launchHook(host)
- }
- if err := runExecGate(ctx); err != nil {
- if ctl != nil {
- ctl.Close()
- }
- return nil, "", err
- }
// Spawned through the cgroup manager (nil-tolerant: no delegation means
// a plain start, exactly as before) so the child lands in
// carlos-exec@<host> and the sampler can account it. build may run
// twice — the manager's clone3-refusal fallback — so everything about
// the Cmd is configured inside it.
+ //
+ // The caller's exec gate (final review F1) is the manager's before-start
+ // hook (launchGate): asked after every step above (the token, the config
+ // read, the control socket's bind) AND the cgroup preparation (the
+ // parent, the child, its limits), immediately before each start attempt,
+ // the PID-move retry included. The lease, a fence or the attempt can
+ // change during any of it. A refusal starts nothing; the manager drops
+ // the child cgroup and the error path below unbinds the listener, as for
+ // a failed spawn. With no gate on ctx (every live-link spawn) there is no
+ // hook and the spawn is exactly as it was.
cmd, settle, err := b.spawnInCgroupAtTier(rt, child, ownTier, tier, func() *exec.Cmd {
cmd := exec.Command(bin, "--socket", sock, "--db", dbPath)
cmd.Env = childEnv(childVars)
@@ -1475,7 +1489,7 @@ func (b *ExecBackend) spawnChildFrom(ctx context.Context, rt registry.Route, soc
// that piping entails.
b.teeStd(cmd, rt)
return cmd
- })
+ }, b.launchGate(ctx, host))
if err != nil {
// The listener was bound before the spawn; nothing will ever answer on
// it now, so unbind it rather than leaving a live socket for a process
diff --git a/internal/activator/containment.go b/internal/activator/containment.go
index d3a526e70..536ca8b7b 100644
--- a/internal/activator/containment.go
+++ b/internal/activator/containment.go
@@ -60,7 +60,7 @@ func (b *ExecBackend) placeFor(kind spawnKind, n uint64) (child string, ownTier
// spawnInCgroupAt is spawnInCgroupAtTier with the live tier: the spawn of a
// placed child whose artifact is the one the live link names.
func (b *ExecBackend) spawnInCgroupAt(rt registry.Route, child string, ownTier bool, build func() *exec.Cmd) (*exec.Cmd, func(), error) {
- return b.spawnInCgroupAtTier(rt, child, ownTier, cgroup.Limit{}, build)
+ return b.spawnInCgroupAtTier(rt, child, ownTier, cgroup.Limit{}, build, nil)
}
// spawnInCgroupAtTier is spawnInCgroup for a placed child, with the
@@ -75,9 +75,15 @@ func (b *ExecBackend) spawnInCgroupAt(rt registry.Route, child string, ownTier b
// owner's group, and a host that is strict is refused rather than started
// outside its strict group. The returned settle is spawnInCgroup's: a no-op
// here. A spawn that fails removes the child it may have created.
-func (b *ExecBackend) spawnInCgroupAtTier(rt registry.Route, child string, ownTier bool, tier cgroup.Limit, build func() *exec.Cmd) (*exec.Cmd, func(), error) {
+//
+// before, when set, is the cgroup manager's before-start hook (final review
+// F1): asked immediately before each start attempt, after the parent and
+// child are prepared, and on the PID-move retry too. A refusal starts
+// nothing, is cleaned up as a failed start is, and comes back as before's
+// own error. Nil is the spawn exactly as it was.
+func (b *ExecBackend) spawnInCgroupAtTier(rt registry.Route, child string, ownTier bool, tier cgroup.Limit, build func() *exec.Cmd, before func() error) (*exec.Cmd, func(), error) {
if child == "" {
- return b.spawnInCgroup(rt, build)
+ return b.spawnInCgroupGated(rt, build, before)
}
start := func() (*exec.Cmd, func(), error) {
t := tier
@@ -88,7 +94,7 @@ func (b *ExecBackend) spawnInCgroupAtTier(rt registry.Route, child string, ownTi
if ownTier {
own = t
}
- cmd, err := b.Cgroups.StartChild(rt.Host, child, t, own, build)
+ cmd, err := b.Cgroups.StartChildGated(rt.Host, child, t, own, build, before)
if err != nil {
b.Cgroups.RemoveChild(rt.Host, child)
return nil, nil, err
diff --git a/internal/activator/exec_gate_test.go b/internal/activator/exec_gate_test.go
index 4d0ebb0c3..b3c44f110 100644
--- a/internal/activator/exec_gate_test.go
+++ b/internal/activator/exec_gate_test.go
@@ -12,6 +12,7 @@ import (
"testing"
"time"
+ "github.com/carlosframework/platform/internal/cgroup"
"github.com/carlosframework/platform/internal/instancectl"
"github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
@@ -371,3 +372,130 @@ func TestSplitBackendCarriesTheExecGate(t *testing.T) {
t.Fatalf("StartStaged = %v, %v (started %d): want the gate's refusal and nothing started", p, err, exec.started)
}
}
+
+// gatedCgroups gives b a cgroup manager on a temp-dir subtree (the
+// fixture delegatedBackend uses) whose log calls flip once a limit is
+// written: a log line naming a memory.max is the manager's preparation
+// (the parent's tier, the child's limits) under way.
+func gatedCgroups(t *testing.T, b *ExecBackend, host string, flip func()) string {
+ t.Helper()
+ root := t.TempDir()
+ for _, f := range []string{"cgroup.procs", "cgroup.subtree_control"} {
+ if err := os.WriteFile(filepath.Join(root, f), nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ var once sync.Once
+ m, err := cgroup.SetupAt(root, func(format string, args ...any) {
+ t.Logf(format, args...)
+ if flip != nil && strings.Contains(format, "memory.max=") {
+ once.Do(flip)
+ }
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ parent := filepath.Join(root, cgroup.ExecName(host))
+ if err := os.MkdirAll(parent, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(parent, "cgroup.controllers"), []byte("cpu io memory pids\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ b.Cgroups = m
+ return parent
+}
+
+// Final review wave 2 (F1 completion, Astra round 2 finding 1): a flat
+// staged spawn's exec gate is the cgroup manager's before-start hook, so
+// it is asked AFTER the cgroup preparation (the parent's tier, the child
+// cgroup and its limits, their log lines) and before EACH start attempt,
+// the PID-move retry after a failed clone3 start included. Ownership lost
+// during that preparation, or between a failed start and its retry,
+// starts nothing: no process, no marker, the control socket unbound, the
+// generation's child cgroup removed, abandonment with no execution
+// evidence. Before the fix the gate ran once, before the cgroup
+// preparation, and each of these launched.
+func TestTheExecGateIsAskedAtEachCgroupStart(t *testing.T) {
+ t.Run("overlap successor, superseded during the cgroup preparation", func(t *testing.T) {
+ f := newSwapFixture(t)
+ b := newLaunchBackend(t, "flat", f.b)
+ b.eb.GenContainment = true
+ parent := gatedCgroups(t, b.eb, b.rt.Host, func() { f.rev.Add(1) })
+ f.a.backend = b
+ mark := filepath.Join(t.TempDir(), "v2.ran")
+ id := markerScript(t, t.TempDir(), "v2", mark)
+ at := f.attempt(ModeOverlap)
+ at.StagedPath, at.Digest = id.StagedPath, id.Digest
+ res := f.a.Swap(context.Background(), at, f.newest)
+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errSwapSuperseded.Error()) || res.Executed {
+ t.Fatalf("Swap = %+v, want abandoned (superseded), nothing executed", res)
+ }
+ gens := b.gens()
+ if len(gens) != 1 {
+ t.Fatalf("the backend was entered for generations %v, want the successor's alone", gens)
+ }
+ time.Sleep(100 * time.Millisecond)
+ if b.launched(t, id, mark) {
+ t.Fatal("the successor was launched after its attempt was superseded during the cgroup preparation")
+ }
+ b.prepGone(t, gens[0])
+ // The preparation did run (the flip is its log line), and nothing was
+ // placed in the child it made. (On cgroupfs the empty child is
+ // removed; in this fixture its limit files are ordinary files, so the
+ // rmdir is refused, as CleanupHost's comment explains.)
+ if b, err := os.ReadFile(filepath.Join(parent, cgroup.GenChild(gens[0]), "cgroup.procs")); err == nil && len(b) != 0 {
+ t.Fatalf("generation %d's child cgroup was given pid %s after a refusal at its start", gens[0], b)
+ }
+ })
+ t.Run("hold's pinned build, lease lost during the cgroup preparation", func(t *testing.T) {
+ f := newSwapFixture(t)
+ b := newLaunchBackend(t, "flat", f.b)
+ gatedCgroups(t, b.eb, b.rt.Host, func() { f.b.lastRep().setErr(errors.New("lease renewal failed")) })
+ f.a.backend = b
+ mark := filepath.Join(t.TempDir(), "v2.ran")
+ id := markerScript(t, t.TempDir(), "v2", mark)
+ at := f.attempt(ModeHold)
+ at.StagedPath, at.Digest = id.StagedPath, id.Digest
+ res := f.a.Swap(context.Background(), at, f.newest)
+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errSwapLeaseLost.Error()) || res.Executed {
+ t.Fatalf("Swap = %+v, want abandoned (the lost lease), nothing executed", res)
+ }
+ time.Sleep(100 * time.Millisecond)
+ if b.launched(t, id, mark) {
+ t.Fatal("the hold's build was launched after the lease was lost during the cgroup preparation")
+ }
+ b.prepGone(t, b.gens()[0])
+ })
+ t.Run("overlap successor, superseded between a failed start and its retry", func(t *testing.T) {
+ f := newSwapFixture(t)
+ b := newLaunchBackend(t, "flat", f.b)
+ b.eb.GenContainment = true
+ // The temp-dir subtree is no cgroupfs: the clone3-into-cgroup start
+ // fails, and the manager retries with the PID move.
+ gatedCgroups(t, b.eb, b.rt.Host, nil)
+ var asked atomic.Int32
+ b.eb.launchHook = func(string) {
+ if asked.Add(1) == 2 {
+ f.rev.Add(1)
+ }
+ }
+ f.a.backend = b
+ mark := filepath.Join(t.TempDir(), "v2.ran")
+ id := markerScript(t, t.TempDir(), "v2", mark)
+ at := f.attempt(ModeOverlap)
+ at.StagedPath, at.Digest = id.StagedPath, id.Digest
+ res := f.a.Swap(context.Background(), at, f.newest)
+ if n := asked.Load(); n != 2 {
+ t.Fatalf("the gate was asked %d times, want twice: the clone3 start and its retry", n)
+ }
+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errSwapSuperseded.Error()) || res.Executed {
+ t.Fatalf("Swap = %+v, want abandoned (superseded), nothing executed", res)
+ }
+ time.Sleep(100 * time.Millisecond)
+ if b.launched(t, id, mark) {
+ t.Fatal("the successor was launched by the retry after its attempt was superseded")
+ }
+ b.prepGone(t, b.gens()[0])
+ })
+}
diff --git a/internal/activator/genbackend.go b/internal/activator/genbackend.go
index 7eb496cd1..8c8b3a7b1 100644
--- a/internal/activator/genbackend.go
+++ b/internal/activator/genbackend.go
@@ -94,6 +94,23 @@ func runExecGate(ctx context.Context) error {
return gate()
}
+// launchGate is a flat staged spawn's before-start hook for the cgroup
+// manager (final review F1): launchHook (the test seam), then ctx's exec
+// gate, asked immediately before each start attempt, after the cgroup's
+// preparation. Nil when ctx carries no gate and no test seam is set, so a
+// live-link spawn hands the manager no hook and starts exactly as it did.
+func (b *ExecBackend) launchGate(ctx context.Context, host string) func() error {
+ if _, ok := ctx.Value(execGateKey{}).(func() error); !ok && b.launchHook == nil {
+ return nil
+ }
+ return func() error {
+ if b.launchHook != nil {
+ b.launchHook(host)
+ }
+ return runExecGate(ctx)
+ }
+}
+
// StartStaged is plan 1's StartGen with perGen true, except for the
// executable, the tier and the health wait, which is the caller's. A flat
// child execs g.StagedPath directly. A nested unit's executable mount names
diff --git a/internal/activator/genbackend_test.go b/internal/activator/genbackend_test.go
index bf20e32d9..a25323035 100644
--- a/internal/activator/genbackend_test.go
+++ b/internal/activator/genbackend_test.go
@@ -263,7 +263,7 @@ func TestExecBackendUsesEachArtifactsTier(t *testing.T) {
if got, err := os.ReadFile(filepath.Join(parent, "memory.max")); err != nil || string(got) != "1610612736" {
t.Fatalf("overlap parent memory.max = %q, %v; want 1 GiB + 512 MiB", got, err)
}
- cmd, settle, err := b.spawnInCgroupAtTier(rt, "g9", true, cgroup.TierChildren, func() *exec.Cmd { return exec.Command("/bin/true") })
+ cmd, settle, err := b.spawnInCgroupAtTier(rt, "g9", true, cgroup.TierChildren, func() *exec.Cmd { return exec.Command("/bin/true") }, nil)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go
index 5c2a74457..de40a60e8 100644
--- a/internal/cgroup/cgroup.go
+++ b/internal/cgroup/cgroup.go
@@ -293,39 +293,83 @@ func (m *Manager) hostDir(host string) string { return filepath.Join(m.root, Exe
// the child process itself — any grandchildren it has already forked by
// then stay charged to the agent's cgroup, not the child's.
func (m *Manager) Start(host string, limit Limit, build func() *exec.Cmd) (*exec.Cmd, error) {
+ return m.StartGated(host, limit, build, nil)
+}
+
+// StartGated is Start with a before-start hook. before, when set, is asked
+// immediately before EACH start attempt, the clone3 one and the PID-move
+// retry alike, so after the cgroup is made and its limits are written
+// (zero-downtime deploys final review F1: the caller's ownership can change
+// during that preparation). A refusal starts nothing, undoes that attempt's
+// preparation as a failed start does (the cgroup dir this spawn would have
+// used is removed if empty), and comes back as before's own error, never
+// joined with an earlier start's. A nil before is Start exactly.
+func (m *Manager) StartGated(host string, limit Limit, build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {
if m == nil {
- cmd := build()
- return cmd, startCmd(cmd)
+ return startPlain(build, before)
+ }
+ return m.startIn(host, m.hostDir(host), limit, build, before)
+}
+
+// startGated starts cmd once before, when set, has agreed. refused reports
+// that before said no: nothing was started, and err is before's own error.
+func startGated(cmd *exec.Cmd, before func() error) (refused bool, err error) {
+ if before != nil {
+ if err := before(); err != nil {
+ return true, err
+ }
+ }
+ return false, startCmd(cmd)
+}
+
+// startPlain is the unaccounted start (no manager, or no cgroup dir): one
+// start behind before. With before nil it is today's `cmd, startCmd(cmd)`.
+func startPlain(build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {
+ cmd := build()
+ refused, err := startGated(cmd, before)
+ if refused {
+ return nil, err
}
- return m.startIn(host, m.hostDir(host), limit, build)
+ return cmd, err
}
// startIn is Start against an explicit cgroup directory: the host's own
// (Start) or one of its children (StartChild). The clone3 attempt, the
-// fallback and its latch are Start's, unchanged.
-func (m *Manager) startIn(host, dir string, limit Limit, build func() *exec.Cmd) (*exec.Cmd, error) {
+// fallback and its latch are Start's, unchanged; before is StartGated's.
+func (m *Manager) startIn(host, dir string, limit Limit, build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {
if err := os.MkdirAll(dir, 0o755); err != nil {
m.logf("cgroup: %s: create cgroup: %v — spawning unaccounted", host, err)
- cmd := build()
- return cmd, startCmd(cmd)
+ return startPlain(build, before)
}
m.applyLimit(host, dir, limit)
if intoCgroupSupported && !m.fallback.Load() {
f, err := os.Open(dir)
if err != nil {
m.logf("cgroup: %s: open cgroup dir: %v — using PID-move fallback for this spawn", host, err)
- return m.startAndMoveIn(host, dir, build)
+ cmd, _, err := m.startAndMoveIn(host, dir, build, before)
+ return cmd, err
}
cmd := build()
setInto(cmd, int(f.Fd()))
- err = startCmd(cmd)
+ refused, err := startGated(cmd, before)
f.Close()
+ if refused {
+ // Nothing started, and no retry: the dir goes as it does after a
+ // spawn that never happened (startAndMoveIn).
+ _ = os.Remove(dir)
+ return nil, err
+ }
if err == nil {
m.biasOOM(host, cmd)
return cmd, nil
}
firstErr := err
- retried, retryErr := m.startAndMoveIn(host, dir, build)
+ retried, refused, retryErr := m.startAndMoveIn(host, dir, build, before)
+ if refused {
+ // The hook said no between the failed clone3 start and its
+ // retry: its own answer, not a spawn failure to join.
+ return nil, retryErr
+ }
if retryErr != nil {
// startAndMoveIn already dropped the ghost dir on its own
// failure; join both errors so the operator can see a clone3
@@ -340,7 +384,8 @@ func (m *Manager) startIn(host, dir string, limit Limit, build func() *exec.Cmd)
}
return retried, nil
}
- return m.startAndMoveIn(host, dir, build)
+ cmd, _, err := m.startAndMoveIn(host, dir, build, before)
+ return cmd, err
}
// applyLimit writes limit into dir. Best-effort in the package's usual
@@ -385,33 +430,36 @@ func (m *Manager) applyLimit(host, dir string, limit Limit) {
}
func (m *Manager) startAndMove(host string, build func() *exec.Cmd) (*exec.Cmd, error) {
- return m.startAndMoveIn(host, m.hostDir(host), build)
+ cmd, _, err := m.startAndMoveIn(host, m.hostDir(host), build, nil)
+ return cmd, err
}
-// startAndMoveIn is startAndMove into an explicit cgroup directory.
-func (m *Manager) startAndMoveIn(host, dir string, build func() *exec.Cmd) (*exec.Cmd, error) {
+// startAndMoveIn is startAndMove into an explicit cgroup directory, behind
+// StartGated's before. refused reports that before said no (err is its
+// own error).
+func (m *Manager) startAndMoveIn(host, dir string, build func() *exec.Cmd, before func() error) (*exec.Cmd, bool, error) {
cmd := build()
- if err := startCmd(cmd); err != nil {
+ if refused, err := startGated(cmd, before); err != nil {
// The spawn never happened: drop the now-ownerless dir so a failed
// wake doesn't leave the sampler reporting permanent zeros for a
// host with nothing running. rmdir-safe: os.Remove refuses a
// non-empty dir, so this can never take a live sibling's dir.
_ = os.Remove(dir)
- return nil, err
+ return nil, refused, err
}
if cmd.Process == nil {
// A broken startCmd seam reporting success with no process: this
// package must never panic the agent over it. Log and skip the
// move; the child, if any, simply stays unaccounted.
m.logf("cgroup: %s: start reported success with no process — skipping pid move", host)
- return cmd, nil
+ return cmd, false, nil
}
procs := filepath.Join(dir, "cgroup.procs")
if err := os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644); err != nil {
m.logf("cgroup: %s: move pid %d: %v — child stays unaccounted", host, cmd.Process.Pid, err)
}
m.biasOOM(host, cmd)
- return cmd, nil
+ return cmd, false, nil
}
// biasOOM writes ExecOOMScoreAdj for a just-started child. Best-effort: a
diff --git a/internal/cgroup/cgroup_linux_test.go b/internal/cgroup/cgroup_linux_test.go
index 5abaa2665..19cd894ec 100644
--- a/internal/cgroup/cgroup_linux_test.go
+++ b/internal/cgroup/cgroup_linux_test.go
@@ -183,3 +183,62 @@ func TestStartJoinsBothErrorsOnTotalFailure(t *testing.T) {
t.Errorf("ghost host dir survived total spawn failure: %v", statErr)
}
}
+
+// Zero-downtime final review wave 2 (F1 completion): the before-start hook
+// is asked again before the PID-move retry that follows a failed clone3
+// start. A refusal there starts nothing more, comes back as the hook's own
+// error (not joined with the clone3 failure), latches nothing, and drops
+// the host dir as a failed spawn does.
+func TestStartGatedAsksAgainBeforeTheRetry(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(cmd *exec.Cmd) error {
+ if cmd.SysProcAttr != nil && cmd.SysProcAttr.UseCgroupFD {
+ return syscall.EPERM
+ }
+ return nil
+ })
+ refused := errors.New("the attempt was superseded")
+ var asked int
+ cmd, err := m.StartGated("h.example.com", Limit{}, buildTrue, func() error {
+ if asked++; asked == 2 {
+ return refused
+ }
+ return nil
+ })
+ if err != refused || cmd != nil {
+ t.Fatalf("StartGated = %v, %v: want the hook's own error, unjoined, and no command", cmd, err)
+ }
+ if asked != 2 || len(*calls) != 1 {
+ t.Fatalf("hook asked %d times, %d starts: want 2 asks and only the clone3 start", asked, len(*calls))
+ }
+ if m.fallback.Load() {
+ t.Fatal("a refused retry latched the PID-move fallback")
+ }
+ if _, err := os.Stat(filepath.Join(root, ExecName("h.example.com"))); !os.IsNotExist(err) {
+ t.Fatalf("host dir after a refused start: %v, want it removed", err)
+ }
+}
+
+// A refusal before the clone3 start itself: nothing starts, no retry.
+func TestStartGatedRefusedBeforeTheFirstStart(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ refused := errors.New("lease lost")
+ if cmd, err := m.StartGated("h.example.com", Limit{}, buildTrue, func() error { return refused }); err != refused || cmd != nil {
+ t.Fatalf("StartGated = %v, %v: want the hook's own error and no command", cmd, err)
+ }
+ if len(*calls) != 0 {
+ t.Fatalf("started %d times after the hook refused, want none", len(*calls))
+ }
+ if _, err := os.Stat(filepath.Join(root, ExecName("h.example.com"))); !os.IsNotExist(err) {
+ t.Fatalf("host dir after a refused start: %v, want it removed", err)
+ }
+}
diff --git a/internal/cgroup/gen.go b/internal/cgroup/gen.go
index 997357278..5105c2611 100644
--- a/internal/cgroup/gen.go
+++ b/internal/cgroup/gen.go
@@ -150,15 +150,23 @@ func (m *Manager) ensureParent(host string, tier Limit) {
// limit alone. parent is the tier the parent takes when this spawn finds
// nothing alive under it. Nil-safe and best-effort like Start.
func (m *Manager) StartChild(host, child string, parent, own Limit, build func() *exec.Cmd) (*exec.Cmd, error) {
+ return m.StartChildGated(host, child, parent, own, build, nil)
+}
+
+// StartChildGated is StartChild with StartGated's before-start hook: asked
+// immediately before each start attempt, after the parent and the child
+// are prepared. A refusal starts nothing, leaves the child removed (if
+// empty) and the parent as a failed start leaves it, and comes back as
+// before's own error. A nil before is StartChild exactly.
+func (m *Manager) StartChildGated(host, child string, parent, own Limit, build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {
if !isChildName(child) {
return nil, fmt.Errorf("cgroup: %s: %q is not a child cgroup name", host, child)
}
if m == nil {
- cmd := build()
- return cmd, startCmd(cmd)
+ return startPlain(build, before)
}
m.ensureParent(host, parent)
- return m.startIn(host, filepath.Join(m.hostDir(host), child), own, build)
+ return m.startIn(host, filepath.Join(m.hostDir(host), child), own, build, before)
}
// SetParentLimit writes l onto host's parent: memory.max, memory.high
diff --git a/internal/cgroup/gen_test.go b/internal/cgroup/gen_test.go
index 367938eab..c04f8061c 100644
--- a/internal/cgroup/gen_test.go
+++ b/internal/cgroup/gen_test.go
@@ -1,6 +1,7 @@
package cgroup
import (
+ "errors"
"os"
"os/exec"
"path/filepath"
@@ -275,3 +276,54 @@ func TestStartKeepsTheSingleLevelLayout(t *testing.T) {
}
}
}
+
+// Zero-downtime final review wave 2 (F1 completion): StartChildGated's
+// before-start hook is asked after the whole preparation, the parent's tier
+// and the generation child's limits both written, and a refusal starts
+// nothing and comes back as the hook's own error. A hook that agrees
+// starts exactly once, as StartChild does.
+func TestStartChildGatedAsksAfterThePreparation(t *testing.T) {
+ const host = "h.example.com"
+ m, parent := genManager(t, host)
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ refused := errors.New("the attempt was superseded")
+ var prepared bool
+ cmd, err := m.StartChildGated(host, GenChild(1), TierDefault, TierDefault, buildTrue, func() error {
+ _, perr := os.Stat(filepath.Join(parent, "memory.max"))
+ _, cerr := os.Stat(filepath.Join(parent, "g1", "memory.max"))
+ prepared = perr == nil && cerr == nil
+ return refused
+ })
+ if err != refused || cmd != nil {
+ t.Fatalf("StartChildGated = %v, %v: want the hook's own error and no command", cmd, err)
+ }
+ if !prepared {
+ t.Fatal("the hook was asked before the parent's tier and the child's limits were written")
+ }
+ if len(*calls) != 0 {
+ t.Fatalf("started %d times after the hook refused, want none", len(*calls))
+ }
+ if _, err := m.StartChildGated(host, GenChild(2), TierDefault, TierDefault, buildTrue, func() error { return nil }); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 1 {
+ t.Fatalf("started %d times with an agreeing hook, want 1", len(*calls))
+ }
+}
+
+// With no manager (no delegation) the hook still guards the one plain
+// start.
+func TestStartGatedWithNoManagerAsksBeforeThePlainStart(t *testing.T) {
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ refused := errors.New("lease lost")
+ var m *Manager
+ if cmd, err := m.StartGated("h.example.com", TierDefault, buildTrue, func() error { return refused }); err != refused || cmd != nil {
+ t.Fatalf("StartGated = %v, %v: want the hook's own error and no command", cmd, err)
+ }
+ if cmd, err := m.StartChildGated("h.example.com", GenChild(1), TierDefault, TierDefault, buildTrue, func() error { return refused }); err != refused || cmd != nil {
+ t.Fatalf("StartChildGated = %v, %v: want the hook's own error and no command", cmd, err)
+ }
+ if len(*calls) != 0 {
+ t.Fatalf("started %d times after the hook refused, want none", len(*calls))
+ }
+}