amadan

carlos / platform Public

provision, release: walk app directories only on a bucket-shaped store

Astra review of 98576596 (reject):

- P1: the fleet door answers ListDirs("apps/") with its own account's
  APP directories. The walk read those as account directories, found
  none known, and returned no records with no error, so reconcile would
  have retired every route on a fleet box. The walk now runs only on a
  store that promises bucket-shaped ListDirs (release.BucketListDirs: the
  S3 store, and a prefix store over one). Every other store is listed
  the old way.
- P2: DirStore lists by walking its whole root, so walking it per app
  was slower than one List. It makes no promise and keeps the old path.
- P2: account discovery was serial. Accounts and apps are now listed
  eight at a time.
- P3: the tests now drive all three readers with real records on a
  bucket-shaped store, prove the fleet-shaped fallback, cover a failed
  account listing as well as a failed app listing, and show reconcile
  keeps a route it could not list.
Agent notes
Mutants checked: walking without the promise, never walking, and reconcile ignoring a partial walk. Each fails the test written for it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude dc8c4296b54ab3eb04e74b7ba2ad6485d6420f6f parent d20e27e
4 files changed, +293 −131
  • internal/host/steering_loop_test.go +0 −6
  • internal/provision/applisting_test.go +189 −75
  • internal/provision/reconcile.go +76 −50
  • internal/release/store.go +28 −0
