amadan

carlos / platform Public

console: automatic non-home eligibility needs complete evidence; a partial read makes nothing non-home eligible (re-review r1)

Two partial-read holes let a failing sibling vanish and the keyed box
speak for its region alone: an unreadable edges object dropped the box
from the fold (regionCandidates ignored snap.Complete), and an unreadable
status snapshot read as not fresh (LoadBoxes skips failed GETs and still
succeeds), so unknown liveness was treated as staleness.

Completeness is now carried separately from staleness. status gains
LoadBoxesComplete, which returns exactly what LoadBoxes returns plus
whether every listed snapshot was read (a failed GET, an oversize or
corrupt body, or a capped listing is incomplete); LoadBoxes' logged-skip
semantics are unchanged for its other callers. readLiveEdgesSnapshot
records it as edgesSnapshot.BoxesComplete, and snap.complete() (edges
Complete AND BoxesComplete) is the bar: on any incomplete read no non-home
region is automatically eligible for that request, whole-request scope,
logged once. The home is exempt as before. The explicit gates (placements
plus NoKeyProven) and the C18/C19/C31 lists are untouched.

At the two doors that render a reason from the candidates, incomplete
evidence for a non-home region is "could not confirm" rather than a C16
word that would misdescribe a transient read failure: the app-derived
instance create answers 503 C28 (the same answer as a failed listing),
and app_default reports reason edges-unreadable.

Tests, RED against 9435b01f: a failed edges GET, a failed status GET and a
corrupt status body on a keyless sibling each leave uk ineligible (and
au, in another region: whole-request); the app-derived uk create is 503
C28 under both failures while an explicit --region uk create and a home
app's create keep their behaviour; the diagnostic is said once; and
LoadBoxesComplete reports skipped reads while LoadBoxes is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude b01bab82d862f8741de8ecacb13d2cfc3ceeb485 parent 9435b01
8 files changed, +271 −11
  • internal/console/apiinstances.go +9 −0
  • internal/console/instancesplacement.go +23 −2
  • internal/console/regioncandidates.go +12 −1
  • internal/console/regioncandidates_test.go +108 −4
  • internal/console/regiondefault.go +5 −1
  • internal/console/regionfill_test.go +34 −0
  • internal/status/fleet.go +26 −3
  • internal/status/status_test.go +54 −0
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go
index 224257efd..6b011d2f4 100644
--- a/internal/console/apiinstances.go
+++ b/internal/console/apiinstances.go
@@ -808,6 +808,15 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request)
jsonError(w, http.StatusServiceUnavailable, msgAppRegionUnconfirmed(s.homeRegionOut(region)))
return
}
+ // Incomplete evidence for a non-home region is "could not confirm"
+ // (C28, the same answer as a failed listing), not C16: the region
+ // may well be fine, and a box that could not be read is the one
+ // that may not be (re-review r1). The home needs no evidence.
+ if region != "" && !snap.complete() {
+ log.Printf("console: api instances create %s/%s: incomplete edges or snapshot read, region %s unconfirmed", acct.Sqid, app, region)
+ jsonError(w, http.StatusServiceUnavailable, msgAppRegionUnconfirmed(s.homeRegionOut(region)))
+ return
+ }
cands := s.regionCands(snap)
reason := "absent"
if c, ok := findCandidate(cands, region); ok {
diff --git a/internal/console/instancesplacement.go b/internal/console/instancesplacement.go
index a3e601c4d..a3102aefd 100644
--- a/internal/console/instancesplacement.go
+++ b/internal/console/instancesplacement.go
@@ -92,16 +92,25 @@ var livenessSaid sync.Map
// person asked for that region and can see it refused later.
func readLiveEdgesSnapshot(ctx context.Context, s release.Store, now time.Time) (edgesSnapshot, error) {
snap, err := readEdgesSnapshot(ctx, s)
- if err != nil || len(snap.Files) == 0 {
+ if err != nil {
return snap, err
}
- boxes, err := status.LoadBoxes(ctx, s)
+ if len(snap.Files) == 0 {
+ snap.BoxesComplete = true // nothing to be incomplete about
+ return snap, nil
+ }
+ // Completeness is carried separately from staleness: LoadBoxesComplete
+ // returns what LoadBoxes would, plus whether every listed snapshot was
+ // actually read. A skipped read leaves that box's liveness UNKNOWN,
+ // and unknown is never "stale".
+ boxes, boxesComplete, err := status.LoadBoxesComplete(ctx, s)
if err != nil {
if _, said := livenessSaid.LoadOrStore("load", true); !said {
log.Printf("console: could not load box snapshots (%v), so no box outside the home counts for automatic region placement until they load", err)
}
return snap, nil
}
+ snap.BoxesComplete = boxesComplete
live := map[string]bool{}
for _, b := range boxes {
if !status.BoxStale(b, now, status.SnapshotPeriod) {
@@ -168,8 +177,20 @@ func readEdgesFiles(ctx context.Context, s release.Store) []edgesFilePlacementVi
type edgesSnapshot struct {
Files []edgesFilePlacementView
Complete bool
+ // BoxesComplete: every listed status snapshot was read, so each file's
+ // Fresh is a KNOWN answer. Set only by readLiveEdgesSnapshot; a plain
+ // readEdgesSnapshot carries no liveness evidence at all. A box whose
+ // snapshot could not be read has unknown liveness, which is not stale
+ // (re-review r1, hole 2).
+ BoxesComplete bool
}
+// complete is the evidence bar for AUTOMATIC non-home eligibility: every
+// listed edges file read and parsed, and every listed status snapshot
+// read. Short of it a failing sibling may simply be missing from the fold,
+// so nothing non-home is eligible for the request (ruling, re-review r1).
+func (s edgesSnapshot) complete() bool { return s.Complete && s.BoxesComplete }
+
// readEdgesSnapshot REPORTS a failed listing instead of answering empty:
// the door must tell "could not read" (503, C28) from "no box publishes
// this code" (codex R2-2). A single unreadable or malformed file is
diff --git a/internal/console/regioncandidates.go b/internal/console/regioncandidates.go
index 5530bc736..c7bf3bf14 100644
--- a/internal/console/regioncandidates.go
+++ b/internal/console/regioncandidates.go
@@ -159,10 +159,21 @@ func regionCandidates(snap edgesSnapshot, homeName, expectedFP string) []regionC
a.regions[f.Region]++
}
}
+ // Complete evidence is the bar for every non-home region (re-review
+ // r1): with one edges file unread, or one status snapshot unread, a
+ // failing sibling may be the box that is missing, and the keyed box
+ // that WAS read would speak for the region alone. Whole-request scope,
+ // deliberately: simpler, and stricter than per region.
+ evidence := snap.complete()
+ if !evidence && len(by) > 1 {
+ if _, said := livenessSaid.LoadOrStore("incomplete", true); !said {
+ log.Printf("console: an incomplete read of the edges files or box snapshots (edges complete %v, snapshots complete %v), so no region outside the home is automatically eligible this request; said once", snap.Complete, snap.BoxesComplete)
+ }
+ }
out := make([]regionCandidate, 0, len(by))
for _, a := range by {
if !a.c.Home {
- a.c.Eligible = a.anyEligible && a.allEligible
+ a.c.Eligible = evidence && a.anyEligible && a.allEligible
}
a.c.AWSRegion = majority(a.c.Code, a.regions)
a.c.NoKeyProven = !a.c.Home && snap.Complete && expectedFP != "" && a.hostingBoxes > 0 && !a.unknownWriter && !a.keyFound
diff --git a/internal/console/regioncandidates_test.go b/internal/console/regioncandidates_test.go
index 1bb2037a9..2e0f3d77f 100644
--- a/internal/console/regioncandidates_test.go
+++ b/internal/console/regioncandidates_test.go
@@ -63,8 +63,111 @@ var (
nzNoHosting = edgeJSON("nz-box", "6.6.6.6", "ap-southeast-6", "nz", edgeOpts{hib: bp(false), shared: bp(true), inbound: bp(true), fp: testKeyFP})
jpNoEIP = edgeJSON("jp-box", "", "ap-northeast-1", "jp", eligibleBox)
fleetBoxFile = edgeJSON("fl-box", "4.4.4.4", "eu-west-2", "fleet/acme", eligibleBox)
+ // ukKeylessSibling is a second uk box reconcile would admit (shared pool,
+ // inbound) that holds no key: the F2 hazard beside ukEligible.
+ ukKeylessSibling = edgeJSON("uk-box-2", "1.2.3.5", "eu-west-2", "uk", edgeOpts{hib: bp(true), shared: bp(true), inbound: bp(true)})
)
+// Partial reads (re-review r1): automatic non-home eligibility needs
+// COMPLETE evidence, the edges snapshot Complete and every listed box's
+// status read succeeded. Otherwise a failing sibling can simply drop out
+// of the fold (its edges GET failed) or read as not fresh (its status GET
+// failed), and the region becomes eligible through the box that was read.
+// On any incomplete read no non-home region is eligible for that request
+// (whole-request scope, the stricter choice); the home stays as it is.
+func TestIncompleteReadsNeverMakeARegionEligible(t *testing.T) {
+ ctx := context.Background()
+ auBox := edgeJSON("au-box", "9.9.9.9", "ap-southeast-2", "au", eligibleBox)
+ s := release.NewDirStore(t.TempDir())
+ for key, body := range map[string]string{"ie-box": homeBox, "uk-box": ukEligible, "uk-box-2": ukKeylessSibling, "au-box": auBox} {
+ if _, err := s.Put(ctx, "control/edges/"+key+".json", []byte(body), ""); err != nil {
+ t.Fatal(err)
+ }
+ setBoxSnapshot(t, s, key, time.Now())
+ }
+ check := func(name string, store release.Store, wantAU bool) {
+ t.Helper()
+ snap, err := readLiveEdgesSnapshot(ctx, store, time.Now())
+ if err != nil {
+ t.Fatalf("%s: %v", name, err)
+ }
+ cands := regionCandidates(snap, "ie", testKeyFP)
+ uk, _ := findCandidate(cands, "uk")
+ au, _ := findCandidate(cands, "au")
+ home, _ := findCandidate(cands, "")
+ if uk.Eligible {
+ t.Errorf("%s: uk = %+v, want ineligible (a keyless sibling was listed)", name, uk)
+ }
+ if au.Eligible != wantAU {
+ t.Errorf("%s: au eligible = %v, want %v", name, au.Eligible, wantAU)
+ }
+ if !home.Eligible {
+ t.Errorf("%s: home = %+v, want eligible regardless", name, home)
+ }
+ }
+ // Control: everything read. uk fails on its keyless sibling, au is clean.
+ check("complete", s, true)
+ // Hole 1: the keyless sibling's EDGES object cannot be read. It drops out
+ // of the fold, and only the keyed box remains to speak for uk.
+ check("edges GET failure", failingGet{Store: s, key: "control/edges/uk-box-2.json"}, false)
+ // Hole 2: the keyless sibling's STATUS snapshot cannot be read. Unknown
+ // liveness is not staleness: it must not be excluded as stale.
+ check("status GET failure", failingGet{Store: s, key: "status/boxes/uk-box-2.json"}, false)
+ // A corrupt status body is an incomplete read too (loadBoxes skips it).
+ corrupt := release.NewDirStore(t.TempDir())
+ for _, key := range []string{"ie-box", "uk-box", "uk-box-2", "au-box"} {
+ body, _, err := s.Get(ctx, "control/edges/"+key+".json")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := corrupt.Put(ctx, "control/edges/"+key+".json", body, ""); err != nil {
+ t.Fatal(err)
+ }
+ setBoxSnapshot(t, corrupt, key, time.Now())
+ }
+ if _, err := corrupt.Put(ctx, "status/boxes/uk-box-2.json", []byte("{not json"), mustEtag(t, corrupt, "status/boxes/uk-box-2.json")); err != nil {
+ t.Fatal(err)
+ }
+ check("corrupt status snapshot", corrupt, false)
+}
+
+func mustEtag(t *testing.T, s release.Store, key string) string {
+ t.Helper()
+ _, etag, err := s.Get(context.Background(), key)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return etag
+}
+
+// Incomplete evidence is said once, like the other liveness diagnostics.
+func TestIncompleteEvidenceIsLoggedOnce(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ for key, body := range map[string]string{"ie-box": homeBox, "uk-box": ukEligible} {
+ if _, err := s.Put(ctx, "control/edges/"+key+".json", []byte(body), ""); err != nil {
+ t.Fatal(err)
+ }
+ setBoxSnapshot(t, s, key, time.Now())
+ }
+ var buf bytes.Buffer
+ restore := captureLog(&buf)
+ defer restore()
+ livenessSaid.Delete("incomplete")
+ for i := 0; i < 2; i++ {
+ snap, err := readLiveEdgesSnapshot(ctx, failingGet{Store: s, key: "status/boxes/uk-box.json"}, time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if c, _ := findCandidate(regionCandidates(snap, "ie", testKeyFP), "uk"); c.Eligible {
+ t.Fatalf("round %d: uk eligible on an incomplete read", i)
+ }
+ }
+ if n := strings.Count(buf.String(), "incomplete"); n != 1 {
+ t.Fatalf("incomplete evidence logged %d times, want once: %s", n, buf.String())
+ }
+}
+
// setEdges creates OR replaces edges files, and gives each box a FRESH
// status snapshot: automatic eligibility needs one (F1), and the fixture
// boxes are all meant to be live unless a test says otherwise through
@@ -112,12 +215,13 @@ func captureLog(w io.Writer) (restore func()) {
return func() { log.SetOutput(prev); log.SetFlags(flags) }
}
-// views is a COMPLETE snapshot of the given edges-file bodies, every box
-// FRESH (F1): the pure fold is handed what readLiveEdgesSnapshot would
-// produce for a fleet that is all reporting. markStale takes one back.
+// views is a COMPLETE snapshot of the given edges-file bodies (every file
+// and every status snapshot read), every box FRESH (F1): the pure fold is
+// handed what readLiveEdgesSnapshot would produce for a fleet that is all
+// reporting and all readable. markStale takes one box back.
func views(t *testing.T, bodies ...string) edgesSnapshot {
t.Helper()
- snap := edgesSnapshot{Complete: true}
+ snap := edgesSnapshot{Complete: true, BoxesComplete: true}
for _, b := range bodies {
var f edgesFilePlacementView
if err := json.Unmarshal([]byte(b), &f); err != nil {
diff --git a/internal/console/regiondefault.go b/internal/console/regiondefault.go
index 2d8fef8f3..5233d0bcb 100644
--- a/internal/console/regiondefault.go
+++ b/internal/console/regiondefault.go
@@ -78,7 +78,11 @@ func (s *Server) appDefault(ctx context.Context, acct *Account, app string) appD
// Nothing is stated about it, and no reason either: the placement
// WAS read (controller ruling PF7).
return v
- case ferr != nil:
+ case ferr != nil, stored != "" && !snap.complete():
+ // A failed listing, or (for a non-home region) a listing with an
+ // edges file or a box snapshot unread: the evidence is not there
+ // to say the region can take an instance, and the create door
+ // would answer C28 for the same reason (re-review r1).
v.Reason = "edges-unreadable"
return v
}
diff --git a/internal/console/regionfill_test.go b/internal/console/regionfill_test.go
index 51100bdc9..02973d467 100644
--- a/internal/console/regionfill_test.go
+++ b/internal/console/regionfill_test.go
@@ -140,6 +140,40 @@ func TestAppDerivedNeedsALiveBox(t *testing.T) {
}
}
+// Partial reads (re-review r1): with a keyless sibling listed beside the
+// keyed uk box, a failed edges GET or a failed status GET on that sibling
+// is incomplete evidence. An app-derived uk create is refused C28 (could
+// not confirm, try again or pass a region), never created through the box
+// that was read. An explicit --region uk create and a home app's create
+// keep their behaviour under both failures: the explicit gates read the
+// files alone and the home is exempt.
+func TestAppDerivedNeedsCompleteEvidence(t *testing.T) {
+ failures := map[string]string{
+ "edges GET": "control/edges/uk-box-2.json",
+ "status GET": "status/boxes/uk-box-2.json",
+ }
+ for name, key := range failures {
+ fx, token, u := fillFixture(t, "uk")
+ setEdges(t, fx, map[string]string{"uk-box-2": ukKeylessSibling})
+ fx.srv.cfg.Releases = failingGet{Store: fx.rel, key: key}
+ status, _, out := create(t, u, token, `{"host":"y.go.tito.io"}`)
+ if status != http.StatusServiceUnavailable || !strings.Contains(out, msgAppRegionUnconfirmed("uk")) {
+ t.Errorf("%s failure, app-derived uk = %d %s, want 503 C28", name, status, out)
+ }
+ if rec := mustCreate(t, u, token, `{"host":"x.go.tito.io","region":"uk"}`); rec.Region != "uk" {
+ t.Errorf("%s failure, explicit uk = %+v, want created in uk as before", name, rec)
+ }
+ }
+ for name, key := range failures {
+ fx, token, u := fillFixture(t, "")
+ setEdges(t, fx, map[string]string{"uk-box-2": ukKeylessSibling})
+ fx.srv.cfg.Releases = failingGet{Store: fx.rel, key: key}
+ if rec := mustCreate(t, u, token, `{"host":"h.go.tito.io"}`); rec.Region != "" {
+ t.Errorf("%s failure, home app = %+v, want created at the home", name, rec)
+ }
+ }
+}
+
func TestAppDerivedEdgesReadFailureIs503(t *testing.T) {
fx, token, u := fillFixture(t, "uk")
fx.srv.cfg.Releases = failingList{fx.rel}
diff --git a/internal/status/fleet.go b/internal/status/fleet.go
index 2f8174f09..eea3f3d2d 100644
--- a/internal/status/fleet.go
+++ b/internal/status/fleet.go
@@ -217,33 +217,56 @@ const (
)
func loadBoxes(ctx context.Context, s release.Store) ([]Box, error) {
+ boxes, _, err := loadBoxesReport(ctx, s)
+ return boxes, err
+}
+
+// LoadBoxesComplete is LoadBoxes that also reports whether EVERY listed
+// snapshot was read: complete is false when any object's read failed, any
+// body was skipped as oversize or corrupt, or the listing was capped. The
+// boxes returned are exactly what LoadBoxes would return, and the skips are
+// logged the same way; the only addition is the accounting. It exists for
+// a reader that must fail CLOSED on partial evidence (the console's
+// automatic region placement: a box whose snapshot could not be read has
+// UNKNOWN liveness, which is not the same as stale), where LoadBoxes'
+// "logged skip, never fatal" posture would quietly drop the one box that
+// mattered.
+func LoadBoxesComplete(ctx context.Context, s release.Store) (boxes []Box, complete bool, err error) {
+ return loadBoxesReport(ctx, s)
+}
+
+func loadBoxesReport(ctx context.Context, s release.Store) (boxes []Box, complete bool, err error) {
keys, err := s.List(ctx, "status/boxes/")
if err != nil {
- return nil, err
+ return nil, false, err
}
+ complete = true
if len(keys) > maxFleetBoxKeys {
log.Printf("status: fleet: %d keys under status/boxes/, capping at %d", len(keys), maxFleetBoxKeys)
keys = keys[:maxFleetBoxKeys]
+ complete = false
}
- var boxes []Box
for _, key := range keys {
body, _, err := s.Get(ctx, key)
if err != nil {
log.Printf("status: fleet: skipping %s: read failed: %v", key, err)
+ complete = false
continue
}
if len(body) > maxFleetBoxBodyBytes {
log.Printf("status: fleet: skipping %s: body %d bytes exceeds %d byte cap", key, len(body), maxFleetBoxBodyBytes)
+ complete = false
continue
}
var b Box
if err := json.Unmarshal(body, &b); err != nil {
log.Printf("status: fleet: skipping %s: corrupt snapshot: %v", key, err)
+ complete = false
continue
}
boxes = append(boxes, b)
}
- return boxes, nil
+ return boxes, complete, nil
}
// sumInstances adds each RouteCounts component across every box — stale
diff --git a/internal/status/status_test.go b/internal/status/status_test.go
index c16eb93d4..4c6f12fbe 100644
--- a/internal/status/status_test.go
+++ b/internal/status/status_test.go
@@ -1221,6 +1221,60 @@ func TestFleetBoxCarriesAdoption(t *testing.T) {
// public feed drops it — this loader must not), and BoxStale answers the same
// "older than 3 periods" question BuildFleet's own boxes table answers, so the
// two renderings can never disagree about which host is dark.
+// oneFailingGet fails the read of one key and serves every other.
+type oneFailingGet struct {
+ release.Store
+ key string
+}
+
+func (f oneFailingGet) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if key == f.key {
+ return nil, "", errors.New("read timeout")
+ }
+ return f.Store.Get(ctx, key)
+}
+
+// LoadBoxesComplete returns what LoadBoxes returns and, separately, whether
+// every listed snapshot was read: a failed GET or a corrupt body is a
+// logged skip for LoadBoxes and an INCOMPLETE read here. The console's
+// automatic region placement fails closed on it (nearest-region re-review
+// r1): a box whose snapshot could not be read has unknown liveness, which
+// is not stale.
+func TestLoadBoxesCompleteReportsSkippedReads(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ for _, label := range []string{"alpha", "beta"} {
+ if err := status.WriteBox(ctx, s, status.Box{Label: label, GeneratedAt: time.Now()}); err != nil {
+ t.Fatal(err)
+ }
+ }
+ boxes, complete, err := status.LoadBoxesComplete(ctx, s)
+ if err != nil || !complete || len(boxes) != 2 {
+ t.Fatalf("clean store: %d boxes, complete %v, err %v; want 2, true, nil", len(boxes), complete, err)
+ }
+ boxes, complete, err = status.LoadBoxesComplete(ctx, oneFailingGet{Store: s, key: "status/boxes/beta.json"})
+ if err != nil || complete || len(boxes) != 1 {
+ t.Fatalf("one failed GET: %d boxes, complete %v, err %v; want 1 (alpha), false, nil", len(boxes), complete, err)
+ }
+ plain, err := status.LoadBoxes(ctx, oneFailingGet{Store: s, key: "status/boxes/beta.json"})
+ if err != nil || len(plain) != 1 {
+ t.Fatalf("LoadBoxes under the same failure: %d boxes, err %v; want the unchanged logged-skip behaviour", len(plain), err)
+ }
+ _, etag, err := s.Get(ctx, "status/boxes/beta.json")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.Put(ctx, "status/boxes/beta.json", []byte("{not json"), etag); err != nil {
+ t.Fatal(err)
+ }
+ if boxes, complete, err := status.LoadBoxesComplete(ctx, s); err != nil || complete || len(boxes) != 1 {
+ t.Fatalf("corrupt body: %d boxes, complete %v, err %v; want 1, false, nil", len(boxes), complete, err)
+ }
+ if _, complete, err := status.LoadBoxesComplete(ctx, release.NewDirStore(t.TempDir())); err != nil || !complete {
+ t.Fatalf("empty prefix: complete %v, err %v; want true, nil", complete, err)
+ }
+}
+
func TestLoadBoxesAndStaleness(t *testing.T) {
s := teststore(t)
ctx := context.Background()