amadan
console: an operator move's own reservation is accepted whatever its keys
Fix round 2 (Important 1). Recovery mode counts an operator's own reservation as evidence, so a console restarted mid-operator-move is in self-serve mode with the switch off and the operator's next restage or start wanted box keys while holding the sentinel: MV-O194, permanently once the box set changed. In reserveBoxes' own retry an operator operation (offlineSelection.Operator, set by offlineRegionChecks for the operator doors) keeps entries it no longer wants (the move's release removes every entry of its id), is covered by a sentinel it holds, and acquires missing keys through the usual conflict check. MV-O194 stays for self-serve operations, whose participants are bound to their row. TestOperatorOwnRetryAcrossModes: staged as the sentinel then recovery mode, staged in recovery mode then out of it, a source box gone, the stager gone and back (each restage and start 202 as main), and a self-serve operation whose keys differ (still MV-O194). RED before.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3 files changed,
+119
−7
internal/console/apiinstancemove_offline.go+1 −1internal/console/offlinereserve.go+23 −6internal/console/offlinereserve_test.go+95 −0
diff --git a/internal/console/apiinstancemove_offline.go b/internal/console/apiinstancemove_offline.go| index 60e9707ee..a9b8b5e64 100644 |
| --- a/internal/console/apiinstancemove_offline.go |
| +++ b/internal/console/apiinstancemove_offline.go |
| @@ -565,7 +565,7 @@ func (s *Server) offlineRegionChecks(ctx context.Context, acct, app, host string |
| if ref := s.offlineSelect(ctx, fleet, host, rec.Region, to); ref != nil { |
| return offlineSelection{}, ref |
| } |
| - sel := offlineSelection{From: rec.Region, To: to, Fleet: fleet, ReturnOf: returnOf} |
| + sel := offlineSelection{From: rec.Region, To: to, Fleet: fleet, ReturnOf: returnOf, Operator: !requireIDs} |
| if ref := s.offlineStageRules(fleet, rec, host, to, stage, &sel); ref != nil { |
| return offlineSelection{}, ref |
| } |
diff --git a/internal/console/offlinereserve.go b/internal/console/offlinereserve.go| index 954f32c60..3f6a4c828 100644 |
| --- a/internal/console/offlinereserve.go |
| +++ b/internal/console/offlinereserve.go |
| @@ -47,6 +47,10 @@ type offlineSelection struct { |
| Stage provision.StageTarget |
| ReturnOf string |
| Sentinel bool |
| + // Operator is set for a selection an operator door made (pinned |
| + // without identities): its own earlier reservation is accepted |
| + // whatever its keys (reserveBoxes step 3). |
| + Operator bool |
| } |
| // reservationKeys are the keys the selection reserves: every participant's |
| @@ -119,10 +123,14 @@ func opOf(e moves.OfflineReservation) offlineOp { return offlineOp{e.Kind, e.ID} |
| // on the first write); |
| // 2. validate the whole object: any malformed entry is unreadable; |
| // 3. the own retry: entries with r's (Kind, ID) are accepted when they |
| -// cover exactly sel's keys (nothing is written and their reserved_at |
| -// stays); a strict subset is a partial reservation whose missing keys |
| -// are acquired below in the same write; any key outside sel's is the |
| -// mismatch refusal; |
| +// cover sel's keys (nothing is written and their reserved_at stays); |
| +// an operation already holding the sentinel covers every box. Missing |
| +// keys are a partial reservation, acquired below in the same write. A |
| +// key outside sel's is the mismatch refusal for a self-serve operation |
| +// (its participants are bound to its row); an operator operation's |
| +// (sel.Operator) is kept, held until the move's release removes every |
| +// entry of it, because an operator selection is recomputed at each door |
| +// and its keys change with the mode and the fleet (fix round 2); |
| // 4. with the switch off (cfg.SelfServeMoves != "offline") every entry of |
| // another operation conflicts: one operation at a time, today's |
| // deployment-wide semantics; |
| @@ -170,13 +178,22 @@ func (s *Server) reserveBoxes(ctx context.Context, sel offlineSelection, r moves |
| own := map[string]moves.OfflineReservation{} |
| for k, e := range table { |
| if opOf(e) == mine { |
| - if !wanted[k] { |
| + if !wanted[k] && !sel.Operator { |
| return offlineReserveMismatch |
| } |
| own[k] = e |
| } |
| } |
| - if len(own) == len(want) { |
| + if _, all := own[moves.OfflineSentinel]; all && sel.Operator { |
| + return nil |
| + } |
| + covered := true |
| + for _, k := range want { |
| + if _, ok := own[k]; !ok { |
| + covered = false |
| + } |
| + } |
| + if covered { |
| return nil |
| } |
| entry := r |
diff --git a/internal/console/offlinereserve_test.go b/internal/console/offlinereserve_test.go| index e94926638..df66dbad6 100644 |
| --- a/internal/console/offlinereserve_test.go |
| +++ b/internal/console/offlinereserve_test.go |
| @@ -1128,3 +1128,98 @@ func sequenceRecord(t *testing.T, rel release.Store, key string) { |
| t.Fatal(err) |
| } |
| } |
| + |
| +// TestOperatorOwnRetryAcrossModes: an operator move's own reservation is |
| +// accepted whatever its keys (fix round 2). A console that restarts in |
| +// recovery mode with the switch off is in self-serve mode, so an operator |
| +// selection staged as the sentinel now wants box keys (and one staged in |
| +// self-serve mode wants the sentinel after it); with the source box set |
| +// changed in between the keys differ again. Each restage and start answers |
| +// as main, and only self-serve operations keep the mismatch. |
| +func TestOperatorOwnRetryAcrossModes(t *testing.T) { |
| + staged := func(t *testing.T, boxes ...offlineBox) *offlineWorld { |
| + t.Helper() |
| + w := newOfflineWorld(t, boxes...) |
| + snapshotReports(t, w, 0, boxes...) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + return w |
| + } |
| + recovery := func(w *offlineWorld, on bool) { |
| + w.fx.srv.cfg.SelfServeMoves = "" |
| + w.fx.srv.selfServeRecovery.Store(on) |
| + } |
| + gone := func(t *testing.T, w *offlineWorld, box string) { |
| + t.Helper() |
| + for _, key := range []string{edgesFilePrefix + box + ".json", "status/boxes/" + box + ".json"} { |
| + if err := w.fx.rel.Delete(t.Context(), key); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + } |
| + t.Run("staged as the sentinel, then recovery mode", func(t *testing.T) { |
| + w := staged(t, oneEach...) |
| + if _, ok := w.table()[moves.OfflineSentinel]; !ok { |
| + t.Fatalf("the stage did not reserve the sentinel: %+v", w.table()) |
| + } |
| + recovery(w, true) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + w.ok("", `{"region":"uk"}`, provision.OfflineStopping) |
| + }) |
| + t.Run("staged in recovery mode, then out of it", func(t *testing.T) { |
| + w := newOfflineWorld(t, oneEach...) |
| + snapshotReports(t, w, 0, oneEach...) |
| + recovery(w, true) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + if _, ok := w.table()["ie-box"]; !ok { |
| + t.Fatalf("the stage in recovery mode did not reserve its boxes: %+v", w.table()) |
| + } |
| + recovery(w, false) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + w.ok("", `{"region":"uk"}`, provision.OfflineStopping) |
| + }) |
| + t.Run("recovery mode, a source box gone", func(t *testing.T) { |
| + w := staged(t, ieOff("ie-a"), ieOff("ie-b"), ukOff("uk-box")) |
| + recovery(w, true) |
| + other := "ie-a" |
| + if w.record().Move.Stager == other { |
| + other = "ie-b" |
| + } |
| + gone(t, w, other) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + w.ok("", `{"region":"uk"}`, provision.OfflineStopping) |
| + }) |
| + t.Run("recovery mode, the stager gone and back", func(t *testing.T) { |
| + w := staged(t, oneEach...) |
| + recovery(w, true) |
| + gone(t, w, "ie-box") |
| + w.fleet(ieOff("ie-other")) |
| + snapshotReports(t, w, 0, ieOff("ie-other")) |
| + w.ok("/stage", `{"region":"uk"}`, provision.OfflineStaging) |
| + w.fleet(ieOff("ie-box")) |
| + snapshotReports(t, w, 0, ieOff("ie-box")) |
| + w.ok("", `{"region":"uk"}`, provision.OfflineStopping) |
| + // Released with the move, every entry of it goes. |
| + id := w.record().Move.ID |
| + if err := w.fx.srv.releaseBoxes(t.Context(), moves.OfflineKindMove, id); err != nil { |
| + t.Fatal(err) |
| + } |
| + if tb := w.table(); len(tb) != 0 { |
| + t.Fatalf("entries left after the release: %+v", tb) |
| + } |
| + }) |
| + t.Run("a self-serve operation whose keys differ is the mismatch", func(t *testing.T) { |
| + w := newReserveWorld(t) |
| + w.fx.srv.cfg.SelfServeMoves = "offline" |
| + w.wantReserve(selOf("ie-1", "uk-1"), mres(mvA, offlineHost, 0), nil) |
| + w.wantReserve(selOf("ie-2", "uk-1"), mres(mvA, offlineHost, 0), refMismatch) |
| + // The same keys under an operator selection are accepted and kept. |
| + op := selOf("ie-2", "uk-1") |
| + op.Operator = true |
| + before := w.table() |
| + w.wantReserve(op, mres(mvA, offlineHost, 0), nil) |
| + tb := w.table() |
| + if tb["ie-1"] != before["ie-1"] || tb["uk-1"] != before["uk-1"] || tb["ie-2"].ID != mvA || len(tb) != 3 { |
| + t.Fatalf("operator retry table = %+v", tb) |
| + } |
| + }) |
| +} |