amadan

carlos / platform Public

zdd plan 2 task 3: strict containment per generation child; parent limit through one API

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude a0b4df33e52a9e0702d49d8072cbc12c987062aa parent e94519f
4 files changed, +463 −4
  • internal/cgroup/strict_gen_linux.go +215 −0
  • internal/cgroup/strict_gen_linux_test.go +213 −0
  • internal/cgroup/strict_linux.go +29 −4
  • internal/cgroup/strict_other.go +6 −0
diff --git a/internal/cgroup/strict_gen_linux.go b/internal/cgroup/strict_gen_linux.go
new file mode 100644
index 000000000..56a392eed
--- /dev/null
+++ b/internal/cgroup/strict_gen_linux.go
@@ -0,0 +1,215 @@
+//go:build linux
+
+package cgroup
+
+import (
+ "errors"
+ "fmt"
+ "os"
+ "os/exec"
+ "slices"
+ "strconv"
+ "strings"
+
+ "golang.org/x/sys/unix"
+)
+
+// tierExpected is the four limit files for l, as the kernel reads them
+// back: memory.high rounded down to the page.
+func tierExpected(l Limit, oomGroup string) map[string]string {
+ page := int64(os.Getpagesize())
+ return map[string]string{
+ "memory.max": strconv.FormatInt(l.MemoryMax, 10),
+ "memory.high": strconv.FormatInt(HighOf(l.MemoryMax)/page*page, 10),
+ "memory.swap.max": strconv.FormatInt(l.MemoryMax, 10),
+ "memory.oom.group": oomGroup,
+ }
+}
+
+// delegateStrict delegates a generation parent's controllers to its
+// children and proves memory is among them: a generation child without it
+// could carry no limit.
+func delegateStrict(dir *os.File) error {
+ ctl, e := strictOpen(dir, "cgroup.controllers", unix.O_RDONLY)
+ if e != nil {
+ return ErrStrictUnsupported
+ }
+ have, e := strictRead(ctl)
+ ctl.Close()
+ if e != nil {
+ return ErrStrictRefused
+ }
+ want := delegatedControllers(have)
+ if !strings.Contains(want, "+memory") {
+ return ErrStrictUnsupported
+ }
+ sub, e := strictOpen(dir, "cgroup.subtree_control", unix.O_RDWR)
+ if e != nil {
+ return ErrStrictUnsupported
+ }
+ defer sub.Close()
+ if e = strictWrite(sub, want); e != nil {
+ return ErrStrictRefused
+ }
+ got, e := strictRead(sub)
+ if e != nil || !slices.Contains(strings.Fields(got), "memory") {
+ return ErrStrictRefused
+ }
+ return nil
+}
+
+// PrepareStrictParent pins host's cgroup as a generation parent for the life
+// of one activator entry (zero-downtime deploys spec §7.3): empty, the tier
+// written with memory.oom.group=0, its controllers delegated to its
+// children. It holds no process: Start refuses it. Each spawn pins and
+// reserves its own child with PrepareGen. The swap moves the parent's limit
+// with SetParentLimit, which keeps the expected-limit record in step.
+func (m *Manager) PrepareStrictParent(host string, tier Limit) (*Prepared, error) {
+ return m.prepareStrict(host, tier, true)
+}
+
+// PrepareGen pins generation n's child under a generation parent: newly
+// empty, reserved for this generation alone, with the tier's full limits
+// and memory.oom.group=1 (spec §7.3). tier is this generation's own and may
+// differ from the incumbent's when a release changes tier.
+func (p *Prepared) PrepareGen(n uint64, tier Limit) (out *Prepared, err error) {
+ if p == nil || p.strictPreparedState == nil || n == 0 || (tier != TierDefault && tier != TierChildren) {
+ return nil, ErrStrictRefused
+ }
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if !p.genParent || p.closed || p.empty || p.draining.Load() {
+ return nil, ErrStrictState
+ }
+ if err = p.identityOK(); err != nil {
+ return nil, err
+ }
+ name := GenChild(n)
+ if err = unix.Mkdirat(int(p.dir.Fd()), name, 0755); err != nil && !errors.Is(err, unix.EEXIST) {
+ return nil, ErrStrictRefused
+ }
+ child := &Prepared{strictPreparedState: &strictPreparedState{parent: p, host: p.host, tier: tier, limits: map[string]*os.File{}, expected: map[string]string{}, entryName: name, start: func(cmd *exec.Cmd) error { return cmd.Start() }}}
+ reserved := false
+ defer func() {
+ if err != nil {
+ child.closeFiles()
+ if reserved {
+ strictReservations.Lock()
+ delete(strictReservations.held, child.identity)
+ strictReservations.Unlock()
+ }
+ }
+ }()
+ fd, e := unix.FcntlInt(p.dir.Fd(), unix.F_DUPFD_CLOEXEC, 0)
+ if e != nil {
+ return nil, ErrStrictUnsupported
+ }
+ child.entryParent = os.NewFile(uintptr(fd), name+"-parent")
+ if child.dir, e = strictOpen(p.dir, name, unix.O_RDONLY|unix.O_DIRECTORY); e != nil {
+ return nil, ErrStrictRefused
+ }
+ if child.identity, e = strictID(child.dir); e != nil {
+ return nil, ErrStrictRefused
+ }
+ strictReservations.Lock()
+ if strictReservations.held[child.identity] {
+ strictReservations.Unlock()
+ return nil, ErrStrictState
+ }
+ strictReservations.held[child.identity] = true
+ reserved = true
+ strictReservations.Unlock()
+ if child.events, e = strictOpen(child.dir, "cgroup.events", unix.O_RDONLY); e != nil {
+ return nil, ErrStrictUnsupported
+ }
+ if child.kill, e = strictOpen(child.dir, "cgroup.kill", unix.O_WRONLY); e != nil {
+ return nil, ErrStrictUnsupported
+ }
+ if populated, e := strictPopulation(child.events); e != nil || populated {
+ return nil, ErrStrictRefused
+ }
+ want := tierExpected(tier, "1")
+ for name := range want {
+ if child.limits[name], e = strictOpen(child.dir, name, unix.O_RDWR); e != nil {
+ return nil, ErrStrictUnsupported
+ }
+ }
+ for _, name := range []string{"memory.max", "memory.high", "memory.swap.max", "memory.oom.group"} {
+ if e = strictWrite(child.limits[name], want[name]); e != nil {
+ return nil, ErrStrictRefused
+ }
+ child.expected[name] = want[name]
+ }
+ if e = child.limitsOK(); e != nil {
+ return nil, e
+ }
+ if e = child.identityOK(); e != nil {
+ return nil, e
+ }
+ return child, nil
+}
+
+// SetParentLimit writes l onto a generation parent and records it as the
+// expected value in the same step (spec §7.3). A child's Start checks the
+// parent with limitsOK and must never see a limit this API did not set.
+// Each file's record moves only after its own write lands, so a partial
+// failure leaves the record matching the kernel. A limit someone else set
+// is refused, not overwritten.
+func (p *Prepared) SetParentLimit(l Limit) error {
+ if p == nil || p.strictPreparedState == nil || !ValidParentLimit(l) {
+ return ErrStrictRefused
+ }
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if !p.genParent || p.closed || p.empty || p.draining.Load() {
+ return ErrStrictState
+ }
+ if e := p.identityOK(); e != nil {
+ return e
+ }
+ if e := p.limitsOK(); e != nil {
+ return e
+ }
+ want := tierExpected(l, "0")
+ for _, name := range []string{"memory.max", "memory.high", "memory.swap.max"} {
+ if e := strictWrite(p.limits[name], want[name]); e != nil {
+ return ErrStrictRefused
+ }
+ p.expected[name] = want[name]
+ }
+ return p.limitsOK()
+}
+
+// CloseRemove is Close for a generation child, then removes the child's
+// directory through the parent descriptor pinned at PrepareGen. Generation
+// numbers are never reused, so a child left behind would stay until the
+// agent restarts. Only an empty child is closed. The handle is closed and
+// its reservation released even when the rmdir fails; that error is
+// returned, and the next agent start's sweep removes the directory.
+func (p *Prepared) CloseRemove() error {
+ if p == nil || p.strictPreparedState == nil {
+ return ErrStrictState
+ }
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ p.draining.Store(true)
+ if p.closed || p.parent == nil || p.entryParent == nil || !isGenName(p.entryName) {
+ return ErrStrictState
+ }
+ if e := p.identityOK(); e != nil {
+ return e
+ }
+ if populated, e := strictPopulation(p.events); e != nil || populated {
+ return ErrStrictRefused
+ }
+ rmErr := unix.Unlinkat(int(p.entryParent.Fd()), p.entryName, unix.AT_REMOVEDIR)
+ p.closed = true
+ p.closeFiles()
+ strictReservations.Lock()
+ delete(strictReservations.held, p.identity)
+ strictReservations.Unlock()
+ if rmErr != nil {
+ return fmt.Errorf("cgroup: %s: remove %s: %w", p.host, p.entryName, rmErr)
+ }
+ return nil
+}
diff --git a/internal/cgroup/strict_gen_linux_test.go b/internal/cgroup/strict_gen_linux_test.go
new file mode 100644
index 000000000..dc6f873ae
--- /dev/null
+++ b/internal/cgroup/strict_gen_linux_test.go
@@ -0,0 +1,213 @@
+//go:build linux
+
+package cgroup
+
+import (
+ "context"
+ "errors"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "syscall"
+ "testing"
+ "time"
+)
+
+// genParentFixture is strictFixture turned into a generation parent: the
+// tier on its files, memory.oom.group=0, the record matching.
+func genParentFixture(t *testing.T) *Prepared {
+ t.Helper()
+ p := strictFixture(t)
+ p.genParent, p.tier = true, TierDefault
+ for name, value := range tierExpected(TierDefault, "0") {
+ if err := strictWrite(p.limits[name], value); err != nil {
+ t.Fatal(err)
+ }
+ p.expected[name] = value
+ }
+ return p
+}
+
+func makeGenFixture(t *testing.T, p *Prepared, n uint64, events string) string {
+ t.Helper()
+ d := filepath.Join(p.dir.Name(), GenChild(n))
+ if err := os.Mkdir(d, 0o700); err != nil {
+ t.Fatal(err)
+ }
+ for name, v := range map[string]string{"cgroup.events": events, "cgroup.kill": "", "memory.max": "", "memory.high": "", "memory.swap.max": "", "memory.oom.group": ""} {
+ if err := os.WriteFile(filepath.Join(d, name), []byte(v), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return d
+}
+
+func TestPrepareGenWritesTheFullTier(t *testing.T) {
+ p := genParentFixture(t)
+ d := makeGenFixture(t, p, 1, "populated 0\n")
+ child, err := p.PrepareGen(1, TierDefault)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer child.Close()
+ want := tierExpected(TierDefault, "1")
+ for name, value := range want {
+ if got := readFixture(t, filepath.Join(d, name)); got != value {
+ t.Errorf("g1/%s = %q, want %q", name, got, value)
+ }
+ }
+ if want["memory.max"] != "536870912" || want["memory.oom.group"] != "1" {
+ t.Fatalf("tierExpected = %v", want)
+ }
+}
+
+func TestPrepareGenReservesPerChild(t *testing.T) {
+ p := genParentFixture(t)
+ makeGenFixture(t, p, 1, "populated 0\n")
+ makeGenFixture(t, p, 2, "populated 0\n")
+ g1, err := p.PrepareGen(1, TierDefault)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer g1.Close()
+ if again, err := p.PrepareGen(1, TierDefault); !errors.Is(err, ErrStrictState) || again != nil {
+ t.Fatalf("second reservation of g1: %v", err)
+ }
+ g2, err := p.PrepareGen(2, TierDefault)
+ if err != nil {
+ t.Fatalf("g2 beside a live g1: %v", err)
+ }
+ defer g2.Close()
+ if _, err := p.PrepareGen(0, TierDefault); !errors.Is(err, ErrStrictRefused) {
+ t.Errorf("generation 0: %v", err)
+ }
+ if _, err := p.PrepareGen(3, Limit{MemoryMax: 64 << 20}); !errors.Is(err, ErrStrictRefused) {
+ t.Errorf("a limit that is not a tier: %v", err)
+ }
+}
+
+func TestPrepareGenAndSetParentLimitRefuseAPlainHostGroup(t *testing.T) {
+ p := strictFixture(t)
+ if _, err := p.PrepareGen(1, TierDefault); !errors.Is(err, ErrStrictState) {
+ t.Errorf("PrepareGen on a leaf host group: %v", err)
+ }
+ if err := p.SetParentLimit(TierDefault); !errors.Is(err, ErrStrictState) {
+ t.Errorf("SetParentLimit on a leaf host group: %v", err)
+ }
+}
+
+func TestGenerationParentRefusesAProcess(t *testing.T) {
+ p := genParentFixture(t)
+ if err := p.Start(exec.Command("/bin/true")); !errors.Is(err, ErrStrictState) {
+ t.Fatalf("a generation parent accepted a process: %v", err)
+ }
+}
+
+// The parent's record moves with the limit it raises, so a child's Start
+// passes limitsOK after a raise; a limit set by anyone else fails it.
+func TestSetParentLimitRecordsWhatItWrites(t *testing.T) {
+ p := genParentFixture(t)
+ makeGenFixture(t, p, 1, "populated 0\n")
+ makeGenFixture(t, p, 2, "populated 0\n")
+ child, err := p.PrepareGen(1, TierDefault)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer child.Close()
+ if err := p.SetParentLimit(OverlapLimit(TierDefault, TierDefault)); err != nil {
+ t.Fatal(err)
+ }
+ if got := readFixture(t, filepath.Join(p.dir.Name(), "memory.max")); got != "1073741824" {
+ t.Fatalf("parent memory.max = %q", got)
+ }
+ // The fixture's start answers ENOSYS: reaching it means every check passed.
+ child.start = func(*exec.Cmd) error { return syscall.ENOSYS }
+ if err := child.Start(exec.Command("/bin/true")); !errors.Is(err, ErrStrictUnsupported) {
+ t.Fatalf("child start after an API raise: %v, want it to reach the spawn", err)
+ }
+ // strictWrite does not truncate a fixture file: "9" over "1073741824"
+ // leaves "9073741824", a limit this API never set.
+ if err := strictWrite(p.limits["memory.max"], "9"); err != nil {
+ t.Fatal(err)
+ }
+ other, err := p.PrepareGen(2, TierDefault)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer other.Close()
+ if err := other.Start(exec.Command("/bin/true")); !errors.Is(err, ErrStrictRefused) {
+ t.Fatalf("child start under a limit the API did not set: %v", err)
+ }
+ if err := p.SetParentLimit(TierDefault); !errors.Is(err, ErrStrictRefused) {
+ t.Fatalf("SetParentLimit papered over a foreign limit: %v", err)
+ }
+}
+
+// Review focus 4: a release that changes tier.
+func TestSetParentLimitAcceptsATierChange(t *testing.T) {
+ p := genParentFixture(t)
+ if err := p.SetParentLimit(OverlapLimit(TierDefault, TierChildren)); err != nil {
+ t.Fatalf("512M + 1G: %v", err)
+ }
+ if got := readFixture(t, filepath.Join(p.dir.Name(), "memory.max")); got != strconv.FormatInt(1536<<20, 10) {
+ t.Fatalf("parent memory.max = %q", got)
+ }
+ if err := p.SetParentLimit(Limit{MemoryMax: 64 << 20}); !errors.Is(err, ErrStrictRefused) {
+ t.Fatalf("a limit that is no tier sum: %v", err)
+ }
+}
+
+func TestGenerationKillAndEmptyTouchesOnlyItsChild(t *testing.T) {
+ p := genParentFixture(t)
+ d1 := makeGenFixture(t, p, 1, "populated 1\n")
+ d2 := makeGenFixture(t, p, 2, "populated 0\n")
+ g1, err := p.PrepareGen(1, TierDefault)
+ if err == nil {
+ t.Fatal("PrepareGen pinned a populated group")
+ }
+ _ = g1
+ if err := os.WriteFile(filepath.Join(d1, "cgroup.events"), []byte("populated 0\n"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if g1, err = p.PrepareGen(1, TierDefault); err != nil {
+ t.Fatal(err)
+ }
+ defer g1.Close()
+ g2, err := p.PrepareGen(2, TierDefault)
+ if err != nil {
+ t.Fatal(err)
+ }
+ ctx, cancel := context.WithTimeout(t.Context(), time.Second)
+ defer cancel()
+ if err := g2.KillAndEmpty(ctx); err != nil {
+ t.Fatal(err)
+ }
+ for dir, want := range map[string]string{d2: "1", d1: "", p.dir.Name(): ""} {
+ if got := readFixture(t, filepath.Join(dir, "cgroup.kill")); got != want {
+ t.Errorf("%s/cgroup.kill = %q, want %q", dir, got, want)
+ }
+ }
+ // CloseRemove: refused while populated; once empty it closes and frees
+ // the reservation even when rmdir fails (a fixture dir holds files).
+ if err := os.WriteFile(filepath.Join(d2, "cgroup.events"), []byte("populated 1\n"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := g2.CloseRemove(); !errors.Is(err, ErrStrictRefused) {
+ t.Fatalf("CloseRemove of a populated child: %v", err)
+ }
+ if err := os.WriteFile(filepath.Join(d2, "cgroup.events"), []byte("populated 0\n"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := g2.CloseRemove(); err == nil {
+ t.Fatal("rmdir of a fixture dir holding files reported success")
+ }
+ if _, err := g2.Populated(); !errors.Is(err, ErrStrictState) {
+ t.Fatalf("handle still open after CloseRemove: %v", err)
+ }
+ again, err := p.PrepareGen(2, TierDefault)
+ if err != nil {
+ t.Fatalf("reservation not released by CloseRemove: %v", err)
+ }
+ again.Close()
+}
diff --git a/internal/cgroup/strict_linux.go b/internal/cgroup/strict_linux.go
index 37a057902..581e22466 100644
--- a/internal/cgroup/strict_linux.go
+++ b/internal/cgroup/strict_linux.go
@@ -9,7 +9,6 @@ import (
"os"
"os/exec"
"path/filepath"
- "strconv"
"strings"
"sync"
"sync/atomic"
@@ -44,6 +43,12 @@ type strictPreparedState struct {
host string
attempted, empty, closed bool
start func(*exec.Cmd) error
+ // genParent marks a generation parent (PrepareStrictParent): it holds
+ // no process, pins PrepareGen children, and moves its limit only
+ // through SetParentLimit. tier is the tier it was prepared with, or a
+ // generation child's own.
+ genParent bool
+ tier Limit
}
func strictOpen(dir *os.File, name string, flags int) (*os.File, error) {
@@ -196,7 +201,14 @@ func (m *Manager) StrictPreflight() error {
// PrepareStrict never uses accounting's fallback. Only the two existing tiers
// are accepted; a zero or missing limit cannot create an uncapped command.
-func (m *Manager) PrepareStrict(host string, limit Limit) (out *Prepared, err error) {
+func (m *Manager) PrepareStrict(host string, limit Limit) (*Prepared, error) {
+ return m.prepareStrict(host, limit, false)
+}
+
+// prepareStrict is PrepareStrict and PrepareStrictParent: with genParent the
+// group is pinned as a generation parent (memory.oom.group=0, controllers
+// delegated to its children, Start refused) instead of a leaf.
+func (m *Manager) prepareStrict(host string, limit Limit, genParent bool) (out *Prepared, err error) {
if m == nil || m.root == "" {
return nil, ErrStrictUnsupported
}
@@ -249,8 +261,11 @@ func (m *Manager) PrepareStrict(host string, limit Limit) (out *Prepared, err er
if populated, e := strictPopulation(p.events); e != nil || populated {
return nil, ErrStrictRefused
}
- high := HighOf(limit.MemoryMax) / int64(os.Getpagesize()) * int64(os.Getpagesize())
- p.expected = map[string]string{"memory.max": strconv.FormatInt(limit.MemoryMax, 10), "memory.high": strconv.FormatInt(high, 10), "memory.swap.max": strconv.FormatInt(limit.MemoryMax, 10), "memory.oom.group": "1"}
+ oomGroup := "1"
+ if genParent {
+ oomGroup = "0"
+ }
+ p.expected = tierExpected(limit, oomGroup)
for name := range p.expected {
p.limits[name], e = strictOpen(p.dir, name, unix.O_RDWR)
if e != nil {
@@ -265,6 +280,12 @@ func (m *Manager) PrepareStrict(host string, limit Limit) (out *Prepared, err er
if e = p.limitsOK(); e != nil {
return nil, e
}
+ if genParent {
+ if e = delegateStrict(p.dir); e != nil {
+ return nil, e
+ }
+ p.genParent, p.tier = true, limit
+ }
if e = p.identityOK(); e != nil {
return nil, e
}
@@ -292,6 +313,10 @@ func (p *Prepared) Start(cmd *exec.Cmd) error {
}
p.mu.Lock()
defer p.mu.Unlock()
+ if p.genParent {
+ // A generation parent holds no process (zero-downtime spec §7.3).
+ return ErrStrictState
+ }
if p.closed || p.attempted || p.empty || p.draining.Load() {
return ErrStrictState
}
diff --git a/internal/cgroup/strict_other.go b/internal/cgroup/strict_other.go
index 34ddee7cb..9e9b73c50 100644
--- a/internal/cgroup/strict_other.go
+++ b/internal/cgroup/strict_other.go
@@ -17,3 +17,9 @@ func (p *Prepared) Start(cmd *exec.Cmd) error { return ErrStrictUns
func (p *Prepared) Populated() (bool, error) { return false, ErrStrictUnsupported }
func (p *Prepared) KillAndEmpty(ctx context.Context) error { return ErrStrictUnsupported }
func (p *Prepared) Close() error { return ErrStrictUnsupported }
+func (m *Manager) PrepareStrictParent(string, Limit) (*Prepared, error) {
+ return nil, ErrStrictUnsupported
+}
+func (p *Prepared) PrepareGen(uint64, Limit) (*Prepared, error) { return nil, ErrStrictUnsupported }
+func (p *Prepared) SetParentLimit(Limit) error { return ErrStrictUnsupported }
+func (p *Prepared) CloseRemove() error { return ErrStrictUnsupported }