feat(oci): flight registry and per-call request state (S32 plan: 8/19)

What

Moves all four of the container request builder's per-call values off the instance onto two channels, and adds the container flight registry.

The four values split two ways. accept and imageName are uniform across a resolution, so they ride one request-scoped context value, RemoteRequestState. The row's url/auth_status/auth_url snapshot and its *UpstreamURLBuilder belong to one container_remote_repositories row, so each position supplies its own through RemoteUpstreamConfigSource. A resolution holds up to 20 rows against one context, so a second field on the request-scoped value would send every position to the first row's host.

FlightRegistry then hands out one shared *remote.SingleFlight per remote repository, bounded by a cap, an idle window, and a throttled sweep. The registry is impossible without the channel move, because remote.NewSingleFlight captures its RequestBuilder for the repository's lifetime.

imageName is the value that must not be missed. target refuses every path outside it, so a registry-held builder that kept it frozen would serve the first image pulled through a remote and fail every other one.

Why the registry is bounded

The cap, idle window, and sweep interval are package constants, copying Maven's maxRemoteFlights / remoteFlightIdleTimeout / remoteFlightSweepInterval trio. Without them the map is an unbounded per-process cache of semaphores on a multi-tenant service. npm's registry has no bound and defers it to #672; this slice bounds where npm deferred.

Eviction removes only the map entry, so a caller already holding a flight finishes. While the two coexist each path pays one uncoalesced fetch.

The frozen cache seam

FlightRegistry.build freezes one cache seam per entry for the flight's life. That is safe on the two paths the flight reaches: the fill write and the freshness bump. Neither reads cache_validity_hours, so no held row pins a freshness verdict. Lookup and the cache-fallback route stay on the caller's own per-fetch store, which is what keeps RemoteOperations per fetch.

Each constructor validates what it reads

NewRemoteOperationsWithFlight first demanded a request builder, a doer, a blob store, and both ids, then discarded all five: a flight carries its own, and only the cache store and the two bounds reach the glue. A caller had to populate five fields that changed nothing, and a reader could not tell which of the eight influence a fetch.

The validator is now split along that line. The direct constructor runs both halves, so its contract is unchanged. The flight-backed one runs the glue half, which is what its own test already pinned.

Behavior

Every standalone remote read behaves identically before and after. Both install sites — the manifest arm's buildUpstreamFill and the blob arm's fill — put the two channels on the context each fetch runs on, so no read reaches the new fail-closed errors.

One operator-facing change: msgBlobFillSurfaceFailed's text now names both rejections its site answers, not only the factory's. A log filter pinned to the old string needs updating. The message appears in no dashboard or doc in this repo.

Tests

Source +575 −272, test +1688 −124.

Unit tests cover each named case: two concurrent builds; two image names through one held builder, each reaching its own URL and its own repository:<image>:pull scope; a settled auth_status not re-discovered; a url change picked up on the next fetch; two builders for two repositories reached concurrently on one request context, each asserting its own host and its own observed-url discovery guard; a key miss against a source holding one sibling position, which refuses the lazy single-entry implementation; fail-closed on a bare context, on state alone, on an empty source, and on a zero-value source; one rejection per moved validation, at the new constructor rather than at the read; and a statement-count assertion at 2 and 20 positions, proving the source itself reads nothing.

The registry gets an identity test, a three-arm sweep test over the cap, the idle window and the throttle, and an in-flight survivor test.

The S16 remote read suites re-run. Two are edited for the constructor's new shape: remote_auth_test.go and remote_e2e_test.go adapt call sites only, and no existing assertion's subject changed.

Diff size

Past the 500-LOC ceiling at 575 source lines, and justified rather than split. The overrun is the two extra values the channel carries, the two shapes it carries them in, the standalone wiring that moves with them, and the registry's eviction. Splitting lands either a registry that cannot be constructed, a context channel with no reader, a build that does not compile, or an unbounded registry in the interim.

Group Lines
New seams (remote_upstream_config.go, remote_flight_registry.go) +388 −0
Request builder (remote_requestbuilder.go) +78 −119
Install sites and glue (remote_manifest.go, remote_blob.go, remote_operations.go, remote_revalidate.go) +100 −92
Composition root (wire_oci_remote.go) +9 −61
Tests +1688 −124

Scope notes

remote_revalidate.go (one line), remote_auth_test.go, and remote_e2e_test.go are in no step's Files list. All three are mechanical adaptations the constructor's new shape forces.

No database review is owed: the diff adds no migration and no file that dispatches a statement.

No e2e scenario changes, because no externally observable behavior changes. No route reaches the registry until Step 15, which owns the end-to-end sharing test.

Owed elsewhere

These belong to other MRs and are deliberately absent here:

  • The plan's Status row, and its "at most a capped number of entries" wording, which overstates a sweep the same bullet mandates copying from Maven. Both go to the batched docs(plans) MR, which is the Status table's single writer. Measured source and test lines for its LOC line: +575 and +1688.
  • An S13 amendment before Step 14 wires the registry. S13 asks for a durable credential read inside BuildRequest on every call; the source is read per fetch from a row already in hand. Nothing regresses today, because the composition root still builds a builder per fetch. S13's own acceptance criterion already records container as non-conformant and names #596.
  • S16's three gap records, which say the container request builder is not hoistable. S16 delegates the choice of fix, so only its bookkeeping is stale.
  • The plan's sentence that the flight-backed constructor "applies the same bounds and validation as NewRemoteOperations", which the validator split makes false in its literal reading. The step's tests never encoded that reading.

Related to #291

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading