amadan
activator: a flag-off stop on a kernel with no cgroup.kill keeps the leader-only proof
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6 files changed,
+67
−4
internal/activator/activator_test.go+20 −0internal/activator/backend_exec.go+4 −0internal/activator/genbackend.go+15 −4internal/cgroup/cleanup.go+3 −0internal/cgroup/cleanup_linux.go+5 −0internal/cgroup/cleanup_linux_test.go+20 −0
diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go| index 7a0add4d9..b5b96db95 100644 |
| --- a/internal/activator/activator_test.go |
| +++ b/internal/activator/activator_test.go |
| @@ -6558,6 +6558,26 @@ func TestEmptyHostDispatch(t *testing.T) { |
| } |
| } |
| +// A kernel before 5.14 has no cgroup.kill, so the host cgroup cannot be |
| +// killed there at all. EmptyHost answers that as no delegation (the |
| +// leader-only proof, ledger 127), not as an unproven kill that would hold |
| +// every flag-off stop's lease on such a box. |
| +func TestEmptyHostWithNoCgroupKillIsTheLeaderOnlyProof(t *testing.T) { |
| + eb := testExecBackend(t) |
| + opened := 0 |
| + eb.openHostCleanup = func(string) (*cgroup.Cleanup, error) { |
| + opened++ |
| + return nil, fmt.Errorf("%w: open cgroup.kill: no such file or directory", cgroup.ErrNoKill) |
| + } |
| + err := eb.EmptyHost(context.Background(), registry.Route{Host: "old-kernel.example.com"}) |
| + if opened != 1 { |
| + t.Fatalf("host cgroup opened %d times, want 1", opened) |
| + } |
| + if !errors.Is(err, cgroup.ErrNoDelegation) || !errors.Is(err, cgroup.ErrNoKill) { |
| + t.Fatalf("EmptyHost with no cgroup.kill = %v, want ErrNoDelegation naming ErrNoKill", err) |
| + } |
| +} |
| + |
| // The pending-stop path (a leader that was still running when stop() asked) |
| // is held to the same proof once the leader exits. |
| func TestAPendingFlagOffStopProvesTheHostCgroupBeforeTheLeaseGoes(t *testing.T) { |
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go| index bf5c3cb93..d4ea0c0a2 100644 |
| --- a/internal/activator/backend_exec.go |
| +++ b/internal/activator/backend_exec.go |
| @@ -261,6 +261,10 @@ type ExecBackend struct { |
| // cgroup manager's own. Tests replace this seam, never the Cgroups |
| // field. |
| cgroupKill func(ctx context.Context, rt registry.Route, child string) error |
| + // openHostCleanup is EmptyHost's open of the host cgroup. Nil: |
| + // Cgroups.OpenCleanup. Tests replace this seam, never the Cgroups |
| + // field. |
| + openHostCleanup func(host string) (*cgroup.Cleanup, error) |
| // strictPreflight is NextSpawnStrict's seam for PrepareStrict's runtime |
| // preflight. Nil: Cgroups.StrictPreflight, the very function |
| // PrepareStrict runs, so S2 and the spawn cannot disagree. |
diff --git a/internal/activator/genbackend.go b/internal/activator/genbackend.go| index 46f1f133c..a74d5ed3b 100644 |
| --- a/internal/activator/genbackend.go |
| +++ b/internal/activator/genbackend.go |
| @@ -532,15 +532,26 @@ func (b *SplitBackend) KillHost(ctx context.Context, rt registry.Route) error { |
| // to recreate. A populated cgroup goes through KillAndEmpty, which the |
| // next start answers by making it fresh (Task 19). An absent cgroup is |
| // empty; no delegated cgroup at all is cgroup.ErrNoDelegation, which the |
| -// caller treats as the leader-only proof. |
| +// caller treats as the leader-only proof. So is a cgroup with no |
| +// cgroup.kill (cgroup.ErrNoKill, a kernel before 5.14): nothing could kill |
| +// a descendant there, and reading it as an unproven kill would hold every |
| +// flag-off stop's lease on such a box. |
| func (b *ExecBackend) EmptyHost(ctx context.Context, rt registry.Route) error { |
| - if b.Cgroups == nil { |
| - return cgroup.ErrNoDelegation |
| + open := b.openHostCleanup |
| + if open == nil { |
| + if b.Cgroups == nil { |
| + return cgroup.ErrNoDelegation |
| + } |
| + open = b.Cgroups.OpenCleanup |
| } |
| - c, err := b.Cgroups.OpenCleanup(rt.Host) |
| + c, err := open(rt.Host) |
| if errors.Is(err, cgroup.ErrNoCgroup) { |
| return nil |
| } |
| + if errors.Is(err, cgroup.ErrNoKill) { |
| + log.Printf("activator: %s: this kernel has no cgroup.kill, so the host cgroup cannot be killed; its leader's exit is the only proof there is", rt.Host) |
| + return fmt.Errorf("%w: %w", cgroup.ErrNoDelegation, err) |
| + } |
| if err != nil { |
| return err |
| } |
diff --git a/internal/cgroup/cleanup.go b/internal/cgroup/cleanup.go| index c48c9295e..8cd26b385 100644 |
| --- a/internal/cgroup/cleanup.go |
| +++ b/internal/cgroup/cleanup.go |
| @@ -5,4 +5,7 @@ import "errors" |
| var ( |
| ErrNoCgroup = errors.New("cgroup: existing host cgroup absent") |
| ErrNoDelegation = errors.New("cgroup: no delegated manager") |
| + // ErrNoKill is a cleanup opened on a cgroup with no cgroup.kill, which |
| + // a kernel before 5.14 does not have. |
| + ErrNoKill = errors.New("cgroup: no cgroup.kill (a kernel before 5.14)") |
| ) |
diff --git a/internal/cgroup/cleanup_linux.go b/internal/cgroup/cleanup_linux.go| index 64bc29806..3d775e74d 100644 |
| --- a/internal/cgroup/cleanup_linux.go |
| +++ b/internal/cgroup/cleanup_linux.go |
| @@ -103,6 +103,11 @@ func (m *Manager) openCleanupAt(host, child string, checkFS func(*os.File) error |
| return nil, err |
| } |
| p.kill, err = strictOpen(p.dir, "cgroup.kill", unix.O_WRONLY) |
| + if errors.Is(err, unix.ENOENT) { |
| + // The cgroup is there (its events were opened above), so a missing |
| + // cgroup.kill is the kernel's, not a cgroup that went away. |
| + return nil, fmt.Errorf("%w: %w", ErrNoKill, err) |
| + } |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/cgroup/cleanup_linux_test.go b/internal/cgroup/cleanup_linux_test.go| index b3c00601d..d9dcdb4a9 100644 |
| --- a/internal/cgroup/cleanup_linux_test.go |
| +++ b/internal/cgroup/cleanup_linux_test.go |
| @@ -188,6 +188,26 @@ func TestCleanupRejectsMissingControlsAndSymlinks(t *testing.T) { |
| } |
| } |
| +// A kernel before 5.14 has no cgroup.kill. A cleanup opened there names |
| +// that (ErrNoKill), so a caller can fall back to the leader-only proof |
| +// instead of reading it as an unproven kill on every stop. A cgroup.kill |
| +// that is there but a symlink is not that case. |
| +func TestCleanupNamesAMissingKillFile(t *testing.T) { |
| + m, host, dir := cleanupFixture(t) |
| + if err := os.Remove(filepath.Join(dir, "cgroup.kill")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if c, err := m.openCleanup(host, func(*os.File) error { return nil }); c != nil || !errors.Is(err, ErrNoKill) { |
| + t.Fatalf("missing cgroup.kill: %v %v, want ErrNoKill", c, err) |
| + } |
| + if err := os.Symlink(filepath.Join(dir, "elsewhere"), filepath.Join(dir, "cgroup.kill")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if c, err := m.openCleanup(host, func(*os.File) error { return nil }); c != nil || err == nil || errors.Is(err, ErrNoKill) { |
| + t.Fatalf("symlinked cgroup.kill: %v %v, want a refusal that is not ErrNoKill", c, err) |
| + } |
| +} |
| + |
| func mkCgroupFixture(t *testing.T, dir, procs, events string) { |
| t.Helper() |
| if err := os.MkdirAll(dir, 0o700); err != nil { |