fix(discovery): hold published bundles while an upstream probe is unsettled - #813
Merged
Merged
Conversation
…ettled Closes the restart window that #810 left open. The upstream-OpenAPI cache is process-local, so every controller restart begins with no entry for any offer. reconcileStaticSite rebuilds the SHARED bundle from that cache on EVERY offer's reconcile — including reconciles belonging to other offers — so an offer that has not reconciled yet was re-rendered from the route-table fallback and visibly lost its advertised routes until its own reconcile landed. Measured on a live nine-offer stack during the v0.14.0-rc3 validation: after a controller image swap, five offers dropped from 11/12/12/12/16 advertised resources to 1 at ~t+30s and were fully recovered by ~t+90s. Buyers crawling discovery inside that window see a single root entry instead of the real paid routes. #810 stopped a FAILED probe being pinned for the whole generation. It could not fix this, because get() returns nil for "never fetched" and "fetched, no document" alike, and the caller cannot tell them apart. getSettled now returns that distinction, and buildOfferBundles takes the currently-published ConfigMap data. While a probe is unsettled the already served openapi.json and x402.json are kept — the ConfigMap survives the restart even though the cache does not, so there is something correct to hold on to. Stale beats thinner. The other direction matters just as much and is tested: once a probe HAS settled with no document, the fallback is the correct final answer and must re-render, or an offer that legitimately drops its upstream OpenAPI would serve the old document forever. A first-ever reconcile with nothing published still renders the fallback rather than an empty document. Verified TestBuildOfferBundles_UnsettledPreservesPublished fails with the preserve branch disabled and passes with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the restart window that #810 left open.
Problem
The upstream-OpenAPI cache is process-local, so every controller restart begins with no entry for any offer.
reconcileStaticSiterebuilds the shared bundle from that cache on every offer's reconcile — including reconciles belonging to other offers — so an offer that has not reconciled yet gets re-rendered from the route-table fallback and visibly loses its advertised routes until its own reconcile lands.Measured on a live nine-offer stack during
v0.14.0-rc3validation, immediately after a controller image swap:Buyers crawling
/.well-known/x402inside that window see a single root entry instead of the real paid routes. It self-heals, but it happens on every controller restart — including the one every upgrade performs.Why #810 couldn't fix it
#810 stopped a failed probe being pinned for the whole generation. It could not address this, because
get()returnsnilfor two different situations and the caller cannot distinguish them:Fix
getSettledreturns that distinction, andbuildOfferBundlestakes the currently-published ConfigMap data. While a probe is unsettled, the already-servedopenapi.jsonandx402.jsonare preserved.The key observation is that the ConfigMap survives the restart even though the cache does not — so there is always correct content to hold onto. Stale beats thinner.
Reading the ConfigMap is best-effort: an error yields
nil, and the caller renders from scratch exactly as before.The other direction is tested just as hard
Getting this wrong would be worse than the bug it fixes. Once a probe has settled with no document, the fallback must re-render — otherwise an offer that legitimately drops its upstream OpenAPI would serve the old document forever. And a first-ever reconcile with nothing published must still render the fallback rather than an empty file.
Three tests in
bundle_unsettled_test.go:TestBuildOfferBundles_UnsettledPreservesPublished— the regression itselfTestBuildOfferBundles_SettledEmptyStillFallsBack— stale content must not pin a settled rebuildTestBuildOfferBundles_UnsettledWithNoPublishedRendersFallback— first reconcile still produces a documentVerified the regression test genuinely catches the bug: with the preserve branch disabled it fails, and passes with it restored.
go build ./...clean, fullserviceoffercontrollersuite green.Scope
Existing test call sites are updated for the new signature;
noUpstreamOpenAPInow returns(nil, true)— a settled probe with no document, which is what those tests always meant.Targets
integration/v0.14.0-rc3for the next RC.v0.14.0-rc3is already published without it, and its release notes carry a[!NOTE]describing this window as known and self-healing.