feat(managementapi): container remote bulk pass (S17 Phase 6 plan: 33/42)

Why

The container bulk-delete routes narrowed to hosted when the Phase 4 write routes landed, so a remote container repository's images, manifests, and tags collections answered the existence-hiding 404 while its cached rows stayed put. This MR serves the remote arms: the routes admit a remote repository, and the mgmtapi:bulk-delete-container worker runs an eviction pass instead of the hosted delete.

Eviction is not deletion, so the pass drops the couplings that only make sense for owned data. It reaches those by calling ContainerRemoteEvictor rather than by branching inside the hosted composer.

Step 33 of the S17 Phase 6 plan.

Depends on

Merge gate: satisfied. The fail-closed worker read this widening relies on merged as c437da899, from fix(managementapi): fail closed on bulk-worker ... (!1736 - merged) • Hayley Swimelar • 19.4, which closed Bulk workers no-op on non-hosted kinds where th... (#723 - closed) • Hayley Swimelar • 19.4.

Deploy gate: open. This route must not widen in production until that fail-closed read is running fleet-wide, so it must not ride the same promotion as c437da899's release. docs/dev/releases.md gates the production manifest upload behind a manual play (runway_manual_production_deploy: true), and staging uploads automatically. Confirm what is actually running by reading the version field of a recent access log row, because the pipeline ends at manifest upload and Flux reconciles the rollout separately, so a green pipeline is not evidence.

The window this closes: during a rolling deploy a new pod's widened route enqueues a remote batch onto the shared mgmtapi:bulk-delete-container kind, and an old pod without the fail-closed read claims it and completes it as a silent no-op, a 202'd eviction that never runs. With the read deployed everywhere a skewed claim returns a typed error and River retries until an armed binary picks it up.

This MR merges only after one of the two clearing routes in Deploy-ordering gate for the remote bulk passes (#755) • Hayley Swimelar • 19.4 is taken.

What (the non-obvious parts)

  • The manifests pass walks created_at DESC and reaches its dependency order by re-attempting refusals, rather than by inverting the sort. AC #108's two clauses pull against each other. Descending puts a row re-cached mid-pass in the region the walk has already left, which is the survival clause and what makes the walk finite, so the dependency order comes from a bounded fixpoint re-sweep of the entries the blocking check refused. Phase 4's subset pass discovers the same order the same way, and the re-sweep only revisits rows the single paged walk already returned. An integration test pins the direction and fails against ASC. One limit worth naming: now() resolves to the inserting transaction's start, so a fill whose transaction opened before the cursor read and commits during the pass is still evicted. That costs one extra upstream fetch, and the walk stays finite because the transactions already open when the page was read are what bounds the set.
  • The route gate is bulk-scoped rather than a widening of the shared resolver. resolveContainerRepository (internal/managementapi/container_list.go) has nine other call sites, and none of them may take the widened path, so the gate dispatches on the already-resolved repositories row kind. TestBulkContainerDeletes_WideningDoesNotLeakToTheSharedResolve pins all nine. Six still answer the remote 404. The other three took their own remote arm from feat(managementapi): serve container remote evi... (!1790 - merged) • Hayley Swimelar • 19.4, so the two deletes answer the eviction arm's child-row miss and the tag upsert answers its hosted-only 405.
  • No acceptance-time predicate on the remote collections. The hosted pass's AcceptedAt bound is deliberately absent, per the spec, so the drain ends on an empty page rather than a short one, and a 200-page per-run budget bounds the walk instead. The budget is shared across the three collections. It is load-bearing for images and tags, which list by name and can meet a row first cached under a name after the cursor. It covers the manifests walk because the drain is shared, not because that walk can outrun it.
  • The manifests arm is the tight one against the attempt deadline. The page budget is 200 pages of containerBulkPageSize, so 20,000 entries, and runRemoteManifestFixpoint re-sweeps the blocked remainder for up to 19 more rounds, an upper bound near 380,000 EvictContainerRemoteManifest calls. Against containerBulkWorkTimeout at 45 minutes that leaves about 7 ms an entry, where each entry is its own transaction with a live-parent probe ahead of the mark. The images arm gets 135 ms for one UPDATE. It is dormant until a writer fills container_remote_manifest_relationships, because nothing makes the fixpoint take a second round today. The arithmetic lives here rather than on the constant because the comment caps in ci: gate Go comment caps on changed blocks (!1828 - merged) • Hayley Swimelar • 19.4 hold an unexported doc to one line.
  • Hosted-only couplings stay off the remote arm. No artifact_registry_artifact_deleted (AC #102 (closed)), asserted in both directions.
  • Two spec-amendment commits ride this MR (b2c85d7aa and a59143538, both on docs/specs/S17-rest-management-api.md). The section fixed a mechanism the shipped pass does not use ("the order is fixed here rather than discovered"), and it never stated the walk-direction constraint that decides the implementation. The second commit corrects the linked Resolutions entry, which the first left asserting the reverse. Precedent for a spec correction riding its implementation on this same spec: chore(managementapi): apply container bulk mani... (!1583 - merged) • Hayley Swimelar • 19.4.
  • Scope beyond the plan's Files list, disclosed under the plan-contradiction rule. Step 33 names only "the container bulk worker and route files (Modify)". Also changed: internal/datastore/container_remote_bulk_scope.go (new, the three keyset scope-page reads), a tag removal on datastore.ContainerRemoteEvictor, internal/datastore/query_names.go, cmd/artifact-registry/container_remote_bulk_evictor.go (new pool binding), cmd/artifact-registry/wire_management.go, internal/managementapi/container_list.go, internal/managementapi/handler.go, api/openapi/v1.yaml, three api/bruno/management-api/container-bulk-deletes/*.bru files, docs/specs/S17-rest-management-api.md. The rebase onto main added a one-line dupl directive to two of step 32's files, cmd/artifact-registry/wire_management_npm_remote_bulk_integration_test.go and internal/managementapi/bulk_npm_worker_remote_fakes_test.go, because that linter anchors its report on either end of a mirrored pair. Six more suppressions and one budget raise came from rebasing onto the merged feat(managementapi): serve container remote evi... (!1790 - merged) • Hayley Swimelar • 19.4, and none of them fire on either side alone: //nolint:iface on both ends of the two evictor pairs, //nolint:funlen on the two Deps literals that grew a seam from each side, and the name budget in internal/metrics/cardinality.go raised from 400 to 450.
  • One boundary move inside that. The plan gives internal/datastore/container_remote_tags.go's hard delete to Step 35 and that file's creation to Step 20. Neither has landed, and the bulk tags pass needs the removal now, so DeleteContainerRemoteTag lands on ContainerRemoteEvictor beside the image and manifest evictions rather than as a second copy in a file that does not exist. It is a removal rather than a mark because container_remote_tags carries no soft_deleted_at column.

Spec coverage

Spec: docs/specs/S17-rest-management-api.md. AC numbers come from the extracted auto-numbered list (123 items), not from counting.

Criterion (container slice) Tests
AC #108: bulk eviction accepts the Phase 4 selectors with the same validation on a remote repository TestBulkContainerDeletes_RemoteRepositoryEnqueues202, TestBulkContainerDeletes_RemoteRoutesResolveTheRemoteChain, TestContainerBulkWorker_RemoteKindRunsTheEvictionPass, TestContainerBulkWorker_RemoteBatchNarrowingIsUnchanged
AC #108: the same no-op rules, so an absent, foreign, chain-broken, or malformed entry skips rather than fails TestContainerBulkWorker_RemoteImagesSubset, ..._RemoteTagsSubset, ..._RemoteManifestsSubset, ..._RemoteChainBreakIsANoOp, TestContainerBulkWorker_RemoteMalformedEntriesTakeTheLoggedSkip, TestContainerBulkWorker_RemoteZeroManifestSkipLeavesTheRefusalIntact, TestBulkContainerDeletes_RemoteImageChainRejections
AC #108: delete_all makes one ordered pass in each collection's list order TestContainerBulkWorker_RemoteDeleteAllIsOneOrderedPass, TestContainerRemoteBulkImagePageStmt, TestContainerRemoteBulkTagPageStmt, TestContainerRemoteBulkManifestPageStmt, TestIntegration_ContainerBulkRemote_DeleteAllDrainsEveryCollection, TestContainerRemoteBulkScope_ScopePagesRideTheirIndex
AC #108: every entry present when the pass reaches its position is evicted, past the page cap TestContainerBulkWorker_RemoteDeleteAllCrossesPageBoundariesAtTheRealCap, ..._RemoteDeleteAllEmptyCollectionAppliesNothing, TestContainerRemoteBulkScope_NameWalksResumeAfterTheirCursor, TestContainerRemoteBulkScope_ManifestsResumeAcrossACreatedAtTie, TestContainerRemoteBulkScope_MarkedRowsLeaveTheScope, TestContainerRemoteBulkScope_PagesAreScopedToTheirParent
AC #108: an entry re-cached behind the pass survives without failing the job TestContainerRemoteBulkScope_ManifestsWalkNewestFirst (the direction pin, fails against ASC), TestIntegration_ContainerBulkRemote_ConcurrentRecacheSurvivesThePass, TestContainerBulkWorker_RemoteDeleteAllIsBoundedUnderEndlessNewCoordinates
AC #108 tail: the manifests pass evicts an index and its children in the one pass TestContainerBulkWorker_RemoteManifestsClearABlockedChainInOnePass (child-before-index, nested fixpoint, and the outside-the-batch survivor), ..._RemoteDeleteAllClearsABlockedChain, ..._RemoteManifestsRoundBoundSettlesTheRemainder, TestIntegration_ContainerBulkRemote_DependencyOrderClearsACachedIndex
AC #62 tail: no remote collection carries the acceptance-time predicate TestContainerBulkWorker_RemoteDeleteAllTakesNoAcceptanceBound, TestContainerRemoteBulkManifestPageStmt_OpeningPageCarriesNoBound
AC #107 interaction: the live-parent 409 is a terminal skip only when no round can clear it, and a marked parent does not block the outside-the-batch subtest of TestContainerBulkWorker_RemoteManifestsClearABlockedChainInOnePass, TestListLiveContainerRemoteParentDigestsStmt_SQL
AC #102 (closed): evictions emit no artifact_registry_artifact_deleted, and the hosted emission is unchanged TestContainerBulkWorker_RemoteEvictionEmitsNoUsageEvent, TestContainerBulkWorker_HostedEmissionIsUnchangedBesideTheRemoteArm
Pass-run log line: the resolved kind plus the applied and skipped counts TestContainerBulkWorker_PassLinesNameTheResolvedKind, TestContainerBulkWorker_RemotePassLogsOnAFailedRun, TestIntegration_ContainerBulkRemote_PassRunLogsItsOutcome

The tag removal this MR adds to the evictor is covered by TestContainerRemoteEvictor_DeleteContainerRemoteTag, TestContainerRemoteEvictor_DeleteContainerRemoteTag_ArgumentGuards, and TestDeleteContainerRemoteTagStmt. AC #105's subtree removal is the S20-A purger's, not this MR's: the images arm marks through the same EvictContainerRemoteImage composer Step 27 landed.

Error cases

Condition Tests
A virtual repository keeps its 404 on every bulk route TestBulkContainerDeletes_VirtualRepositoryIs404
A hosted repository keeps the hosted chain unchanged TestBulkContainerDeletes_HostedRoutesKeepTheHostedChain, TestContainerBulkWorker_RemoteBatchNarrowingIsUnchanged
A scope read that fails ends the pass, because the cursor cannot advance past a page nobody saw TestContainerBulkWorker_RemoteScopeReadFailureEndsThePass, TestContainerBulkWorker_RemoteReadFailureIsNotAMiss
A full page that resolved no entry stops before advancing the cursor TestContainerBulkWorker_RemoteDrainStallsOnAnAllFailedPage
One entry's failure does not stop the pass, and a dead context aborts it TestContainerBulkWorker_RemoteEntryFailureDoesNotStopThePass, TestContainerBulkWorker_RemoteDeadContextAbortsThePass
An unserviceable namespace retries TestContainerBulkWorker_RemoteUnserviceableNamespaceRetries
A child resolve failure is a 500 rather than a hidden 404 TestBulkContainerDeletes_RemoteChildResolveFailureIs500
Wiring: a nil client panics at construction, and the pool resolves per call TestNewContainerRemoteBulkScope_NilClientPanics, TestNewContainerRemoteEvictor_NilClientPanics, TestContainerRemoteBulkBindings_ResolveThePoolPerCall, TestIntegration_WireManagementAPI_ContainerRemoteBulkRouteReachesTheEvictor
Injection: every statement built through go-jet with bound parameters TestContainerRemoteBulkImagePageStmt, TestContainerRemoteBulkTagPageStmt, TestContainerRemoteBulkManifestPageStmt, TestContainerRemoteEvictionMarkStmts, TestDeleteContainerRemoteTagStmt

Test plan

Every gate below ran on 32c0df088. Three later rounds re-ran them on the head each produced, the last being 081ac8482: the first rebase onto main, the comment-cap conformance pass plus that round's review fixes, and the rebase onto the merged feat(managementapi): serve container remote evi... (!1790 - merged) • Hayley Swimelar • 19.4. Re-run each time: go build ./..., go vet ./..., go vet -tags=integration ./..., golangci-lint run ./... at 0 issues, the full unit suite, and -race on the three touched packages. Integration re-ran for ./internal/managementapi/ and for the two WireManagementAPI remote bulk-route cases. ./internal/datastore/ integration runs in CI.

  • go build ./... and go vet ./...: clean.
  • go test ./... -count=1: 63 of the 64 packages carrying tests ok, 11 with no test files. The one failure, TestUpstreamAuthenticator_ReauthorizeGetsAFreshBudget in internal/format/oci, is a flake outside this branch's diff. The branch touches no file in that package, and the case passes on three full re-runs of the package and three of the test alone.
  • go test ./... needs GOOGLE_APPLICATION_CREDENTIALS set, or three TestWireStorage_CloudCDN* cases fail on missing ADC. Those fail identically on pristine origin/main.
  • go test -tags=integration -count=1 against PostgreSQL 17.10: ./internal/datastore/ ok (591.5 s), ./internal/managementapi/ ok (67.6 s), ./cmd/artifact-registry/ ok (135.3 s).
  • go test -race -count=1 ./internal/managementapi/ ./internal/datastore/ ./cmd/artifact-registry/: ok (30.6 s / 2.3 s / 5.7 s), no race reported.
  • golangci-lint 2.12.2 untagged, --max-same-issues=0 --max-issues-per-linter=0: 0 issues.
  • Integration-tagged on the three touched package trees, same zero caps plus --uniq-by-line=false: 71 findings land on this branch's files. Seventy are contextcheck in internal/datastore/container_remote_eviction_integration_test.go, and one is a dupl pair-half on the container wiring walk, which only fires under the integration tag. Left unsuppressed to match the merged Maven and npm remote eviction suites, which carry 147 of the same finding, against a repo-wide integration-tagged baseline of 3,907 that CI lint cannot see.
  • scripts/ci/check-migration-immutability.sh origin/main: OK, and the branch touches no migration.
  • TestEveryStatementIsInstrumented and TestQueryNames_EachUsedExactlyOnce: pass, covering the four new query names.
  • EXPLAIN pins on the three scope-page reads: TestContainerRemoteBulkScope_ScopePagesRideTheirIndex, plus TestContainerRemoteEvictor_EvictionStatements_SinglePartitionPrune on the eviction statements.

No e2e scenario is added, and none is affected. docs/testing/e2e/oci.md scopes virtual and remote repositories out of the catalog until the capability ships, and no e2e catalog carries a management-API bulk-delete scenario.

No conformance run: this changes no Maven, npm, or Container/OCI protocol behavior, only the management API under /api/v1/.

Diff size

38 files, +7,122 / -255, past the 500 reviewable LOC docs/dev/development-model.md asks a justification for.

Group Files LOC
Production Go 12 +1,417 / -185
Test Go 21 +5,630 / -43
Contracts (OpenAPI, Bruno) 4 +66 / -24
Spec 1 +9 / -3

The plan's own size estimate for this step is ~250 production, ~600 test, ~850 total, so the actual is close to ten times it. The estimate assumed the plan's Files list, which named the worker and route files only. The step turned out to need three keyset scope-page reads, a tag removal on the evictor, a pool binding, and the wiring, which is the same omission the plan-contradiction disclosure above covers. The test mass then tracks that widened surface: three collections' scope reads, the worker's remote arms, the route gate and its non-leak proof across all nine sharing routes, the boot-level composition walk, and the query-plan pins.

Splitting does not help. The route widening, the worker's remote arm, and the scope reads have to land in one MR, or the widened route enqueues batches whose worker cannot dispatch them, which is the exact failure this step exists to prevent.

The plan's Status table is untouched here. Step 33's row on main is still empty, and the table has a single writer, so a docs(plans) batch or standing MR records it.

Context for LLM agents

Rationale

  • Inverting the manifests sort to ASC to get dependency order. Rejected because ASC walks toward the position every cache fill lands in, which evicts the rows AC #108 promises to spare and never terminates under a steady pull. The re-sweep gets the order without giving up either property.
  • Widening resolveContainerRepository itself. Rejected because all nine of its call sites, the single manifest delete included, still answer the remote 404 today, and the eviction arms that change three of them land in Step 30.
  • Marking cached tags rather than removing them. Rejected because container_remote_tags carries no soft_deleted_at column: a tag is a mutable pointer re-pointed in place by the next revalidation, so it holds no unique slot a tombstone would have to release.
  • Waiting for Step 20 to create internal/datastore/container_remote_tags.go before adding the tag removal. Rejected because the bulk tags pass needs it now, and a second copy in an unlanded file would have to be reconciled later.

Non-goals

  • Reaping what the marks leave behind. The S20-A purger owns the subtree removal AC #105 names, and no container remote reap has a production consumer yet.
  • The single-artifact eviction handler arms and the remote tag-upsert 405. Those are Step 30, in feat(managementapi): serve container remote evi... (!1790 - merged) • Hayley Swimelar • 19.4.
  • The remote tag untag route. That is Step 35, which also moves the hard delete into internal/datastore/container_remote_tags.go once Step 20 creates it.
  • The Maven and npm PackagesBulkDeleteAccepted response text in api/openapi/v1.yaml, which still defines delete_all completion by an acceptance boundary those formats' remote arms have no equivalent for, a gap feat(managementapi): maven remote bulk pass (S1... (!1756 - merged) • Hayley Swimelar • 19.4 flagged. It is not this MR's surface: the container routes answer through their own ContainerBulkDeleteAccepted, which already promises the pass rather than an end state on a remote repository, so only the three operation descriptions and the selector schema needed qualifying here.

Related to #314

Edited by Hayley Swimelar

Merge request reports

Loading
Loading