diff --git a/internal/host/steering_loop_test.go b/internal/host/steering_loop_test.go
index 6d7dda5fe..6e2ba9add 100644
--- a/internal/host/steering_loop_test.go
+++ b/internal/host/steering_loop_test.go
@@ -294,12 +294,6 @@ func TestSteeringPassFailsClosedOnRecordsRead(t *testing.T) {
return failingListStore{Store: s, prefix: "apps/", err: boom}
},
},
- {
- name: "one app's records listing fails",
- wrap: func(s release.Store) release.Store {
- return failingListStore{Store: s, prefix: provision.RecordsPrefix(domains.Sqid(12), "app"), err: boom}
- },
- },
{
name: "a record object will not Get",
wrap: func(s release.Store) release.Store {
diff --git a/internal/provision/applisting_test.go b/internal/provision/applisting_test.go
index d055094da..135eea120 100644
--- a/internal/provision/applisting_test.go
+++ b/internal/provision/applisting_test.go
@@ -12,28 +12,74 @@ import (
"github.com/carlosframework/platform/internal/release"
)
-// listCountingStore records every List prefix asked of it.
-type listCountingStore struct {
+// bucketShaped is a store that promises bucket-shaped ListDirs, as the S3
+// store does, and records every listing asked of it. DirStore's own ListDirs
+// has that shape; it just does not advertise it.
+type bucketShaped struct {
release.Store
mu sync.Mutex
lists []string
+ dirs []string
+ // failDirs and failList fail one prefix each.
+ failDirs, failList string
}
-func (s *listCountingStore) List(ctx context.Context, prefix string) ([]string, error) {
+func (s *bucketShaped) BucketShapedListDirs() bool { return true }
+
+func (s *bucketShaped) List(ctx context.Context, prefix string) ([]string, error) {
s.mu.Lock()
s.lists = append(s.lists, prefix)
s.mu.Unlock()
+ if prefix == s.failList {
+ return nil, errors.New("list refused")
+ }
return s.Store.List(ctx, prefix)
}
+func (s *bucketShaped) ListDirs(ctx context.Context, prefix string) ([]string, error) {
+ s.mu.Lock()
+ s.dirs = append(s.dirs, prefix)
+ s.mu.Unlock()
+ if prefix == s.failDirs {
+ return nil, errors.New("listdirs refused")
+ }
+ return s.Store.ListDirs(ctx, prefix)
+}
+
+func (s *bucketShaped) listedAllOfApps() bool {
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ for _, p := range s.lists {
+ if p == "apps/" {
+ return true
+ }
+ }
+ return false
+}
+
+// fleetShaped answers ListDirs the way the fleet door does: scoped to one
+// account, ListDirs("apps/") is that account's APP directories. It makes no
+// bucket-shape promise, which is the point.
+type fleetShaped struct {
+ release.Store
+ acct string
+}
+
+func (s fleetShaped) ListDirs(ctx context.Context, prefix string) ([]string, error) {
+ if prefix == "apps/" {
+ return s.Store.ListDirs(ctx, "apps/"+s.acct+"/")
+ }
+ return s.Store.ListDirs(ctx, prefix)
+}
+
// A bucket with everything a production one holds under apps/: records of
// both kinds, the logs and release artifacts that dwarf them, an account that
-// has not materialized, a legacy account-less app, and an app named like a
-// release-plane word.
+// has not materialized, a legacy account-less app, and apps named like the
+// release plane's own words.
func seedAppListingBucket(t *testing.T) release.Store {
t.Helper()
s := release.NewDirStore(t.TempDir())
- keys := []string{
+ for _, k := range []string{
"apps/bab/shop/instances/a.example.com.json",
"apps/bab/shop/instances/b.example.com.json",
"apps/bab/shop/instances.json",
@@ -50,8 +96,7 @@ func seedAppListingBucket(t *testing.T) release.Store {
"apps/legacyapp/channels/edge.json", // account-less
"apps/bab/channels/instances/f.example.com.json",
"apps/bab/releases/instances/g.example.com.json",
- }
- for _, k := range keys {
+ } {
if _, err := s.Put(context.Background(), k, []byte(`{}`), ""); err != nil {
t.Fatalf("seed %s: %v", k, err)
}
@@ -59,14 +104,23 @@ func seedAppListingBucket(t *testing.T) release.Store {
return s
}
-// The narrow walk finds exactly the keys the full listing did, for both
-// record kinds, and never lists the whole of apps/.
-func TestAppSubtreeListingMatchesTheFullListing(t *testing.T) {
+func parsedRecords(keys []string, parse func(string) bool) []string {
+ var out []string
+ for _, k := range keys {
+ if parse(k) {
+ out = append(out, k)
+ }
+ }
+ sort.Strings(out)
+ return out
+}
+
+// On a bucket-shaped store the walk finds exactly the records the full
+// listing did, for both record kinds, and never lists the whole of apps/.
+func TestAppSubtreeWalkMatchesTheFullListing(t *testing.T) {
inner := seedAppListingBucket(t)
known := map[string]bool{"bab": true, "bad": true}
- ctx := context.Background()
-
- all, err := inner.List(ctx, "apps/")
+ all, err := inner.List(context.Background(), "apps/")
if err != nil {
t.Fatal(err)
}
@@ -77,27 +131,13 @@ func TestAppSubtreeListingMatchesTheFullListing(t *testing.T) {
{"instances/", func(k string) bool { _, _, _, ok := ParseRecordKey(k, known); return ok }},
{"sidecars/", func(k string) bool { _, _, _, ok := ParseSidecarKey(k, known); return ok }},
} {
- var want []string
- for _, k := range all {
- if c.parse(k) {
- want = append(want, k)
- }
- }
- sort.Strings(want)
-
- s := &listCountingStore{Store: inner}
- got, partial, err := listAppSubtree(ctx, s, known, c.sub)
+ s := &bucketShaped{Store: inner}
+ got, partial, err := listAppSubtree(context.Background(), s, known, c.sub)
if err != nil || partial {
t.Fatalf("%s: partial=%v err=%v", c.sub, partial, err)
}
- var parsed []string
- for _, k := range got {
- if c.parse(k) {
- parsed = append(parsed, k)
- }
- }
- if !reflect.DeepEqual(parsed, want) {
- t.Fatalf("%s: narrow walk found %v, the full listing %v", c.sub, parsed, want)
+ if g, w := parsedRecords(got, c.parse), parsedRecords(all, c.parse); !reflect.DeepEqual(g, w) {
+ t.Fatalf("%s: the walk found %v, the full listing %v", c.sub, g, w)
}
for _, p := range s.lists {
if !strings.HasSuffix(p, "/"+c.sub) {
@@ -107,61 +147,135 @@ func TestAppSubtreeListingMatchesTheFullListing(t *testing.T) {
}
}
-// The passes that read records on every serial bump use the narrow walk, and
-// never the full listing: on the flagship, apps/ held 59,288 objects (50,695
-// of them logs) to find 152 records.
-func TestRecordReadersNeverListAllOfApps(t *testing.T) {
- s := &listCountingStore{Store: seedAppListingBucket(t)}
- // No account manifests: ops is always known, so the walk has bab to do.
- if _, err := ListAllRecords(context.Background(), s); err != nil {
- t.Fatalf("ListAllRecords: %v", err)
+// A store that makes no bucket-shape promise is listed the old way. The fleet
+// door's ListDirs("apps/") is its account's app directories; walked as if it
+// were account directories, it yields no records at all, which reconcile
+// would read as every route retired.
+func TestAppSubtreeListsTheOldWayWithoutTheBucketShape(t *testing.T) {
+ fleet := fleetShaped{Store: seedAppListingBucket(t), acct: "bab"}
+ known := map[string]bool{"bab": true}
+ parse := func(k string) bool { _, _, _, ok := ParseRecordKey(k, known); return ok }
+ got, partial, err := listAppSubtree(context.Background(), fleet, known, "instances/")
+ if err != nil || partial {
+ t.Fatalf("partial=%v err=%v", partial, err)
}
- walked := false
- for _, p := range s.lists {
- if p == "apps/" {
- t.Fatal("ListAllRecords listed the whole of apps/")
- }
- if strings.HasPrefix(p, "apps/bab/") {
- walked = true
- }
+ want := []string{
+ "apps/bab/blog/instances/c.example.com.json",
+ "apps/bab/shop/instances/a.example.com.json",
+ "apps/bab/shop/instances/b.example.com.json",
+ }
+ if g := parsedRecords(got, parse); !reflect.DeepEqual(g, want) {
+ t.Fatalf("records on a fleet-shaped store = %v, want %v", g, want)
}
- if !walked {
- t.Fatalf("ListAllRecords listed %v and never an app of ops's", s.lists)
+ if release.HasBucketListDirs(release.NewDirStore(t.TempDir())) {
+ t.Fatal("DirStore promises bucket-shaped ListDirs; it walks its whole root per List, so it must be listed the old way")
}
}
-// failListStore fails List for one prefix only.
-type failListStore struct {
- release.Store
- fail string
+// One account's or one app's listing failing is partial, not fatal: the rest
+// still come back, and the caller is told some were not seen.
+func TestAppSubtreeOneListingFailingIsPartial(t *testing.T) {
+ known := map[string]bool{"bab": true, "bad": true}
+ parse := func(k string) bool { _, _, _, ok := ParseRecordKey(k, known); return ok }
+ for _, c := range []struct {
+ name string
+ s *bucketShaped
+ want []string
+ }{
+ {"an app", &bucketShaped{failList: "apps/bab/blog/instances/"}, []string{
+ "apps/bab/shop/instances/a.example.com.json",
+ "apps/bab/shop/instances/b.example.com.json",
+ "apps/bad/site/instances/d.example.com.json",
+ }},
+ {"an account", &bucketShaped{failDirs: "apps/bab/"}, []string{
+ "apps/bad/site/instances/d.example.com.json",
+ }},
+ } {
+ c.s.Store = seedAppListingBucket(t)
+ got, partial, err := listAppSubtree(context.Background(), c.s, known, "instances/")
+ if err != nil {
+ t.Fatalf("%s: one failed listing failed the walk: %v", c.name, err)
+ }
+ if !partial {
+ t.Fatalf("%s: a failed listing was not reported as partial", c.name)
+ }
+ if g := parsedRecords(got, parse); !reflect.DeepEqual(g, c.want) {
+ t.Fatalf("%s: records = %v, want %v", c.name, g, c.want)
+ }
+ }
}
-func (s failListStore) List(ctx context.Context, prefix string) ([]string, error) {
- if prefix == s.fail {
- return nil, errors.New("list refused")
+// All three record readers, on a bucket-shaped store with real records: they
+// find the records, mint the same route they mint off a plain store, and
+// never list the whole of apps/.
+func TestRecordReadersWalkABucketShapedStore(t *testing.T) {
+ inner, sqid := setup(t)
+ if err := CreateRecord(ctx, inner, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
}
- return s.Store.List(ctx, prefix)
-}
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
-// One app's listing failing is partial, not fatal: every other app's keys
-// still come back, and the caller is told some were not seen.
-func TestAppSubtreeListingOneAppFailingIsPartial(t *testing.T) {
- s := failListStore{Store: seedAppListingBucket(t), fail: "apps/bab/blog/instances/"}
- got, partial, err := listAppSubtree(context.Background(), s, map[string]bool{"bab": true}, "instances/")
+ plainReg := regWith(t)
+ if err := Reconcile(ctx, inner, plainReg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ s := &bucketShaped{Store: inner}
+ reg := regWith(t)
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ want, got := *mustGet(t, plainReg, "acme.go.tito.io"), *mustGet(t, reg, "acme.go.tito.io")
+ want.CreatedAt, got.CreatedAt = 0, 0
+ if got != want {
+ t.Fatalf("minted over the walk = %+v, over the full listing %+v", got, want)
+ }
+
+ recs, err := ListAllRecords(ctx, s)
if err != nil {
- t.Fatalf("one app's failed listing failed the walk: %v", err)
+ t.Fatal(err)
}
- if !partial {
- t.Fatal("a failed app listing was not reported as partial")
+ if len(recs) != 1 || recs[0].Host != "acme.go.tito.io" {
+ t.Fatalf("ListAllRecords = %+v, want the one acme record", recs)
}
- var records []string
- for _, k := range got {
- if _, _, _, ok := ParseRecordKey(k, map[string]bool{"bab": true}); ok {
- records = append(records, k)
- }
+ if err := ReconcileSidecars(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ if s.listedAllOfApps() {
+ t.Fatalf("a record reader listed the whole of apps/: %v", s.lists)
+ }
+}
+
+// Reconcile treats a partial walk as it treats an unreadable record: the
+// routes it could not see are not retired.
+func TestReconcileKeepsRoutesItCouldNotList(t *testing.T) {
+ inner, sqid := setup(t)
+ if err := CreateRecord(ctx, inner, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
+ }
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
+ reg := regWith(t)
+ if err := Reconcile(ctx, inner, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ s := &bucketShaped{Store: inner, failList: RecordsPrefix(sqid, "tito")}
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ if rt, err := reg.Get("acme.go.tito.io"); err != nil || rt == nil {
+ t.Fatalf("a route whose records could not be listed was removed: %+v, %v", rt, err)
+ }
+}
+
+// ListAllRecords refuses a partial walk rather than report a short record
+// set: the moves and steering passes act on what it returns, and a missing
+// record reads to them as an instance that no longer exists.
+func TestListAllRecordsRefusesAPartialWalk(t *testing.T) {
+ inner, sqid := setup(t)
+ if err := CreateRecord(ctx, inner, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
}
- want := []string{"apps/bab/shop/instances/a.example.com.json", "apps/bab/shop/instances/b.example.com.json"}
- if !reflect.DeepEqual(records, want) {
- t.Fatalf("records = %v, want %v", records, want)
+ s := &bucketShaped{Store: inner, failList: RecordsPrefix(sqid, "tito")}
+ if recs, err := ListAllRecords(ctx, s); err == nil {
+ t.Fatalf("ListAllRecords over a failed app listing = %+v, nil; want an error", recs)
}
}
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go
index a2e8c04a4..0551541bb 100644
--- a/internal/provision/reconcile.go
+++ b/internal/provision/reconcile.go
@@ -1361,83 +1361,109 @@ func ListAllRecords(ctx context.Context, s release.Store) ([]Record, error) {
return recs, nil
}
-// listAppSubtree returns every key under apps/<acct>/<app>/<sub> for each
-// known account and each of its apps, sorted.
+// listAppSubtree returns the keys a record reader needs to see: every key
+// under apps/<acct>/<app>/<sub> for each known account and each of its apps,
+// sorted — or, on a store that cannot be walked that way, every key under
+// apps/, as the readers always listed. The callers parse every key exactly as
+// before (ParseRecordKey, ParseSidecarKey); this changes only which keys they
+// are shown.
//
-// It exists because the record readers used to List(ctx, "apps/") and filter:
-// that listing returns every object under every app — every release artifact
-// and every log chunk — to find the records among them. On the flagship on
-// 2026-10-05 apps/ held 59,288 objects, 50,695 of them logs, for 152 instance
-// records: sixty list pages, several times over, on every serial bump (#231).
-// This walks the same tree by its directories instead (ListDirs, the
-// delimiter-aware listing the mail sweep already uses): one page of accounts,
-// one page of apps per account, then one small listing per app.
+// It exists because List(ctx, "apps/") returns every object under every app —
+// every release artifact and every log chunk — to find the records among
+// them. On the flagship on 2026-10-05 apps/ held 59,288 objects, 50,695 of
+// them logs, for 152 instance records: sixty list pages per reader, three
+// readers on every serial bump, and most of why a box took 16 to 43 seconds
+// to notice a promote (#231). The walk is one ListDirs of apps/, one per
+// known account, and one small List per app.
//
// It finds the same records the full listing did. A record's account is
-// always one of known (ParseRecordKey and ParseSidecarKey refuse any other),
-// and its app is a directory directly under the account, so no record lives
-// anywhere this does not look. The callers still parse every key exactly as
-// before; this changes only which keys they are shown.
+// always one of known (the parsers refuse any other), and its app is a
+// directory directly under the account, so no record lives anywhere the walk
+// does not look — PROVIDED ListDirs answers in the bucket's shape. Only a
+// store that says so (release.HasBucketListDirs: the S3 store, and a prefix
+// store over one) is walked; the fleet door, whose ListDirs("apps/") is its
+// own account's app directories, and DirStore, which lists by walking its
+// whole root, are listed the old way. Assuming the shape on the fleet door
+// found no records at all, and the reconcile pass would have read that as
+// every route retired.
//
// Failure comes in two sizes. The top-level listing failing is an error, as
// the single List failing was. One account's or one app's listing failing is
// PARTIAL: the keys that were listed come back, and partial reports that some
-// were not. The callers treat that exactly as they treat a record they could
-// not read — removal is vetoed for the whole pass, everything that was seen
-// still converges — because a partial record set must never read as "the
-// bucket retired the rest", and one app's bad minute must not stop every
-// other app's routes from being minted.
+// were not. The callers treat that as they treat a record they could not
+// read — removal is vetoed for the whole pass, everything that was seen still
+// converges — because a partial record set must never read as "the bucket
+// retired the rest", and one app's bad minute must not stop every other app's
+// routes from being minted.
func listAppSubtree(ctx context.Context, s release.Store, known map[string]bool, sub string) (keys []string, partial bool, err error) {
+ if !release.HasBucketListDirs(s) {
+ keys, err := s.List(ctx, "apps/")
+ return keys, false, err
+ }
accts, err := s.ListDirs(ctx, "apps/")
if err != nil {
return nil, false, err
}
- var prefixes []string
+ var acctPrefixes []string
for _, ap := range accts {
- acct := strings.TrimSuffix(strings.TrimPrefix(ap, "apps/"), "/")
- if !known[acct] {
- continue
+ if known[strings.TrimSuffix(strings.TrimPrefix(ap, "apps/"), "/")] {
+ acctPrefixes = append(acctPrefixes, ap)
}
+ }
+
+ var (
+ mu sync.Mutex
+ prefixes []string
+ out []string
+ )
+ fail := func(what string, err error) {
+ log.Printf("provision: listing %s: %v — its records are unseen this pass", what, err)
+ partial = true
+ }
+ // A handful at a time, accounts and apps alike: each is one small round
+ // trip, and a box walks a few dozen accounts and a few hundred apps.
+ forEach(acctPrefixes, func(ap string) {
apps, err := s.ListDirs(ctx, ap)
+ mu.Lock()
+ defer mu.Unlock()
if err != nil {
- log.Printf("provision: listing %s: %v — its records are unseen this pass", ap, err)
- partial = true
- continue
+ fail(ap, err)
+ return
}
for _, p := range apps {
prefixes = append(prefixes, p+sub)
}
- }
+ })
+ forEach(prefixes, func(p string) {
+ keys, err := s.List(ctx, p)
+ mu.Lock()
+ defer mu.Unlock()
+ if err != nil {
+ fail(p, err)
+ return
+ }
+ out = append(out, keys...)
+ })
+ slices.Sort(out)
+ return out, partial, nil
+}
- // A handful at a time: each is one small round trip, and a box walks
- // a few hundred apps.
- const listers = 8
- var (
- mu sync.Mutex
- out []string
- wg sync.WaitGroup
- sem = make(chan struct{}, listers)
- )
- for _, p := range prefixes {
+// forEach runs f over items, listWalkers at a time, and returns when all
+// have finished.
+func forEach(items []string, f func(string)) {
+ const listWalkers = 8
+ var wg sync.WaitGroup
+ sem := make(chan struct{}, listWalkers)
+ for _, it := range items {
wg.Add(1)
sem <- struct{}{}
- go func(p string) {
+ go func(it string) {
defer wg.Done()
defer func() { <-sem }()
- keys, err := s.List(ctx, p)
- mu.Lock()
- defer mu.Unlock()
- if err != nil {
- log.Printf("provision: listing %s: %v — its records are unseen this pass", p, err)
- partial = true
- return
- }
- out = append(out, keys...)
- }(p)
+ f(it)
+ }(it)
}
wg.Wait()
- slices.Sort(out)
- return out, partial, nil
}
// poolServed reports whether pool is one this box serves. An empty
diff --git a/internal/release/store.go b/internal/release/store.go
index c296106f7..93edf1c67 100644
--- a/internal/release/store.go
+++ b/internal/release/store.go
@@ -79,6 +79,30 @@ type prefixStore struct {
base string // always "" or ends in "/"
}
+// BucketListDirs is the optional half of Store that says ListDirs answers in
+// the bucket's own shape: ListDirs("apps/") is the account directories, and
+// ListDirs("apps/<acct>/") that account's app directories, exactly as an S3
+// delimiter listing of the deployment bucket would be. A reader may then walk
+// the tree by its directories instead of listing every key under it.
+//
+// Optional, because not every Store has that shape. The fleet door scopes a
+// box to its own account and answers ListDirs("apps/") with that account's
+// APP directories, and DirStore lists by walking its whole root, so walking
+// it per app is slower than one List. A Store that does not say so is read
+// the old way.
+type BucketListDirs interface {
+ BucketShapedListDirs() bool
+}
+
+// HasBucketListDirs reports whether s promises bucket-shaped ListDirs.
+func HasBucketListDirs(s Store) bool {
+ b, ok := s.(BucketListDirs)
+ return ok && b.BucketShapedListDirs()
+}
+
+// BucketShapedListDirs: rebasing keeps the shape of the store underneath.
+func (p prefixStore) BucketShapedListDirs() bool { return HasBucketListDirs(p.Store) }
+
// NewPrefixStore wraps s so every key is rebased under prefix. An empty
// prefix returns s UNCHANGED (not a prefixStore with an empty base) — every
// deployment that never sets the namespacing env var (every platform box
@@ -739,6 +763,10 @@ func (s *S3Store) ListSizes(ctx context.Context, prefix string) ([]KeySize, erro
return out, nil
}
+// BucketShapedListDirs: S3Store IS the deployment bucket, and its ListDirs
+// is the delimiter listing of it.
+func (s *S3Store) BucketShapedListDirs() bool { return true }
+
// ListDirs is the Delimiter:"/" listing: S3 rolls everything below one level
// up into a CommonPrefix, so the response is bounded by the number of child
// prefixes rather than by the total object count beneath them. Objects AT