amadan

carlos / platform Public

Sidecar isolation Task 6 (5/6): an uncertain nested worker start is not "nothing ran" (D19, R2-1)

A respawn whose worker start came back uncertain leaves the obligation on
the entry (unitUnproven): the next respawn proves the worker unit gone
through roleGone before it execs anything, and a terminal stop proves it
before the lease goes, whatever the generations switch says. ReplaceWorker's
unchanged-incarnation branch reads an executed-or-uncertain start as the
attempt's build having run: a hold restores nothing and stops the entry; an
overlap puts the replaced build back only once the attempted unit is proven
gone. Both abandon, since the build was not seen to fail.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude 3f52a36b0870a95f252bef160205ee2f0dd27436 parent bee95bc
4 files changed, +237 −2
  • internal/activator/activator.go +49 −1
  • internal/activator/opset.go +35 −1
  • internal/activator/worker_nested_test.go +126 −0
  • internal/activator/workerswap.go +27 −0
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index 63139231f..48a6e81ff 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -759,6 +759,15 @@ type actEntry struct {
// exited (reapShedLocked): a terminal stop keeps the lease until then,
// and no respawn execs beside it.
shed *shedChild
+
+ // unitUnproven is a nested worker's obligation after a respawn whose
+ // start came back uncertain (sidecar isolation D19, R2-1): root may have
+ // started the worker unit, no process handle came back, and nothing
+ // proves it gone. Until roleGone for the worker role proves it, no
+ // respawn execs beside it (respawnWorkerLocked's retired proof) and no
+ // terminal stop releases the lease (stopUnitProven). Written under
+ // a.mu; cleared by the proof that holds.
+ unitUnproven bool
}
// shedChild is a respawn child that was exec'd and then refused at its
@@ -3613,6 +3622,21 @@ func (a *Service) respawnWorkerLocked(e *actEntry, stopOld Proc, why string, ont
retired = e.shed == nil
a.mu.Unlock()
}
+ // A nested worker unit an earlier respawn's start may have left
+ // running (unitUnproven; sidecar isolation D19, R2-1) is no process
+ // of ours and runs in no cgroup KillHost reaches: only the fenced
+ // /stop and the unit's own facts prove it gone (roleGone). Unproven,
+ // nothing execs beside it, whatever the switch says.
+ if retired && a.unitUnprovenLocked(e) {
+ if perr := a.roleGone(rt, instancectl.RoleWorker); perr != nil {
+ log.Printf("activator: worker %s: an earlier start's worker unit is not proven gone (%v); nothing execs beside it", host, perr)
+ retired = false
+ } else {
+ a.mu.Lock()
+ e.unitUnproven = false
+ a.mu.Unlock()
+ }
+ }
// Still wanted? Asked HERE, before anything is spawned: past this
// point a stop can only be raced, never prevented. The gate's
@@ -3664,6 +3688,15 @@ func (a *Service) respawnWorkerLocked(e *actEntry, stopOld Proc, why string, ont
a.mu.Lock()
e.workerRespawning = false
+ if legacyStartMayRun(err) {
+ // The nested arm's uncertain start (sidecar isolation D19): root
+ // may have started the worker unit, and nothing proves it gone.
+ // The obligation stays on the entry until roleGone holds: the
+ // next respawn (the sweep's crash restart, a restore, a
+ // fallback) proves it before it execs, and a terminal stop
+ // proves it before the lease goes (R2-1).
+ e.unitUnproven = true
+ }
// And once more before the child is installed: ownership lost after
// the exec sheds the child (final review F1). ownedLocked covers what
// the old liveness check did (the entry is the host's, awake, not
@@ -3778,7 +3811,10 @@ func (a *Service) respawnWorkerLocked(e *actEntry, stopOld Proc, why string, ont
// refuses a recovery onto changed bytes, and that refusal must
// outlast the retry (ruling R17), so the next crash restart asks
// for the same captured build again. Only a converge, admitted
- // on its own terms, moves the worker onto another build.
+ // on its own terms, moves the worker onto another build. An
+ // uncertain nested start left its obligation on the entry
+ // above (unitUnproven): the crash restart proves the unit gone
+ // before it execs anything.
a.mu.Unlock()
log.Printf("activator: WORKER %s failed to respawn (%s): %v — the sweep's crash restart takes it from here", host, why, err)
emitEvent(a.Logs, rt, "error", "sidecar could not be restarted. retrying")
@@ -4185,6 +4221,18 @@ func (a *Service) stop(e *actEntry) {
a.mu.Unlock()
return
}
+ if !a.stopUnitProven(e) {
+ // A nested worker unit an uncertain respawn start may have left
+ // running (sidecar isolation D19, R2-1): proven gone through the
+ // fenced /stop first, whatever the switch says. Unproven, the stop
+ // stays pending with the lease, and the sweep retries the proof
+ // (reapCurrentGen).
+ log.Printf("activator: stop %s: an earlier start's worker unit is not proven gone; keeping the lease until it is", e.host)
+ a.mu.Lock()
+ a.keepPendingLocked(e, outcome)
+ a.mu.Unlock()
+ return
+ }
if !a.gens && !a.stopHostProven(e) {
// Flag off, the leader's exit says nothing of what it forked (Task
// 8, F6): the host cgroup is proven empty first. Unproven, the stop
diff --git a/internal/activator/opset.go b/internal/activator/opset.go
index 262fa2d0d..8be0bb855 100644
--- a/internal/activator/opset.go
+++ b/internal/activator/opset.go
@@ -420,6 +420,37 @@ func (a *Service) stopHostProven(e *actEntry) bool {
return false
}
+// unitUnprovenLocked reads e.unitUnproven under a.mu.
+func (a *Service) unitUnprovenLocked(e *actEntry) bool {
+ a.mu.Lock()
+ defer a.mu.Unlock()
+ return e.unitUnproven
+}
+
+// stopUnitProven is a terminal stop's proof for a nested worker whose
+// respawn start came back uncertain (actEntry.unitUnproven; sidecar
+// isolation D19, R2-1): root may have started the worker unit, no handle
+// came back, and it runs in no cgroup KillHost or EmptyHost reaches. Only
+// roleGone for the worker role, the fenced /stop and the unit's own facts,
+// proves it gone; a proof that holds discharges the obligation. Nothing to
+// prove answers true. Never called with a.mu held.
+func (a *Service) stopUnitProven(e *actEntry) bool {
+ a.mu.Lock()
+ unproven, rt := e.unitUnproven, e.route
+ a.mu.Unlock()
+ if !unproven {
+ return true
+ }
+ if err := a.roleGone(rt, instancectl.RoleWorker); err != nil {
+ log.Printf("activator: stop %s: an earlier start's worker unit is not proven gone: %v", rt.Host, err)
+ return false
+ }
+ a.mu.Lock()
+ e.unitUnproven = false
+ a.mu.Unlock()
+ return true
+}
+
// wakeHostProven is a flag-off wake's proof, under its lease and before
// Restore (Task 8 fix round 1, finding 4), that carlos-exec@<host> holds
// nothing: EmptyHost reads cgroup.events and kills only what is left, so a
@@ -712,7 +743,10 @@ func (a *Service) reapCurrentGen(e *actEntry) {
if a.gens {
proven = a.releasable
}
- if !proven(e) {
+ // A nested worker unit an uncertain respawn start may have left
+ // running is proven gone first, whatever the switch (stopUnitProven;
+ // sidecar isolation D19, R2-1).
+ if !a.stopUnitProven(e) || !proven(e) {
a.mu.Lock()
e.genReaping = false
a.mu.Unlock()
diff --git a/internal/activator/worker_nested_test.go b/internal/activator/worker_nested_test.go
index 24e4a846c..eb6423db0 100644
--- a/internal/activator/worker_nested_test.go
+++ b/internal/activator/worker_nested_test.go
@@ -927,3 +927,129 @@ func TestFailedNestedWorkerWakeKeepsLeaseUntilRoleGone(t *testing.T) {
t.Fatal("the proven wake is still retained")
}
}
+
+// ---- D19 replacements (R2-1): an uncertain start is not "nothing ran" ----
+
+// uncertainWorkerStart is the nested arm's answer to a worker start root
+// may have carried out: marked both ways.
+func uncertainWorkerStart() error {
+ return markLegacyStartUncertain(markStartUncertain(errors.New("start carlos-instance-worker@bot.service: context deadline exceeded")))
+}
+
+// R2-1: a hold ReplaceWorker whose worker start answers uncertain restores
+// nothing (the attempt's build may have migrated the database) and stops
+// the entry; the stop keeps the lease until RoleGone for the worker role
+// proves the attempted unit gone.
+func TestHoldReplaceWorkerUncertainStartRestoresNothing(t *testing.T) {
+ shrinkFirstWake(t, 55*time.Second, 25*time.Second, 300*time.Millisecond)
+ a, b, clk, _, _ := stagedActivator(t)
+ fb := b.fakeBackend
+ host := "bot.x.internal"
+ if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil {
+ t.Fatal(err)
+ }
+ b.smu.Lock()
+ b.failNextStaged = uncertainWorkerStart()
+ b.smu.Unlock()
+ fb.mu.Lock()
+ fb.legacyStopErr = errors.New("unit is deactivating")
+ fb.mu.Unlock()
+ good := fakeIdentity("v1")
+ res := a.ReplaceWorker(context.Background(), workerAttempt(CauseRelease, ModeHold, fakeIdentity("v2")), &good, one)
+ if res.Outcome != SwapAbandoned || res.Executed != true {
+ t.Fatalf("ReplaceWorker = %+v, want abandoned with the build possibly executed", res)
+ }
+ if calls := b.calls(); len(calls) != 2 {
+ t.Fatalf("spawns %v: a hold attempt whose start is uncertain restored a build over a database the new one may have migrated", calls)
+ }
+ // The entry is stopping, and its stop pends on the proof: the lease is
+ // kept while RoleGone fails.
+ for i := 0; i < 3; i++ {
+ clk.advance(time.Minute)
+ a.sweepOnce()
+ }
+ waitFor(t, "the worker proof to be asked", func() bool { return len(fb.roleGoneRoles()) > 0 })
+ if fb.releaseCount() != 0 || !a.Owns(host) {
+ t.Fatalf("releases %d, owns %v: the lease went back over an unproven worker unit", fb.releaseCount(), a.Owns(host))
+ }
+ for _, r := range fb.roleGoneRoles() {
+ if r != instancectl.RoleWorker {
+ t.Fatalf("RoleGone asked for %s, want the worker role only", r)
+ }
+ }
+ fb.mu.Lock()
+ fb.legacyStopErr = nil
+ fb.mu.Unlock()
+ clk.advance(time.Minute)
+ a.sweepOnce()
+ waitFor(t, "the entry to stop once the unit is proven gone", func() bool { return !a.Owns(host) && fb.releaseCount() == 1 })
+ if calls := b.calls(); len(calls) != 2 {
+ t.Fatalf("spawns %v: something was restored after all", calls)
+ }
+}
+
+// R2-1: an overlap ReplaceWorker whose worker start answers uncertain falls
+// back only after RoleGone has proven the attempted unit gone. While the
+// proof fails nothing execs and the entry keeps its lease; once it holds,
+// the build the attempt replaced comes back.
+func TestOverlapReplaceWorkerFallsBackOnlyAfterRoleGone(t *testing.T) {
+ for _, gens := range []bool{false, true} {
+ t.Run(fmt.Sprintf("gens=%v", gens), func(t *testing.T) {
+ shrinkFirstWake(t, 55*time.Second, 25*time.Second, 300*time.Millisecond)
+ a, b, clk, _, _ := stagedActivator(t)
+ a.gens = gens
+ fb := b.fakeBackend
+ host := "bot.x.internal"
+ if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil {
+ t.Fatal(err)
+ }
+ prev := a.entryServing(host)
+ b.smu.Lock()
+ b.failNextStaged = uncertainWorkerStart()
+ b.smu.Unlock()
+ fb.mu.Lock()
+ fb.legacyStopErr = errors.New("unit is deactivating")
+ fb.mu.Unlock()
+ good := fakeIdentity("v1")
+ res := a.ReplaceWorker(context.Background(), workerAttempt(CauseRelease, ModeOverlap, fakeIdentity("v2")), &good, one)
+ if res.Outcome != SwapAbandoned || !res.Executed {
+ t.Fatalf("ReplaceWorker = %+v, want abandoned with the build possibly executed", res)
+ }
+ waitFor(t, "the restore's proof to be asked", func() bool { return len(fb.roleGoneRoles()) > 0 })
+ waitFor(t, "the restore to stand down", func() bool { return !a.entryRespawning(host) })
+ if calls := b.calls(); len(calls) != 2 {
+ t.Fatalf("spawns %v: a build was exec'd beside a worker unit not proven gone", calls)
+ }
+ // The sweep retries; still unproven, nothing execs and the lease
+ // stays.
+ clk.advance(time.Minute)
+ a.sweepOnce()
+ waitFor(t, "a retried proof", func() bool { return len(fb.roleGoneRoles()) >= 2 })
+ waitFor(t, "the retry to stand down", func() bool { return !a.entryRespawning(host) })
+ if calls := b.calls(); len(calls) != 2 || fb.releaseCount() != 0 || !a.Owns(host) {
+ t.Fatalf("spawns %v, releases %d, owns %v", calls, fb.releaseCount(), a.Owns(host))
+ }
+ for _, r := range fb.roleGoneRoles() {
+ if r != instancectl.RoleWorker {
+ t.Fatalf("RoleGone asked for %s, want the worker role only", r)
+ }
+ }
+ // Proven gone: the replaced build comes back, on the same entry.
+ fb.mu.Lock()
+ fb.legacyStopErr = nil
+ fb.mu.Unlock()
+ clk.advance(time.Minute)
+ a.sweepOnce()
+ waitFor(t, "the replaced build back", func() bool {
+ calls := b.calls()
+ return len(calls) == 3 && !a.entryRespawning(host) && a.entryServing(host) == prev
+ })
+ if calls := b.calls(); *calls[2] != prev {
+ t.Fatalf("respawned %+v, want the build the attempt replaced", calls[2])
+ }
+ if fb.releaseCount() != 0 || !a.Owns(host) {
+ t.Fatal("the entry lost its lease across the fallback")
+ }
+ })
+ }
+}
diff --git a/internal/activator/workerswap.go b/internal/activator/workerswap.go
index 8edf3ba9c..f3d9083aa 100644
--- a/internal/activator/workerswap.go
+++ b/internal/activator/workerswap.go
@@ -37,6 +37,10 @@ import (
// The scheduler runs the cohort-failure transition from the result, then
// calls PublishHeldServing for a fallback's identity. newest is called with
// a.mu held, as Swap's is.
+// errSwapStartUncertain abandons a socketless attempt whose worker start may
+// have run without a handle coming back (sidecar isolation D19, R2-1).
+var errSwapStartUncertain = errors.New("the worker unit may have started; nothing is restored until it is proven gone")
+
func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *release.StagedIdentity, newest func(host string) uint64) (out SwapResult) {
if a.cfg.HostWrites.Enabled {
return abandon(ErrClaimIsolationUnavailable)
@@ -149,6 +153,29 @@ 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 e.workerInc == inc0 && executed {
+ // No new incarnation, and the start's error says the attempt's
+ // build may have run (startMayHaveRun): a nested worker's start root
+ // may have carried out (sidecar isolation D19, R2-1). That is not
+ // "nothing ran". The respawn left the obligation on the entry
+ // (unitUnproven), and this reads as the attempt's build having run:
+ // a hold restores nothing and stops the entry, as a hold build
+ // exec'd before a refusal does (final review F2; spec §6.4, §8.3),
+ // and its stop proves the unit gone before the lease goes; an
+ // overlap puts the replaced build back only after the attempted
+ // unit is proven gone, which the restore's own respawn requires
+ // before it execs (roleGone), execing nothing until it holds.
+ // Abandoned, not a failure of the build: it was not seen to fail.
+ if at.Mode == ModeHold {
+ log.Printf("activator: worker %s: hold attempt onto %s has an uncertain start (the unit may have run); the build it replaced (%s) is not restored, and the worker stops once the unit is proven gone", at.Host, onto.Version, prev.Version)
+ a.beginStopLocked(e, StopRelease)
+ } else {
+ log.Printf("activator: worker %s: attempt onto %s has an uncertain start (the unit may have run); the build it replaced (%s) comes back once the unit is proven gone", at.Host, onto.Version, prev.Version)
+ a.restoreLocked(e, prev)
+ }
+ a.mu.Unlock()
+ return abandon(fmt.Errorf("staging: worker start onto %s is uncertain: %w", onto.Version, errSwapStartUncertain))
+ }
if e.workerInc == inc0 || !sameArtifact(e.workerServing, onto) {
// Nothing of the attempt's build ran: no new incarnation was
// installed (the exec failed, its build could not be published