amadan

carlos / platform Public

provision: only the claim-backed retry advances the recovery epoch (MoveRetryRecovery); the legacy retry writes the bytes it always did

Answers Astra's P3 on ff72310f: with HostClaim off, the legacy resolve
door's MoveRetry added recovery_retry_epoch to the record. MoveRetry now
leaves it alone; hostops' ConsoleMoveRetry uses MoveRetryRecovery.
TestLegacyMoveRetryLeavesTheEpochAlone pins the legacy bytes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 1208e627fc5d56679adc0e6abdb6ad3e33d0ada0 parent ff72310
5 files changed, +48 −11
  • internal/host/movesprobeattempts_test.go +3 −3
  • internal/hostops/console_move_grant.go +1 −1
  • internal/hostops/console_move_grant_test.go +1 −1
  • internal/provision/move_test.go +24 −2
  • internal/provision/provision.go +19 −4

CI passed — run details

diff --git a/internal/host/movesprobeattempts_test.go b/internal/host/movesprobeattempts_test.go
index 1861e80fb..3c72cee6d 100644
--- a/internal/host/movesprobeattempts_test.go
+++ b/internal/host/movesprobeattempts_test.go
@@ -478,7 +478,7 @@ func TestRecoveryProbeAttemptsOperatorRetryBudget(t *testing.T) {
if err := transition(provision.MoveMarkStuck(g, provision.StuckRecoveryProbeAttempts)); err != nil {
t.Fatal(err)
}
- if err := transition(provision.MoveRetry(g)); err != nil {
+ if err := transition(provision.MoveRetryRecovery(g)); err != nil {
t.Fatal(err)
}
if got := f.ops.CurrentAttempt(f.start); got.ID != "" {
@@ -512,7 +512,7 @@ func TestRecoveryProbeAttemptsOperatorRetryBudget(t *testing.T) {
}
}
if budget > 0 {
- if err := transition(provision.MoveRetry(g)); !errors.Is(err, provision.ErrMoveConflict) {
+ if err := transition(provision.MoveRetryRecovery(g)); !errors.Is(err, provision.ErrMoveConflict) {
t.Fatalf("retry replay: %v", err)
}
}
@@ -533,7 +533,7 @@ func TestRecoveryProbeAttemptsOperatorRetryBudget(t *testing.T) {
f.ops = previousProvider
}
newSession()
- if err := transition(provision.MoveRetry(g)); !errors.Is(err, provision.ErrMoveConflict) {
+ if err := transition(provision.MoveRetryRecovery(g)); !errors.Is(err, provision.ErrMoveConflict) {
t.Fatalf("exhausted retry replay: %v", err)
}
if err := f.ops.Reserve(ctx, f.start, 0); !errors.Is(err, ErrRecoveryProbeExhausted) {
diff --git a/internal/hostops/console_move_grant.go b/internal/hostops/console_move_grant.go
index 5870f5632..16fd95009 100644
--- a/internal/hostops/console_move_grant.go
+++ b/internal/hostops/console_move_grant.go
@@ -108,7 +108,7 @@ func consoleMoveTransition(a ConsoleMoveAction, g provision.MoveGuard, op hostcl
if op.Kind != hostclaim.KindRestoring && op.Kind != hostclaim.KindReverting {
return nil, hostclaim.ErrWrongState
}
- return provision.MoveRetry(g), nil
+ return provision.MoveRetryRecovery(g), nil
}
return nil, ErrIdentity
}
diff --git a/internal/hostops/console_move_grant_test.go b/internal/hostops/console_move_grant_test.go
index 71d9e4698..bd60a3dbf 100644
--- a/internal/hostops/console_move_grant_test.go
+++ b/internal/hostops/console_move_grant_test.go
@@ -92,7 +92,7 @@ func TestJ17CutAbortRetryWriteOnlyTheirTypedTransition(t *testing.T) {
rec.Move.State, rec.Move.StuckReason = provision.MoveStuck, provision.StuckRecoveryProbeAttempts
rec.Move.Rollback = &provision.MoveRollback{ID: "rb_abcdefghijklmnopqrstuvwxyz", TargetPhase: provision.RbTargetReleased, SourcePhase: provision.RbSourceProbing, StuckAt: provision.RbSourceProbing}
}, func(r *provision.Record) {
- if err := provision.MoveRetry(provision.MoveGuard{ID: "move", Incarnation: r.Move.Incarnation})(r); err != nil {
+ if err := provision.MoveRetryRecovery(provision.MoveGuard{ID: "move", Incarnation: r.Move.Incarnation})(r); err != nil {
panic(err)
}
}},
diff --git a/internal/provision/move_test.go b/internal/provision/move_test.go
index 43ab35d84..e7cdd0aa0 100644
--- a/internal/provision/move_test.go
+++ b/internal/provision/move_test.go
@@ -1137,7 +1137,7 @@ func TestMoveRetryEpochRefusals(t *testing.T) {
rec.Move.Rollback = &MoveRollback{ID: rbTestID, StuckAt: RbSourceProbing}
}
before, _ := json.Marshal(rec)
- err := MoveRetry(g)(rec)
+ err := MoveRetryRecovery(g)(rec)
if epoch == ^uint64(0) {
after, _ := json.Marshal(rec)
if !errors.Is(err, ErrMoveConflict) || string(before) != string(after) {
@@ -1149,7 +1149,7 @@ func TestMoveRetryEpochRefusals(t *testing.T) {
t.Fatalf("retry epoch: %v %+v", err, rec.Move)
}
before, _ = json.Marshal(rec)
- err = MoveRetry(g)(rec)
+ err = MoveRetryRecovery(g)(rec)
after, _ := json.Marshal(rec)
if !errors.Is(err, ErrMoveConflict) || string(before) != string(after) {
t.Fatalf("retry replay mutated record: %v %+v", err, rec.Move)
@@ -1158,6 +1158,28 @@ func TestMoveRetryEpochRefusals(t *testing.T) {
}
}
+// The legacy retry (flag off) must write the same bytes it always did: it
+// never touches the recovery epoch, set or not.
+func TestLegacyMoveRetryLeavesTheEpochAlone(t *testing.T) {
+ for _, rollback := range []bool{false, true} {
+ for _, epoch := range []uint64{0, 7, ^uint64(0)} {
+ g := cutTestGuard()
+ rec := &Record{Move: &MoveState{ID: g.ID, Incarnation: g.Incarnation,
+ State: MoveStuck, StuckReason: StuckRecoveryProbeAttempts, RecoveryRetryEpoch: epoch}}
+ if rollback {
+ rec.Move.Rollback = &MoveRollback{ID: rbTestID, StuckAt: RbSourceProbing}
+ }
+ if err := MoveRetry(g)(rec); err != nil || rec.Move.RecoveryRetryEpoch != epoch {
+ t.Fatalf("legacy retry: %v epoch %d, want %d", err, rec.Move.RecoveryRetryEpoch, epoch)
+ }
+ raw, _ := json.Marshal(rec.Move)
+ if epoch == 0 && strings.Contains(string(raw), "recovery_retry_epoch") {
+ t.Fatalf("legacy retry wrote the epoch: %s", raw)
+ }
+ }
+ }
+}
+
func TestStuckAndRetryReturnToThePhase(t *testing.T) {
g := cutTestGuard()
now := time.Date(2026, 10, 6, 9, 0, 0, 0, time.UTC)
diff --git a/internal/provision/provision.go b/internal/provision/provision.go
index bd81a4d22..94542d08e 100644
--- a/internal/provision/provision.go
+++ b/internal/provision/provision.go
@@ -2185,9 +2185,22 @@ func MoveMarkStuck(g MoveGuard, reason string) func(*Record) error {
// recovery it stopped. A rollback returns to rolling_back with the source
// at the phase StuckAt names, so a mismatch verifies again and a probe
// probes again; a revert (no Rollback) returns to reverting. It clears
-// Move.StuckReason and StuckAt and advances RecoveryRetryEpoch: the move's
-// cause (Move.Reason) and the rollback's (Rollback.Reason) stay.
+// Move.StuckReason and StuckAt: the move's cause (Move.Reason) and the
+// rollback's (Rollback.Reason) stay. It leaves RecoveryRetryEpoch alone, so
+// the legacy (flag-off) retry writes the same bytes it always did; the
+// claim-backed retry is MoveRetryRecovery.
func MoveRetry(g MoveGuard) func(*Record) error {
+ return moveRetry(g, false)
+}
+
+// MoveRetryRecovery is MoveRetry for the claim-backed retry: in the same
+// transition it advances RecoveryRetryEpoch, which grants the recovery probe
+// a fresh durable start budget.
+func MoveRetryRecovery(g MoveGuard) func(*Record) error {
+ return moveRetry(g, true)
+}
+
+func moveRetry(g MoveGuard, advanceEpoch bool) func(*Record) error {
return func(rec *Record) error {
if err := g.check(rec); err != nil {
return err
@@ -2196,7 +2209,7 @@ func MoveRetry(g MoveGuard) func(*Record) error {
if m.State != MoveStuck {
return fmt.Errorf("%w: %s", ErrMoveConflict, RefusedNotStuck)
}
- if m.RecoveryRetryEpoch == ^uint64(0) {
+ if advanceEpoch && m.RecoveryRetryEpoch == ^uint64(0) {
return ErrMoveConflict
}
if rb := m.Rollback; rb != nil {
@@ -2208,7 +2221,9 @@ func MoveRetry(g MoveGuard) func(*Record) error {
m.State = MoveReverting
}
m.StuckReason = ""
- m.RecoveryRetryEpoch++
+ if advanceEpoch {
+ m.RecoveryRetryEpoch++
+ }
return nil
}
}