amadan

carlos / platform Public

settings-4 task 7: fix round 1: a retained copy nudges root; failed and unread holds never run the link

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 9293c114a4a5bef40e394401172fed13ab30f8e4 parent ab13192
4 files changed, +249 −12
  • internal/activator/wakecheck.go +41 −4
  • internal/activator/wakecheck_test.go +143 −3
  • internal/host/buildstate.go +21 −5
  • internal/host/buildstate_test.go +44 −0
diff --git a/internal/activator/wakecheck.go b/internal/activator/wakecheck.go
index dedc674e6..dca5e029d 100644
--- a/internal/activator/wakecheck.go
+++ b/internal/activator/wakecheck.go
@@ -58,11 +58,14 @@ const heldPoll = 250 * time.Millisecond
// it captures against it (heldIdentityOK);
// - with no planner and direct set (the caller can exec a named file), a
// retained verified copy of the target, which the caller runs as its
-// identity, recorded first;
+// identity, recorded first; root is asked to install the target too,
+// without waiting (R-RETAINED-NUDGE);
// - otherwise root is asked to install the target (RequestStage, the
// stage relay) and the wake waits for the live link to name it, within
// its own deadline and never past wakeWaitBound, the bound its waiters
-// have. A wait that ends first is errStaleLink.
+// have. A wait that ends first is errStaleLink. The answer is asked
+// again every heldReask polls, and a changed answer ends the wait at
+// once with its own refusal (unknown, failed) or errStaleLink.
//
// It never sets e.pin (Astra I2 #4): with a planner, what boots after root's
// install is the plan's. The string is the target ("" when the wake is not
@@ -89,7 +92,16 @@ func (a *Service) wakeCheck(ctx context.Context, host string, direct bool) (stri
return h.Target, nil, nil
}
if direct && a.Planner == nil && h.Staged != nil && h.Staged.Version == h.Target {
- return h.Target, h.Staged, nil // run the retained copy; step 2b records it first
+ // Run the retained copy; step 2b records it first. Root is still
+ // asked to install the target, without waiting (ruling
+ // R-RETAINED-NUDGE): until it repoints, the live link names another
+ // build than the one running, and the sweep's convergence would
+ // cycle the instance after ConvergeGrace, waking it onto the same
+ // copy each time, until root's own pass.
+ if a.RequestStage != nil {
+ a.RequestStage(host, h.Target)
+ }
+ return h.Target, h.Staged, nil
}
if a.RequestStage != nil {
a.RequestStage(host, h.Target)
@@ -98,10 +110,26 @@ func (a *Service) wakeCheck(ctx context.Context, host string, direct bool) (stri
defer cancel()
tick := time.NewTicker(heldPoll)
defer tick.Stop()
- for {
+ for n := 1; ; n++ {
if a.liveVersion(host) == h.Target {
return h.Target, nil, nil
}
+ if n%heldReask == 0 {
+ // The answer is asked again while the wake waits: a member's
+ // change (a pin, Update now, auto) or root's verdict (failed,
+ // unknown) ends this wait now, rather than at its bound onto a
+ // target that is no longer the instance's. The next wake checks
+ // afresh.
+ if next := a.HeldTarget(host); !sameHeld(next, h) {
+ switch {
+ case next.Held && (next.Unknown || next.Target == ""):
+ return "", nil, errBuildUnknown
+ case next.Held && next.Failed && a.Planner == nil:
+ return "", nil, &targetFailedError{host: host, build: next.Target}
+ }
+ return "", nil, fmt.Errorf("%w: %s's target changed from %s while it waited", errStaleLink, host, h.Target)
+ }
+ }
select {
case <-wait.Done():
return "", nil, fmt.Errorf("%w: %s is on %q, wants %s", errStaleLink, host, installed, h.Target)
@@ -110,6 +138,15 @@ func (a *Service) wakeCheck(ctx context.Context, host string, direct bool) (stri
}
}
+// heldReask is how many polls a held wake's wait makes between asking
+// HeldTarget again (which may read the bucket).
+const heldReask = 8
+
+// sameHeld reports whether two answers send the wake to the same place.
+func sameHeld(x, y HeldWake) bool {
+ return x.Held == y.Held && x.Target == y.Target && x.Unknown == y.Unknown && x.Failed == y.Failed
+}
+
// liveVersion is what live/<host> names now, "" when it cannot be read.
func (a *Service) liveVersion(host string) string {
if a.liveVer == nil {
diff --git a/internal/activator/wakecheck_test.go b/internal/activator/wakecheck_test.go
index 28768495a..e7d41f464 100644
--- a/internal/activator/wakecheck_test.go
+++ b/internal/activator/wakecheck_test.go
@@ -157,21 +157,32 @@ func TestWithNoPlannerAFailedTargetIsRefusedAtTheLiveLink(t *testing.T) {
}
// Astra I3 #5: with no planner, a retained, verified staged copy of the
-// target runs directly, recorded first; root's repoint is not needed.
+// target runs directly, recorded first; root's repoint is not needed. Root
+// is still asked to install the target, without the wake waiting for it
+// (ruling R-RETAINED-NUDGE): here root never repoints, and the wake runs
+// the retained file at once.
func TestWithNoPlannerARetainedCopyRunsWithoutARepoint(t *testing.T) {
- shrinkWakeWait(t, 2*time.Second)
+ shrinkWakeWait(t, 10*time.Second)
a, b := identityActivator(t, "v3") // stale v3 installed
retained := stagedID(t, t.TempDir(), "v1")
a.HeldTarget = func(string) HeldWake { return HeldWake{Target: "v1", Held: true, Staged: &retained} }
- a.RequestStage = func(string, string) { t.Error("asked root to repoint, with the retained copy on the box") }
+ r := &asks{} // records, never repoints
+ a.RequestStage = r.ask
l := &execLog{spawns: b.startCount}
a.BeforeExec = l.hook
rt := identityRoute(t, "one.example.com")
+ began := time.Now()
done, err := a.beginRequest(context.Background(), rt)
if err != nil {
t.Fatalf("the wake waited for root instead of running the retained copy: %v", err)
}
done()
+ if took := time.Since(began); took >= time.Second {
+ t.Fatalf("the wake took %v: it waited for root", took)
+ }
+ if got := r.seen(); !slices.Equal(got, []string{"one.example.com@v1"}) {
+ t.Fatalf("asked root %v, want v1 asked for once, not waited on", got)
+ }
x, ok := b.lastExe()
calls, at, _ := l.seen()
if !ok || x.id != retained || !slices.Equal(calls, []string{"one.example.com@v1"}) || at[0] != 0 {
@@ -185,6 +196,46 @@ func TestWithNoPlannerARetainedCopyRunsWithoutARepoint(t *testing.T) {
}
}
+// Ruling R-RETAINED-NUDGE: a retained wake runs v1 while the live link
+// still names v3, which the sweep's convergence reads as drift and cycles
+// after ConvergeGrace. Root, asked at the wake, repoints inside the grace:
+// the drift is gone and the instance keeps serving. The control (root never
+// repoints) shows the sweep does cycle it.
+func TestARetainedWakeIsNotCycledOnceRootRepoints(t *testing.T) {
+ for _, repoint := range []bool{true, false} {
+ a, b, clk, _ := testActivator(t, time.Hour)
+ b.setIdentity("v3")
+ retained := stagedID(t, t.TempDir(), "v1")
+ a.HeldTarget = func(string) HeldWake { return HeldWake{Target: "v1", Held: true, Staged: &retained} }
+ r := &asks{}
+ a.RequestStage = r.ask
+ host := "one.example.com"
+ done, err := a.beginRequest(context.Background(), identityRoute(t, host))
+ if err != nil {
+ t.Fatal(err)
+ }
+ done()
+ if len(r.seen()) != 1 {
+ t.Fatalf("asked root %v, want once", r.seen())
+ }
+ a.sweepOnce() // the drift is seen: v1 runs, v3 is linked
+ clk.advance(ConvergeGrace / 2)
+ if repoint {
+ b.setLive("v1") // root's pass, asked at the wake, lands inside the grace
+ }
+ a.sweepOnce()
+ clk.advance(ConvergeGrace)
+ a.sweepOnce()
+ _, awake := a.entryGen(host)
+ if repoint && !awake {
+ t.Fatal("the retained wake was cycled although root repointed inside the grace")
+ }
+ if !repoint && awake {
+ t.Fatal("control: a retained wake on a link root never moves was not cycled; the sweep fixture proves nothing")
+ }
+ }
+}
+
// Anchors D18: a retained copy is a staged file run that no plan or pin
// published. The wake publishes it as pending (ruling R-COLDPUBLISH) before
// it records, so root's sweep cannot remove the file between the capture
@@ -402,3 +453,92 @@ func TestHeldIdentityOK(t *testing.T) {
}
}
}
+
+// The wait asks for the answer again: a changed one ends it at once, with
+// its own refusal, rather than at the bound onto a stale target.
+func TestAHeldWakeEndsItsWaitWhenTheAnswerChanges(t *testing.T) {
+ cases := []struct {
+ name string
+ next HeldWake
+ want error
+ }{
+ {"unknown", HeldWake{Held: true, Unknown: true}, errBuildUnknown},
+ {"failed", HeldWake{Target: "v1", Held: true, Failed: true}, errTargetFailed},
+ {"another target", HeldWake{Target: "v2", Held: true}, errStaleLink},
+ {"no longer held", HeldWake{}, errStaleLink},
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ shrinkWakeWait(t, 20*time.Second)
+ a, b := identityActivator(t, "v3")
+ var asked atomic.Int32
+ a.HeldTarget = func(string) HeldWake {
+ if asked.Add(1) == 1 {
+ return HeldWake{Target: "v1", Held: true}
+ }
+ return c.next
+ }
+ a.RequestStage = func(string, string) {} // root never installs v1
+ began := time.Now()
+ _, err := a.beginRequest(context.Background(), identityRoute(t, "one.example.com"))
+ if !errors.Is(err, c.want) {
+ t.Fatalf("err = %v, want %v", err, c.want)
+ }
+ if took := time.Since(began); took >= 10*time.Second {
+ t.Fatalf("the wait ran %v: the changed answer did not end it", took)
+ }
+ if b.startCount() != 0 {
+ t.Fatal("a wake whose answer changed ran something")
+ }
+ })
+ }
+}
+
+// countingRoutes is a RouteLookup whose nth Get runs a hook.
+type countingRoutes struct {
+ rt registry.Route
+ n atomic.Int32
+ onGet func(n int32)
+}
+
+func (c *countingRoutes) Get(string) (*registry.Route, error) {
+ if c.onGet != nil {
+ c.onGet(c.n.Add(1))
+ }
+ rt := c.rt
+ return &rt, nil
+}
+
+// A held unit-backed row (systemd execs the live link; nothing can pin its
+// file): root installs the target, then the link moves after the wait. The
+// read just before the start refuses it, and nothing starts.
+func TestAHeldUnitRowRefusesALinkThatMovedAfterTheWait(t *testing.T) {
+ exec, unit := &fakeBackend{}, &fakeBackend{}
+ exec.setIdentity("v3") // live/<host>: SplitBackend.LiveVersion reads the exec half's link
+ split := &SplitBackend{Exec: exec, Unit: struct{ Backend }{unit}}
+ reg, err := registry.Open(filepath.Join(t.TempDir(), "registry.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ a := New(Config{Bucket: "test-bucket", Idle: time.Hour}, split, reg)
+ ur := identityRoute(t, "u.example.com")
+ ur.Backing = registry.BackingUnit
+ routes := &countingRoutes{rt: ur}
+ // Get 1 is step 0's; Get 2 is the re-read after the wait.
+ routes.onGet = func(n int32) {
+ if n == 2 {
+ exec.setLive("v2")
+ }
+ }
+ a.Routes = routes
+ a.HeldTarget = heldOn("v1")
+ a.RequestStage = func(string, string) { exec.setLive("v1") }
+ _, err = a.beginRequest(context.Background(), ur)
+ if !errors.Is(err, errStaleLink) {
+ t.Fatalf("err = %v, want errStaleLink", err)
+ }
+ if unit.startCount() != 0 || exec.startCount() != 0 {
+ t.Fatalf("unit starts %d, exec starts %d: a moved link was started", unit.startCount(), exec.startCount())
+ }
+ waitFor(t, "the lease to go back", func() bool { return unit.releaseCount() == 1 })
+}
diff --git a/internal/host/buildstate.go b/internal/host/buildstate.go
index f0e8dfef1..a7d3565f0 100644
--- a/internal/host/buildstate.go
+++ b/internal/host/buildstate.go
@@ -457,15 +457,31 @@ func (b *buildStates) heldTarget(host string) activator.HeldWake {
return held(bs.State.Resolved.Target)
}
}
- heldByRoot := c != nil && (c.Reason == string(release.ReasonManual) || c.Reason == string(release.ReasonMajor) || rootUnknown)
+ le := ""
+ if bs.State != nil {
+ le = bs.State.LastExecuted
+ }
+ // Rule 3 (held): manual, or a major hold in force by root's word. Root
+ // rewrites a major hold whose last_executed it holds as failed to
+ // Reason failed with that build as Detail, so that decision is a major
+ // hold too (ruling R-HELD-FAILED): held on it, and Failed (rootFailed).
+ majorFailed := c != nil && u.Major != nil && le != "" && c.Reason == string(release.ReasonFailed) && c.Detail == le
+ heldByRoot := c != nil && (c.Reason == string(release.ReasonManual) || c.Reason == string(release.ReasonMajor) || rootUnknown) || majorFailed
if u.Updates != release.UpdatesManual && !heldByRoot {
return activator.HeldWake{}
}
- if rootUnknown {
+ // Ruling R-HELD-UNKNOWN, as root's Target decides rule 3: an unread copy
+ // is unknown, whatever root last wrote; a read one with last_executed
+ // targets it, even over a stale unknown of root's (root reaches the same
+ // answer from the same copy, and the wake asks it to). Root's unknown
+ // stands only for a read copy with no last_executed.
+ switch {
+ case !bs.Known:
+ return activator.HeldWake{Held: true, Unknown: true}
+ case le != "":
+ return held(le)
+ case rootUnknown:
return activator.HeldWake{Held: true, Unknown: true}
- }
- if bs.State != nil && bs.State.LastExecuted != "" {
- return held(bs.State.LastExecuted)
}
return activator.HeldWake{} // no build state: whatever root installed is the instance's
}
diff --git a/internal/host/buildstate_test.go b/internal/host/buildstate_test.go
index 6b2c3048d..91863757d 100644
--- a/internal/host/buildstate_test.go
+++ b/internal/host/buildstate_test.go
@@ -984,3 +984,47 @@ func (g *getCounter) n() int {
defer g.mu.Unlock()
return g.gets
}
+
+// Ruling R-HELD-FAILED: root's rule 3 rewrites a major hold whose
+// last_executed this box holds as failed to Reason failed, Detail that
+// build. The wake still reads the major hold: held on it, and failed, so
+// with no planner the wake is refused rather than run the live link.
+func TestAMajorHoldOnAFailedBuildStaysHeldAndFailed(t *testing.T) {
+ b, _, reg := testBuildStates(t)
+ two := 2
+ setSettings(t, reg, release.UpdateSettings{Updates: release.UpdatesAuto, Channel: "stable", Major: &two})
+ setCopy(t, reg, release.BuildStateCopy{Known: true, Replica: true, State: &release.BuildState{LastExecuted: "v2"}})
+ writeConsumed(t, b, release.Consumed{Reason: string(release.ReasonFailed), Detail: "v2"})
+ if h := b.heldTarget("a.example.com"); !h.Held || !h.Failed || h.Target != "v2" {
+ t.Fatalf("heldTarget = %+v; want v2 held and failed", h)
+ }
+ // Root's failed head under rule 4 (Detail is the head, not
+ // last_executed) is not a hold.
+ writeConsumed(t, b, release.Consumed{Reason: string(release.ReasonFailed), Detail: "v3"})
+ if h := b.heldTarget("a.example.com"); h.Held {
+ t.Fatalf("heldTarget = %+v; a failed head under adoption is not a hold", h)
+ }
+}
+
+// Ruling R-HELD-UNKNOWN: rule 3 as root's Target decides it. An unread copy
+// is unknown whatever root last wrote; a read copy with last_executed
+// targets it over a stale unknown; root's unknown stands for a read copy
+// with no last_executed.
+func TestAHeldWakeReadsTheCopyAsRootsTargetDoes(t *testing.T) {
+ b, _, reg := testBuildStates(t)
+ setSettings(t, reg, release.UpdateSettings{Updates: release.UpdatesManual, Channel: "stable"})
+ setCopy(t, reg, release.BuildStateCopy{}) // never read
+ writeConsumed(t, b, release.Consumed{Reason: string(release.ReasonManual), Desired: "v2"})
+ if h := b.heldTarget("a.example.com"); !h.Held || !h.Unknown {
+ t.Fatalf("an unread copy under manual = %+v; want held, unknown", h)
+ }
+ setCopy(t, reg, release.BuildStateCopy{Known: true, Replica: true, State: &release.BuildState{LastExecuted: "v2"}})
+ writeConsumed(t, b, release.Consumed{Reason: string(release.ReasonUnknown)}) // written while the copy was unread
+ if h := b.heldTarget("a.example.com"); !h.Held || h.Unknown || h.Target != "v2" {
+ t.Fatalf("a read copy with last_executed under root's stale unknown = %+v; want held on v2", h)
+ }
+ setCopy(t, reg, release.BuildStateCopy{Known: true, Replica: true})
+ if h := b.heldTarget("a.example.com"); !h.Held || !h.Unknown {
+ t.Fatalf("a read copy with no last_executed and root's unknown = %+v; want unknown", h)
+ }
+}