amadan
zdd-readiness: the stage relay gives up after an hour; a populated legacy parent refuses generation placement
A relay head root has neither installed nor refused for an hour is dropped with a log line (the next blocked pass notes it again). ensureParent reads the parent's own cgroup.procs and returns ErrLegacyParentPopulated whenever a process is a direct member, with or without g* children, before any child is created or exec'd (amadan #272, real-unit check (d)). The activator reads it as an abandonment with Executed false, never as may-have-run or a failed build; a member sees the maintenance-fence text. Kernel-gated test added for the dev-box rerun.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
11 files changed,
+482
−16
internal/activator/containment_test.go+34 −0internal/activator/genparent_kernel_linux_test.go+81 −0internal/activator/logsink.go+10 −0internal/activator/logsink_test.go+12 −0internal/activator/swap.go+6 −2internal/activator/swap_executed_test.go+10 −0internal/activator/swap_overlap_test.go+6 −0internal/cgroup/gen.go+57 −12internal/cgroup/gen_test.go+120 −0internal/host/stagerelay.go+49 −2internal/host/stagerelay_test.go+97 −0
diff --git a/internal/activator/containment_test.go b/internal/activator/containment_test.go| index feae25cfb..57bb6017b 100644 |
| --- a/internal/activator/containment_test.go |
| +++ b/internal/activator/containment_test.go |
| @@ -281,6 +281,40 @@ func TestSpawnInCgroupAtRefusesAStrictHost(t *testing.T) { |
| } |
| } |
| +// Amadan #272: a host whose parent cgroup still holds a process directly |
| +// (a legacy flat start) gets no generation, in any child shape. The refusal |
| +// comes back before the exec, leaves no child, and reads as nothing of the |
| +// build having run, so no swap or wake counts it as "may have run" or as a |
| +// failure of the build. |
| +func TestSpawnInCgroupAtRefusesAPopulatedLegacyParent(t *testing.T) { |
| + const host = "h.example.com" |
| + b, parent := delegatedBackend(t, host) |
| + b.GenContainment = true |
| + if err := os.WriteFile(filepath.Join(parent, "cgroup.procs"), []byte("4242\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + ran := false |
| + build := func() *exec.Cmd { ran = true; return exec.Command("/bin/true") } |
| + for _, child := range []string{"g7", "role-seed", "role-sidecar"} { |
| + cmd, _, err := b.spawnInCgroupAt(registry.Route{Host: host}, child, child == "g7", build) |
| + if !errors.Is(err, cgroup.ErrLegacyParentPopulated) || cmd != nil { |
| + t.Fatalf("%s under a populated parent: %v, %v", child, cmd, err) |
| + } |
| + if startMayHaveRun(nil, err) { |
| + t.Errorf("%s: the refusal reads as a start that may have run", child) |
| + } |
| + if verdictOf(err) != gateNoOwnership { |
| + t.Errorf("%s: the refusal is not an abandonment", child) |
| + } |
| + if _, serr := os.Stat(filepath.Join(parent, child)); !os.IsNotExist(serr) { |
| + t.Errorf("%s: a refused spawn made a child cgroup: %v", child, serr) |
| + } |
| + } |
| + if ran { |
| + t.Error("the build was prepared for exec under a populated parent") |
| + } |
| +} |
| + |
| // zdd-readiness Task 6: errStartedThenExited is found through every wrap |
| // spawnUnitFrom gives it (the uncertain mark, and beside errGenUnproven when |
| // the kill is unproven), and the text is the one it always was: a flag-off |
diff --git a/internal/activator/genparent_kernel_linux_test.go b/internal/activator/genparent_kernel_linux_test.go| new file mode 100644 |
| index 000000000..19b11a573 |
| --- /dev/null |
| +++ b/internal/activator/genparent_kernel_linux_test.go |
| @@ -0,0 +1,81 @@ |
| +//go:build linux |
| + |
| +package activator |
| + |
| +import ( |
| + "errors" |
| + "os" |
| + "os/exec" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/cgroup" |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// Amadan #272, real-unit check (d): on the dev box (arm64, kernel 6.12, |
| +// systemd 257) a generation started under a carlos-exec@<host> that still |
| +// held a process directly ran with no memory limit, silently, while the |
| +// strict path refused. This runs the same shape against the real cgroup |
| +// filesystem: a flat start (the older agent's layout) puts a process |
| +// directly in the parent, and a generation start beside it must be refused |
| +// before it creates a g* child or execs anything. The unit-level cases |
| +// (internal/cgroup gen_test.go, containment_test.go) use a fake cgroupfs; |
| +// only this one proves the kernel's side. Same gate as the claim-spawn |
| +// kernel test: a disposable delegated user service, and the synthetic |
| +// child TestClaimSpawnKernelChild. |
| +func TestGenerationStartRefusedBesideADirectParentMember(t *testing.T) { |
| + if os.Getenv("CARLOS_CLAIM_SPAWN_KERNEL") != "1" { |
| + t.Skip("requires a disposable delegated user service") |
| + } |
| + manager := cgroup.Setup(t.Logf) |
| + if manager == nil { |
| + t.Fatal("missing delegated cgroup") |
| + } |
| + const host = "genparent.example" |
| + parent := filepath.Join(manager.Root(), cgroup.ExecName(host)) |
| + t.Cleanup(func() { manager.CleanupHost(host) }) |
| + |
| + legacy, err := manager.Start(host, cgroup.TierDefault, func() *exec.Cmd { |
| + cmd := exec.Command(os.Args[0], "-test.run=^TestClaimSpawnKernelChild$", "-test.count=1") |
| + cmd.Env = []string{"CARLOS_CLAIM_SPAWN_CHILD=1"} |
| + cmd.Dir = t.TempDir() |
| + return cmd |
| + }) |
| + if err != nil || legacy == nil || legacy.Process == nil { |
| + t.Fatalf("flat start: %v, %v", legacy, err) |
| + } |
| + t.Cleanup(func() { |
| + _ = legacy.Process.Kill() |
| + _ = legacy.Wait() |
| + }) |
| + procs, err := os.ReadFile(filepath.Join(parent, "cgroup.procs")) |
| + if err != nil || !strings.Contains(string(procs), "\n") { |
| + t.Fatalf("the flat start's process is not a direct member of %s: %q, %v", parent, procs, err) |
| + } |
| + |
| + b := testExecBackend(t) |
| + b.Cgroups = manager |
| + b.GenContainment = true |
| + ran := false |
| + cmd, _, err := b.spawnInCgroupAt(registry.Route{Host: host}, cgroup.GenChild(1), true, func() *exec.Cmd { |
| + ran = true |
| + return exec.Command("/bin/true") |
| + }) |
| + if !errors.Is(err, cgroup.ErrLegacyParentPopulated) || cmd != nil { |
| + t.Fatalf("generation start beside a direct member = %v, %v: want ErrLegacyParentPopulated and no command", cmd, err) |
| + } |
| + if ran { |
| + t.Error("the generation's command was built for exec") |
| + } |
| + ents, err := os.ReadDir(parent) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, e := range ents { |
| + if e.IsDir() { |
| + t.Errorf("the refused start left %s under the parent", e.Name()) |
| + } |
| + } |
| +} |
diff --git a/internal/activator/logsink.go b/internal/activator/logsink.go| index faa705ba2..f07459b60 100644 |
| --- a/internal/activator/logsink.go |
| +++ b/internal/activator/logsink.go |
| @@ -11,6 +11,7 @@ import ( |
| "regexp" |
| "strings" |
| + "github.com/carlosframework/platform/internal/cgroup" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| @@ -57,6 +58,15 @@ func memberSafeWakeReason(err error) string { |
| // operator's (persistGenerations logs it). |
| err = ErrFenced |
| } |
| + if errors.Is(err, cgroup.ErrLegacyParentPopulated) { |
| + // A generation start refused because the host's parent cgroup still |
| + // holds a process directly (amadan #272). It is the box's state to |
| + // clear, and the cgroup detail (the host, the count) is the |
| + // operator's: the agent logs it. The member gets the maintenance |
| + // fence's existing text, as for the other holds the platform lifts |
| + // itself. |
| + err = ErrFenced |
| + } |
| if errors.Is(err, errWakeUnproven) || genStartUnproven(err) || errors.Is(err, errOutcomeUndelivered) { |
| // A wake refused while the host's last failed wake is not proven |
| // stopped (zero-downtime spec §7.4), or that failed wake itself: a |
diff --git a/internal/activator/logsink_test.go b/internal/activator/logsink_test.go| index d1908d5b8..c95508100 100644 |
| --- a/internal/activator/logsink_test.go |
| +++ b/internal/activator/logsink_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "context" |
| "errors" |
| "fmt" |
| + "github.com/carlosframework/platform/internal/cgroup" |
| "github.com/carlosframework/platform/internal/instancectl" |
| "strings" |
| "testing" |
| @@ -195,3 +196,14 @@ func TestMemberSafeWakeReasonHidesTheClaimsAdmissionWrapper(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Amadan #272: a wake refused because the host's parent cgroup holds a |
| +// process directly reads to a member as the maintenance fence's approved |
| +// text, not as cgroup detail. |
| +func TestMemberSafeWakeReasonHidesALegacyParentRefusal(t *testing.T) { |
| + err := fmt.Errorf("h.example.com: %w: h.example.com holds 2 process(es)", cgroup.ErrLegacyParentPopulated) |
| + got := memberSafeWakeReason(err) |
| + if got != memberSafeWakeReason(ErrFenced) || strings.Contains(got, "cgroup") || strings.Contains(got, "process") { |
| + t.Errorf("member reason = %q, want the fence text alone", got) |
| + } |
| +} |
diff --git a/internal/activator/swap.go b/internal/activator/swap.go| index 2fd96e99f..4a7abb793 100644 |
| --- a/internal/activator/swap.go |
| +++ b/internal/activator/swap.go |
| @@ -175,7 +175,7 @@ const ( |
| // verdictOf types an attempt-gate error. An error that is not the gate's |
| // (a failure of the build) is gateProceed: the gate did not refuse. |
| // |
| -// Besides the gate's own refusals, three kinds of refusal that stop a spawn |
| +// Besides the gate's own refusals, four kinds of refusal that stop a spawn |
| // before anything of the build runs are NoOwnership, so the one classifier |
| // reads them as abandonment, never as a failed build: |
| // - plan 2's strict-host gate on placed spawns (index P2-8): |
| @@ -185,6 +185,10 @@ const ( |
| // so it is named here; |
| // - a row that cannot run a staged generation at all: errUnitNoGens, |
| // errSwapNoGens; |
| +// - the cgroup manager refusing a generation start because the host's |
| +// parent cgroup holds a process directly (cgroup.ErrLegacyParentPopulated, |
| +// amadan #272): it checks before it creates the child or execs, and the |
| +// condition is the box's, not the build's; |
| // - root refusing the nested staged executable (instancectl.ErrStagedExe, |
| // Task 9): it validates before it mounts or starts anything. |
| // |
| @@ -207,7 +211,7 @@ func verdictOf(err error) gateVerdict { |
| errors.Is(err, hostguard.ErrNoHolder), errors.Is(err, hostguard.ErrMalformedClaim), |
| errors.Is(err, hostbox.ErrChanged), errors.Is(err, errClaimsAdmission), |
| errors.Is(err, ErrStrictRequired), errors.Is(err, ErrStrictGroupInUse), errors.Is(err, ErrStrictCleanupPending), |
| - errors.Is(err, ErrClaimIsolationUnavailable), |
| + errors.Is(err, ErrClaimIsolationUnavailable), errors.Is(err, cgroup.ErrLegacyParentPopulated), |
| errors.Is(err, errUnitNoGens), errors.Is(err, errSwapNoGens), errors.Is(err, instancectl.ErrStagedExe): |
| return gateNoOwnership |
| } |
diff --git a/internal/activator/swap_executed_test.go b/internal/activator/swap_executed_test.go| index 1cb563549..4e020cc32 100644 |
| --- a/internal/activator/swap_executed_test.go |
| +++ b/internal/activator/swap_executed_test.go |
| @@ -11,6 +11,7 @@ import ( |
| "testing" |
| "time" |
| + "github.com/carlosframework/platform/internal/cgroup" |
| "github.com/carlosframework/platform/internal/instancectl" |
| "github.com/carlosframework/platform/internal/registry" |
| ) |
| @@ -35,6 +36,15 @@ func TestSwapResultReportsWhetherTheAttemptsBuildExecuted(t *testing.T) { |
| {"overlap, the spawn fails", ModeOverlap, func(f *swapFixture) { |
| f.b.genErr = errors.New("exec format error") |
| }, SwapFailed, false}, |
| + // Amadan #272: a parent that holds a legacy process directly refuses |
| + // the generation start before any exec. That is the box's condition, |
| + // not a failure of the build, so the attempt is abandoned unspent. |
| + {"overlap, the host's parent holds a process", ModeOverlap, func(f *swapFixture) { |
| + f.b.genErr = fmt.Errorf("h.example.com: %w: holds 1 process(es)", cgroup.ErrLegacyParentPopulated) |
| + }, SwapAbandoned, false}, |
| + {"hold, the host's parent holds a process", ModeHold, func(f *swapFixture) { |
| + f.b.genErr = fmt.Errorf("h.example.com: %w: holds 1 process(es)", cgroup.ErrLegacyParentPopulated) |
| + }, SwapAbandoned, false}, |
| {"overlap, superseded at the switch", ModeOverlap, func(f *swapFixture) { |
| f.a.genHealth = func(context.Context, Gen) error { f.rev.Add(1); return nil } |
| }, SwapAbandoned, true}, |
diff --git a/internal/activator/swap_overlap_test.go b/internal/activator/swap_overlap_test.go| index 0d646cbcb..797d93da7 100644 |
| --- a/internal/activator/swap_overlap_test.go |
| +++ b/internal/activator/swap_overlap_test.go |
| @@ -291,6 +291,9 @@ func TestClassifyStep(t *testing.T) { |
| {"a closed claims launch", live, fmt.Errorf("launch: %w", ErrClaimIsolationUnavailable), stepStale}, |
| // A row that cannot run a staged generation at all. |
| {"a unit row", live, errUnitNoGens, stepStale}, |
| + // Amadan #272: the generation start refused before any exec because |
| + // the host's parent holds a process directly. |
| + {"a populated legacy parent", live, fmt.Errorf("h: %w: holds 1 process(es)", cgroup.ErrLegacyParentPopulated), stepStale}, |
| // Root refused the nested staged executable before it mounted or |
| // started anything (Task 9): an abandonment, not a failed build. |
| {"root refused the staged exe", live, fmt.Errorf("start unit: %w: not blog's staged v2", instancectl.ErrStagedExe), stepStale}, |
| @@ -341,6 +344,9 @@ func TestVerdictOf(t *testing.T) { |
| {ErrClaimIsolationUnavailable, gateNoOwnership}, |
| {fmt.Errorf("start: %w", ErrClaimIsolationUnavailable), gateNoOwnership}, |
| {claimAdmission(ErrClaimIsolationUnavailable, false), gateNoOwnership}, |
| + // Amadan #272: a populated legacy parent refuses the start before any |
| + // exec, and no build is at fault. |
| + {fmt.Errorf("h: %w: holds 1 process(es)", cgroup.ErrLegacyParentPopulated), gateNoOwnership}, |
| // A row that cannot run a staged generation: nothing staged spawns. |
| {errUnitNoGens, gateNoOwnership}, |
| {instancectl.ErrStagedExe, gateNoOwnership}, |
diff --git a/internal/cgroup/gen.go b/internal/cgroup/gen.go| index 5105c2611..faf8c3873 100644 |
| --- a/internal/cgroup/gen.go |
| +++ b/internal/cgroup/gen.go |
| @@ -118,30 +118,71 @@ func parentPopulated(dir string) bool { |
| return !strings.Contains(string(b), "populated 0") |
| } |
| -// ensureParent makes carlos-exec@<host> a generation parent. When nothing |
| -// is alive under it (a cold wake), it delegates the controllers and writes |
| -// the tier with memory.oom.group=0. With a live generation under it, the |
| -// parent is left as it is, because a swap raised it on purpose. Best-effort, |
| -// like the rest of the accounting path: a failure is one logged line and |
| -// children that run without their limits, never a refused spawn. |
| -func (m *Manager) ensureParent(host string, tier Limit) { |
| +// ErrLegacyParentPopulated is a generation start refused because the host's |
| +// parent cgroup holds a process directly. A cgroup with processes of its own |
| +// cannot delegate the memory controller to children (cgroup v2's |
| +// no-internal-process rule), so a generation started under it would run with |
| +// no memory limit at all, silently (amadan #272, real-unit check (d), kernel |
| +// 6.12, systemd 257). An older agent's flat start is the usual way a parent |
| +// gets one. The start fails closed instead: nothing is created and nothing |
| +// is exec'd, and it is no fault of the build. |
| +var ErrLegacyParentPopulated = errors.New("cgroup: the host's parent cgroup holds a process directly") |
| + |
| +// parentProcs counts the processes dir holds directly: its own cgroup.procs, |
| +// not cgroup.events, whose "populated" also counts every descendant. An |
| +// absent file is a directory nothing has run in. A file that cannot be read |
| +// is an error, so the caller never takes it for empty. |
| +func parentProcs(dir string) (int, error) { |
| + b, err := os.ReadFile(filepath.Join(dir, "cgroup.procs")) |
| + if errors.Is(err, os.ErrNotExist) { |
| + return 0, nil |
| + } |
| + if err != nil { |
| + return 0, err |
| + } |
| + return len(strings.Fields(string(b))), nil |
| +} |
| + |
| +// ensureParent makes carlos-exec@<host> a generation parent. It first |
| +// refuses a parent that holds a process directly, whatever children it has |
| +// (plan review #12): a stale empty g<n> beside a legacy process does not make |
| +// the parent one. Then, when nothing is alive under it (a cold wake), it |
| +// delegates the controllers and writes the tier with memory.oom.group=0. With |
| +// a live generation under it, the parent is left as it is, because a swap |
| +// raised it on purpose. The refusal is the one failure that is not |
| +// best-effort: the rest of the accounting path logs a line and runs children |
| +// without their limits, but a refusal-free start here would run a generation |
| +// uncapped with nothing to say so. The error is ErrLegacyParentPopulated, |
| +// and an unreadable cgroup.procs is refused as the same (unknown is not |
| +// empty). |
| +func (m *Manager) ensureParent(host string, tier Limit) error { |
| dir := m.hostDir(host) |
| if err := os.MkdirAll(dir, 0o755); err != nil { |
| m.logf("cgroup: %s: create parent cgroup: %v (children run unaccounted)", host, err) |
| - return |
| + return nil |
| + } |
| + n, err := parentProcs(dir) |
| + if err != nil { |
| + m.logf("cgroup: %s: generation start refused: cannot read the parent's cgroup.procs: %v", host, err) |
| + return fmt.Errorf("%w (cgroup.procs unreadable, treated as populated): %v", ErrLegacyParentPopulated, err) |
| + } |
| + if n > 0 { |
| + m.logf("cgroup: %s: generation start refused: the parent cgroup holds %d process(es) directly, so no memory limit could apply to a generation under it; stop them or let them exit", host, n) |
| + return fmt.Errorf("%w: %s holds %d process(es)", ErrLegacyParentPopulated, host, n) |
| } |
| if parentPopulated(dir) { |
| - return |
| + return nil |
| } |
| if err := enableChildren(dir); err != nil { |
| m.logf("cgroup: %s: delegate controllers to generations: %v (children run uncapped)", host, err) |
| - return |
| + return nil |
| } |
| if tier.MemoryMax > 0 { |
| if err := m.SetParentLimit(host, tier); err != nil { |
| m.logf("cgroup: %s: parent limit: %v", host, err) |
| } |
| } |
| + return nil |
| } |
| // StartChild is Start for the generation layout: the child lands in |
| @@ -157,7 +198,9 @@ func (m *Manager) StartChild(host, child string, parent, own Limit, build func() |
| // 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. |
| +// before's own error. A nil before is StartChild exactly. A parent that |
| +// holds a process directly refuses the start with ErrLegacyParentPopulated |
| +// before anything is created or exec'd (ensureParent). |
| 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) |
| @@ -165,7 +208,9 @@ func (m *Manager) StartChildGated(host, child string, parent, own Limit, build f |
| if m == nil { |
| return startPlain(build, before) |
| } |
| - m.ensureParent(host, parent) |
| + if err := m.ensureParent(host, parent); err != nil { |
| + return nil, err |
| + } |
| return m.startIn(host, filepath.Join(m.hostDir(host), child), own, build, before) |
| } |
diff --git a/internal/cgroup/gen_test.go b/internal/cgroup/gen_test.go| index c04f8061c..3361e47a9 100644 |
| --- a/internal/cgroup/gen_test.go |
| +++ b/internal/cgroup/gen_test.go |
| @@ -2,10 +2,12 @@ package cgroup |
| import ( |
| "errors" |
| + "fmt" |
| "os" |
| "os/exec" |
| "path/filepath" |
| "strconv" |
| + "strings" |
| "testing" |
| ) |
| @@ -327,3 +329,121 @@ func TestStartGatedWithNoManagerAsksBeforeThePlainStart(t *testing.T) { |
| t.Fatalf("started %d times after the hook refused, want none", len(*calls)) |
| } |
| } |
| + |
| +// legacyParent is genManager over a manager whose log lines are kept, with |
| +// the parent's own cgroup.procs listing pids: the shape the dev box showed |
| +// (amadan #272, check (d)), a carlos-exec@<host> an older agent started a |
| +// process in directly. |
| +func legacyParent(t *testing.T, host, procs string) (*Manager, string, *[]string) { |
| + t.Helper() |
| + m, parent := genManager(t, host) |
| + var lines []string |
| + m.logf = func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) } |
| + writeFixture(t, filepath.Join(parent, "cgroup.procs"), procs) |
| + return m, parent, &lines |
| +} |
| + |
| +func noGenerationChild(t *testing.T, parent string) { |
| + t.Helper() |
| + ents, err := os.ReadDir(parent) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, e := range ents { |
| + if e.IsDir() { |
| + t.Errorf("the refused start created %s under the parent", e.Name()) |
| + } |
| + } |
| +} |
| + |
| +// Flag blocker (amadan #272): a generation started under a parent that |
| +// still holds a process directly runs with no memory limit, because a |
| +// cgroup with processes of its own cannot delegate the memory controller. |
| +// The start is refused, names the host and the count, and creates nothing. |
| +func TestAPopulatedLegacyParentIsRefusedBeforeAnyChildExists(t *testing.T) { |
| + const host = "h.example.com" |
| + m, parent, lines := legacyParent(t, host, "4242\n4243\n") |
| + calls := fakeStart(t, func(*exec.Cmd) error { return nil }) |
| + cmd, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, buildTrue) |
| + if !errors.Is(err, ErrLegacyParentPopulated) || cmd != nil { |
| + t.Fatalf("StartChild = %v, %v: want ErrLegacyParentPopulated and no command", cmd, err) |
| + } |
| + if len(*calls) != 0 { |
| + t.Fatalf("started %d times under a populated parent", len(*calls)) |
| + } |
| + noGenerationChild(t, parent) |
| + if _, serr := os.Stat(filepath.Join(parent, "cgroup.subtree_control")); !os.IsNotExist(serr) { |
| + t.Errorf("the refused start delegated controllers anyway (%v)", serr) |
| + } |
| + if len(*lines) != 1 || !strings.Contains((*lines)[0], host) || !strings.Contains((*lines)[0], "2 process") { |
| + t.Errorf("log lines = %q, want one naming %s and 2 processes", *lines, host) |
| + } |
| +} |
| + |
| +// A parent that holds a process directly is refused whatever children it |
| +// has (plan review #12): an empty g1 left from an earlier generation does |
| +// not make the parent a generation parent. |
| +func TestAPopulatedParentWithAStaleEmptyGenerationIsRefused(t *testing.T) { |
| + const host = "h.example.com" |
| + m, parent, _ := legacyParent(t, host, "4242\n") |
| + if err := os.MkdirAll(filepath.Join(parent, "g1"), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + writeFixture(t, filepath.Join(parent, "g1", "cgroup.procs"), "") |
| + writeFixture(t, filepath.Join(parent, "cgroup.events"), "populated 1\nfrozen 0\n") |
| + calls := fakeStart(t, func(*exec.Cmd) error { return nil }) |
| + if _, err := m.StartChild(host, GenChild(2), TierDefault, TierDefault, buildTrue); !errors.Is(err, ErrLegacyParentPopulated) { |
| + t.Fatalf("StartChild = %v, want ErrLegacyParentPopulated", err) |
| + } |
| + if len(*calls) != 0 { |
| + t.Fatalf("started %d times", len(*calls)) |
| + } |
| + if _, serr := os.Stat(filepath.Join(parent, "g2")); !os.IsNotExist(serr) { |
| + t.Errorf("the refused start created g2 (%v)", serr) |
| + } |
| +} |
| + |
| +// The refusal is about direct members only. A parent whose processes all |
| +// live in generation children is the layout working: cgroup.procs is empty |
| +// and cgroup.events reports the descendants. |
| +func TestAnEmptyParentWithLiveGenerationsStillStartsASuccessor(t *testing.T) { |
| + const host = "h.example.com" |
| + m, parent, _ := legacyParent(t, host, "") |
| + if err := os.MkdirAll(filepath.Join(parent, "g1"), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + writeFixture(t, filepath.Join(parent, "g1", "cgroup.procs"), "5151\n") |
| + writeFixture(t, filepath.Join(parent, "cgroup.events"), "populated 1\nfrozen 0\n") |
| + writeFixture(t, filepath.Join(parent, "memory.max"), "1073741824") |
| + calls := fakeStart(t, func(*exec.Cmd) error { return nil }) |
| + if _, err := m.StartChild(host, GenChild(2), TierDefault, TierDefault, buildTrue); err != nil { |
| + t.Fatalf("StartChild beside a live generation: %v", err) |
| + } |
| + if len(*calls) != 1 { |
| + t.Fatalf("started %d times, want 1", len(*calls)) |
| + } |
| + if got := readFixture(t, filepath.Join(parent, "memory.max")); got != "1073741824" { |
| + t.Errorf("the successor's start lowered the raised parent to %q", got) |
| + } |
| +} |
| + |
| +// A parent cgroup.procs that cannot be read is not known to be empty, so it |
| +// is refused as populated is (parentPopulated's rule: unreadable counts as |
| +// populated). A parent that has no cgroup.procs yet is a fresh directory. |
| +func TestAnUnreadableParentProcsFileIsRefusedAndAnAbsentOneIsNot(t *testing.T) { |
| + const host = "h.example.com" |
| + m, parent, _ := legacyParent(t, host, "") |
| + if err := os.Remove(filepath.Join(parent, "cgroup.procs")); err != nil { |
| + t.Fatal(err) |
| + } |
| + fakeStart(t, func(*exec.Cmd) error { return nil }) |
| + if _, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, buildTrue); err != nil { |
| + t.Fatalf("a parent with no cgroup.procs file: %v", err) |
| + } |
| + if err := os.MkdirAll(filepath.Join(parent, "cgroup.procs"), 0o755); err != nil { // reading a directory fails |
| + t.Fatal(err) |
| + } |
| + if _, err := m.StartChild(host, GenChild(2), TierDefault, TierDefault, buildTrue); !errors.Is(err, ErrLegacyParentPopulated) { |
| + t.Fatalf("StartChild over an unreadable cgroup.procs = %v, want ErrLegacyParentPopulated", err) |
| + } |
| +} |
diff --git a/internal/host/stagerelay.go b/internal/host/stagerelay.go| index 64be9d048..277b43e70 100644 |
| --- a/internal/host/stagerelay.go |
| +++ b/internal/host/stagerelay.go |
| @@ -2,6 +2,7 @@ package host |
| import ( |
| "context" |
| + "log" |
| "sync" |
| "sync/atomic" |
| "time" |
| @@ -12,6 +13,15 @@ import ( |
| "github.com/carlosframework/platform/internal/rollout" |
| ) |
| +// relayGiveUp is how long the relay asks about one head. Root can fail to |
| +// answer for good (its unit is broken, or the head is one it will never |
| +// stage and never refuse), and without a bound the agent would touch the |
| +// marker every stageRequestEvery for as long as the head stayed on the |
| +// channel, each touch starting a root pass. After an hour it stops and says |
| +// so; the next blocked pass that still finds the head unstaged notes it |
| +// again, which starts a fresh hour (Astra R1 T15). |
| +const relayGiveUp = time.Hour |
| + |
| // stageRelay retransmits the stage request (spec §7.7). carlos-stage.path |
| // does not act on a change made while carlos-stage.service is running, so a |
| // touch that lands mid-pass is lost; this asks again every stageRequestEvery |
| @@ -25,6 +35,25 @@ type stageRelay struct { |
| // over the data dir): nil when there is none. |
| refused func(host string) (*release.Refusal, error) |
| touch func() |
| + // now and logf are the clock and the log line; nil is time.Now and |
| + // log.Printf. They are seams for the give-up. |
| + now func() time.Time |
| + logf func(format string, args ...any) |
| +} |
| + |
| +func (r *stageRelay) clock() time.Time { |
| + if r.now != nil { |
| + return r.now() |
| + } |
| + return time.Now() |
| +} |
| + |
| +func (r *stageRelay) log(format string, args ...any) { |
| + if r.logf != nil { |
| + r.logf(format, args...) |
| + return |
| + } |
| + log.Printf(format, args...) |
| } |
| // pendingHead is a head the relay is asking root to stage, with the version |
| @@ -36,6 +65,9 @@ type pendingHead struct { |
| ver string |
| base string |
| baseSet bool // false while the row could not be read at note time |
| + // since is when the relay first noted this head: relayGiveUp counts |
| + // from it. A head noted again keeps it; a newer head starts its own. |
| + since time.Time |
| } |
| // installed is the host's installed Version as of now: ok is false when the |
| @@ -67,7 +99,7 @@ func (r *stageRelay) note(host, ver string) { |
| } |
| return |
| } |
| - r.pending[host] = pendingHead{ver: ver, base: base, baseSet: ok} |
| + r.pending[host] = pendingHead{ver: ver, base: base, baseSet: ok, since: r.clock()} |
| } |
| // tick touches the marker once if any head is still outstanding. A head is |
| @@ -77,7 +109,9 @@ func (r *stageRelay) note(host, ver string) { |
| // record names it, or when the route is gone. Without the baseline rule a |
| // head that was overtaken would be asked for every stageRequestEvery, each |
| // touch starting a root pass, until the route went. A row or a record that |
| -// cannot be read is no outcome, so it keeps the head pending. |
| +// cannot be read is no outcome, so it keeps the head pending, until |
| +// relayGiveUp has passed since it was first noted: then it is dropped with a |
| +// log line, and a later blocked pass notes it afresh. |
| func (r *stageRelay) tick() bool { |
| r.mu.Lock() |
| snap := make(map[string]pendingHead, len(r.pending)) |
| @@ -87,6 +121,19 @@ func (r *stageRelay) tick() bool { |
| r.mu.Unlock() |
| outstanding := false |
| for host, p := range snap { |
| + if waited := r.clock().Sub(p.since); waited >= relayGiveUp { |
| + r.mu.Lock() |
| + cur, have := r.pending[host] |
| + gone := have && cur.ver == p.ver && cur.since.Equal(p.since) // a newer head noted meanwhile stays |
| + if gone { |
| + delete(r.pending, host) |
| + } |
| + r.mu.Unlock() |
| + if gone { |
| + r.log("host: stage relay: giving up on %s for %s after %v: root has neither installed nor refused it; the next blocked pass will ask again", p.ver, host, waited.Round(time.Second)) |
| + } |
| + continue |
| + } |
| rt, err := r.get(host) |
| if err != nil { |
| outstanding = true |
diff --git a/internal/host/stagerelay_test.go b/internal/host/stagerelay_test.go| index f3e096282..601a920a9 100644 |
| --- a/internal/host/stagerelay_test.go |
| +++ b/internal/host/stagerelay_test.go |
| @@ -3,8 +3,10 @@ package host |
| import ( |
| "context" |
| "errors" |
| + "fmt" |
| "os" |
| "path/filepath" |
| + "strings" |
| "sync/atomic" |
| "testing" |
| "time" |
| @@ -271,3 +273,98 @@ func TestTheFlagOffLeavesTheStageRelayUnarmed(t *testing.T) { |
| t.Fatal("CARLOS_SWAP unset armed the stage relay") |
| } |
| } |
| + |
| +// Astra R1 T15: root can fail to answer for good, and the relay must not |
| +// touch the marker for as long as the head stays on the channel. After |
| +// relayGiveUp with neither an install nor a refusal it drops the head with a |
| +// log line and stops touching; the next blocked pass notes it again, which |
| +// asks for another relayGiveUp. |
| +func TestTheRelayGivesUpOnAHeadRootNeverAnswersAndNotesItAgain(t *testing.T) { |
| + rows := map[string]*registry.Route{"a": {Host: "a", Version: "v1"}} |
| + now := time.Date(2026, 10, 12, 9, 0, 0, 0, time.UTC) |
| + touches := 0 |
| + var lines []string |
| + r := &stageRelay{pending: map[string]pendingHead{}, |
| + get: func(h string) (*registry.Route, error) { return rows[h], nil }, |
| + refused: noRefusal, |
| + touch: func() { touches++ }, |
| + now: func() time.Time { return now }, |
| + logf: func(f string, a ...any) { lines = append(lines, fmt.Sprintf(f, a...)) }} |
| + r.note("a", "v2") |
| + now = now.Add(relayGiveUp - time.Second) |
| + if !r.tick() || touches != 1 { |
| + t.Fatalf("just under the give-up: outstanding or touches wrong (touches %d)", touches) |
| + } |
| + if len(lines) != 0 { |
| + t.Fatalf("logged before the give-up: %q", lines) |
| + } |
| + now = now.Add(time.Second) |
| + if r.tick() || touches != 1 { |
| + t.Fatalf("at the give-up the relay still asked (touches %d)", touches) |
| + } |
| + if len(lines) != 1 || !strings.Contains(lines[0], "v2") || !strings.Contains(lines[0], "a") { |
| + t.Fatalf("give-up log = %q, want one line naming the head and the host", lines) |
| + } |
| + if len(r.pending) != 0 { |
| + t.Fatalf("the dropped head is still pending: %v", r.pending) |
| + } |
| + if r.tick() || touches != 1 || len(lines) != 1 { |
| + t.Fatalf("a tick after the drop touched or logged again (touches %d, lines %q)", touches, lines) |
| + } |
| + r.note("a", "v2") // the next blocked pass |
| + if !r.tick() || touches != 2 { |
| + t.Fatalf("the head noted again was not asked for (touches %d)", touches) |
| + } |
| + now = now.Add(relayGiveUp - time.Second) |
| + if !r.tick() { |
| + t.Fatal("the fresh hour ended early: a re-noted head must count from the new note") |
| + } |
| +} |
| + |
| +// The hour counts from the first note of a head, not from each note: a |
| +// blocked pass every few seconds re-notes the same head, and that must not |
| +// keep it alive for ever. A newer head is a different request and starts its |
| +// own hour. |
| +func TestTheRelayCountsTheHourFromTheFirstNoteOfAHead(t *testing.T) { |
| + now := time.Date(2026, 10, 12, 9, 0, 0, 0, time.UTC) |
| + touches := 0 |
| + r := &stageRelay{pending: map[string]pendingHead{}, |
| + get: func(h string) (*registry.Route, error) { return ®istry.Route{Host: h, Version: "v1"}, nil }, |
| + refused: noRefusal, |
| + touch: func() { touches++ }, |
| + now: func() time.Time { return now }, |
| + logf: func(string, ...any) {}} |
| + r.note("a", "v2") |
| + for range 4 { |
| + now = now.Add(15 * time.Minute) |
| + r.note("a", "v2") |
| + } |
| + if r.tick() { |
| + t.Fatal("re-noting the same head kept it past the hour") |
| + } |
| + r.note("a", "v3") |
| + now = now.Add(relayGiveUp - time.Minute) |
| + if !r.tick() { |
| + t.Fatal("a newer head did not get its own hour") |
| + } |
| +} |
| + |
| +// Root's outcome still wins over the clock: a head installed or refused |
| +// inside the hour is dropped quietly, with no give-up line. |
| +func TestTheRelayLogsNoGiveUpForAnAnsweredHead(t *testing.T) { |
| + rows := map[string]*registry.Route{"a": {Host: "a", Version: "v1"}} |
| + now := time.Date(2026, 10, 12, 9, 0, 0, 0, time.UTC) |
| + var lines []string |
| + r := &stageRelay{pending: map[string]pendingHead{}, |
| + get: func(h string) (*registry.Route, error) { return rows[h], nil }, |
| + refused: noRefusal, |
| + touch: func() {}, |
| + now: func() time.Time { return now }, |
| + logf: func(f string, a ...any) { lines = append(lines, fmt.Sprintf(f, a...)) }} |
| + r.note("a", "v2") |
| + now = now.Add(time.Minute) |
| + rows["a"].Version = "v2" |
| + if r.tick() || len(lines) != 0 { |
| + t.Fatalf("an installed head: outstanding or logged %q", lines) |
| + } |
| +} |