plans: Astra P6 fixed (ownership gate on every exec, sidecar cgroup proof, failure before worker publication, claims-on graceful retire, incarnation-gated attempts, flag-independent worker pins)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+1185
−121
docs/superpowers/plans/2026-10-07-zero-downtime-deploys-0-index.md+64 −5docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md+521 −76docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md+600 −40
CI passed — run details
diff --git a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-0-index.md b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-0-index.md| index a295d5598..b121ba846 100644 |
| --- a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-0-index.md |
| +++ b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-0-index.md |
| @@ -214,7 +214,10 @@ type StagedIdentity struct { |
| plan 5 ships, while an **unreadable** file means an incomplete pin |
| set; |
| - so plan 5's agent writes the file **before its first spawn**, and |
| - rewrites it at every change to an operation set. |
| + rewrites it at every change to an operation set; |
| + - the agent writes it **whatever `CARLOS_SWAP` says** (P5 review |
| + finding 6): plan 4's captured-artifact recycle runs with the flag off, |
| + so a worker's captured serving build must be pinned with it off too. |
| - **Plan 2's decisions D1–D6, accepted, with one guard on D1.** |
| - **D1:** each nested generation unit gets its slice from a runtime |
| drop-in plus `daemon-reload`. **Measure `daemon-reload` on dev**, on a |
| @@ -480,6 +483,57 @@ Per finding: |
| - **3** (plan 3 Tasks 4, 10): the hostguard refusals from the claimed |
| spawn's last `Work.With` classify as abandonment. |
| +## Fixes for Astra's P6 review of the plans (2026-10-08) |
| + |
| +`CODEX-FINDINGS-ZDD-PLANS-P6.md`: three partial P5 fixes and eight new |
| +findings. The rules that close them, each with its own test: |
| + |
| +- **Ownership at every exec** (`ownedLocked`, plan 3 Task 3): the gate's |
| + ownership half, apart from supersession. Every spawn asks it at its |
| + exec: a worker respawn (crash restart, attempt, fallback, restore, |
| + whatever the flag), a planned wake's fallback (`wakeOwned`), a claimed |
| + planned wake (`StartPolicy.Gate`), a legacy sidecar (`startSidecar`). |
| + Planned wakes also re-check the selected build at the exec, after the |
| + restore (`WakePlan.Check`, `checkPlan`, `StartPolicy.PinCheck`). |
| +- **A termination proof for every process kind**: legacy sidecars get |
| + `sidecarTerminated` (the leader's exit and an empty sidecar cgroup, |
| + `KillSidecar`). |
| +- **Failure before publication everywhere**: a wake publishes a worker's |
| + identity only after `afterWake` (so after `GenFellBack`'s record); a |
| + `ReplaceWorker` fallback holds its publication until the scheduler's |
| + `done()` has recorded the failure (`PublishHeldServing`). |
| + |
| +Per finding: |
| + |
| +- **P1 #6, completed** (plan 5 Tasks 9, 11): see the third rule. Tested |
| + on the real fallback wake with a blocked callback. |
| +- **New #1, completed** (plan 5 Task 11): the fallback respawn after B's |
| + crash asks `ownedLocked` at its exec; a lost lease spawns nothing. |
| +- **New #3, completed** (plan 3 Tasks 4, 10): `ErrNoClaim`, the other |
| + claim-read sentinels and any error a claims-on spawn meets before its |
| + exec hook (`claimAdmission`, `errClaimsAdmission`) are NoOwnership. |
| +- **1** (plan 5 Task 11): as new #1. |
| +- **2** (plan 3 Tasks 2, 7): a discarded, replaced or co-stopped sidecar is |
| + forgotten only once `sidecarTerminated` holds; otherwise it is a tracked |
| + stray (`straySidecars` now keeps its route), nothing starts a sidecar |
| + beside it, and `releasable` keeps the lease. |
| +- **3** (plan 5 Tasks 9, 10, 12): the planner's `Check` refuses a first |
| + wake's build that a failure recorded since the plan holds; the wake is |
| + replanned once on the restored state, or abandoned (`errPlanStale`, |
| + recording nothing). A replanned build is published before it execs. |
| +- **4** (plan 3 Task 10): `stopMember` retires a serving claims-on |
| + generation with `claimProcess.Retire` (SIGTERM, `actStopTimeout`, then |
| + `KillAndEmpty` and the proof); an unready candidate is still killed at |
| + once. |
| +- **5** (plan 5 Task 11): `ReplaceWorker` counts its attempt only if |
| + `workerInc` moved; a failed exec that left the old child is a staging |
| + refusal, never `SwapDone`. |
| +- **6** (plan 5 Tasks 12, 14): the agent sets `GenerationsFile` and |
| + `OnWorkerServing` whatever the flag (`armAlways`). |
| +- **7** (plan 5 Tasks 9, 11): as P1 #6. |
| +- **8** (plan 5 Task 11): an empty version is published through the same |
| + ordered writer, clearing the obsolete `ServingVersion`. |
| + |
| ## Cross-plan interface ledger |
| Every type, function, method or field that one plan defines and another |
| @@ -551,10 +605,12 @@ many line numbers in plans 1, 3 and 5; find each use by its quoted name. |
| | `instancectl.Client.StartSidecarExe(ctx, host string, memoryMax int64, x StagedExe) (string, error)`; `Server.ensureHostExeAt(host, x)` | P3 T9 | P3 only | |
| | `func (a *Service) swapChecksLocked(e *actEntry, at Attempt, newest func(string) uint64) error` (now `attemptCurrentLocked(e, at, newest, false)`); `errSwapNotAwake`, `errSwapBusy`, `errSwapKind`, `errSwapStopped`; `func abandon(err error) SwapResult` | P3 T3, T4 | P5 T1 L172, T11 (`ReplaceWorker`, its `attemptGate`) | |
| | **The attempt gate**: `func (a *Service) attemptCurrentLocked(e *actEntry, at Attempt, newest func(string) uint64, waking bool) error`; `memberCurrentLocked(e, m, mctx, at, newest)`. Every exec point and completion path asks it: overlap Step 3a, the switch, `spawnSuccessor`'s claims `before` hook, a hold's `pinCheck` and result, `ReplaceWorker` | P3 T3 | P3 T4, T6, T10; P5 T11 (through `swapChecksLocked`) | |
| -| **The outcome classifier**: `type stepOutcome`; `stepOK`, `stepCancelled`, `stepStale`, `stepFailed`; `func classifyStep(lifecycle context.Context, err error) stepOutcome`; `stepResult`; the gate's typed verdict: `type gateVerdict`, `gateProceed`, `gateSuperseded`, `gateNoOwnership`, `func verdictOf(err error) gateVerdict` (every non-Proceed verdict, hostguard claims refusals included, classifies `stepStale`) | P3 T4 | P3 T4, T6, T10; P5 T9 (`spawnPlanned`, `spawnPlannedWorker`: only `stepFailed` records a failure or falls back), T11 (`ReplaceWorker`'s respawn gate: only `gateSuperseded` may restore) | |
| +| **The ownership check**: `func (a *Service) ownedLocked(e *actEntry, waking bool) error` (the gate's ownership half, apart from supersession; every refusal is `gateNoOwnership`). Asked at EVERY exec: `startSidecar`, every worker respawn, a planned wake's fallback (`wakeOwned`), a claimed planned wake (`StartPolicy.Gate`) | P3 T3 | P3 T7; P5 T9 (`wakeOwned`, `doClaimWake`), T11 (`respawnWorkerLocked`'s "still wanted?") | |
| +| **The outcome classifier**: `type stepOutcome`; `stepOK`, `stepCancelled`, `stepStale`, `stepFailed`; `func classifyStep(lifecycle context.Context, err error) stepOutcome`; `stepResult`; the gate's typed verdict: `type gateVerdict`, `gateProceed`, `gateSuperseded`, `gateNoOwnership`, `func verdictOf(err error) gateVerdict` (every non-Proceed verdict, hostguard claims refusals included, `ErrNoClaim` too, classifies `stepStale`); `errClaimsAdmission`, `func claimAdmission(err error, reached bool) error` (a claims-on spawn's error before its exec hook, a failed claim read included, is NoOwnership) | P3 T4 (used by T10's `startClaimedSuccessor` and `startAdmitted`) | P3 T4, T6, T10; P5 T9 (`spawnPlanned`, `spawnPlannedWorker`: only `stepFailed` records a failure or falls back), T11 (`ReplaceWorker`'s respawn gate: only `gateSuperseded` may restore) | |
| | **The termination proof**: `func (a *Service) genTerminated(rt registry.Route, g Gen, p Proc) bool`; claims on: `func (p *claimProcess) provenStopped(ctx) bool`; workers: `Gen.Worker bool`, `type hostKiller interface{ KillHost(ctx, rt) error }`, `ExecBackend.KillHost`, `SplitBackend.KillHost` (P3 T11; no nested-layout exemption); the lease rule: `func (a *Service) releasable(e *actEntry) bool` (no member, no stray sidecar, current generation `genTerminated`) | P3 T2, T11 | every lease- or set-giving path in P3 (member stop, reaper, `stop()` and `reapCurrentGen` through `releasable`, `cleanupFailedWake`, `reapUnprovenWakesLocked`, a worker respawn's retire and shed, a nil-`stopOld` respawn); P5 T9 (`killFailed` for a worker) | |
| -| `StartPolicy.Gate func() error` (the attempt gate at every claims-on exec, in `startAdmitted`'s `beforeStart`) | P3 T10 (set by T6's `doClaimWake` hold block) | P3 only | |
| -| `func (a *Service) discardSidecar(host string, sc Proc)`; `trackStraySidecar`, `reapStraySidecarsLocked`, `Service.straySidecars` (`sidecarJoinBound` is gone: with generations on the join has no timeout) | P3 T7 (`straySidecars` declared in T2, read by `releasable`) | P3 only | |
| +| `StartPolicy.Gate func() error` (the attempt gate, or a planned wake's ownership check, at every claims-on exec, in `startAdmitted`'s `beforeStart`); `StartPolicy.PinCheck func(pin Gen) error` (the planner's selection gate for the pin about to exec) | P3 T10 (set by T6's `doClaimWake` hold block) | P5 T9 (`doClaimWake` sets both for a planned wake) | |
| +| `func (p *claimProcess) Retire(ctx context.Context, grace time.Duration) error` (SIGTERM and the drain, then `Stop`'s `KillAndEmpty` and proof); `stopMember` retires a claims-on `roleRetiring` member through it | P3 T10 | P3 only | |
| +| `func (a *Service) discardSidecar(rt registry.Route, sc Proc)`; **the sidecar termination proof** `func (a *Service) sidecarTerminated(rt registry.Route, sc Proc) bool`; `type sidecarKiller interface{ KillSidecar(ctx, rt) error }`, `ExecBackend.KillSidecar`, `SplitBackend.KillSidecar`; `func (a *Service) startSidecar(ctx, e *actEntry, rt, g Gen, waking bool) (Proc, error)` (ownership at the exec; refuses while a stray is tracked); `errSidecarUnproven`; `trackStraySidecarLocked`, `reapStraySidecarsLocked`, `Service.straySidecars map[string][]*straySidecar` (`sidecarJoinBound` is gone: with generations on the join has no timeout) | P3 T7 (`straySidecars` and `type straySidecar` declared in T2, read by `releasable`) | P3 only | |
| | `var closeStrictParent = (*cgroup.Prepared).Close`; `Service.unclosedParents`, `func (a *Service) retryUnclosedParentsLocked()` (an empty strict parent is closed, not `CloseRemove`d, and its handle kept until `Close` succeeds) | P3 T10 | P3 only (consumes P2 T3's `Prepared.Close`) | |
| | `actEntry.ops opSet` (`members map[uint64]*genMember`, `genMember.gen`); `addMemberLocked`; `leaveSet` | P3 T2 | P5 T1, T12 L6170, L6366, L6419 | |
| | `swapOverlap`'s `// Step 3a` gate block, before `g, proc, spawnErr := a.spawnSuccessor(mctx, gb, e, rt, work, m.gen, gate)` (`spawnSuccessor` and `startClaimedSuccessor` gain `gate func() error`) | P3 T4, T10 | P5 T12 (persist point, before Step 3a; a failed write abandons the attempt there) | |
| @@ -599,7 +655,10 @@ many line numbers in plans 1, 3 and 5; find each use by its quoted name. |
| | `Manager.served`, `publishServing`, `acknowledged`, `writeGood`/`writeGoodLocked`, `Manager.goodMu` (every serving- and good-build write is for the generation the activator serves, serialised; `done` publishes before it frees the slot) | P5 T8 | P5 T10 (`OnGen`) | |
| | `generationsLocked` includes `unprovenWakes` | P5 T12 | consumes P3 T2's `unprovenWakes` | |
| | `WakePlan.FailedUnder`, `GenEvent.FailedUnder` (a first wake's failing pointer, read by the planner before the wake runs) | P5 T9, T10 | P5 only (`buildFailed`) | |
| -| `Service.OnWorkerServing func(host string, id release.StagedIdentity)` (flag-independent; the agent writes `ServingVersion` with it); `func (a *Service) publishWorkerServing(e *actEntry, id release.StagedIdentity, inc uint64)`, `Service.servingMu`, `actEntry.workerInc` (delivered only for the entry's latest worker incarnation) | P5 T11, T14 | P6 T13 (`carlos sidecars ls` reads `ServingVersion`) | |
| +| `WakePlan.Check func(build string) error` (the planner's selection gate); `errPlanStale` (wraps `errSwapSuperseded`); `func (a *Service) checkPlan(ctx, e, rt, plan WakePlan) (WakePlan, error)` (at the exec, after the restore; replans once); `func (a *Service) wakeOwned(ctx, e) error` (ownership at a fallback's exec); `func (m *Manager) planCheck(rt, plan) func(string) error` | P5 T9, T10 | P5 T9 (`spawnPlanned`, `spawnPlannedWorker`, `doClaimWake`'s `PinCheck`), T12 (`checkPlan` publishes a replanned build) | |
| +| `Service.OnWorkerServing func(host string, id release.StagedIdentity)` (flag-independent; the agent writes `ServingVersion` with it, an empty version included); `func (a *Service) publishWorkerServing(e *actEntry, id release.StagedIdentity, inc uint64)`, `Service.servingMu`, `actEntry.workerInc` (delivered only for the entry's latest worker incarnation; a `ReplaceWorker` attempt counts only once it moved); published after `afterWake`, never before | P5 T11, T14 | P6 T13 (`carlos sidecars ls` reads `ServingVersion`) | |
| +| `func (a *Service) PublishHeldServing(host string)`; `type heldServing`; `actEntry.holdServing`, `actEntry.heldServing` (a `ReplaceWorker` fallback's publication, released after `done()` records the failure); `rollout.Activator.PublishHeldServing`, `fakeAct.onHeldPublish` | P5 T11 | P5 T11 (`execute`) | |
| +| `func armAlways(svc *activator.Service, reg *registry.Registry, dd string)` (whatever the flag: `OnWorkerServing` and `GenerationsFile`) | P5 T14 | P5 only | |
| | `func (m *Manager) setServingLocked(host string, g activator.Gen)` (the scheduler's only `SetServingVersion`; skips a `Gen.Worker` generation, whose serving build is the activator's alone) | P5 T8 | P5 T10 (`OnGen`), T11 | |
| | `fakeRoutes.setServingHook` (test seam: runs at each `SetServingVersion`) | P5 T8 | P5 T10 | |
| | `func plannedWorkerReady(ctx context.Context, p Proc) error` (spec §6.5 for planned socketless wakes) | P5 T9 | P5 only | |
diff --git a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md| index 0225ac6b9..412fdfe91 100644 |
| --- a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md |
| +++ b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-3-swap.md |
| @@ -1347,7 +1347,7 @@ func TestAStopKeepsTheLeaseWhileAStraySidecarRuns(t *testing.T) { |
| x := awakeForBarrier(t, true) |
| stray := &fakeProc{label: "sidecar", rec: x.b, stuck: true} |
| x.a.mu.Lock() |
| - x.a.straySidecars = map[string][]Proc{x.host: {stray}} |
| + x.a.straySidecars = map[string][]*straySidecar{x.host: {{proc: stray, rt: testRoute(x.host)}}} |
| x.a.mu.Unlock() |
| rep := x.b.lastRep() |
| x.a.Bounce(x.host) |
| @@ -2010,7 +2010,17 @@ func (a *Service) releasable(e *actEntry) bool { |
| } |
| ``` |
| -Add the field it reads to `Service` now: `straySidecars map[string][]Proc // legacy sidecars whose stop is unproven, per host; Task 7 tracks into it; under a.mu`. |
| +Add the field it reads to `Service` now: `straySidecars map[string][]*straySidecar // legacy sidecars whose termination is unproven, per host; Task 7 tracks into it; under a.mu`, and the type beside `genMember` in `opset.go` (Task 7 documents and uses it): |
| + |
| +```go |
| +// straySidecar is a legacy sidecar whose termination is unproven, with the |
| +// route its proof needs (Task 7). reaping marks a re-proof in flight. |
| +type straySidecar struct { |
| + proc Proc |
| + rt registry.Route |
| + reaping bool |
| +} |
| +``` |
| Add to `claimwake.go`, beside `claimProcess.Stop`: |
| @@ -2193,7 +2203,8 @@ git commit -m "zdd plan 3 task 2: the entry's operation set, the barrier in stop |
| func (a *Service) selectWork(ctx context.Context, host string) (hostwrite.Work, error) |
| func (a *Service) admit(ctx context.Context, work hostwrite.Work, fn func() error) error |
| func (a *Service) swapChecksLocked(e *actEntry, at Attempt, newest func(string) uint64) error // = attemptCurrentLocked(e, at, newest, false) |
| - func (a *Service) attemptCurrentLocked(e *actEntry, at Attempt, newest func(string) uint64, waking bool) error // THE attempt gate |
| + func (a *Service) attemptCurrentLocked(e *actEntry, at Attempt, newest func(string) uint64, waking bool) error // THE attempt gate: ownedLocked, then supersession |
| + func (a *Service) ownedLocked(e *actEntry, waking bool) error // the gate's ownership half, asked at EVERY exec (wake, fallback, respawn, restore, sidecar) |
| func (a *Service) memberCurrentLocked(e *actEntry, m *genMember, mctx context.Context, at Attempt, newest func(string) uint64) error |
| // test helpers reused by Tasks 4 and 6: swapWork, swapSelector, errClaimChanged |
| ``` |
| @@ -2316,6 +2327,38 @@ func TestSwapChecks(t *testing.T) { |
| // Claims off: hostwrite's legacy work. The check runs alone, under the |
| // entry lock that fn takes itself. |
| +// Spec §7.4: ownership is checked apart from supersession, so an exec that |
| +// is not an attempt's (a crash restart, a fallback, a restore, a sidecar) |
| +// can ask it. A superseded attempt still owns the host; a lost lease does |
| +// not, whatever the revision says. |
| +func TestOwnershipIsCheckedApartFromSupersession(t *testing.T) { |
| + a, b, _, _ := testActivator(t, time.Hour) |
| + host := "owned.example.com" |
| + arrangeAwake(t, a, testRoute(host)) |
| + owned := func(waking bool) error { |
| + a.mu.Lock() |
| + defer a.mu.Unlock() |
| + return a.ownedLocked(a.entries[host], waking) |
| + } |
| + if err := owned(false); err != nil { |
| + t.Fatalf("an awake, leased, unfenced entry: %v", err) |
| + } |
| + b.lastRep().setErr(errors.New("renewal failed")) |
| + if err := owned(false); !errors.Is(err, errSwapLeaseLost) { |
| + t.Fatalf("lease lost: %v", err) |
| + } |
| + if err := owned(true); err != nil { |
| + t.Fatalf("a waking entry's lease is the wake's own: %v", err) |
| + } |
| + b.lastRep().setErr(nil) |
| + a.mu.Lock() |
| + a.fences = map[string]uint64{host: 1} |
| + a.mu.Unlock() |
| + if err := owned(true); !errors.Is(err, errSwapFenced) { |
| + t.Fatalf("fenced while waking: %v", err) |
| + } |
| +} |
| + |
| func TestAdmitClaimsOffRunsTheCheckAlone(t *testing.T) { |
| a := New(Config{}, &fakeBackend{}, nil) |
| work, err := a.selectWork(context.Background(), "h.example.com") |
| @@ -2363,8 +2406,8 @@ func TestAdmitClaimsOnValidatesFirstAndNeverFallsBack(t *testing.T) { |
| - [ ] **Step 2: Run the tests to verify they fail** |
| -Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestSwapChecks|TestAdmit' -count=1` |
| -Expected: build failure: `undefined: errSwapSuperseded`, `a.selectWork undefined`. |
| +Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestSwapChecks|TestOwnership|TestAdmit' -count=1` |
| +Expected: build failure: `undefined: errSwapSuperseded`, `a.selectWork undefined`, `a.ownedLocked undefined`. |
| - [ ] **Step 3: Write the code** |
| @@ -2437,19 +2480,38 @@ func (a *Service) swapChecksLocked(e *actEntry, at Attempt, newest func(string) |
| // are checked. Otherwise the entry must be awake with its lease. The caller |
| // holds a.mu. |
| func (a *Service) attemptCurrentLocked(e *actEntry, at Attempt, newest func(string) uint64, waking bool) error { |
| + if e == nil || a.entries[at.Host] != e { |
| + return errSwapNotAwake |
| + } |
| + if err := a.ownedLocked(e, waking); err != nil { |
| + return err |
| + } |
| + if newest(at.Host) != at.SchedRev { |
| + return errSwapSuperseded |
| + } |
| + return nil |
| +} |
| + |
| +// ownedLocked is the gate's OWNERSHIP check, apart from supersession (spec |
| +// §7.4). Every exec asks it, an attempt's or not: a wake's and its |
| +// fallback's, a worker respawn's (a crash restart, an attempt, its fallback, |
| +// its restore) and a legacy sidecar's. Its refusals (errSwapNotAwake, |
| +// errSwapLeaseLost, errSwapFenced, errSwapShutdown) are all gateNoOwnership: |
| +// nothing spawns, a restore included. waking is an entry still waking, |
| +// whose lease the wake holds itself (doWake's lease, not e.rep): the entry |
| +// must be the host's, with no fence and no shutdown. The caller holds a.mu. |
| +func (a *Service) ownedLocked(e *actEntry, waking bool) error { |
| switch { |
| - case e == nil || a.entries[at.Host] != e: |
| + case e == nil || a.entries[e.host] != e: |
| return errSwapNotAwake |
| case !waking && e.state != actAwake: // swapsim:M5 |
| return errSwapNotAwake |
| case !waking && (e.rep == nil || e.rep.Err() != nil): |
| return errSwapLeaseLost |
| - case a.fences[at.Host] != 0: |
| + case a.fences[e.host] != 0: |
| return errSwapFenced |
| case a.shuttingDown: |
| return errSwapShutdown |
| - case newest(at.Host) != at.SchedRev: |
| - return errSwapSuperseded |
| } |
| return nil |
| } |
| @@ -2471,7 +2533,7 @@ func (a *Service) memberCurrentLocked(e *actEntry, m *genMember, mctx context.Co |
| - [ ] **Step 4: Run the tests to verify they pass** |
| -Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestSwapChecks|TestAdmit' -count=1` |
| +Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestSwapChecks|TestOwnership|TestAdmit' -count=1` |
| Expected: `ok`. |
| - [ ] **Step 5: Commit** |
| @@ -2500,7 +2562,9 @@ git commit -m "zdd plan 3 task 3: swap admission through hostwrite, claims off a |
| type stepOutcome int; const stepOK, stepCancelled, stepStale, stepFailed stepOutcome |
| func classifyStep(lifecycle context.Context, err error) stepOutcome // THE outcome classifier: only stepFailed is a failure of the build |
| type gateVerdict int; const gateProceed, gateSuperseded, gateNoOwnership gateVerdict |
| - func verdictOf(err error) gateVerdict // the attempt gate's typed verdict; NoOwnership (lease, fence, shutdown, a claims refusal such as hostguard.ErrStampMismatch) stops every spawn, restores included |
| + func verdictOf(err error) gateVerdict // the attempt gate's typed verdict; NoOwnership (lease, fence, shutdown, a claims refusal such as hostguard.ErrStampMismatch or ErrNoClaim, a claim read that failed) stops every spawn, restores included |
| + var errClaimsAdmission error // a claims-on spawn's error before its exec hook ran: NoOwnership |
| + func claimAdmission(err error, reached bool) error |
| func stepResult(lifecycle context.Context, err error) (SwapResult, bool) |
| // Service gains: genHealth func(ctx context.Context, g Gen) error (test seam) |
| var errSwapOff, errSwapBusy, errSwapKind, errSwapStopped, errSuccessorExited, errSwapMode error |
| @@ -2556,6 +2620,9 @@ type fakeSwapBackend struct { |
| // sidecarStopGate, when set, is installed on every sidecar started from |
| // then on: its Stop blocks until the gate closes. |
| sidecarStopGate chan struct{} |
| + // sidecarDescendant: a process a sidecar forked is still in its cgroup, |
| + // so KillSidecar (Task 7) cannot prove it empty. |
| + sidecarDescendant bool |
| // tiers is each build's own tier (GenTier); a build not here is the |
| // default tier. |
| tiers map[string]cgroup.Limit |
| @@ -3072,6 +3139,11 @@ func TestVerdictOf(t *testing.T) { |
| {errSwapNotAwake, gateNoOwnership}, |
| {fmt.Errorf("with: %w", hostguard.ErrStampMismatch), gateNoOwnership}, |
| {fmt.Errorf("with: %w", hostguard.ErrFenced), gateNoOwnership}, |
| + // P5 review partial new #3: an absent claim, and a claim read that |
| + // failed before the exec, are no failure of the build either. |
| + {fmt.Errorf("with: %w", hostguard.ErrNoClaim), gateNoOwnership}, |
| + {claimAdmission(errors.New("s3: GetObject: request timeout"), false), gateNoOwnership}, |
| + {claimAdmission(errors.New("GET / answered 500"), true), gateProceed}, |
| } { |
| if got := verdictOf(tc.err); got != tc.want { |
| t.Errorf("verdictOf(%v) = %d, want %d", tc.err, got, tc.want) |
| @@ -3256,12 +3328,31 @@ func verdictOf(err error) gateVerdict { |
| errors.Is(err, hostguard.ErrInactive), errors.Is(err, hostguard.ErrRetiredStamp), |
| errors.Is(err, hostguard.ErrOldStamp), errors.Is(err, hostguard.ErrClaimIncarnation), |
| errors.Is(err, hostguard.ErrRecordIncarnation), errors.Is(err, hostguard.ErrNotParticipant), |
| - errors.Is(err, hostbox.ErrChanged): |
| + errors.Is(err, hostguard.ErrNoClaim), errors.Is(err, hostguard.ErrNoClaims), |
| + errors.Is(err, hostguard.ErrNoHolder), errors.Is(err, hostguard.ErrMalformedClaim), |
| + errors.Is(err, hostbox.ErrChanged), errors.Is(err, errClaimsAdmission): |
| return gateNoOwnership |
| } |
| return gateProceed |
| } |
| +// errClaimsAdmission marks an error a claims-on spawn met before its exec |
| +// hook ran (claimAdmission): the claim read failed (a store error, a |
| +// timeout), the claim was absent or malformed, or the work's validation |
| +// refused before the spawn. Nothing of the build ran, so it is never a |
| +// failure of the build (spec §7.4). |
| +var errClaimsAdmission = errors.New("the claims admission refused before the exec") |
| + |
| +// claimAdmission wraps err in errClaimsAdmission when the spawn never |
| +// reached its exec hook (reached false). An error after the hook ran is the |
| +// spawn's own, and is returned as it is. |
| +func claimAdmission(err error, reached bool) error { |
| + if err == nil || reached { |
| + return err |
| + } |
| + return fmt.Errorf("%w: %w", errClaimsAdmission, err) |
| +} |
| + |
| // stepResult is classifyStep for a swap step: a cancelled or stale step |
| // abandons the attempt (over is true); a failure of the build is the |
| // caller's to report (over is false). |
| @@ -4681,17 +4772,25 @@ git commit -m "zdd plan 3 task 6: the hold swap, a pinned wake installed by fini |
| func (b *ExecBackend) StartSidecarGen(ctx context.Context, rt registry.Route, g Gen) (Proc, error) |
| func (b *ExecBackend) startSidecarChildFrom(ctx context.Context, rt registry.Route, bin string) (Proc, error) // plan 2's body, placement kept; only the binary is a parameter |
| func (b *SplitBackend) StartSidecarGen(ctx context.Context, rt registry.Route, g Gen) (Proc, error) |
| - func (a *Service) startSidecar(ctx context.Context, rt registry.Route, g Gen) (Proc, error) |
| - func (a *Service) replaceSidecar(e *actEntry) // joins an in-flight supervised start, then replaces |
| - func (a *Service) discardSidecar(host string, sc Proc) // flag off: today's async stop; on: synchronous, before the join closes; unproven: tracked |
| - func (a *Service) trackStraySidecar(host string, sc Proc) |
| - func (a *Service) reapStraySidecarsLocked() |
| - // Service gains straySidecars map[string][]Proc; with generations on, stop() joins an in-flight sidecar start with no timeout |
| + func (a *Service) startSidecar(ctx context.Context, e *actEntry, rt registry.Route, g Gen, waking bool) (Proc, error) // ownedLocked at the exec; refuses while a stray is tracked |
| + var errSidecarUnproven error |
| + func (a *Service) replaceSidecar(e *actEntry) // joins an in-flight supervised start, then replaces once the old one is proven gone |
| + func (a *Service) discardSidecar(rt registry.Route, sc Proc) // flag off: today's async stop; on: synchronous, before the join closes; unproven: tracked |
| + func (a *Service) sidecarTerminated(rt registry.Route, sc Proc) bool // THE sidecar termination proof: leader exited AND its sidecar cgroup empty |
| + type sidecarKiller interface{ KillSidecar(ctx context.Context, rt registry.Route) error } |
| + func (b *ExecBackend) KillSidecar(ctx context.Context, rt registry.Route) error // role-sidecar child (flat), the sidecar unit's stop (nested) |
| + func (b *SplitBackend) KillSidecar(ctx context.Context, rt registry.Route) error |
| + // type straySidecar struct{ proc Proc; rt registry.Route; reaping bool } (declared in Task 2 with the field) |
| + func (a *Service) trackStraySidecarLocked(rt registry.Route, sc Proc) |
| + func (a *Service) reapStraySidecarsLocked() // re-stops and re-proves each, off the lock; forgets one only once proven |
| + // Service.straySidecars map[string][]*straySidecar (declared in Task 2); with generations on, stop() joins an in-flight sidecar start with no timeout |
| ``` |
| Three rules this task keeps (findings 2, 10 and 11 of the P1 review): |
| - **Placement.** `startSidecarChildFrom` is plan 2 Task 10's `startSidecarChild` body with one change: the binary. Plan 2's `placeFor(spawnSidecar, 0)`, `spawnInCgroupAt(rt, child, false, …)` and the goroutine's `RemoveChild(rt.Host, child)` stay. Going back to `spawnInCgroup` would put the tier and `memory.oom.group=1` back on the generation parent (spec §7.3) and, with controllers delegated, try to place a process in a parent that must hold none. |
| - **No live-link read for a nested sidecar.** The nested unit binds whatever root lays out, so a check of `LiveVersion` before the start is a race with a repoint, and after a fallback it refuses for ever. Until Task 9 the nested arm refuses outright (`errNestedSidecarGen`), the way Task 1's nested `StartStaged` does. Task 9 hands root the generation's captured staged executable, which root validates under the same rule as the run unit's. |
| - - **Replacement is never skipped, and never runs beside the old one.** A supervised restart in flight captured the generation it read when it began, which can be the one just retired. `replaceSidecar` waits for it (`sidecarStartDone`) and then replaces whatever it installed. The supervisor installs a sidecar it started for a retired generation (it does not discard it asynchronously), so it stays tracked: `replaceSidecar` stops it synchronously and starts the new one only once it has exited, and a terminal stop stops it like any sidecar. An old sidecar whose stop is unproven is kept as the entry's sidecar, nothing starts beside it, and the sweep retries the replacement (`actEntry.sidecarGen`, `wokenInstance.sidecarGen`: the generation a sidecar runs). |
| + - **Replacement is never skipped, and never runs beside the old one.** A supervised restart in flight captured the generation it read when it began, which can be the one just retired. `replaceSidecar` waits for it (`sidecarStartDone`) and then replaces whatever it installed. The supervisor installs a sidecar it started for a retired generation (it does not discard it asynchronously), so it stays tracked: `replaceSidecar` stops it synchronously and starts the new one only once it is proven gone, and a terminal stop stops it like any sidecar. An old sidecar whose termination is unproven becomes a tracked stray, nothing starts beside it, and the sweep re-proves it (`actEntry.sidecarGen`, `wokenInstance.sidecarGen`: the generation a sidecar runs). |
| + - **A sidecar is gone only when its cgroup is empty** (spec §7.4, §7.5; P5 review finding 2). `childProc.Stop` signals only the leader, so `Exited()` is not termination: a process the sidecar forked lives on in `role-sidecar`. `sidecarTerminated` is the leader's exit AND the sidecar cgroup proven empty (`KillSidecar`). Every path that would forget a sidecar asks it: a discard, a replacement, a terminal stop's co-stop, the stray reaper. An unproven one is tracked; while one is tracked nothing starts a sidecar for the host (`startSidecar`), no wake runs (`errWakeUnproven`), and no stop releases the lease (`releasable`). |
| + - **Ownership at the exec** (spec §7.4; P5 review rule a). `startSidecar` asks `ownedLocked` immediately before every sidecar spawn: the wake tail (waking), the supervisor and a replacement (awake). |
| - [ ] **Step 1: Write the failing tests** |
| @@ -4717,6 +4816,23 @@ func (g *fakeSwapBackend) StartSidecarGen(_ context.Context, _ registry.Route, g |
| return p, nil |
| } |
| +// KillSidecar is the sidecar cgroup's proof: it fails while a descendant |
| +// is "left" (setSidecarDescendant). |
| +func (g *fakeSwapBackend) KillSidecar(context.Context, registry.Route) error { |
| + g.gmu.Lock() |
| + defer g.gmu.Unlock() |
| + if g.sidecarDescendant { |
| + return errors.New("role-sidecar: still populated") |
| + } |
| + return nil |
| +} |
| + |
| +func (g *fakeSwapBackend) setSidecarDescendant(left bool) { |
| + g.gmu.Lock() |
| + g.sidecarDescendant = left |
| + g.gmu.Unlock() |
| +} |
| + |
| func (g *fakeSwapBackend) sidecarGenPaths() []string { |
| g.gmu.Lock() |
| defer g.gmu.Unlock() |
| @@ -4982,12 +5098,13 @@ func TestAStraySidecarIsNeverForgotten(t *testing.T) { |
| f.a.Bounce(f.host) |
| waitFor(t, "the instance asleep", func() bool { return f.a.entryCount() == 0 }) |
| stray := &fakeProc{label: "sidecar", rec: f.b.fakeBackend, stuck: true} |
| - f.a.discardSidecar(f.host, stray) |
| + f.a.discardSidecar(testRoute(f.host), stray) |
| if _, err := f.a.beginRequest(context.Background(), testRoute(f.host)); err == nil || !strings.Contains(err.Error(), errWakeUnproven.Error()) { |
| t.Fatalf("a wake beside an unproven sidecar = %v, want refused", err) |
| } |
| stray.exit() |
| - f.a.sweepOnce() |
| + f.a.sweepOnce() // the reaper re-proves it off the lock |
| + waitFor(t, "the stray proven gone", func() bool { return f.a.strayCount(f.host) == 0 }) |
| release, err := f.a.beginRequest(context.Background(), testRoute(f.host)) |
| if err != nil { |
| t.Fatalf("the wake once the stray exited: %v", err) |
| @@ -4995,6 +5112,48 @@ func TestAStraySidecarIsNeverForgotten(t *testing.T) { |
| release() |
| } |
| +func (a *Service) strayCount(host string) int { |
| + a.mu.Lock() |
| + defer a.mu.Unlock() |
| + return len(a.straySidecars[host]) |
| +} |
| + |
| +// Spec §7.4, §7.5: a legacy sidecar is gone only when its cgroup is empty. |
| +// childProc.Stop signals only the leader, so a process the sidecar forked |
| +// outlives its exit in role-sidecar. Here the leader exits but a descendant |
| +// stays (KillSidecar cannot empty the cgroup): the replacement does not |
| +// start beside it, and a terminal stop keeps the lease until it is proven |
| +// gone. Accepting Exited() started the new sidecar beside the descendant |
| +// and released the lease under it. |
| +func TestASidecarsDescendantBlocksItsReplacementAndTheLease(t *testing.T) { |
| + f := newSwapFixture(t) |
| + f.b.sidecarStaged = true |
| + f.a.sweepOnce() |
| + waitFor(t, "late attachment", func() bool { return f.a.sidecarSettled(f.host) && len(f.b.sidecarGenPaths()) == 1 }) |
| + f.b.setSidecarDescendant(true) |
| + if res := f.a.Swap(context.Background(), f.attempt(ModeOverlap), f.newest); res.Outcome != SwapDone { |
| + t.Fatalf("Swap = %+v", res) |
| + } |
| + f.a.sweepOnce() // a supervisor pass must not start one either |
| + time.Sleep(50 * time.Millisecond) |
| + if n := len(f.b.sidecarGenPaths()); n != 1 { |
| + t.Fatalf("%d sidecar starts: a new sidecar started beside the old one's descendant", n) |
| + } |
| + rep := f.b.lastRep() |
| + f.a.Bounce(f.host) |
| + time.Sleep(100 * time.Millisecond) |
| + f.a.sweepOnce() |
| + time.Sleep(100 * time.Millisecond) |
| + if rep.isClosed() { |
| + t.Fatal("the lease was released while the sidecar's descendant still ran") |
| + } |
| + f.b.setSidecarDescendant(false) |
| + waitFor(t, "the lease release once the cgroup is empty", func() bool { |
| + f.a.sweepOnce() |
| + return rep.isClosed() |
| + }) |
| +} |
| + |
| // Under plan 2's containment the sidecar keeps its role-sidecar child: no |
| // limit of its own, and the generation parent keeps memory.oom.group=0 and |
| // holds no process (spec §7.3, plan 2 D4). Spawning through spawnInCgroup |
| @@ -5082,7 +5241,7 @@ var _ SidecarGenStarter = (*SplitBackend)(nil) |
| - [ ] **Step 2: Run the tests to verify they fail** |
| -Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestLegacySidecar|TestSwapSidecar|TestSwapStopJoins|TestAStraySidecar|TestExecBackendStartSidecarGen|TestStartSidecarGenKeeps' -count=1` (`TestSwapSidecar` covers `TestSwapSidecarNeverRunsBesideTheRetiredGenerationsSidecar`) |
| +Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestLegacySidecar|TestSwapSidecar|TestSwapStopJoins|TestAStraySidecar|TestASidecarsDescendant|TestExecBackendStartSidecarGen|TestStartSidecarGenKeeps' -count=1` (`TestSwapSidecar` covers `TestSwapSidecarNeverRunsBesideTheRetiredGenerationsSidecar`) |
| Expected: build failure: `undefined: SidecarGenStarter`, `b.StartSidecarGen undefined`, `undefined: errNestedSidecarGen`. |
| - [ ] **Step 3: Write the backend side** |
| @@ -5189,7 +5348,22 @@ Add to `activator.go`, after `bounceLocked`: |
| // (zero-downtime spec §7.5). With swaps on and a generation whose staged |
| // path is known, it execs that path. Otherwise it is today's StartSidecar, |
| // live link and all. |
| -func (a *Service) startSidecar(ctx context.Context, rt registry.Route, g Gen) (Proc, error) { |
| +// |
| +// Immediately before the spawn it asks the gate's ownership check (spec |
| +// §7.4): e must still be the host's, unfenced, with no shutdown, and (awake) |
| +// its lease held; waking is the wake tail, whose lease is the wake's own. |
| +// While a stray sidecar of the host is tracked (its termination unproven) |
| +// nothing starts beside it. |
| +func (a *Service) startSidecar(ctx context.Context, e *actEntry, rt registry.Route, g Gen, waking bool) (Proc, error) { |
| + a.mu.Lock() |
| + err := a.ownedLocked(e, waking) |
| + if err == nil && len(a.straySidecars[rt.Host]) > 0 { |
| + err = errSidecarUnproven |
| + } |
| + a.mu.Unlock() |
| + if err != nil { |
| + return nil, err |
| + } |
| if a.gens && g.StagedPath != "" { |
| if s, ok := a.backend.(SidecarGenStarter); ok { |
| return s.StartSidecarGen(ctx, rt, g) |
| @@ -5220,7 +5394,9 @@ func (a *Service) replaceSidecar(e *actEntry) { |
| } |
| a.mu.Lock() |
| } |
| - if a.entries[e.host] != e || e.state != actAwake || !e.sidecarWanted { |
| + if a.entries[e.host] != e || e.state != actAwake || !e.sidecarWanted || len(a.straySidecars[e.host]) > 0 { |
| + // A stray still unproven: the reaper proves it first, and the |
| + // supervisor then starts the current generation's sidecar. |
| a.mu.Unlock() |
| return |
| } |
| @@ -5242,24 +5418,29 @@ func (a *Service) replaceSidecar(e *actEntry) { |
| log.Printf("activator: stop sidecar %s for its replacement: %v", e.host, err) |
| } |
| cancel() |
| - if !old.Exited() { |
| - // Unproven (spec §7.4, §7.5): it stays tracked as the entry's |
| - // sidecar, so a terminal stop still stops it, and no second |
| - // sidecar starts beside it. The sweep retries the replacement. |
| + if !a.sidecarTerminated(rt, old) { |
| + // Unproven (spec §7.4, §7.5): the leader still runs, or something |
| + // it forked is left in its cgroup. It becomes a tracked stray, in |
| + // the same critical section that takes it off the entry, so no |
| + // supervisor sees the entry without a sidecar and nothing to wait |
| + // for. Nothing starts beside it, a stop keeps the lease |
| + // (releasable), and the reaper proves it. |
| a.mu.Lock() |
| if a.entries[e.host] == e { |
| + e.sidecar = nil |
| e.sidecarStarting, e.sidecarStartDone, e.sidecarUncertain = false, nil, true |
| } |
| + a.trackStraySidecarLocked(rt, old) |
| a.mu.Unlock() |
| - log.Printf("activator: SIDECAR for %s did not stop for its replacement; keeping it tracked (the sweep retries)", e.host) |
| + log.Printf("activator: SIDECAR for %s is not proven gone for its replacement; tracking it (the sweep re-proves it)", e.host) |
| return |
| } |
| } |
| ctx, cancel := context.WithTimeout(context.Background(), instancectl.DeadlineControl) |
| - sc, err := a.startSidecar(ctx, rt, g) |
| + sc, err := a.startSidecar(ctx, e, rt, g, false) |
| cancel() |
| var discard Proc |
| - defer func() { a.discardSidecar(e.host, discard) }() // after the unlock, before done closes |
| + defer func() { a.discardSidecar(rt, discard) }() // after the unlock, before done closes |
| a.mu.Lock() |
| defer a.mu.Unlock() |
| if a.entries[e.host] != e { |
| @@ -5285,7 +5466,7 @@ func (a *Service) replaceSidecar(e *actEntry) { |
| `doWake` step 6 (line 1347): replace `if sc, err := a.backend.StartSidecar(ctx, rt); err != nil {` with: |
| ```go |
| - if sc, err := a.startSidecar(ctx, rt, w.gen); err != nil { |
| + if sc, err := a.startSidecar(ctx, e, rt, w.gen, true); err != nil { |
| ``` |
| `sweepOnce`'s supervisor (lines 2673–2680): capture the generation with the route, and spawn through `startSidecar`: |
| @@ -5299,7 +5480,7 @@ func (a *Service) replaceSidecar(e *actEntry) { |
| go func() { |
| defer close(done) |
| ctx, cancel := context.WithTimeout(context.Background(), instancectl.DeadlineControl) |
| - sc, err := a.startSidecar(ctx, route, gen) |
| + sc, err := a.startSidecar(ctx, e0, route, gen, false) |
| ``` |
| In the same goroutine, directly after the existing `if e0.state != actAwake { … stopDiscardedSidecar(sc); return }` block, add: |
| @@ -5331,7 +5512,7 @@ and in the same goroutine's two success installs (`e0.sidecar = sc` in the error |
| ```go |
| var discard Proc |
| - defer func() { a.discardSidecar(host, discard) }() // after the unlock, before done closes |
| + defer func() { a.discardSidecar(route, discard) }() // after the unlock, before done closes |
| ``` |
| Replace the first branch's `stopDiscardedSidecar(sc)` with `discard = sc`. Replace the second branch with: |
| @@ -5358,8 +5539,9 @@ Add to `activator.go`, beside `stopDiscardedSidecar`: |
| // generations off it is today's stopDiscardedSidecar, fire-and-forget. With |
| // them on it is synchronous and bounded, and the caller runs it before it |
| // closes the join (sidecarStartDone) a terminal stop waits on (spec §7.4). |
| -// A sidecar whose exit it cannot prove stays tracked (trackStraySidecar). |
| -func (a *Service) discardSidecar(host string, sc Proc) { |
| +// A sidecar whose termination it cannot prove stays tracked |
| +// (trackStraySidecarLocked). |
| +func (a *Service) discardSidecar(rt registry.Route, sc Proc) { |
| if sc == nil { |
| return |
| } |
| @@ -5368,61 +5550,169 @@ func (a *Service) discardSidecar(host string, sc Proc) { |
| return |
| } |
| ctx, cancel := context.WithTimeout(context.Background(), sidecarStopTimeout) |
| + err := sc.Stop(ctx) |
| + cancel() |
| + if !a.sidecarTerminated(rt, sc) { |
| + log.Printf("activator: %s: a discarded SIDECAR is not proven gone (%v); keeping it tracked", rt.Host, err) |
| + a.mu.Lock() |
| + a.trackStraySidecarLocked(rt, sc) |
| + a.mu.Unlock() |
| + } |
| +} |
| + |
| +var errSidecarUnproven = errors.New("a legacy sidecar of this host is not proven gone") |
| + |
| +// sidecarKiller is the backend's legacy-sidecar kill: whatever is left in |
| +// the host's sidecar cgroup is killed, and it returns once that cgroup is |
| +// empty. |
| +type sidecarKiller interface { |
| + KillSidecar(ctx context.Context, rt registry.Route) error |
| +} |
| + |
| +// sidecarTerminated is THE termination proof for a legacy sidecar (spec |
| +// §7.4, §7.5), as genTerminated is for a generation: its leader has exited |
| +// AND its sidecar cgroup is proven empty. childProc.Stop signals only the |
| +// leader, so its exit says nothing of a process it forked. A backend with |
| +// no sidecar kill (a test double), or one that answers errNoGenContainment |
| +// (the sidecar shares the host cgroup, flag off), has nothing more to |
| +// prove. A nil process never ran. Never called with a.mu held. The cgroup |
| +// is this host's legacy sidecar's alone: one entry per host, and the |
| +// interlock and the join mean no second sidecar of the host runs while one |
| +// is being proven. |
| +func (a *Service) sidecarTerminated(rt registry.Route, sc Proc) bool { |
| + if sc == nil { |
| + return true |
| + } |
| + if !sc.Exited() { |
| + return false |
| + } |
| + k, ok := a.backend.(sidecarKiller) |
| + if !ok { |
| + return true |
| + } |
| + ctx, cancel := context.WithTimeout(context.Background(), instancectl.DeadlineControl) |
| defer cancel() |
| - if err := sc.Stop(ctx); err != nil || !sc.Exited() { |
| - log.Printf("activator: %s: a discarded SIDECAR did not prove its exit (%v); keeping it tracked", host, err) |
| - a.trackStraySidecar(host, sc) |
| + if err := k.KillSidecar(ctx, rt); err != nil && !errors.Is(err, errNoGenContainment) { |
| + log.Printf("activator: %s: the sidecar's leader exited but its cgroup is not proven empty: %v", rt.Host, err) |
| + return false |
| } |
| + return true |
| } |
| -// trackStraySidecar keeps a legacy sidecar whose stop is unproven: it is |
| -// never forgotten (spec §7.4). The host takes no new wake while one is |
| -// tracked (errWakeUnproven), and the sweep (reapStraySidecarsLocked) stops |
| -// it again until it exits. |
| -func (a *Service) trackStraySidecar(host string, sc Proc) { |
| - a.mu.Lock() |
| - defer a.mu.Unlock() |
| +// trackStraySidecarLocked keeps a legacy sidecar whose termination is |
| +// unproven: it is never forgotten (spec §7.4). While one is tracked the host |
| +// takes no new wake (errWakeUnproven), starts no sidecar (startSidecar), and |
| +// no stop of it releases its lease (releasable); the sweep |
| +// (reapStraySidecarsLocked) re-stops and re-proves it. The caller holds a.mu. |
| +func (a *Service) trackStraySidecarLocked(rt registry.Route, sc Proc) { |
| if a.straySidecars == nil { |
| - a.straySidecars = make(map[string][]Proc) |
| + a.straySidecars = make(map[string][]*straySidecar) |
| } |
| - a.straySidecars[host] = append(a.straySidecars[host], sc) |
| + a.straySidecars[rt.Host] = append(a.straySidecars[rt.Host], &straySidecar{proc: sc, rt: rt}) |
| } |
| -// reapStraySidecarsLocked retries each stray sidecar's stop and forgets it |
| -// once it has exited. The caller holds a.mu. |
| +// reapStraySidecarsLocked re-stops each stray sidecar and re-proves it off |
| +// the lock (the proof is a kernel or root call), forgetting it only once |
| +// sidecarTerminated holds. The caller holds a.mu. |
| func (a *Service) reapStraySidecarsLocked() { |
| - for host, procs := range a.straySidecars { |
| - var keep []Proc |
| - for _, sc := range procs { |
| - if sc.Exited() { |
| + for _, strays := range a.straySidecars { |
| + for _, s := range strays { |
| + if s.reaping { |
| continue |
| } |
| - keep = append(keep, sc) |
| - go func(sc Proc) { |
| + s.reaping = true |
| + go func(s *straySidecar) { |
| ctx, cancel := context.WithTimeout(context.Background(), sidecarStopTimeout) |
| - _ = sc.Stop(ctx) |
| + _ = s.proc.Stop(ctx) |
| cancel() |
| - }(sc) |
| - } |
| - if len(keep) == 0 { |
| - delete(a.straySidecars, host) |
| - } else { |
| - a.straySidecars[host] = keep |
| + proven := a.sidecarTerminated(s.rt, s.proc) |
| + a.mu.Lock() |
| + defer a.mu.Unlock() |
| + s.reaping = false |
| + if !proven { |
| + return |
| + } |
| + host := s.rt.Host |
| + keep := slices.DeleteFunc(a.straySidecars[host], func(x *straySidecar) bool { return x == s }) |
| + if len(keep) == 0 { |
| + delete(a.straySidecars, host) |
| + } else { |
| + a.straySidecars[host] = keep |
| + } |
| + }(s) |
| } |
| } |
| } |
| ``` |
| -(`Service.straySidecars` is Task 2's, and Task 2's `releasable` already keeps a stop's lease while the host has one.) Call `a.reapStraySidecarsLocked()` beside Task 2's `a.reapUnprovenWakesLocked()` at the top of `finishPendingStops`. In `doWake`, extend Task 2's unproven-wake refusal so a tracked stray also refuses: `_, unproven := a.unprovenWakes[host]; unproven = unproven || len(a.straySidecars[host]) > 0`. |
| +(`Service.straySidecars` and `straySidecar` are Task 2's, and Task 2's `releasable` already keeps a stop's lease while the host has one. Add `slices` to `activator.go`'s imports if it is not there.) Call `a.reapStraySidecarsLocked()` beside Task 2's `a.reapUnprovenWakesLocked()` at the top of `finishPendingStops`. In `doWake`, extend Task 2's unproven-wake refusal so a tracked stray also refuses: `_, unproven := a.unprovenWakes[host]; unproven = unproven || len(a.straySidecars[host]) > 0`. |
| -In `stop()`'s co-stop, with generations on, a sidecar whose stop is unproven is tracked as well. After main's `outcome.sidecarStop, outcome.sidecarExited = err == nil, e.sidecar.Exited()`, add: |
| +In `stop()`'s co-stop, with generations on, a sidecar whose termination is unproven is tracked as well: a leader still running, or one that exited with something it forked still in its cgroup. After main's `outcome.sidecarStop, outcome.sidecarExited = err == nil, e.sidecar.Exited()`, add: |
| ```go |
| - if a.gens && !outcome.sidecarExited { |
| - a.trackStraySidecar(e.host, e.sidecar) |
| + if a.gens { |
| + a.mu.Lock() |
| + rt := e.route |
| + a.mu.Unlock() |
| + if !a.sidecarTerminated(rt, e.sidecar) { |
| + a.mu.Lock() |
| + a.trackStraySidecarLocked(rt, e.sidecar) |
| + a.mu.Unlock() |
| + } |
| } |
| ``` |
| +The `KillSidecar` that `sidecarTerminated` asks for: add to `genbackend.go`: |
| + |
| +```go |
| +// KillSidecar kills whatever is left of the host's legacy sidecar and |
| +// returns once its cgroup is empty (zero-downtime spec §7.4, §7.5): the |
| +// leader's exit says nothing of a process it forked. Flat with containment: |
| +// the role-sidecar child (plan 2 D4). Nested: the sidecar unit's stop, which |
| +// returns once systemd has emptied its cgroup (KillMode=mixed sends SIGKILL |
| +// to whatever SIGTERM left). Without containment the sidecar shares the |
| +// host cgroup with the instance, and the terminal stop's evidence on that |
| +// cgroup is the proof (errNoGenContainment, as KillGen answers). |
| +func (b *ExecBackend) KillSidecar(ctx context.Context, rt registry.Route) error { |
| + switch p := b.pathsFor(rt); p.layout { |
| + case LayoutNested: |
| + return b.Control.Stop(ctx, rt.Host, instancectl.RoleSidecar) |
| + case LayoutFenced, LayoutConflict: |
| + return fmt.Errorf("%s: refusing to kill a sidecar: %s", rt.Host, p.reason) |
| + } |
| + if !b.GenContainment { |
| + return errNoGenContainment |
| + } |
| + if b.Cgroups == nil { |
| + return cgroup.ErrNoDelegation |
| + } |
| + c, err := b.Cgroups.OpenCleanupChild(rt.Host, cgroup.RoleChild("sidecar")) |
| + if errors.Is(err, cgroup.ErrNoCgroup) { |
| + return nil // removed after the leader's exit, so empty |
| + } |
| + if err != nil { |
| + return err |
| + } |
| + if err := c.KillAndEmpty(ctx); err != nil { |
| + _ = c.Close() |
| + return err |
| + } |
| + return c.Close() |
| +} |
| + |
| +// KillSidecar forwards to the exec half for exec rows; systemd owns a unit |
| +// row's tree. |
| +func (b *SplitBackend) KillSidecar(ctx context.Context, rt registry.Route) error { |
| + if rt.Backing == registry.BackingUnit { |
| + return nil |
| + } |
| + if k, ok := b.Exec.(sidecarKiller); ok { |
| + return k.KillSidecar(ctx, rt) |
| + } |
| + return nil |
| +} |
| +``` |
| + |
| `replaceSidecar` (above) already sheds through `discardSidecar` and installs onto a stopping entry. |
| In `stop()`, the join on a supervised start is bounded by `sidecarStopTimeout` today, and a stop whose join times out goes on to release the lease. With generations on the join has no timeout of its own (P4 review, partial P2 #3): the start goroutine is bounded by its own `DeadlineControl` and its synchronous discard by `sidecarStopTimeout`, and whatever it leaves unproven stays tracked, so the stop never completes past a sidecar it did not prove. Replace |
| @@ -5470,7 +5760,7 @@ In `sweepOnce`, beside the supervisor (inside the same per-entry block, before i |
| - [ ] **Step 5: Run the tests to verify they pass** |
| -Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestLegacySidecar|TestSwapSidecar|TestExecBackendStartSidecarGen|TestStartSidecarGenKeeps' -count=1 -v 2>&1 | tail -20` |
| +Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestLegacySidecar|TestSwapSidecar|TestSwapStopJoins|TestAStraySidecar|TestASidecarsDescendant|TestExecBackendStartSidecarGen|TestStartSidecarGenKeeps' -count=1 -v 2>&1 | tail -20` |
| Expected: every subtest `--- PASS`, then `ok`. |
| - [ ] **Step 6: Run the whole package (the existing sidecar tests are the flag-off proof)** |
| @@ -6738,7 +7028,11 @@ Claims-on wakes spawn through the claim runtime's provider (`ClaimStart.Build`), |
| ```go |
| // StartPolicy gains Gen Gen; ClaimStart gains Gen Gen; ClaimRuntime gains Generations bool |
| // StartPolicy gains Fallback *Gen; HealthBy time.Time (a claimed first wake's fallback and health cap, spec §8.2; plan 5 sets them) |
| - // StartPolicy gains Gate func() error: the attempt gate, run in startAdmitted's beforeStart at every exec (a claims-on hold's pinCheck) |
| + // StartPolicy gains Gate func() error: the attempt gate, run in startAdmitted's beforeStart at every exec (a claims-on hold's pinCheck, a planned wake's ownership) |
| + // StartPolicy gains PinCheck func(pin Gen) error: plan 5's selection gate for the pin about to exec (Pin, or the Fallback that replaced it) |
| + var errClaimsAdmission error; func claimAdmission(err error, reached bool) error // (Task 4) an error before the exec hook is the admission's, never the build's |
| + func (p *claimProcess) Retire(ctx context.Context, grace time.Duration) error // a serving claims-on generation's graceful retirement: SIGTERM, the drain, then Stop's KillAndEmpty and proof |
| + // stopMember retires a claims-on roleRetiring member through Retire; an unready candidate is still killed at once (Stop) |
| // startAdmitted records pinFailed and falls back only when classifyStep says stepFailed |
| // actEntry gains pinFailed *Gen; pinFailReason string (the pinned build a claimed start saw fail before it was ready) |
| // Service gains: claimWakeSpawn func(ctx context.Context, s ClaimStart, g Gen, before func() error) (*exec.Cmd, *cgroup.Prepared, error) (test seam) |
| @@ -6974,7 +7268,7 @@ func TestAnEmptyStrictParentIsClosedAndKeptUntilItCloses(t *testing.T) { |
| // attempt is abandoned, never SwapFailed: a failure record would halt the |
| // cohort for a build that never executed. |
| func TestAClaimsRefusalAtTheExecIsNoFailure(t *testing.T) { |
| - for _, refusal := range []error{hostguard.ErrStampMismatch, hostguard.ErrFenced} { |
| + for _, refusal := range []error{hostguard.ErrStampMismatch, hostguard.ErrFenced, hostguard.ErrNoClaim} { |
| f := newSwapFixture(t) |
| f.a.cfg.HostWrites = hostwrite.Config{Enabled: true, Selector: &swapSelector{work: &swapWork{}}} |
| f.a.cfg.Claims = &ClaimRuntime{Pins: true, Generations: true} |
| @@ -6985,6 +7279,22 @@ func TestAClaimsRefusalAtTheExecIsNoFailure(t *testing.T) { |
| t.Fatalf("%v at the exec: Swap = %+v, want abandoned", refusal, res) |
| } |
| } |
| + // A claim read that fails (the store timing out) in startClaimedSuccessor |
| + // itself: an admission refusal, so abandoned. Returned raw it read as |
| + // SwapFailed and halted the cohort for a build that never ran. |
| + f := newSwapFixture(t) |
| + f.a.cfg.HostWrites = hostwrite.Config{Enabled: true, Selector: &swapSelector{work: &swapWork{}}} |
| + f.a.cfg.Claims = &ClaimRuntime{Pins: true, Generations: true, |
| + Read: func(context.Context, string) (hostclaim.Observed, error) { |
| + return hostclaim.Observed{}, errors.New("s3: GetObject: request timeout") |
| + }, |
| + Prepare: func(context.Context, registry.Route, hostclaim.Observed, startPolicy) (ClaimStart, error) { |
| + t.Fatal("prepared after a failed claim read") |
| + return ClaimStart{}, nil |
| + }} |
| + if res := f.a.Swap(context.Background(), f.attempt(ModeOverlap), f.newest); res.Outcome != SwapAbandoned { |
| + t.Fatalf("a failed claim read: Swap = %+v, want abandoned", res) |
| + } |
| } |
| // claimedStartFor is a ClaimStart for obs with every hook a no-op, version v. |
| @@ -7119,7 +7429,7 @@ Add `"fmt"`, `"sync"`, `"sync/atomic"`, `"github.com/carlosframework/platform/in |
| - [ ] **Step 2: Run the tests to verify they fail** |
| -Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestClaimsOnSwapNeedsGenerations|TestStartAdmittedRefusesAGenerationMismatch|TestClaimGenAt|TestStartAdmittedFallsBack|TestClaimsOnSuccessorReChecks|TestAClaimsOnHoldRunsTheGate|TestAClaimedWakeCancelled|TestAClaimsRefusalAtTheExec|TestAnEmptyStrictParent' -count=1` |
| +Run: `TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestClaimsOnSwapNeedsGenerations|TestStartAdmittedRefusesAGenerationMismatch|TestClaimGenAt|TestStartAdmittedFallsBack|TestClaimsOnSuccessorReChecks|TestAClaimsOnHoldRunsTheGate|TestAClaimedWakeCancelled|TestAClaimsRefusalAtTheExec|TestAnEmptyStrictParent|TestARetiringClaimed' -count=1` |
| Expected: build failure: `undefined: errClaimsNoGenerations`, `unknown field Gen in struct literal of type ClaimStart`, `a.claimGenAt undefined`, `unknown field Fallback in struct literal of type StartPolicy`. |
| - [ ] **Step 3: Extend the claim types and `startAdmitted`** |
| @@ -7146,6 +7456,10 @@ Expected: build failure: `undefined: errClaimsNoGenerations`, `unknown field Gen |
| // inside the selected work immediately before every exec, after the |
| // admission and Restore (spec §6.3, §7.4). |
| Gate func() error |
| + // PinCheck, when set, is the planner's selection gate (plan 5's |
| + // WakePlan.Check) for the pin about to exec: Pin, or the Fallback that |
| + // replaced it. startAdmitted runs it beside Gate, at every exec. |
| + PinCheck func(pin Gen) error |
| ``` |
| In `startAdmitted`'s attempt loop, replace main's |
| @@ -7155,17 +7469,36 @@ In `startAdmitted`'s attempt loop, replace main's |
| ``` |
| with |
| ```go |
| + reached := false // the exec hook ran: an error before it is the admission's (claimAdmission) |
| beforeStart := func() error { |
| + reached = true |
| if policy.Gate != nil { |
| - // The attempt gate at the exec (Task 6's pinCheck). |
| + // The attempt gate at the exec (Task 6's pinCheck), or a |
| + // planned wake's ownership check (plan 5). |
| if err := policy.Gate(); err != nil { |
| return err |
| } |
| } |
| + if policy.PinCheck != nil && policy.Pin != nil { |
| + // The planner's selection gate for the build about to exec |
| + // (plan 5): a failure recorded since planning holds it. |
| + if err := policy.PinCheck(*policy.Pin); err != nil { |
| + return err |
| + } |
| + } |
| return budget.start(ctx, s.OpenProbe) |
| } |
| ``` |
| +Directly after the spawn (the `switch` below that sets `startErr`), add: |
| + |
| +```go |
| + // A refusal by the work's own validation (a claim read, an absent |
| + // claim, a changed stamp) came back before the exec hook: nothing of |
| + // the build ran (P5 review partial new #3). |
| + startErr = claimAdmission(startErr, reached) |
| +``` |
| + |
| Add to `actEntry` (`activator.go`, after Task 6's `holdNext`): |
| ```go |
| @@ -7564,7 +7897,9 @@ func (a *Service) startClaimedSuccessor(ctx context.Context, e *actEntry, rt reg |
| } |
| obs, err := r.Read(ctx, e.host) |
| if err != nil { |
| - return g, nil, err |
| + // A claim read that failed (a store error, an absent claim) is a |
| + // refusal of the admission, not the build's failure (spec §7.4). |
| + return g, nil, claimAdmission(err, false) |
| } |
| want, pin := a.claimGenAt(rt, g.N), g |
| s, err := r.Prepare(ctx, rt, obs, startPolicy{MaxStarts: 1, Window: 30 * time.Second, Gen: want, Pin: &pin}) |
| @@ -7587,10 +7922,19 @@ func (a *Service) startClaimedSuccessor(ctx context.Context, e *actEntry, rt reg |
| if g.Tier != (cgroup.Limit{}) { |
| tier = g.Tier // the successor's own artifact's tier, captured at admission (Task 1) |
| } |
| - cmd, child, err := spawnClaimedGen(ctx, parent, g.N, tier, work, s.Identity, s.Build, gate) |
| + // The work's With validates the claim and the stamp before its body |
| + // runs; anything it refuses with, a claim read included, comes back |
| + // before the exec hook. Only an error after that hook is the spawn's own |
| + // (P5 review partial new #3). |
| + reached := false |
| + before := func() error { |
| + reached = true |
| + return gate() |
| + } |
| + cmd, child, err := spawnClaimedGen(ctx, parent, g.N, tier, work, s.Identity, s.Build, before) |
| p := newClaimProcess(cmd, child) |
| if err != nil { |
| - return g, p, err |
| + return g, p, claimAdmission(err, reached) |
| } |
| g.Socket, g.Control = want.Socket, want.Control |
| return g, p, nil |
| @@ -7605,6 +7949,107 @@ Add to `Service` (activator.go, beside `genHealth`): |
| claimSpawn func(ctx context.Context, e *actEntry, rt registry.Route, work hostwrite.Work, g Gen) (Gen, Proc, error) |
| ``` |
| +**A serving claims-on generation retires gracefully** (spec §7.5 step 6; P5 review finding 4). `claimProcess.Stop` is `KillAndEmpty` at once, which is right for a candidate that never became ready and wrong for the generation being retired: it kills the requests it is still answering. Add to `claimwake.go`, beside `Stop` (imports `os`, `syscall`, `time`): |
| + |
| +```go |
| +// Retire is a serving claims-on generation's retirement (zero-downtime spec |
| +// §7.5 step 6): SIGTERM to its leader, the app's own drain for up to grace, |
| +// then Stop, whose KillAndEmpty on the generation's own child and the |
| +// emptiness proof are unchanged. A candidate that never became ready is not |
| +// retired: its owner Stops it, which kills at once. |
| +func (p *claimProcess) Retire(ctx context.Context, grace time.Duration) error { |
| + if p.cmd != nil && p.cmd.Process != nil && !p.Exited() { |
| + err := p.cmd.Process.Signal(syscall.SIGTERM) |
| + if err == nil || errors.Is(err, os.ErrProcessDone) { |
| + t := time.NewTimer(grace) |
| + select { |
| + case <-p.done: |
| + case <-t.C: |
| + case <-ctx.Done(): |
| + } |
| + t.Stop() |
| + } |
| + } |
| + return p.Stop(ctx) |
| +} |
| +``` |
| + |
| +In `opset.go`'s `stopMember` (Task 2), replace |
| + |
| +```go |
| + ctx, cancel := context.WithTimeout(context.Background(), actStopTimeout) |
| + defer cancel() |
| + err := m.proc.Stop(ctx) |
| +``` |
| +with |
| +```go |
| + var err error |
| + if cp, ok := m.proc.(*claimProcess); ok && m.role == roleRetiring { |
| + // A serving claims-on generation drains like a flat one (spec §7.5 |
| + // step 6): SIGTERM and actStopTimeout first, then KillAndEmpty and |
| + // the proof, with their own bound. |
| + ctx, cancel := context.WithTimeout(context.Background(), actStopTimeout+instancectl.DeadlineControl) |
| + err = cp.Retire(ctx, actStopTimeout) |
| + cancel() |
| + } else { |
| + ctx, cancel := context.WithTimeout(context.Background(), actStopTimeout) |
| + err = m.proc.Stop(ctx) |
| + cancel() |
| + } |
| +``` |
| + |
| +Append to `claimgen_test.go` (the test runs `sleep`, and skips where it is not on `PATH`): |
| + |
| +```go |
| +// orderContainment records whether the leader had already exited when |
| +// KillAndEmpty ran, then kills and waits like the real one. |
| +type orderContainment struct { |
| + p *claimProcess |
| + exitedAtKill bool |
| +} |
| + |
| +func (c *orderContainment) KillAndEmpty(context.Context) error { |
| + c.exitedAtKill = c.p.Exited() |
| + if c.p.cmd.Process != nil { |
| + _ = c.p.cmd.Process.Kill() |
| + } |
| + <-c.p.done |
| + return nil |
| +} |
| + |
| +func (c *orderContainment) Close() error { return nil } |
| + |
| +// Spec §7.5 step 6: a retiring claims-on generation gets SIGTERM and its |
| +// drain before KillAndEmpty, so its in-flight requests finish. An unready |
| +// candidate is still killed at once. Stop alone (cgroup.kill first) killed |
| +// the retiring generation's requests. |
| +func TestARetiringClaimedGenerationDrainsBeforeItsKill(t *testing.T) { |
| + if _, err := exec.LookPath("sleep"); err != nil { |
| + t.Skip("no sleep") |
| + } |
| + for _, tc := range []struct { |
| + role genRole |
| + drained bool |
| + }{{roleRetiring, true}, {roleSuccessor, false}} { |
| + cmd := exec.Command("sleep", "30") |
| + if err := cmd.Start(); err != nil { |
| + t.Fatal(err) |
| + } |
| + p := newClaimProcess(cmd, nil) |
| + c := &orderContainment{p: p} |
| + p.prepared = c |
| + a := &Service{} |
| + e := &actEntry{host: "claims.example.com"} |
| + if !a.stopMember(e, &genMember{gen: Gen{N: 2}, role: tc.role, proc: p}) { |
| + t.Fatalf("%s: stop not proven", tc.role) |
| + } |
| + if c.exitedAtKill != tc.drained { |
| + t.Fatalf("%s: leader exited before KillAndEmpty = %v, want %v", tc.role, c.exitedAtKill, tc.drained) |
| + } |
| + } |
| +} |
| +``` |
| + |
| - [ ] **Step 5: Route the swap through the claims arms** |
| In `swapOverlap`, after `work, err := a.selectWork(...)` and its error check, add: |
| @@ -7686,7 +8131,7 @@ The kernel path of a claims-on generation spawn (`spawnClaimedGen` under a real |
| - [ ] **Step 6: Run the tests to verify they pass** |
| -Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestClaimsOnSwapNeedsGenerations|TestStartAdmittedRefuses|TestStartAdmittedFallsBack|TestClaimGenAt|TestSwapClaimsOn|TestSwapHoldClaimsOn|TestClaim' -count=1` |
| +Run: `go vet ./internal/activator && TMPDIR=/tmp/k nice -n 19 go test ./internal/activator -run 'TestClaimsOnSwapNeedsGenerations|TestStartAdmittedRefuses|TestStartAdmittedFallsBack|TestClaimGenAt|TestSwapClaimsOn|TestSwapHoldClaimsOn|TestClaim|TestAClaims|TestARetiringClaimed|TestAnEmptyStrictParent' -count=1` |
| Expected: `ok`. |
| - [ ] **Step 7: Run the whole package (generations off: claims-on wakes unchanged)** |
diff --git a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md| index eb475e251..e4cc03573 100644 |
| --- a/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md |
| +++ b/docs/superpowers/plans/2026-10-07-zero-downtime-deploys-5-scheduler.md |
| @@ -16,7 +16,7 @@ |
| - **Line numbers** are from `origin/main` at `17362eaf` (2026-10-07). Plans 1 to 4 move them again; find every edit site by the quoted code, and re-read line numbers against the branch point before starting (index amendments). |
| - **Gate before every commit:** `go build ./...`, `go vet ./...`, `test -z "$(gofmt -l .)"`, and the touched packages' tests with `-count=1`. Use the default Go build cache (`~/.cache/go-build`), never one under `/tmp`. Packages that bind sockets (`internal/activator`, `internal/host`) run with `TMPDIR=/tmp/k` (create it with `mkdir -p /tmp/k`), per the index. |
| - **Cross-compile:** darwin, windows and linux-arm64 must build. Nothing in this plan adds a syscall; `internal/host` is linked into the CLI. |
| -- **The flag** (spec §15): `CARLOS_SWAP=overlap` arms everything: `activator.SetGenerations(true)` (plan 1; plan 3's `Swap` refuses every attempt while it is off, and adds no switch of its own), `ExecBackend.GenContainment = true` (plan 2), the generations file, and the manager. Any other value, including unset, leaves every path as it was: generations off, `GenContainment` false, `svc.Planner`, `svc.OnGen`, `svc.OnDrift`, `svc.CoalesceRecycle` and `svc.GenerationsFile` unset, the converge leg is `convergeAdopted`, the restart leg calls `Bounce`, config sync calls `Bounce` and acks at delivery. |
| +- **The flag** (spec §15): `CARLOS_SWAP=overlap` arms everything: `activator.SetGenerations(true)` (plan 1; plan 3's `Swap` refuses every attempt while it is off, and adds no switch of its own), `ExecBackend.GenContainment = true` (plan 2), and the manager. Any other value, including unset, leaves every path as it was: generations off, `GenContainment` false, `svc.Planner`, `svc.OnGen`, `svc.OnDrift` and `svc.CoalesceRecycle` unset, the converge leg is `convergeAdopted`, the restart leg calls `Bounce`, config sync calls `Bounce` and acks at delivery. Two things are set whatever the flag (`armAlways`, Task 14), because plan 4's captured-artifact recycle runs whatever the flag: the generations file (a worker's captured serving build is in root's pin sets), and `svc.OnWorkerServing` (`ServingVersion` for `carlos sidecars ls`). |
| - **The generations file** (index amendments, plan 4 Task 5): `/data/zdd/generations.json`, written only through `release.WriteGenerations(release.GenerationsPath(dataDir), g)`, before the agent's first spawn and again on every change to an operation set (a generation spawning, becoming current, retiring or ending). An absent file means no generations; an unreadable one is an incomplete pin set, and root then neither restages nor sweeps that pass. |
| - **Retry** (spec rev 14 §8.4): a failure record keeps the `PromotedAt` of the channel pointer it failed under, and clears when this box sees a pointer naming the same build with a newer `PromotedAt` (`carlos promote --retry`, on `main` since `9e628024`). There is no signed retry object. |
| - **App policy** (spec §4.1): `swap` is `overlap` or `hold`, default `overlap`; `swap_concurrency` is an integer ≥ 1, default 1. They live in the app's default config bundle (`apps/<acct>/<app>/config.json`) as `"swap"` and `"swap_concurrency"`, additive to schema 1. Plan 6 adds the doors that write them. |
| @@ -4547,10 +4547,14 @@ git commit -m "zdd-5 task 8: the manager and the per-app scheduler" |
| - Modify: `internal/activator/zdd5_surface_test.go` (the `startPinned` pin) |
| **Interfaces:** |
| -- Consumes: plan 3's `startPinned`, `observeSettle`, `genHealth`, `SwapBackend`, `e.pin`, `StartPolicy.Pin/Fallback/HealthBy`, `e.pinFailed`, `e.pinFailReason` (Tasks 1, 4, 5, 6, 10); plan 1's `e.gen`, `w.gen`, `Service.gens`; plan 4's `startWorker`, `e.workerServing`; plan 2's `(*ExecBackend).KillGen(ctx, rt, n uint64) error` and its `SplitBackend` forward (Task 10). |
| +- Consumes: plan 3's `startPinned`, `observeSettle`, `genHealth`, `SwapBackend`, `e.pin`, `ownedLocked`, `errSwapSuperseded`, `StartPolicy.Pin/Fallback/HealthBy/Gate/PinCheck`, `e.pinFailed`, `e.pinFailReason` (Tasks 1, 4, 5, 6, 10); plan 1's `e.gen`, `w.gen`, `Service.gens`; plan 4's `startWorker`, `e.workerServing`; plan 2's `(*ExecBackend).KillGen(ctx, rt, n uint64) error` and its `SplitBackend` forward (Task 10). |
| - Produces: |
| ```go |
| - type WakePlan struct { Boot release.StagedIdentity; Mode SwapMode; FirstWake bool; Fallback *release.StagedIdentity; Refuse string } |
| + type WakePlan struct { Boot release.StagedIdentity; Mode SwapMode; FirstWake bool; Fallback *release.StagedIdentity; Refuse string; FailedUnder time.Time; Check func(build string) error } |
| + var errPlanStale error // wraps errSwapSuperseded: records nothing |
| + func (a *Service) checkPlan(ctx context.Context, e *actEntry, rt registry.Route, plan WakePlan) (WakePlan, error) // the selection gate at the exec; replans once on the restored state |
| + func (a *Service) wakeOwned(ctx context.Context, e *actEntry) error // plan 3's ownedLocked(e, true) at a fallback's exec |
| + // doClaimWake: a planned claimed wake sets StartPolicy.Gate (ownership) and StartPolicy.PinCheck (WakePlan.Check) |
| type GenEventKind string // GenHealthy, GenSettled, GenFailed, GenFellBack, GenCancelled |
| type GenEvent struct { Kind GenEventKind; Route registry.Route; Gen Gen; Failed release.StagedIdentity; Mode SwapMode; FirstWake bool; Reason string; At time.Time } |
| // Service gains: Planner func(ctx context.Context, rt registry.Route) WakePlan; OnGen func(GenEvent); OnDrift func(host string) |
| @@ -4814,6 +4818,70 @@ func TestAHoldFirstWakeNeverFallsBack(t *testing.T) { |
| } |
| } |
| +// Spec §7.4, §8.1–8.4 (P5 review finding 3): a planned wake re-checks its |
| +// build at the exec, after the lease and the restore. Here a sibling's |
| +// failure holds v2 between the planning and the exec: the wake is planned |
| +// again on the restored state and boots the good build, and v2 never |
| +// execs. Without the re-check v2 ran after its hold. |
| +func TestAPlannedWakeRechecksItsBuildAtTheExec(t *testing.T) { |
| + dir := t.TempDir() |
| + good, next := stagedID(t, dir, "v1"), stagedID(t, dir, "v2") |
| + a, b, evs := planActivator(t, WakePlan{}) |
| + var held atomic.Bool |
| + check := func(build string) error { |
| + if build == "v2" && held.Load() { |
| + return errors.New("v2 failed on a sibling") |
| + } |
| + return nil |
| + } |
| + var plans atomic.Int32 |
| + a.Planner = func(context.Context, registry.Route) WakePlan { |
| + if plans.Add(1) == 1 { |
| + held.Store(true) // the sibling's failure lands after this plan is made |
| + return WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good, Check: check} |
| + } |
| + return WakePlan{Boot: good, Mode: ModeOverlap, Check: check} |
| + } |
| + done, err := a.beginRequest(context.Background(), testRoute("one.example.com")) |
| + if err != nil { |
| + t.Fatalf("the wake: %v", err) |
| + } |
| + done() |
| + if gens := b.started(); len(gens) != 1 || gens[0].Build != "v1" { |
| + t.Fatalf("spawned %+v, want the good v1 alone", gens) |
| + } |
| + if k := evs.kinds(); slices.Contains(k, GenFailed) || slices.Contains(k, GenFellBack) { |
| + t.Fatalf("events %v: a build that never ran was recorded", k) |
| + } |
| +} |
| + |
| +// Spec §7.4 (P5 review rule a): a wake's fallback passes the ownership |
| +// check at its own exec. Here the host is fenced while B fails its health: |
| +// the good build is not started. B's failure is still reported. Checked |
| +// only before B's exec, the fallback ran on a fenced host. |
| +func TestAFallbackOnAFencedHostIsNotStarted(t *testing.T) { |
| + dir := t.TempDir() |
| + good, next := stagedID(t, dir, "v1"), stagedID(t, dir, "v2") |
| + a, b, evs := planActivator(t, WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good}) |
| + const host = "one.example.com" |
| + a.genHealth = func(_ context.Context, g Gen) error { |
| + if g.Build != "v2" { |
| + return nil |
| + } |
| + a.mu.Lock() |
| + a.fences = map[string]uint64{host: 1} // maintenance fences the host meanwhile |
| + a.mu.Unlock() |
| + return errors.New("GET / answered 500") |
| + } |
| + if _, err := a.beginRequest(context.Background(), testRoute(host)); err == nil { |
| + t.Fatal("a fenced host's fallback served") |
| + } |
| + if gens := b.started(); len(gens) != 1 || gens[0].Build != "v2" { |
| + t.Fatalf("spawned %+v, want B alone", gens) |
| + } |
| + evs.wait(t, GenFailed) |
| +} |
| + |
| func TestAnUnprovenKillStartsNoSecondGeneration(t *testing.T) { |
| dir := t.TempDir() |
| good, next := stagedID(t, dir, "v1"), stagedID(t, dir, "v2") |
| @@ -5319,7 +5387,7 @@ func TestBounceForNamesTheCause(t *testing.T) { |
| - [ ] **Step 2: Run them to see them fail** |
| -Run: `mkdir -p /tmp/k && TMPDIR=/tmp/k go test ./internal/activator/ -run 'TestFirstWake|TestAHoldFirstWake|TestAnUnprovenKill|TestAnExitInside|TestAnIntentionalStop|TestAFirstWakeThat|TestARefusedPlan|TestAClaimedFirstWake|TestAPlannedFirstWakeOnTheRealExecBackend|TestAPlannedOneShotWorker|TestAFirstWakeIsObserved|TestAPlannedWorkerFirstWake|TestACancelledFirstWake|TestAFailedWorkerFirstWakeFallsBack|TestWithoutAPlanner|TestDriftFeeds|TestBounceFor' -count=1` |
| +Run: `mkdir -p /tmp/k && TMPDIR=/tmp/k go test ./internal/activator/ -run 'TestFirstWake|TestAHoldFirstWake|TestAPlannedWakeRechecks|TestAFallbackOnAFenced|TestAnUnprovenKill|TestAnExitInside|TestAnIntentionalStop|TestAFirstWakeThat|TestARefusedPlan|TestAClaimedFirstWake|TestAPlannedFirstWakeOnTheRealExecBackend|TestAPlannedOneShotWorker|TestAFirstWakeIsObserved|TestAPlannedWorkerFirstWake|TestACancelledFirstWake|TestAFailedWorkerFirstWakeFallsBack|TestWithoutAPlanner|TestDriftFeeds|TestBounceFor' -count=1` |
| Expected: FAIL, `undefined: WakePlan`. |
| - [ ] **Step 3: Write the plan types and helpers** |
| @@ -5361,10 +5429,64 @@ type WakePlan struct { |
| // published with no blocking read (spec §8.1), as dispatch.FailedUnder |
| // is for an attempt. Zero: the failure's own time stands in. |
| FailedUnder time.Time |
| + // Check is the planner's selection gate (spec §7.4, §8.1–8.4): nil when |
| + // build may still boot. Planning runs before the lease and the restore, |
| + // so a sibling's failure recorded since can hold the build it picked. |
| + // The activator asks it at every planned exec, after the restore; the |
| + // good build always passes. Nil: nothing to re-check. |
| + Check func(build string) error |
| } |
| func (p WakePlan) planned() bool { return p.Boot.StagedPath != "" } |
| +// errPlanStale: the planned build is no longer the one to boot. A |
| +// supersession (gateSuperseded), so it records nothing and falls back to |
| +// nothing (classifyStep's stepStale). |
| +var errPlanStale = fmt.Errorf("the planned build is no longer the one to boot: %w", errSwapSuperseded) |
| + |
| +// checkPlan is a planned wake's selection gate at the exec (spec §7.4, |
| +// §8.1–8.4), after the lease and the restore: a refused plan is planned |
| +// again once, on the state already restored, and a second refusal abandons |
| +// the wake (errPlanStale). It returns the plan to boot. |
| +func (a *Service) checkPlan(ctx context.Context, e *actEntry, rt registry.Route, plan WakePlan) (WakePlan, error) { |
| + if plan.Check == nil { |
| + return plan, nil |
| + } |
| + err := plan.Check(plan.Boot.Version) |
| + if err == nil { |
| + return plan, nil |
| + } |
| + if a.Planner == nil { |
| + return plan, fmt.Errorf("%w: %v", errPlanStale, err) |
| + } |
| + next := a.Planner(ctx, rt) |
| + if next.Refuse != "" || !next.planned() { |
| + return plan, fmt.Errorf("%w: %v; replanned: %s", errPlanStale, err, next.Refuse) |
| + } |
| + if next.Check != nil { |
| + if nerr := next.Check(next.Boot.Version); nerr != nil { |
| + return plan, fmt.Errorf("%w: %v", errPlanStale, nerr) |
| + } |
| + } |
| + a.mu.Lock() |
| + e.plan, e.firstWake = next, next.FirstWake |
| + a.mu.Unlock() |
| + return next, nil |
| +} |
| + |
| +// wakeOwned is the ownership check at a wake's exec (spec §7.4): the wake's |
| +// lifecycle has not ended, and plan 3's ownedLocked holds for a waking |
| +// entry (its lease is the wake's own). A planned wake's fallback asks it |
| +// before its exec, seconds after doWake's own check before the first. |
| +func (a *Service) wakeOwned(ctx context.Context, e *actEntry) error { |
| + if err := ctx.Err(); err != nil { |
| + return err |
| + } |
| + a.mu.Lock() |
| + defer a.mu.Unlock() |
| + return a.ownedLocked(e, true) |
| +} |
| + |
| func genOf(id release.StagedIdentity) Gen { |
| return Gen{Build: id.Version, StagedPath: id.StagedPath, Digest: id.Digest, Demand: id.Demand} |
| } |
| @@ -5487,6 +5609,12 @@ func (a *Service) killFailed(rt registry.Route, g Gen, p Proc, worker bool) erro |
| // executed (a staging refusal from verifyStaged), which is no failure of the |
| // build. |
| func (a *Service) spawnPlanned(ctx context.Context, e *actEntry, rt registry.Route, plan WakePlan, w *wokenInstance) (Proc, string, Gen, error) { |
| + // The selection gate at the exec (P5 review finding 3): the plan was |
| + // made before the lease and the restore. |
| + plan, err := a.checkPlan(ctx, e, rt, plan) |
| + if err != nil { |
| + return nil, "", Gen{}, err |
| + } |
| canFall := plan.FirstWake && plan.Fallback != nil && plan.Mode == ModeOverlap |
| var healthBy time.Time |
| if canFall { |
| @@ -5512,6 +5640,12 @@ func (a *Service) spawnPlanned(ctx context.Context, e *actEntry, rt registry.Rou |
| if kerr := a.killFailed(rt, g, proc, false); kerr != nil { |
| return proc, ver, g, fmt.Errorf("%w; its end was not proven, so the good build was not started: %v", err, kerr) |
| } |
| + if oerr := a.wakeOwned(ctx, e); oerr != nil { |
| + // The ownership check at the fallback's exec (spec §7.4; P5 review |
| + // rule a): a fence, a shutdown or a lost entry since B's exec means |
| + // nothing more runs. B's failure is still recorded (w.failed). |
| + return proc, ver, g, fmt.Errorf("%w; the good build was not started: %v", err, oerr) |
| + } |
| fproc, fver, fg, ferr := a.startPinned(ctx, rt, genOf(*plan.Fallback), time.Time{}) |
| if ferr != nil { |
| return fproc, fver, fg, fmt.Errorf("%w; the good build %s failed too: %v", err, plan.Fallback.Version, ferr) |
| @@ -5531,6 +5665,10 @@ func (a *Service) spawnPlannedWorker(ctx context.Context, e *actEntry, rt regist |
| } |
| return p, got, err |
| } |
| + plan, err := a.checkPlan(ctx, e, rt, plan) // the selection gate at the exec (P5 review finding 3) |
| + if err != nil { |
| + return nil, release.StagedIdentity{}, err |
| + } |
| canFall := plan.FirstWake && plan.Fallback != nil && plan.Mode == ModeOverlap |
| proc, id, err := start(plan.Boot) |
| if err == nil || proc == nil { |
| @@ -5550,6 +5688,9 @@ func (a *Service) spawnPlannedWorker(ctx context.Context, e *actEntry, rt regist |
| if kerr := a.killFailed(rt, Gen{}, proc, true); kerr != nil { |
| return proc, id, fmt.Errorf("%w; its end was not proven, so the good build was not started: %v", err, kerr) |
| } |
| + if oerr := a.wakeOwned(ctx, e); oerr != nil { |
| + return proc, id, fmt.Errorf("%w; the good build was not started: %v", err, oerr) // ownership at the fallback's exec (spec §7.4) |
| + } |
| fproc, fid, ferr := start(*plan.Fallback) |
| if ferr != nil { |
| return fproc, fid, fmt.Errorf("%w; the good build %s failed too: %v", err, plan.Fallback.Version, ferr) |
| @@ -5902,6 +6043,29 @@ Directly after the `policy := startPolicy{…, Pin: e.pin}` line (and plan 3 Tas |
| policy.Fallback = &fb |
| policy.HealthBy = e.wakeStart.Add(firstWakeBudget - firstWakeKillReserve) |
| } |
| + if p := e.plan; p.planned() { |
| + // At every claimed planned exec, B and its fallback alike, inside |
| + // the work after the admission and Restore (plan 3's beforeStart): |
| + // the ownership check (spec §7.4), and the planner's selection gate |
| + // for the pin about to exec (§8.1–8.4; P5 review finding 3). A |
| + // refusal there records nothing and starts nothing; the wake fails |
| + // and the next one is planned afresh. |
| + if policy.Gate == nil { |
| + policy.Gate = func() error { |
| + a.mu.Lock() |
| + defer a.mu.Unlock() |
| + return a.ownedLocked(e, true) |
| + } |
| + } |
| + if p.Check != nil { |
| + policy.PinCheck = func(pin Gen) error { |
| + if err := p.Check(pin.Build); err != nil { |
| + return fmt.Errorf("%w: %v", errPlanStale, err) |
| + } |
| + return nil |
| + } |
| + } |
| + } |
| ``` |
| At the end of `doClaimWake`, route both returns through `claimedPlanResult`, so `afterWake` sees the claimed wake's outcome as it sees an unclaimed one's. The failure return |
| @@ -5995,7 +6159,8 @@ git commit -m "zdd-5 task 9: planned wakes, the first-wake budget and fallback, |
| type wakeBook interface { Blocks(acct, app, build, host string, served bool) bool; HeldDown(acct, app, build, host string) bool } |
| type wakeInputs struct { Route registry.Route; OptDir string; Policy AppPolicy; Book wakeBook; Demand func(version, stagedPath string) (string, string, error) } |
| func decideWake(in wakeInputs) (activator.WakePlan, error) |
| - func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePlan // refuses while the host's attempt is running; installs a cold managed host first (Deps.Materialize), never an empty plan |
| + func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePlan // refuses while the host's attempt is running; installs a cold managed host first (Deps.Materialize), never an empty plan; sets WakePlan.Check |
| + func (m *Manager) planCheck(rt registry.Route, plan activator.WakePlan) func(build string) error // a first wake's build held by a failure recorded since the plan may not boot |
| func (m *Manager) served(host string, g activator.Gen); func (m *Manager) publishServing(host string, g activator.Gen); func (m *Manager) acknowledged(host string, g activator.Gen) bool |
| // every serving- and good-build write is acknowledged (g is what the activator serves) and serialised by Manager.goodMu; done() publishes before it frees the slot |
| func (m *Manager) attemptRunning(host string) bool |
| @@ -6313,6 +6478,31 @@ func TestAColdManagedHostIsInstalledBeforeItIsPlanned(t *testing.T) { |
| } |
| } |
| +// Spec §7.4, §8.1–8.4 (P5 review finding 3): a plan carries its selection |
| +// gate. A sibling's failure recorded after the plan was made refuses the |
| +// planned build at the exec; the good build still passes. |
| +func TestAPlansCheckRefusesABuildHeldSinceThePlan(t *testing.T) { |
| + h := newHarness(t, "a") |
| + h.stage("v2") |
| + h.install("v2", "a") |
| + p := h.m.Plan(context.Background(), h.rows.get("a")) |
| + if !p.FirstWake || p.Boot.Version != "v2" || p.Check == nil { |
| + t.Fatalf("plan %+v: want a first wake onto v2 with its check", p) |
| + } |
| + if err := p.Check("v2"); err != nil { |
| + t.Fatalf("nothing holds v2 yet: %v", err) |
| + } |
| + if _, err := h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "b", At: t0}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := p.Check("v2"); err == nil { |
| + t.Fatal("v2 failed on a sibling after the plan, and the check still let it boot") |
| + } |
| + if err := p.Check("v1"); err != nil { |
| + t.Fatalf("the good build was refused: %v", err) |
| + } |
| +} |
| + |
| // Spec §2.9 (rev 17), §6.4, §8.3: an overlap attempt on a foreign-layout row |
| // runs as a hold, and the activator says so (SwapResult.Mode). Its failure |
| // is a hold build's failure: held down, so no wake boots the good build in |
| @@ -6344,7 +6534,7 @@ Add `"sync/atomic"`, `"time"` and `"github.com/carlosframework/platform/internal |
| - [ ] **Step 2: Run them to see them fail** |
| -Run: `go test ./internal/rollout/ -run 'TestDecideWake|TestPlanBoots|TestAFellBack|TestOnlyASettled|TestAStaleSuccess|TestAFailedHold|TestAFailedFirstWakeIsRecorded|TestAFallbackHolds|TestTheSchedulerLeavesAWorkers|TestAWakeActivates|TestASleepersConfig|TestAForeignRowsFailedHold|TestAColdManagedHost' -count=1` |
| +Run: `go test ./internal/rollout/ -run 'TestDecideWake|TestPlanBoots|TestAFellBack|TestOnlyASettled|TestAStaleSuccess|TestAFailedHold|TestAFailedFirstWakeIsRecorded|TestAFallbackHolds|TestTheSchedulerLeavesAWorkers|TestAWakeActivates|TestASleepersConfig|TestAForeignRowsFailedHold|TestAColdManagedHost|TestAPlansCheck' -count=1` |
| Expected: FAIL, `undefined: decideWake`. |
| - [ ] **Step 3: Write the planner and the outcome handler** |
| @@ -6487,9 +6677,32 @@ func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePla |
| // published with no blocking read (buildFailed, spec §8.1). |
| plan.FailedUnder = m.failedUnder(rt, plan.Boot.Version) |
| } |
| + if plan.planned() { |
| + plan.Check = m.planCheck(rt, plan) |
| + } |
| return plan |
| } |
| +// planCheck is a plan's selection gate (WakePlan.Check), asked by the |
| +// activator at each planned exec, after the lease and the restore (spec |
| +// §7.4, §8.1–8.4): a first wake's build that a failure record now holds for |
| +// this instance (a sibling's failure since the plan, or a hold-down) may not |
| +// boot. The good build always may: it is what a held build falls back to. |
| +// The book is in memory; nothing here blocks. |
| +func (m *Manager) planCheck(rt registry.Route, plan activator.WakePlan) func(build string) error { |
| + acct, app, host := release.RouteAccount(rt), rt.App, rt.Host |
| + boot := plan.Boot.Version |
| + return func(build string) error { |
| + if !plan.FirstWake || build != boot { |
| + return nil |
| + } |
| + if m.d.Book.HeldDown(acct, app, build, host) || m.d.Book.Blocks(acct, app, build, host, false) { |
| + return fmt.Errorf("build %s is held by a failure recorded since the wake was planned", build) |
| + } |
| + return nil |
| + } |
| +} |
| + |
| // attemptRunning reports whether host has an attempt in flight: admitted |
| // and not yet finished by done(). |
| func (m *Manager) attemptRunning(host string) bool { |
| @@ -6589,10 +6802,10 @@ git commit -m "zdd-5 task 10: the wake planner and planned-wake outcomes" |
| - Create: `internal/activator/workerswap.go`, `internal/activator/workerswap_test.go` |
| - Modify: `internal/activator/activator.go` (`actEntry`: `attemptBuilds`, `retireTook`; plan 4's `respawnWorkerLocked`: time the retire of the old child) |
| - Create: `internal/rollout/worker.go`, `internal/rollout/worker_test.go` |
| -- Modify: `internal/rollout/scheduler.go` (`execute`) |
| +- Modify: `internal/rollout/scheduler.go` (`execute`), `internal/rollout/manager.go` (`Activator` gains `PublishHeldServing`), `internal/rollout/manager_test.go` (`fakeAct.PublishHeldServing`) |
| **Interfaces:** |
| -- Consumes: plan 4 Task 9's `respawnWorkerLocked(e *actEntry, stopOld Proc, why string, onto *release.StagedIdentity)`, `e.workerServing`, `stagedActivator`, `stagedFake`, `fakeIdentity`; `e.respawnDone`, `e.workerRespawning` (on `main`); plan 3's `swapChecksLocked`, `abandon`, `errSwapKind`, `errSwapBusy`, `errSwapStopped`, `observeSettle`, `settleOK`/`settleExited`/`settleCancelled` (Tasks 3–5); Task 9's `e.firstWatch`, `e.unexpectedExit`, `firstWakeSettle`, `shrinkFirstWake`, `genOf`, `bounceText`, `procCrashed` (on `main`); Task 8's `Activator`, `resolve`, `done`; Task 6's `gate`, `start`. |
| +- Consumes: plan 4 Task 9's `respawnWorkerLocked(e *actEntry, stopOld Proc, why string, onto *release.StagedIdentity)`, `e.workerServing`, `stagedActivator`, `stagedFake`, `fakeIdentity`; `e.respawnDone`, `e.workerRespawning` (on `main`); plan 3's `ownedLocked` (every respawn's ownership check), `verdictOf`, `swapChecksLocked`, `abandon`, `errSwapKind`, `errSwapBusy`, `errSwapStopped`, `observeSettle`, `settleOK`/`settleExited`/`settleCancelled` (Tasks 3–5); Task 9's `e.firstWatch`, `e.unexpectedExit`, `firstWakeSettle`, `shrinkFirstWake`, `genOf`, `bounceText`, `procCrashed` (on `main`); Task 8's `Activator`, `resolve`, `done`; Task 6's `gate`, `start`. |
| - Produces: |
| ```go |
| // activator |
| @@ -6600,10 +6813,16 @@ git commit -m "zdd-5 task 10: the wake planner and planned-wake outcomes" |
| // actEntry gains: attemptBuilds []string (the builds a socketless attempt in flight may exec, for the generations file, Task 12) |
| // retireTook time.Duration (how long the last respawn took to retire its old child) |
| // attemptGate func() error; attemptRestore *release.StagedIdentity; attemptVeto error |
| - // Service gains OnWorkerServing func(host string, id release.StagedIdentity): a worker's captured identity on every |
| - // successful wake and respawn, whatever CARLOS_SWAP says (the agent writes registry ServingVersion with it) |
| // (the attempt's §7.4 checks, re-run by the respawn at the exec; what it execs instead; its refusal) |
| + // workerInc uint64 (bumped at every install; an attempt counts only if it moved); holdServing bool; heldServing *heldServing |
| + // Service gains OnWorkerServing func(host string, id release.StagedIdentity): a worker's captured identity (an empty |
| + // version included) on every successful wake (after afterWake) and respawn, whatever CARLOS_SWAP says; servingMu |
| + func (a *Service) publishWorkerServing(e *actEntry, id release.StagedIdentity, inc uint64) // delivered only for the latest incarnation |
| + func (a *Service) PublishHeldServing(host string) // a ReplaceWorker fallback's publication, released after the failure record |
| + type heldServing struct{ id release.StagedIdentity; inc uint64 } |
| + // every respawn (crash restart, attempt, fallback, restore) asks plan 3's ownedLocked at its exec, whatever the flag |
| // rollout |
| + // Activator gains PublishHeldServing(host string); execute calls it after done() for a socketless attempt |
| func (s *scheduler) executeWorker(ctx context.Context, rt registry.Route, a activator.Attempt) activator.SwapResult |
| func (q *appQueue) admitOne(host string, now time.Time) (dispatch, bool) |
| func (m *Manager) CoalesceRecycle(host string) bool // plan 4's Service.CoalesceRecycle |
| @@ -6859,7 +7078,7 @@ func TestReplaceWorkerSupersededSpawnsNothing(t *testing.T) { |
| - [ ] **Step 2: Run them to see them fail** |
| -Run: `TMPDIR=/tmp/k go test ./internal/activator/ -run 'TestReplaceWorker' -count=1` |
| +Run: `TMPDIR=/tmp/k go test ./internal/activator/ -run 'TestReplaceWorker|TestAWorkerFallback|TestAFailedRestartExec|TestAnUnknownVersion|TestAFallbackWakeRecords' -count=1` |
| Expected: FAIL, `a.ReplaceWorker undefined`. |
| - [ ] **Step 3: Time the retire, and add the entry fields** |
| @@ -6898,7 +7117,17 @@ becomes |
| ```go |
| a.mu.Lock() |
| - wanted := a.entries[host] == e && e.state == actAwake && !a.shuttingDown |
| + // The ownership check at EVERY respawn's exec (spec §7.4; P5 review |
| + // finding 1): a crash restart, an attempt, its fallback and its |
| + // restore alike, whatever the flag. It is plan 3's ownedLocked: the |
| + // entry, awake, its lease held, no fence, no shutdown. A respawn that |
| + // fails it spawns nothing, and the entry stops through the sweep's |
| + // lease-loss or crash path. |
| + oerr := a.ownedLocked(e, false) |
| + wanted := oerr == nil |
| + if !wanted && e.attemptGate != nil { |
| + e.attemptVeto = oerr |
| + } |
| if wanted && onto != nil && e.attemptGate != nil { |
| // A socketless attempt's checks, again, at the exec (spec §6.3, |
| // §7.4): a newer release or a halted cohort (whose halt bumps |
| @@ -6925,7 +7154,7 @@ becomes |
| a.mu.Unlock() |
| ``` |
| -(`onto` is the goroutine's own copy of the parameter, so reassigning it touches nothing else.) |
| +(`onto` is the goroutine's own copy of the parameter, so reassigning it touches nothing else. `ownedLocked` covers main's three conditions, the entry, `actAwake` and `!a.shuttingDown`, and adds the lease and the fences. With the flag off this changes one thing: a worker on a host whose lease is lost or that is fenced is not respawned, where today the respawn runs.) |
| In plan 4's `respawnWorkerLocked` goroutine, replace the whole retire block |
| @@ -7019,6 +7248,11 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea |
| } |
| // Spec §9.4: the bounce event names the attempt's cause. |
| emitEvent(a.Logs, e.route, "info", bounceText(true, at.Cause)) |
| + // The incarnation before the respawn: the attempt's child counts only if |
| + // the respawn installed a NEW one (P5 review finding 5). A failed exec |
| + // keeps the old e.proc and its identity, which for a restart or a config |
| + // attempt is the same artifact, so sameArtifact alone passed it. |
| + inc0 := e.workerInc |
| a.respawnWorkerLocked(e, e.proc, "attempt ("+string(at.Cause)+")", &onto) |
| done := e.respawnDone |
| a.mu.Unlock() |
| @@ -7045,12 +7279,15 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea |
| a.mu.Unlock() |
| return abandon(fmt.Errorf("re-checked at the spawn: %w", veto)) |
| } |
| - if !sameArtifact(e.workerServing, onto) { |
| - // Nothing of the attempt's build ran: the backend refused the staged |
| - // copy (release.ErrStagedCopyChanged), the config gate stood the |
| - // worker down, or the exec failed. Put the build it replaced back, |
| - // as a failed restart does (spec §6.5). This is a staging refusal, |
| - // never a failure of the build. |
| + if e.workerInc == inc0 || !sameArtifact(e.workerServing, onto) { |
| + // Nothing of the attempt's build ran: no new incarnation was |
| + // installed (the exec failed, and the entry still holds the old |
| + // child, perhaps exited cleanly), the backend refused the staged |
| + // copy (release.ErrStagedCopyChanged), or the config gate stood the |
| + // worker down. Put the build it replaced back, as a failed restart |
| + // does (spec §6.5). This is a staging refusal, never a failure of the |
| + // build, and never a success: a restart or config attempt whose child |
| + // never ran acknowledges nothing (spec §8.1). |
| if prev.StagedPath != "" && !e.workerRespawning { |
| a.respawnWorkerLocked(e, nil, "restoring "+prev.Version, &prev) |
| } |
| @@ -7114,18 +7351,49 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea |
| a.mu.Unlock() |
| return SwapResult{Outcome: SwapFailed, Reason: reason + "; no good build to fall back to"} |
| } |
| + // The fallback's publication waits for the failure transition (spec |
| + // §8.1; P5 review finding 7): the respawn stores it (holdServing), and |
| + // the scheduler delivers it with PublishHeldServing once done() has |
| + // recorded B's failure. A synchronous publication here would sit in |
| + // front of that record, and a blocked callback would leave B unheld. |
| + e.holdServing = true |
| + inc1 := e.workerInc |
| a.respawnWorkerLocked(e, nil, "falling back to "+fallback.Version, fallback) |
| done = e.respawnDone |
| a.mu.Unlock() |
| a.awaitRespawn(ctx, done) |
| a.mu.Lock() |
| defer a.mu.Unlock() |
| - if a.entries[at.Host] == e && sameArtifact(e.workerServing, *fallback) { |
| + e.holdServing = false |
| + if a.entries[at.Host] == e && e.workerInc != inc1 && sameArtifact(e.workerServing, *fallback) { |
| return SwapResult{Outcome: SwapFailed, Gen: served(*fallback), Reason: reason} |
| } |
| return SwapResult{Outcome: SwapFailed, Reason: reason + "; the good build did not start either"} |
| } |
| +// PublishHeldServing delivers a worker's publication that a fallback held |
| +// back (holdServing) until the caller had recorded the failure it falls |
| +// back from (spec §8.1). Incarnation-ordered like any other: a newer install |
| +// since makes it a no-op. Called with no lock held. |
| +func (a *Service) PublishHeldServing(host string) { |
| + a.mu.Lock() |
| + e := a.entries[host] |
| + var held *heldServing |
| + if e != nil { |
| + held, e.heldServing = e.heldServing, nil |
| + } |
| + a.mu.Unlock() |
| + if held != nil { |
| + a.publishWorkerServing(e, held.id, held.inc) |
| + } |
| +} |
| + |
| +// heldServing is a worker publication held for PublishHeldServing. |
| +type heldServing struct { |
| + id release.StagedIdentity |
| + inc uint64 |
| +} |
| + |
| // sameArtifact reports whether the backend exec'd exactly the chosen bytes: |
| // the staged path and its digest (the release demand is not the backend's |
| // to report). |
| @@ -7166,8 +7434,13 @@ A worker child that crashes during the observation is not recycled by the sweep |
| // while e is the host's entry and inc its current incarnation, checked and |
| // delivered under servingMu, so an older publication can never land after a |
| // newer one. Called with no lock held. |
| +// |
| +// An empty version is published too (P5 review finding 8): a child exec'd |
| +// from an external live-link target has no version to capture, and the |
| +// row's previous ServingVersion must be cleared, not left naming a build |
| +// that no longer runs. |
| func (a *Service) publishWorkerServing(e *actEntry, id release.StagedIdentity, inc uint64) { |
| - if a.OnWorkerServing == nil || id.Version == "" { |
| + if a.OnWorkerServing == nil { |
| return |
| } |
| a.servingMu.Lock() |
| @@ -7182,21 +7455,48 @@ func (a *Service) publishWorkerServing(e *actEntry, id release.StagedIdentity, i |
| } |
| ``` |
| -In `wake()`, in its success branch, beside plan 4's `e.workerServing = w.spawnIdentity`, add `e.workerInc++` and capture `servingID, inc, publish := e.workerServing, e.workerInc, e.worker`; after that block's `a.mu.Unlock()`, before `close(e.ready)`, add: |
| +**Failure before publication** (spec §8.1; P5 review finding 7). A publication is a callback that may block (its mutex, a registry write), so it never runs ahead of a failure transition. In `wake()`, in its success branch, beside plan 4's `e.workerServing = w.spawnIdentity`, add `e.workerInc++` and capture `servingID, inc, publish := e.workerServing, e.workerInc, e.worker`. Publish only after Task 9's `a.afterWake(e, w, err)` (which runs after `close(e.ready)`), never before it: for a fallback wake, `afterWake` emits `GenFellBack`, and the manager's `OnGen` records B's failure synchronously, so B is held before the fallback's identity is published: |
| ```go |
| + a.afterWake(e, w, err) // Task 9's line: the failure transition first |
| if err == nil && publish { |
| a.publishWorkerServing(e, servingID, inc) |
| } |
| ``` |
| -In plan 4's `respawnWorkerLocked` goroutine, in the `default:` arm, beside `e.workerServing = ident` add `e.workerInc++; inc := e.workerInc`, and after the arm's `a.mu.Unlock()`, before its `log.Printf("activator: worker %s respawned …`, add: |
| +In plan 4's `respawnWorkerLocked` goroutine, in the `default:` arm, beside `e.workerServing = ident` add: |
| ```go |
| - a.publishWorkerServing(e, ident, inc) |
| + e.workerInc++ |
| + inc := e.workerInc |
| + hold := e.holdServing // a ReplaceWorker fallback: the scheduler publishes after the failure record |
| + if hold { |
| + e.heldServing = &heldServing{id: ident, inc: inc} |
| + } |
| ``` |
| -(`ident` is the identity plan 4's arm installed as `e.workerServing`.) Task 8's `served` and `publishServing` already leave a `Gen.Worker` generation's serving build alone (`setServingLocked`). Mark a worker's generations: `Current` returns `g := genOf(e.workerServing); g.Worker = true`, Task 9's `w.planned = genOf(ident)` becomes `w.planned = genOf(ident); w.planned.Worker = true`, and `ReplaceWorker`'s `served` helper sets `g.Worker = true`. Append to `workerswap_test.go`: |
| +and after the arm's `a.mu.Unlock()`, before its `log.Printf("activator: worker %s respawned …`, add: |
| + |
| +```go |
| + if !hold { |
| + a.publishWorkerServing(e, ident, inc) |
| + } |
| +``` |
| + |
| +(`ident` is the identity plan 4's arm installed as `e.workerServing`.) Add to `actEntry`, beside `workerInc`: `holdServing bool // ReplaceWorker's fallback respawn: hold its publication; under a.mu` and `heldServing *heldServing // that held publication, for PublishHeldServing; under a.mu`. Add `PublishHeldServing(host string)` to Task 8's `Activator` interface (`manager.go`), and to `fakeAct` in Task 8's `manager_test.go` a field `onHeldPublish func(host string)` and: |
| + |
| +```go |
| +// PublishHeldServing runs onHeldPublish, when set: a test's view of the |
| +// moment the scheduler releases a held worker publication. |
| +func (f *fakeAct) PublishHeldServing(host string) { |
| + f.mu.Lock() |
| + hook := f.onHeldPublish |
| + f.mu.Unlock() |
| + if hook != nil { |
| + hook(host) |
| + } |
| +} |
| +``` Task 8's `served` and `publishServing` already leave a `Gen.Worker` generation's serving build alone (`setServingLocked`). Mark a worker's generations: `Current` returns `g := genOf(e.workerServing); g.Worker = true`, Task 9's `w.planned = genOf(ident)` becomes `w.planned = genOf(ident); w.planned.Worker = true`, and `ReplaceWorker`'s `served` helper sets `g.Worker = true`. Append to `workerswap_test.go`: |
| ```go |
| // Spec §7.9: with CARLOS_SWAP unset (no generations, no rollout manager), a |
| @@ -7260,9 +7560,179 @@ func TestAnOlderWorkerPublicationIsDropped(t *testing.T) { |
| t.Fatalf("published %v: an older incarnation's publication was delivered after a newer one", got) |
| } |
| } |
| + |
| +// Spec §7.9 (P5 review finding 8): a child exec'd from an external live-link |
| +// target has no captured version. Its publication still goes out, empty, |
| +// so the row stops naming the build the previous child ran. Skipping empty |
| +// versions left ServingVersion=v1 on display indefinitely. |
| +func TestAnUnknownVersionClearsTheServingVersion(t *testing.T) { |
| + a, _, _, _, _ := stagedActivator(t) |
| + var mu sync.Mutex |
| + var got []string |
| + a.OnWorkerServing = func(_ string, id release.StagedIdentity) { |
| + mu.Lock() |
| + got = append(got, id.Version) |
| + mu.Unlock() |
| + } |
| + host := "bot.x.internal" |
| + if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + waitFor(t, "the wake's identity", func() bool { mu.Lock(); defer mu.Unlock(); return len(got) == 1 }) |
| + a.mu.Lock() |
| + e := a.entries[host] |
| + e.workerInc++ // a replacement exec'd from an external live-link target |
| + inc := e.workerInc |
| + a.mu.Unlock() |
| + a.publishWorkerServing(e, release.StagedIdentity{StagedPath: "/srv/external/app"}, inc) |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if len(got) != 2 || got[1] != "" { |
| + t.Fatalf("published %q, want the empty version last: the old one must be cleared", got) |
| + } |
| +} |
| + |
| +// Spec §7.4 (P5 review finding 1): every respawn passes the ownership check |
| +// at its exec, a fallback included. Here B execs, the lease is lost, and |
| +// then B crashes: the good build is NOT respawned. Without the check the |
| +// fallback ran A against a database the box no longer owned. |
| +func TestAWorkerFallbackWithoutOwnershipSpawnsNothing(t *testing.T) { |
| + shrinkFirstWake(t, 55*time.Second, 25*time.Second, 300*time.Millisecond) |
| + a, b, _, _, _ := stagedActivator(t) |
| + host := "bot.x.internal" |
| + if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + good := fakeIdentity("v1") |
| + go func() { |
| + waitFor(t, "the v2 child", func() bool { return len(b.calls()) == 2 }) |
| + b.lastRep().setErr(errors.New("lease renewal failed")) // ownership lost after B's exec |
| + time.Sleep(50 * time.Millisecond) |
| + b.lastWorker().crash() |
| + }() |
| + res := a.ReplaceWorker(context.Background(), workerAttempt(CauseRelease, ModeOverlap, fakeIdentity("v2")), &good, one) |
| + if res.Outcome != SwapFailed || res.Gen.Build != "" { |
| + t.Fatalf("ReplaceWorker = %+v, want failed with nothing serving", res) |
| + } |
| + if calls := b.calls(); len(calls) != 2 { |
| + t.Fatalf("spawns %v: the fallback exec'd after the lease was lost", calls) |
| + } |
| +} |
| + |
| +// Spec §6.5, §8.1 (P5 review finding 5): a restart counts only once a NEW |
| +// child is installed. Here the restart's exec fails after the old child |
| +// stopped cleanly: the entry still holds that child, with the same |
| +// artifact, and its clean exit read as a one-shot's success, so the |
| +// scheduler acknowledged config that never ran. |
| +func TestAFailedRestartExecIsNeverDone(t *testing.T) { |
| + shrinkFirstWake(t, 55*time.Second, 25*time.Second, 100*time.Millisecond) |
| + a, b, _, _, _ := stagedActivator(t) |
| + host := "bot.x.internal" |
| + if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + serving := a.entryServing(host) |
| + b.mu.Lock() |
| + b.workerErr = errors.New("exec: text file busy") |
| + b.mu.Unlock() |
| + res := a.ReplaceWorker(context.Background(), workerAttempt(CauseRestart, ModeOverlap, serving), nil, one) |
| + if res.Outcome == SwapDone { |
| + t.Fatalf("ReplaceWorker = %+v: a restart whose child never ran was done", res) |
| + } |
| +} |
| + |
| +// Spec §8.1 (P5 review finding 7): a ReplaceWorker fallback holds its |
| +// publication until the scheduler has recorded B's failure and calls |
| +// PublishHeldServing. Published from the respawn, a blocked callback sat |
| +// in front of the SwapFailed result, and so of the failure record. |
| +func TestAWorkerFallbackPublishesOnlyWhenReleased(t *testing.T) { |
| + shrinkFirstWake(t, 55*time.Second, 25*time.Second, 300*time.Millisecond) |
| + a, b, _, _, _ := stagedActivator(t) |
| + var mu sync.Mutex |
| + var got []string |
| + block := make(chan struct{}) |
| + a.OnWorkerServing = func(_ string, id release.StagedIdentity) { |
| + mu.Lock() |
| + got = append(got, id.Version) |
| + n := len(got) |
| + mu.Unlock() |
| + if n > 2 { |
| + <-block // a slow registry write for the fallback's identity (the wake's v1, then B's v2, come first) |
| + } |
| + } |
| + host := "bot.x.internal" |
| + if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + good := fakeIdentity("v1") |
| + go func() { |
| + waitFor(t, "the v2 child", func() bool { return len(b.calls()) == 2 }) |
| + time.Sleep(50 * time.Millisecond) |
| + b.lastWorker().crash() |
| + }() |
| + results := make(chan SwapResult, 1) |
| + go func() { |
| + results <- a.ReplaceWorker(context.Background(), workerAttempt(CauseRelease, ModeOverlap, fakeIdentity("v2")), &good, one) |
| + }() |
| + select { |
| + case res := <-results: |
| + if res.Outcome != SwapFailed || res.Gen.Build != "v1" { |
| + t.Fatalf("ReplaceWorker = %+v", res) |
| + } |
| + case <-time.After(5 * time.Second): |
| + close(block) |
| + t.Fatal("the failure result waited on the fallback's publication") |
| + } |
| + mu.Lock() |
| + n := len(got) |
| + mu.Unlock() |
| + if n != 2 { |
| + t.Fatalf("published %v before the failure was recorded", got) |
| + } |
| + close(block) |
| + a.PublishHeldServing(host) // the scheduler, after done() |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if len(got) != 3 || got[2] != "v1" { |
| + t.Fatalf("published %v, want the fallback's v1 once released", got) |
| + } |
| +} |
| + |
| +// Spec §8.1 (P5 review finding 7): on a planned worker wake that falls back, |
| +// the failure transition (GenFellBack, whose OnGen records B's failure) |
| +// completes before the fallback's identity is published, and a blocked |
| +// publication does not hold it up. Publishing before afterWake left B |
| +// unheld while the callback blocked. |
| +func TestAFallbackWakeRecordsTheFailureBeforeItPublishes(t *testing.T) { |
| + shrinkFirstWake(t, 55*time.Second, 25*time.Second, 300*time.Millisecond) |
| + a, b, _, _, _ := stagedActivator(t) |
| + a.gens = true |
| + evs := &genLog{} |
| + a.OnGen = evs.add |
| + dir := t.TempDir() |
| + good, next := stagedID(t, dir, "v1"), stagedID(t, dir, "v2") |
| + a.Planner = func(context.Context, registry.Route) WakePlan { |
| + return WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good} |
| + } |
| + block := make(chan struct{}) |
| + var sawFailure atomic.Bool |
| + a.OnWorkerServing = func(string, release.StagedIdentity) { |
| + sawFailure.Store(slices.Contains(evs.kinds(), GenFellBack)) |
| + <-block |
| + } |
| + go func() { |
| + waitFor(t, "the v2 child", func() bool { return len(b.calls()) == 1 }) |
| + time.Sleep(50 * time.Millisecond) |
| + b.lastWorker().crash() |
| + }() |
| + go a.WakeWorker(alwaysOnWorkerRoute("bot.x.internal")) |
| + evs.wait(t, GenFellBack) // arrives while the publication is still blocked |
| + close(block) |
| + waitFor(t, "the publication", func() bool { return sawFailure.Load() }) |
| +} |
| ``` |
| -(add `"sync"` to that file's imports). |
| +(add `"slices"`, `"sync"` and `"github.com/carlosframework/platform/internal/registry"` to that file's imports). |
| - [ ] **Step 5: Run the activator tests** |
| @@ -7278,6 +7748,7 @@ package rollout |
| import ( |
| "testing" |
| + "time" |
| "github.com/carlosframework/platform/internal/activator" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -7326,6 +7797,33 @@ func TestAFailedWorkerReleaseFallsBackAndHalts(t *testing.T) { |
| } |
| } |
| +// Spec §8.1 (P5 review finding 7): the fallback's held publication is |
| +// released only after done() has recorded the failure it falls back from. |
| +func TestAWorkerFallbackIsPublishedAfterItsFailureIsRecorded(t *testing.T) { |
| + h := workerHarness(t, "bot") |
| + h.stage("v2") |
| + h.act.worker = func(a activator.Attempt, fb *release.StagedIdentity) activator.SwapResult { |
| + return activator.SwapResult{Outcome: activator.SwapFailed, Reason: "exited within 10s of starting"} |
| + } |
| + recorded := make(chan bool, 1) |
| + h.act.mu.Lock() |
| + h.act.onHeldPublish = func(string) { |
| + _, ok := h.book.Lookup("bab", "blog", "v2") |
| + recorded <- ok |
| + } |
| + h.act.mu.Unlock() |
| + h.install("v2", "bot") |
| + h.pass() |
| + select { |
| + case ok := <-recorded: |
| + if !ok { |
| + t.Fatal("the fallback's publication was released before v2's failure was recorded") |
| + } |
| + case <-time.After(5 * time.Second): |
| + t.Fatal("the held publication was never released") |
| + } |
| +} |
| + |
| // Spec §7.9: a recycle that falls due coalesces with a queued release attempt |
| // only when the scheduler would admit that attempt now. |
| func TestARecycleCoalescesOnlyWithAnAdmissibleRelease(t *testing.T) { |
| @@ -7445,8 +7943,11 @@ with |
| ```go |
| if rt.IsSidecar() { |
| // Workers and native sidecar members keep stop-then-start on the |
| - // same entry (spec §7.9). |
| + // same entry (spec §7.9). done() runs the cohort-failure transition |
| + // synchronously; only then may a fallback's identity be published |
| + // (spec §8.1), which the activator held back for this call. |
| s.done(d, s.executeWorker(ctx, *rt, a), a.Mode) |
| + m.d.Act.PublishHeldServing(rt.Host) |
| return |
| } |
| m.emit(*rt, EvSwapping, "info", textSwapping(a.Build, a.Cause)) |
| @@ -7482,7 +7983,7 @@ git commit -m "zdd-5 task 11: socketless attempts on the same entry, and the rec |
| - Consumes: plan 4's `release.Generations`, `GenerationsPath`, `WriteGenerations`, `ReadGenerations`, `GenerationPins`, `BoxPins`; plan 3's `e.ops.members`, `e.pin`, `swapOverlap`'s `a.spawnSuccessor(` call (Task 10); Task 9's `e.plan`; Task 11's `e.attemptBuilds`. |
| - Produces: |
| ```go |
| - // Service gains: GenerationsFile string (empty: never written, the flag off) |
| + // Service gains: GenerationsFile string (empty: never written; the agent sets it whatever the flag, Task 14) |
| func (a *Service) generationsLocked() release.Generations |
| func (a *Service) persistGenerations() error // nil when written, or when GenerationsFile is empty |
| var errGenerationsUnpublished error |
| @@ -7585,6 +8086,26 @@ func TestARetainedFailedWakeStaysPinned(t *testing.T) { |
| } |
| } |
| +// Spec §4.2, §7.9, §8.5 (P5 review finding 6): a worker's captured serving |
| +// build is what its recycle respawns onto whatever the flag says, so it is |
| +// published whatever the flag says. With generations off the file still |
| +// names it. Root pinned only the installed, previous and good builds, and |
| +// with B and C installed meanwhile it could sweep or restage the A the |
| +// worker runs and recycles onto. |
| +func TestAWorkersArtifactIsPinnedWithTheFlagOff(t *testing.T) { |
| + a, _, _, _, _ := stagedActivator(t) // generations off, no planner |
| + a.GenerationsFile = filepath.Join(t.TempDir(), "zdd", "generations.json") |
| + host := "bot.x.internal" |
| + if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + want := a.entryServing(host).Version |
| + waitFor(t, "the worker's build in the generations file", func() bool { |
| + g, _ := release.ReadGenerations(a.GenerationsFile) |
| + return g != nil && slices.Contains(g.Hosts[host], want) |
| + }) |
| +} |
| + |
| // unwritableGenerationsFile is a path whose parent is a regular file, so |
| // every write fails. |
| func unwritableGenerationsFile(t *testing.T) string { |
| @@ -7703,7 +8224,7 @@ func TestAnUnreadableGenerationsFileSkipsTheSweep(t *testing.T) { |
| - [ ] **Step 2: Run them to see them fail** |
| -Run: `TMPDIR=/tmp/k go test ./internal/activator/ ./internal/release/ ./internal/host/ -run 'TestTheGenerationsFile|TestAnUnpublishedBuild|TestARetainedFailedWake|TestBoxPinsNamesTheGoodBuild|TestSweepKeeps|TestAnUnreadableGenerations' -count=1` |
| +Run: `TMPDIR=/tmp/k go test ./internal/activator/ ./internal/release/ ./internal/host/ -run 'TestTheGenerationsFile|TestAnUnpublishedBuild|TestARetainedFailedWake|TestAWorkersArtifactIsPinned|TestBoxPinsNamesTheGoodBuild|TestSweepKeeps|TestAnUnreadableGenerations' -count=1` |
| Expected: FAIL, `a.GenerationsFile undefined`, `undefined: errGenerationsUnpublished`, `undefined: sweepStagedVersionsWith`, and the `BoxPins` assertion. |
| - [ ] **Step 3: Write the generations writer** |
| @@ -7811,12 +8332,15 @@ Add to `Service` in `activator.go`, beside `Planner`: |
| ```go |
| // GenerationsFile, when set, is where every live or pending generation's |
| - // build is published for root's pin set (release.GenerationsPath). Set |
| - // only with CARLOS_SWAP=overlap; empty writes nothing. |
| + // build is published for root's pin set (release.GenerationsPath). The |
| + // agent sets it whatever CARLOS_SWAP says (P5 review finding 6): a |
| + // worker's captured serving build is recycled onto with the flag off |
| + // too, and root pins only the installed, previous and good builds |
| + // without it. Empty writes nothing. |
| GenerationsFile string |
| ``` |
| -Call `a.persistGenerations()` (never while holding `a.mu`) at each of these points. The four **before a spawn** refuse the spawn on an error; the rest only log it (the function already does). |
| +Call `a.persistGenerations()` (never while holding `a.mu`) at each of these points. The five **before a spawn** refuse the spawn on an error; the rest only log it (the function already does). |
| Before a spawn: |
| - `doWake`: directly after the planning block Task 9 added (after `a.mu.Unlock()` that stores `e.plan`), and before step 1, outside the `if a.Planner != nil && e.pin == nil {` block so it covers a hold's pinned wake too: |
| @@ -7860,6 +8384,12 @@ Before a spawn: |
| } |
| } |
| ``` |
| +- Task 9's `checkPlan`, after it stores a replanned `next` on the entry and before it returns it (the replanned build must be on disk before it execs): |
| + ```go |
| + if err := a.persistGenerations(); err != nil { |
| + return plan, err |
| + } |
| + ``` |
| After a change (log only): |
| - `Start`: as its first statement, so the file exists before the first spawn (`_ = a.persistGenerations()`). |
| @@ -8335,6 +8865,7 @@ git commit -m "zdd-5 task 13: the rollout report in box status" |
| ```go |
| func armedRollout() *rollout.Manager // nil unless CARLOS_SWAP=overlap; plan 6 calls it in fastConvergeLoop |
| func armRollout(ctx context.Context, getenv func(string) string, svc *activator.Service, reg *registry.Registry, dd string, sink activator.EventSink, openStore func(context.Context) (release.Store, error)) *rollout.Manager |
| + func armAlways(svc *activator.Service, reg *registry.Registry, dd string) // whatever the flag: OnWorkerServing and GenerationsFile |
| func rolloutLegFor(m *rollout.Manager, reg *registry.Registry) func() |
| func feedRestarts(routes []registry.Route, moved [][2]string, scoped []scopedRestart, gen map[[2]string]string, feed func(host, id string) bool) int |
| func rolloutStatusProbe() *status.RolloutReport // with the flag on and the scheduler unarmed: only Unavailable |
| @@ -8392,6 +8923,24 @@ func TestTheFlagOffArmsNothing(t *testing.T) { |
| } |
| } |
| +// Spec §4.2, §7.9, §8.5 (P5 review finding 6): with the flag off a worker's |
| +// captured serving build is still recycled onto, so the generations file is |
| +// still written (root's sweep and restage pin what it names), and the |
| +// serving version still published. armRollout returning early left both |
| +// unset, and root could delete the artifact the next recycle needs. |
| +func TestTheFlagOffStillPublishesWorkerArtifacts(t *testing.T) { |
| + svc, reg, dd := rolloutFixture(t) |
| + armAlways(svc, reg, dd) |
| + armRollout(context.Background(), func(string) string { return "" }, svc, reg, dd, nil, |
| + func(context.Context) (release.Store, error) { return nil, errors.New("unused") }) |
| + if svc.GenerationsFile != release.GenerationsPath(dd) { |
| + t.Fatalf("generations file = %q with the flag off, want %q", svc.GenerationsFile, release.GenerationsPath(dd)) |
| + } |
| + if svc.OnWorkerServing == nil { |
| + t.Fatal("no worker serving publication with the flag off") |
| + } |
| +} |
| + |
| func TestTheFlagOnArmsTheScheduler(t *testing.T) { |
| svc, reg, dd := rolloutFixture(t) |
| t.Cleanup(func() { rolloutMgr.Store(nil) }) |
| @@ -8553,6 +9102,24 @@ func refuseManaged(svc *activator.Service, reason string) { |
| } |
| } |
| +// armAlways wires what the activator needs whatever CARLOS_SWAP says. A |
| +// worker's serving identity (an empty version included) becomes the |
| +// registry's ServingVersion, which `carlos sidecars ls` reads (spec §7.9, |
| +// Task 11). And every live and pending artifact is published for root's |
| +// pin sets (spec §4.2, §8.5; P5 review finding 6): a worker's captured |
| +// serving build is what its recycle respawns onto with the flag off too, |
| +// and root pins only the installed, previous and good builds without the |
| +// file. With the flag off the file names only that (generations are off, |
| +// so no generation, plan or attempt adds to it). |
| +func armAlways(svc *activator.Service, reg *registry.Registry, dd string) { |
| + svc.OnWorkerServing = func(host string, id release.StagedIdentity) { |
| + if err := reg.SetServingVersion(host, id.Version); err != nil { |
| + log.Printf("agent: %s: record the worker's serving build %q: %v", host, id.Version, err) |
| + } |
| + } |
| + svc.GenerationsFile = release.GenerationsPath(dd) |
| +} |
| + |
| // armRollout builds the scheduler when CARLOS_SWAP=overlap (spec §15) and |
| // wires it into svc, which must not have started yet. With the flag off it |
| // returns nil and touches nothing. With the flag on but no deploy store, no |
| @@ -8692,19 +9259,12 @@ In `runAgent`, directly before `svc = activator.New(*cfg, &activator.SplitBacken |
| activator.SetGenerations(swapOn) |
| eb.GenContainment = swapOn |
| rolloutMaterialize = eb.Materialize |
| - // Whatever the flag (Task 11): a worker's serving identity is the |
| - // registry's ServingVersion, which `carlos sidecars ls` reads. |
| - workerServing := func(host string, id release.StagedIdentity) { |
| - if err := reg.SetServingVersion(host, id.Version); err != nil { |
| - log.Printf("agent: %s: record the worker's serving build %s: %v", host, id.Version, err) |
| - } |
| - } |
| ``` |
| After `svc.Logs = logSink` (line 393, still before `startActivator(...)`), add: |
| ```go |
| - svc.OnWorkerServing = workerServing // flag on or off |
| + armAlways(svc, reg, dd) // flag on or off |
| var rolloutSink activator.EventSink |
| if logSink != nil { // a typed nil in the interface would not be nil |
| rolloutSink = logSink |