cgroup: the arm-time placement check fits a 4 s budget, the shim's waits included (#272)
VerifyPlacement bounds itself at placementCheckBudget (4 s, or the caller's sooner deadline) and passes that instant to the shim through startViaShimBy, which caps both of the shim's waits at it. startViaShim is unchanged for tenant starts: each wait stays shimDeadline (Ruling 9). A box whose shim stalls now declines cgroup-placement-failed in about 4 s, well inside the 20 s `carlos system update` gate, where before it could take about 20 s. awaitProbe judges a probe by its reaped status, never by which select case was ready first: a probe that exited before ctx ended is judged by its own exit, and only one the SIGKILL ended fails on ctx's error. placementCheck in internal/host drops its 15 s ctx; the budget lives in VerifyPlacement, where the kernel test proves it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 files changed,
+236
−46
internal/cgroup/shim_linux.go+39 −15internal/cgroup/shim_linux_test.go+39 −9internal/cgroup/verify_linux.go+50 −17internal/cgroup/verify_linux_test.go+103 −0internal/host/rollout.go+5 −5
diff --git a/internal/cgroup/shim_linux.go b/internal/cgroup/shim_linux.go| index 1dd8cad71..a38c1b0c0 100644 |
| --- a/internal/cgroup/shim_linux.go |
| +++ b/internal/cgroup/shim_linux.go |
| @@ -45,6 +45,15 @@ func shimAvailable() bool { return shimHooked.Load() || shimForcedForTest.Load() |
| // the status stream proves no R; a start that may have exec'd is reported |
| // started, so its caller owns the process and nothing runs a tenant twice. |
| func (m *Manager) startViaShim(dir string, cmd *exec.Cmd, before func() error) (refused bool, err error) { |
| + return m.startViaShimBy(dir, cmd, before, time.Time{}) |
| +} |
| + |
| +// startViaShimBy is startViaShim with neither of its two waits running past |
| +// by. The zero time caps nothing: each wait is shimDeadline, as for every |
| +// tenant start (Ruling 9). Only VerifyPlacement sets by, so the arm-time |
| +// check fits its budget whatever the shim does; past by, a start fails and |
| +// is reaped exactly as one past shimDeadline is. |
| +func (m *Manager) startViaShimBy(dir string, cmd *exec.Cmd, before func() error, by time.Time) (refused bool, err error) { |
| // The hook itself, not shimAvailable: FakePlacementForTest's override |
| // must never send a binary without the hook to start itself as a shim. |
| if !shimHooked.Load() { |
| @@ -74,7 +83,19 @@ func (m *Manager) startViaShim(dir string, cmd *exec.Cmd, before func() error) ( |
| dirFile.Close() |
| return false, &placementError{fmt.Errorf("%s is not a cgroup below /sys/fs/cgroup", dir)} |
| } |
| - return m.startShimCore(dir, dirFile, "/"+rel, cmd, before) |
| + return m.startShimCore(dir, dirFile, "/"+rel, cmd, before, by) |
| +} |
| + |
| +// shimWait is one of the parent's waits on the shim: shimDeadline from now, |
| +// or by when that is sooner (and set). within is the wait it allows, for the |
| +// error that names it. |
| +func shimWait(by time.Time) (deadline time.Time, within time.Duration) { |
| + now := time.Now() |
| + deadline = now.Add(shimDeadline) |
| + if by.IsZero() || !by.Before(deadline) { |
| + return deadline, shimDeadline |
| + } |
| + return by, max(by.Sub(now), 0).Round(time.Millisecond) |
| } |
| // envHasNUL reports whether Start would refuse cmd's environment for a NUL. |
| @@ -94,7 +115,8 @@ func envHasNUL(cmd *exec.Cmd) bool { |
| // startShimCore is startViaShim after its checks: dirFile is dir opened, |
| // and expected is dir as /proc/self/cgroup will show it. It closes dirFile. |
| -func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, cmd *exec.Cmd, before func() error) (refused bool, err error) { |
| +// by caps both waits, as startViaShimBy describes. |
| +func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, cmd *exec.Cmd, before func() error, by time.Time) (refused bool, err error) { |
| // The tenant, captured before cmd is rewritten into the shim's start: |
| // exactly what exec.Cmd.Start would have passed. |
| path := cmd.Path |
| @@ -160,7 +182,7 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| cmd.ExtraFiles = append(slices.Clip(cmd.ExtraFiles), specR, dirFile, statusW) |
| // One deadline covers the spec and the wait for placement. |
| - deadline := time.Now().Add(shimDeadline) |
| + deadline, within := shimWait(by) |
| done := m.beginPlacement(dir) |
| startErr := startCmd(cmd) |
| specR.Close() |
| @@ -181,7 +203,7 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| reap(cmd) |
| done() |
| if errors.Is(err, os.ErrDeadlineExceeded) { |
| - return false, &placementError{fmt.Errorf("no answer from the shim within %v", shimDeadline)} |
| + return false, &placementError{fmt.Errorf("no answer from the shim within %v", within)} |
| } |
| return false, &placementError{fmt.Errorf("the shim exited before placement (spec: %v)", err)} |
| } |
| @@ -189,7 +211,7 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| _ = statusR.SetReadDeadline(deadline) |
| br := bufio.NewReader(statusR) |
| line, rerr := br.ReadBytes('\n') |
| - placed, perr := classifyPlacement(line, rerr != nil && !errors.Is(rerr, io.EOF)) |
| + placed, perr := classifyPlacement(line, rerr != nil && !errors.Is(rerr, io.EOF), within) |
| if !placed { |
| // P, EOF or the deadline: no release byte was sent, so the shim |
| // cannot have exec'd. reap's SIGKILL ends one still running. |
| @@ -218,7 +240,8 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| _ = specW.SetWriteDeadline(time.Time{}) |
| _, _ = specW.Write([]byte{'X'}) |
| closeSpecW() |
| - _ = statusR.SetReadDeadline(time.Now().Add(shimDeadline)) |
| + deadline, within = shimWait(by) |
| + _ = statusR.SetReadDeadline(deadline) |
| if shimAfterRelease != nil { |
| shimAfterRelease() |
| } |
| @@ -229,7 +252,7 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| // loading: started, and nothing is killed. An E line is the shim's |
| // answer. Otherwise kill the shim, which was not released into an |
| // exec it has announced, and read what it wrote before it died. |
| - started, err := classifyExec(path, wd, rest, false) |
| + started, err := classifyExec(path, wd, rest, false, within) |
| if started { |
| return false, nil |
| } |
| @@ -243,7 +266,7 @@ func (m *Manager) startShimCore(dir string, dirFile *os.File, expected string, c |
| rest = append(rest, more...) |
| killed = true |
| } |
| - started, err := classifyExec(path, wd, rest, killed) |
| + started, err := classifyExec(path, wd, rest, killed, within) |
| if started { |
| return false, nil |
| } |
| @@ -262,8 +285,9 @@ func reap(cmd *exec.Cmd) { |
| } |
| // classifyPlacement reads the stream up to placement (parent step 7). |
| -// placed is true only for "A". timedOut is the read deadline having passed. |
| -func classifyPlacement(b []byte, timedOut bool) (placed bool, err error) { |
| +// placed is true only for "A". timedOut is the read deadline, which allowed |
| +// within, having passed. |
| +func classifyPlacement(b []byte, timedOut bool, within time.Duration) (placed bool, err error) { |
| if i := bytes.IndexByte(b, '\n'); i >= 0 { |
| line := string(b[:i]) |
| switch { |
| @@ -276,19 +300,19 @@ func classifyPlacement(b []byte, timedOut bool) (placed bool, err error) { |
| } |
| } |
| if timedOut { |
| - return false, &placementError{fmt.Errorf("no answer from the shim within %v", shimDeadline)} |
| + return false, &placementError{fmt.Errorf("no answer from the shim within %v", within)} |
| } |
| return false, &placementError{errors.New("the shim exited before placement")} |
| } |
| // classifyExec reads the stream after the release byte (parent step 9). |
| -// killed means the deadline passed with no R, the shim was SIGKILLed, and |
| -// b is everything read until EOF after that. |
| +// killed means the deadline (which allowed within) passed with no R, the |
| +// shim was SIGKILLed, and b is everything read until EOF after that. |
| // |
| // R, and no "E exec" after it, is started: the tenant may have run. An |
| // "E chdir" before R, or an "E exec" after it, is the command's own error, |
| // as exec.Cmd.Start reports it. No R is a placement error. |
| -func classifyExec(path, wd string, b []byte, killed bool) (started bool, err error) { |
| +func classifyExec(path, wd string, b []byte, killed bool, within time.Duration) (started bool, err error) { |
| sawR := false |
| for _, line := range strings.Split(string(b), "\n") { |
| if line == "R" { |
| @@ -308,7 +332,7 @@ func classifyExec(path, wd string, b []byte, killed bool) (started bool, err err |
| return true, nil |
| } |
| if killed { |
| - return false, &placementError{fmt.Errorf("the shim did not exec within %v", shimDeadline)} |
| + return false, &placementError{fmt.Errorf("the shim did not exec within %v", within)} |
| } |
| return false, &placementError{errors.New("the shim exited before exec")} |
| } |
diff --git a/internal/cgroup/shim_linux_test.go b/internal/cgroup/shim_linux_test.go| index 32f3b5613..306479c8b 100644 |
| --- a/internal/cgroup/shim_linux_test.go |
| +++ b/internal/cgroup/shim_linux_test.go |
| @@ -11,6 +11,7 @@ import ( |
| "strings" |
| "syscall" |
| "testing" |
| + "time" |
| ) |
| // These run without a delegated cgroup: a temp directory is not cgroupfs, |
| @@ -95,7 +96,7 @@ func TestShimPlacementFailureIsReported(t *testing.T) { |
| defer f.Close() |
| marker := filepath.Join(t.TempDir(), "ran") |
| cmd := exec.Command("/bin/sh", "-c", "touch "+marker) |
| - _, err = m.startShimCore(m.root, f, "/nowhere", cmd, nil) |
| + _, err = m.startShimCore(m.root, f, "/nowhere", cmd, nil, time.Time{}) |
| if !isPlacementError(err) { |
| t.Fatalf("err=%v, want a placement error", err) |
| } |
| @@ -135,7 +136,7 @@ func TestShimBeforeRefusalRegistersNothing(t *testing.T) { |
| } |
| defer f.Close() |
| cmd = exec.Command("/bin/sh", "-c", "touch "+marker) |
| - refused, err = m.startShimCore(m.root, f, "/nowhere", cmd, before) |
| + refused, err = m.startShimCore(m.root, f, "/nowhere", cmd, before, time.Time{}) |
| if refused || !isPlacementError(err) { |
| t.Fatalf("core: refused=%v err=%v, want a placement error", refused, err) |
| } |
| @@ -187,7 +188,7 @@ func TestShimRefusesNULInEnv(t *testing.T) { |
| } |
| defer f.Close() |
| cmd := newCmd() |
| - refused, err := m.startShimCore(m.root, f, "/nowhere", cmd, nil) |
| + refused, err := m.startShimCore(m.root, f, "/nowhere", cmd, nil, time.Time{}) |
| if refused || err == nil || err.Error() != want || isPlacementError(err) { |
| t.Fatalf("refused=%v err=%v, want the command's error %q", refused, err, want) |
| } |
| @@ -220,7 +221,7 @@ func TestShimStatusClassification(t *testing.T) { |
| {"garbage\n", false, false}, |
| {"A", false, false}, // no newline: not the shim's answer |
| } { |
| - placed, err := classifyPlacement([]byte(c.in), c.timedOut) |
| + placed, err := classifyPlacement([]byte(c.in), c.timedOut, shimDeadline) |
| if placed != c.placed { |
| t.Fatalf("classifyPlacement(%q, %v) placed=%v, want %v", c.in, c.timedOut, placed, c.placed) |
| } |
| @@ -231,13 +232,13 @@ func TestShimStatusClassification(t *testing.T) { |
| t.Fatalf("classifyPlacement(%q, %v) err=%v", c.in, c.timedOut, err) |
| } |
| } |
| - if _, err := classifyPlacement([]byte("P procs: permission denied\n"), false); !strings.Contains(err.Error(), "procs: permission denied") { |
| + if _, err := classifyPlacement([]byte("P procs: permission denied\n"), false, shimDeadline); !strings.Contains(err.Error(), "procs: permission denied") { |
| t.Fatalf("the shim's reason is lost: %v", err) |
| } |
| - if _, err := classifyPlacement(nil, true); !strings.Contains(err.Error(), "no answer") { |
| + if _, err := classifyPlacement(nil, true, shimDeadline); !strings.Contains(err.Error(), "no answer") { |
| t.Fatalf("the deadline is not named: %v", err) |
| } |
| - if _, err := classifyPlacement(nil, false); !strings.Contains(err.Error(), "exited before placement") { |
| + if _, err := classifyPlacement(nil, false, shimDeadline); !strings.Contains(err.Error(), "exited before placement") { |
| t.Fatalf("the early exit is not named: %v", err) |
| } |
| @@ -259,7 +260,7 @@ func TestShimStatusClassification(t *testing.T) { |
| {"R\ngarbage\n", false, true, "", 0}, // R and no E after it: started |
| {"R", true, true, "", 0}, // R without its newline still counts |
| } { |
| - started, err := classifyExec("/bin/app", "/wd", []byte(c.in), c.killed) |
| + started, err := classifyExec("/bin/app", "/wd", []byte(c.in), c.killed, shimDeadline) |
| if started != c.started { |
| t.Fatalf("classifyExec(%q, %v) started=%v, want %v", c.in, c.killed, started, c.started) |
| } |
| @@ -287,7 +288,36 @@ func TestShimStatusClassification(t *testing.T) { |
| t.Fatalf("classifyExec(%q, %v) = %#v, want Op %q Path %q Err %v", c.in, c.killed, pe, c.op, wantPath, c.errno) |
| } |
| } |
| - if _, err := classifyExec("/bin/app", "/wd", nil, false); !strings.Contains(err.Error(), "exited before exec") { |
| + if _, err := classifyExec("/bin/app", "/wd", nil, false, shimDeadline); !strings.Contains(err.Error(), "exited before exec") { |
| t.Fatalf("the early exit is not named: %v", err) |
| } |
| } |
| + |
| +// TestShimWaitCapsAtBy: with by unset or later than shimDeadline from now, |
| +// each wait is shimDeadline, as for every tenant start (Ruling 9); with by |
| +// sooner, the wait ends at by and the error names what was left of it. |
| +func TestShimWaitCapsAtBy(t *testing.T) { |
| + before := time.Now() |
| + d, within := shimWait(time.Time{}) |
| + if within != shimDeadline || d.Before(before.Add(shimDeadline)) || d.After(time.Now().Add(shimDeadline)) { |
| + t.Fatalf("no cap: deadline %v within %v, want shimDeadline from now", d.Sub(before), within) |
| + } |
| + late := time.Now().Add(shimDeadline + time.Hour) |
| + if d, within := shimWait(late); within != shimDeadline || !d.Before(late) { |
| + t.Fatalf("a later by: deadline %v within %v, want shimDeadline from now", d, within) |
| + } |
| + soon := time.Now().Add(time.Second) |
| + if d, within := shimWait(soon); !d.Equal(soon) || within <= 0 || within > time.Second { |
| + t.Fatalf("a sooner by: deadline %v within %v, want by and at most 1s", d, within) |
| + } |
| + past := time.Now().Add(-time.Second) |
| + if d, within := shimWait(past); !d.Equal(past) || within != 0 { |
| + t.Fatalf("a past by: deadline %v within %v, want by and 0", d, within) |
| + } |
| + if _, err := classifyPlacement(nil, true, 1500*time.Millisecond); !strings.Contains(err.Error(), "within 1.5s") { |
| + t.Fatalf("the capped wait is not named: %v", err) |
| + } |
| + if _, err := classifyExec("/bin/app", "/wd", nil, true, 1500*time.Millisecond); !strings.Contains(err.Error(), "within 1.5s") { |
| + t.Fatalf("the capped wait is not named: %v", err) |
| + } |
| +} |
diff --git a/internal/cgroup/verify_linux.go b/internal/cgroup/verify_linux.go| index 0b1de37b7..6f11d5d04 100644 |
| --- a/internal/cgroup/verify_linux.go |
| +++ b/internal/cgroup/verify_linux.go |
| @@ -12,6 +12,8 @@ import ( |
| "path/filepath" |
| "strconv" |
| "strings" |
| + "syscall" |
| + "time" |
| "golang.org/x/sys/unix" |
| ) |
| @@ -25,6 +27,14 @@ const placementCheckName = "carlos-placement-check" |
| // change it, to stand in a probe that misreports or never exits. |
| var placementProbeArgs = []string{ProbeVerb} |
| +// placementCheckBudget bounds the whole of VerifyPlacement, the shim's two |
| +// waits and the probe's run included, so a decline is decided in time for |
| +// the restarted edge to answer `carlos system update`'s 20 s gate |
| +// (systemGateWait) with the rest of its boot work. What runs after the |
| +// budget is the reap of a SIGKILLed shim or probe and an rmdir: milliseconds, |
| +// which the second of headroom under 5 s covers. |
| +const placementCheckBudget = 4 * time.Second |
| + |
| // VerifyPlacement does one real placement, through the shim, into a scratch |
| // child cgroup with the default tier's limits, and answers nil only if the |
| // placed process saw itself in that cgroup and the limit reads back. It is |
| @@ -32,9 +42,11 @@ var placementProbeArgs = []string{ProbeVerb} |
| // check found a box that armed and then failed every generation start, |
| // because no placement was possible under carlos-edge.service's hardening. |
| // |
| -// It calls startViaShim itself, never the placeStart seam, so a test's |
| -// FakePlacementForTest cannot make it pass. The scratch cgroup is removed |
| -// once the probe is reaped; a removal failure is logged, not returned. |
| +// It calls the shim itself (startViaShimBy), never the placeStart seam, so a |
| +// test's FakePlacementForTest cannot make it pass. It ends at ctx's deadline |
| +// or after placementCheckBudget, whichever is sooner; the shim's waits are |
| +// capped at the same instant. The scratch cgroup is removed once the probe |
| +// is reaped; a removal failure is logged, not returned. |
| func (m *Manager) VerifyPlacement(ctx context.Context) error { |
| if m == nil { |
| return ErrNoDelegation |
| @@ -42,6 +54,9 @@ func (m *Manager) VerifyPlacement(ctx context.Context) error { |
| if !shimAvailable() { |
| return errors.New("cgroup: placement check: the placement shim is not available in this process") |
| } |
| + ctx, cancel := context.WithTimeout(ctx, placementCheckBudget) |
| + defer cancel() |
| + by, _ := ctx.Deadline() |
| dir := filepath.Join(m.root, placementCheckName) |
| // A leftover from an earlier run is empty: the unit's restart killed |
| // its members. One that will not go fails the Mkdir below. |
| @@ -73,23 +88,13 @@ func (m *Manager) VerifyPlacement(ctx context.Context) error { |
| cmd := exec.Command(shimExe, placementProbeArgs...) |
| cmd.Env = []string{} |
| cmd.Stdout = &out |
| - if _, err := m.startViaShim(dir, cmd, nil); err != nil { |
| - // A placement error or not, the check failed; startViaShim has |
| + if _, err := m.startViaShimBy(dir, cmd, nil, by); err != nil { |
| + // A placement error or not, the check failed; the shim call has |
| // reaped anything it started. |
| return fmt.Errorf("cgroup: placement check: %w", err) |
| } |
| - waited := make(chan error, 1) |
| - go func() { waited <- cmd.Wait() }() |
| - var werr error |
| - select { |
| - case werr = <-waited: |
| - case <-ctx.Done(): |
| - _ = cmd.Process.Kill() |
| - <-waited // bounded: the probe is dead and holds no other process |
| - return fmt.Errorf("cgroup: placement check: the probe did not exit: %w", ctx.Err()) |
| - } |
| - if werr != nil { |
| - return fmt.Errorf("cgroup: placement check: the probe failed: %w", werr) |
| + if err := awaitProbe(ctx, cmd); err != nil { |
| + return fmt.Errorf("cgroup: placement check: %w", err) |
| } |
| // What the probe saw in /proc/self/cgroup: dir as startViaShim names it |
| // to the shim, below /sys/fs/cgroup. |
| @@ -106,3 +111,31 @@ func (m *Manager) VerifyPlacement(ctx context.Context) error { |
| } |
| return nil |
| } |
| + |
| +// awaitProbe reaps the started probe: nil when it exited 0, its exit as the |
| +// error otherwise. When ctx ends first the probe is SIGKILLed and reaped |
| +// before this returns, and the check fails on ctx's error. The judgement is |
| +// the reaped status, never which channel was ready first: a probe that had |
| +// already exited when ctx ended is a zombie the kill cannot touch, so it is |
| +// judged by its own exit, not declined as one that hung. |
| +func awaitProbe(ctx context.Context, cmd *exec.Cmd) error { |
| + waited := make(chan error, 1) |
| + go func() { waited <- cmd.Wait() }() |
| + var werr error |
| + select { |
| + case werr = <-waited: |
| + case <-ctx.Done(): |
| + _ = cmd.Process.Kill() |
| + werr = <-waited // bounded: the probe is dead and holds no other process |
| + var ee *exec.ExitError |
| + if errors.As(werr, &ee) { |
| + if ws, ok := ee.Sys().(syscall.WaitStatus); ok && ws.Signaled() && ws.Signal() == syscall.SIGKILL { |
| + return fmt.Errorf("the probe did not exit: %w", ctx.Err()) |
| + } |
| + } |
| + } |
| + if werr != nil { |
| + return fmt.Errorf("the probe failed: %w", werr) |
| + } |
| + return nil |
| +} |
diff --git a/internal/cgroup/verify_linux_test.go b/internal/cgroup/verify_linux_test.go| index dfdbedd98..75ddd53cf 100644 |
| --- a/internal/cgroup/verify_linux_test.go |
| +++ b/internal/cgroup/verify_linux_test.go |
| @@ -5,7 +5,9 @@ package cgroup |
| import ( |
| "context" |
| "errors" |
| + "fmt" |
| "os" |
| + "os/exec" |
| "path/filepath" |
| "strings" |
| "testing" |
| @@ -81,6 +83,61 @@ func TestVerifyPlacementKernelChild(t *testing.T) { |
| time.Sleep(time.Minute) |
| } |
| +// awaitProbe judges a probe that has already exited by its own exit, never |
| +// by which of its two channels the select saw first: with ctx already done |
| +// and the probe a zombie, an exit 0 passes and an exit 1 fails as the |
| +// probe's failure, every time. One still running is killed, reaped, and |
| +// fails on ctx's error. Ungated: plain processes, no cgroup. |
| +func TestAwaitProbeJudgesAnExitedProbeByItsExit(t *testing.T) { |
| + done, cancel := context.WithCancel(context.Background()) |
| + cancel() |
| + for i := 0; i < 50; i++ { |
| + ok := exitedProbe(t, "/bin/true") |
| + if err := awaitProbe(done, ok); err != nil { |
| + t.Fatalf("round %d: a probe that exited 0 before ctx ended: %v, want nil", i, err) |
| + } |
| + bad := exitedProbe(t, "/bin/false") |
| + err := awaitProbe(done, bad) |
| + if err == nil || errors.Is(err, context.Canceled) || !strings.Contains(err.Error(), "the probe failed: exit status 1") { |
| + t.Fatalf("round %d: a probe that exited 1 before ctx ended: %v, want its own exit", i, err) |
| + } |
| + } |
| + hung := exec.Command("/bin/sleep", "60") |
| + if err := hung.Start(); err != nil { |
| + t.Fatal(err) |
| + } |
| + err := awaitProbe(done, hung) |
| + if !errors.Is(err, context.Canceled) || !strings.Contains(err.Error(), "did not exit") { |
| + t.Fatalf("a running probe with ctx done: %v, want ctx's error", err) |
| + } |
| + if hung.ProcessState == nil { |
| + t.Fatal("the killed probe was not reaped") |
| + } |
| +} |
| + |
| +// exitedProbe starts path and returns once the process has exited and is |
| +// a zombie, not yet reaped. |
| +func exitedProbe(t *testing.T, path string) *exec.Cmd { |
| + t.Helper() |
| + cmd := exec.Command(path) |
| + if err := cmd.Start(); err != nil { |
| + t.Fatal(err) |
| + } |
| + stat := fmt.Sprintf("/proc/%d/stat", cmd.Process.Pid) |
| + for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(time.Millisecond) { |
| + raw, err := os.ReadFile(stat) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // The state follows the parenthesised command name. |
| + if i := strings.LastIndexByte(string(raw), ')'); i >= 0 && strings.HasPrefix(string(raw[i:]), ") Z") { |
| + return cmd |
| + } |
| + } |
| + t.Fatalf("%s did not exit", path) |
| + return nil |
| +} |
| + |
| func TestVerifyPlacementKernel(t *testing.T) { |
| m := shimKernelManager(t) |
| if err := m.VerifyPlacement(context.Background()); err != nil { |
| @@ -164,3 +221,49 @@ func TestVerifyPlacementKernelKillsAProbeThatHangs(t *testing.T) { |
| t.Logf("hung probe: %v", err) |
| assertNoPlacementCheckDir(t, m.root) |
| } |
| + |
| +// A shim that stalls, before placement or after the release, cannot hold |
| +// the arm-time check past its budget: the decline, the reap and the removal |
| +// of the scratch cgroup all land within placementCheckBudget plus a few |
| +// milliseconds, under 5 s, with the caller's ctx carrying no deadline at |
| +// all, as placementCheck's does not. A sooner ctx deadline caps the shim's |
| +// waits the same way. |
| +func TestVerifyPlacementKernelStalledShimDeclinesWithinBudget(t *testing.T) { |
| + const hard = 5 * time.Second |
| + for _, c := range []struct { |
| + name string |
| + hold int |
| + ctx time.Duration // 0: context.Background() |
| + want time.Duration // the wait that must end it |
| + }{ |
| + {"before placement", 1, 0, placementCheckBudget}, |
| + {"after release", 2, 0, placementCheckBudget}, |
| + {"before placement, sooner ctx", 1, 300 * time.Millisecond, 300 * time.Millisecond}, |
| + } { |
| + t.Run(c.name, func(t *testing.T) { |
| + m := shimKernelManager(t) |
| + setShimVar(t, &shimTestHold, c.hold) |
| + ctx := context.Background() |
| + if c.ctx > 0 { |
| + var cancel context.CancelFunc |
| + ctx, cancel = context.WithTimeout(ctx, c.ctx) |
| + defer cancel() |
| + } |
| + start := time.Now() |
| + err := m.VerifyPlacement(ctx) |
| + took := time.Since(start) |
| + t.Logf("hold %d: took %v: %v", c.hold, took, err) |
| + if err == nil || !isPlacementError(err) { |
| + t.Fatalf("a stalled shim: %v, want a placement error", err) |
| + } |
| + if took < c.want-50*time.Millisecond { |
| + t.Fatalf("declined after %v, before the %v wait ended: not the stall the test is about", took, c.want) |
| + } |
| + if took > c.want+500*time.Millisecond || took > hard { |
| + t.Fatalf("declined after %v, want within %v plus 500ms and under %v", took, c.want, hard) |
| + } |
| + assertNoPlacementCheckDir(t, m.root) |
| + assertNothingInFlight(t, m) |
| + }) |
| + } |
| +} |
diff --git a/internal/host/rollout.go b/internal/host/rollout.go| index d09feb71a..6173a2b69 100644 |
| --- a/internal/host/rollout.go |
| +++ b/internal/host/rollout.go |
| @@ -147,12 +147,12 @@ func swapDelegation(eb *activator.ExecBackend) string { |
| // placementCheck is swapArming's arm-time placement check: one real shim |
| // placement into a scratch child of the delegated subtree |
| -// (cgroup.Manager.VerifyPlacement), bounded at 15 s. A seam host tests |
| -// replace, since a temp-dir root can never pass it. |
| +// (cgroup.Manager.VerifyPlacement). VerifyPlacement bounds itself, the |
| +// shim's waits included, at 4 s, so a decline lands well inside the 20 s |
| +// `carlos system update` gives the restarted edge (systemGateWait). A seam |
| +// host tests replace, since a temp-dir root can never pass it. |
| var placementCheck = func(m *cgroup.Manager) error { |
| - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) |
| - defer cancel() |
| - return m.VerifyPlacement(ctx) |
| + return m.VerifyPlacement(context.Background()) |
| } |
| // swapArming decides, before the activator exists, whether CARLOS_SWAP=overlap |