amadan

carlos / platform Public

reliability polish: one spawn seam pins the tier, biasOOM is silent for a dead child, unit path gets MemoryHigh parity

Paul Campbell pushed by paul@keymail.dev aa1507e36986a3e2d5091a0d7605500afa995f94 parent 5d9b658
6 files changed, +78 −3
  • internal/activator/backend_exec.go +12 −3
  • internal/activator/backend_exec_test.go +35 −0
  • internal/caps/render.go +4 −0
  • internal/caps/testdata/keymail.service.conf +1 −0
  • internal/cgroup/cgroup.go +23 −0
  • internal/host/units/carlos-app@.service +3 −0

CI passed — run details

diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 580e63fe9..ddd0893b5 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -130,6 +130,15 @@ func (b *ExecBackend) tier(rt registry.Route) cgroup.Limit {
return b.Tier(rt)
}
+// spawnInCgroup is the ONE seam all three child shapes — seed, instance,
+// sidecar — start through: the route's cgroup, the route's tier. A spawn
+// site that bypassed it would compile and silently ship uncapped children;
+// keeping the tier plumbing here makes that a diff a reviewer can see and
+// TestSpawnInCgroupAppliesTheTier can pin.
+func (b *ExecBackend) spawnInCgroup(rt registry.Route, build func() *exec.Cmd) (*exec.Cmd, error) {
+ return b.Cgroups.Start(rt.Host, b.tier(rt), build)
+}
+
func (b *ExecBackend) liveDir() string { return filepath.Join(b.OptDir, "live") }
func (b *ExecBackend) liveLink(host string) string { return filepath.Join(b.liveDir(), host) }
func (b *ExecBackend) defaultDBPath(host string) string {
@@ -325,7 +334,7 @@ func (b *ExecBackend) Seed(ctx context.Context, rt registry.Route) error {
// cost is that instance's work. No CleanupHost here — the instance spawn
// reuses the dir moments later, and its exit path (or the next agent
// start's sweep) removes it.
- cmd, err := b.Cgroups.Start(host, b.tier(rt), func() *exec.Cmd {
+ cmd, err := b.spawnInCgroup(rt, func() *exec.Cmd {
cmd := exec.CommandContext(ctx, b.liveLink(host), argv...)
// The control-channel vars are agent-owned in BOTH directions here too
// (Start's discipline, see withoutVars), and here they are always
@@ -633,7 +642,7 @@ func (b *ExecBackend) Start(ctx context.Context, rt registry.Route) (Proc, strin
// 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.
- cmd, err := b.Cgroups.Start(host, b.tier(rt), func() *exec.Cmd {
+ cmd, err := b.spawnInCgroup(rt, func() *exec.Cmd {
cmd := exec.Command(bin, "--socket", rt.Socket, "--db", dbPath)
cmd.Env = childEnv(childVars)
// The child logs through the agent's own stderr/stdout (journald
@@ -836,7 +845,7 @@ func (b *ExecBackend) StartSidecar(_ context.Context, rt registry.Route) (Proc,
// Same host cgroup as the instance: a sidecar's load is that instance's
// work, and attributing it elsewhere would be a lie in the other
// direction.
- cmd, err := b.Cgroups.Start(rt.Host, b.tier(rt), func() *exec.Cmd {
+ cmd, err := b.spawnInCgroup(rt, func() *exec.Cmd {
cmd := exec.Command(b.liveLink(rt.Host), "sidecar", "run")
cmd.Env = childEnv(fileEnv)
// Like the instance child: logs through the agent's own streams
diff --git a/internal/activator/backend_exec_test.go b/internal/activator/backend_exec_test.go
index b1f165250..6bc46a846 100644
--- a/internal/activator/backend_exec_test.go
+++ b/internal/activator/backend_exec_test.go
@@ -1,6 +1,9 @@
package activator
import (
+ "os"
+ "os/exec"
+ "path/filepath"
"testing"
"github.com/carlosframework/platform/internal/cgroup"
@@ -21,3 +24,35 @@ func TestExecBackendTierSeam(t *testing.T) {
t.Fatalf("set Tier: %v, want TierChildren", got)
}
}
+
+// TestSpawnInCgroupAppliesTheTier: the seam every spawn site uses really
+// writes the route's tier into the host cgroup — a regression that swapped
+// a site back to a bare Cgroups.Start(…, cgroup.Limit{}, …) compiles fine
+// and this is what catches the class. Real cgroup.SetupAt against a
+// temp-dir fixture; the spawn is /bin/true.
+func TestSpawnInCgroupAppliesTheTier(t *testing.T) {
+ 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)
+ }
+ }
+ m, err := cgroup.SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ b := ExecBackend{Cgroups: m, Tier: func(registry.Route) cgroup.Limit { return cgroup.TierChildren }}
+ rt := registry.Route{Host: "h.example.com"}
+ cmd, err := b.spawnInCgroup(rt, func() *exec.Cmd { return exec.Command("/bin/true") })
+ if err != nil {
+ t.Fatal(err)
+ }
+ _ = cmd.Wait()
+ got, err := os.ReadFile(filepath.Join(root, "carlos-exec@h.example.com", "memory.max"))
+ if err != nil {
+ t.Fatalf("memory.max not written: %v", err)
+ }
+ if string(got) != "1073741824" {
+ t.Errorf("memory.max = %q, want the children tier", got)
+ }
+}
diff --git a/internal/caps/render.go b/internal/caps/render.go
index 2cdadc905..c4c2f01ae 100644
--- a/internal/caps/render.go
+++ b/internal/caps/render.go
@@ -91,6 +91,10 @@ func RenderService(app string, s *Set, g Grants) ([]byte, error) {
"KillMode=control-group",
"TasksMax=1024",
"MemoryMax=1G",
+ // Throttle-and-reclaim before the kill, like the exec path's
+ // memory.high (internal/cgroup applyLimit): the same declared
+ // capability behaves the same on both backings.
+ "MemoryHigh=920M",
)
}
diff --git a/internal/caps/testdata/keymail.service.conf b/internal/caps/testdata/keymail.service.conf
index 766f71f78..facb583d8 100644
--- a/internal/caps/testdata/keymail.service.conf
+++ b/internal/caps/testdata/keymail.service.conf
@@ -8,6 +8,7 @@ RuntimeDirectoryPreserve=restart
KillMode=control-group
TasksMax=1024
MemoryMax=1G
+MemoryHigh=920M
DynamicUser=no
RestrictSUIDSGID=yes
User=carlos-keymail
diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go
index b6a38a8dc..94ccaff70 100644
--- a/internal/cgroup/cgroup.go
+++ b/internal/cgroup/cgroup.go
@@ -360,10 +360,33 @@ func (m *Manager) biasOOM(host string, cmd *exec.Cmd) {
return
}
if err := os.WriteFile(oomScoreAdjPath(cmd.Process.Pid), []byte(ExecOOMScoreAdj), 0o644); err != nil {
+ // A child that crashed in its first milliseconds is a zombie by the
+ // time this write lands, and a zombie's oom_score_adj refuses the
+ // write (seen live: birthday-alarm's crash loop logged one of these
+ // per wake). A dead child needs no bias — say nothing.
+ if procGone(cmd.Process.Pid) {
+ return
+ }
m.logf("cgroup: %s: oom_score_adj for pid %d: %v — the agent and this child score alike at an edge-tree OOM", host, cmd.Process.Pid, err)
}
}
+// procGone reports whether pid is dead — absent from /proc, or a zombie
+// (state Z in /proc/<pid>/stat, third field after the parenthesised comm).
+func procGone(pid int) bool {
+ b, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "stat"))
+ if err != nil {
+ return true
+ }
+ // comm may contain spaces and parens; the state letter follows the LAST ')'.
+ s := string(b)
+ i := strings.LastIndexByte(s, ')')
+ if i < 0 || i+2 >= len(s) {
+ return false
+ }
+ return s[i+2] == 'Z'
+}
+
// CleanupHost removes host's per-host cgroup dir after its children exit.
// Nil-safe and rmdir-tolerant: while a sibling (instance vs sidecar) is
// still alive the dir is non-empty and the remove fails — the LAST exiter's
diff --git a/internal/host/units/carlos-app@.service b/internal/host/units/carlos-app@.service
index 2fc9dde79..613eea27b 100644
--- a/internal/host/units/carlos-app@.service
+++ b/internal/host/units/carlos-app@.service
@@ -57,6 +57,9 @@ CapabilityBoundingSet=
RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX
MemoryDenyWriteExecute=yes
MemoryMax=512M
+# Throttle-and-reclaim before the kill — the same band the exec path's
+# memory.high gives (internal/cgroup applyLimit, 90% of max).
+MemoryHigh=460M
TasksMax=256
[Install]