Keep waiting for build image while registry import is in flight#310
Keep waiting for build image while registry import is in flight#310hiroTamada wants to merge 1 commit into
Conversation
The registry imports pushed images asynchronously (OCI layout copy + conversion enqueue), and under concurrent build bursts that import can take longer than the image manager's 30s existence deadline in WaitForReady. The build then fails with "image conversion failed: get image: image not found" even though the push succeeded and the image converts fine seconds later. Since waitForImageReady only runs after the builder has successfully pushed, treat not-found as import-still-in-flight and keep retrying until the build context expires instead of failing on the first existence timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 24d9b52. Configure here.
| err := mgr.waitForImageReady(ctx, "builds/test-build-never-imported") | ||
| require.Error(t, err) | ||
| assert.ErrorIs(t, err, images.ErrNotFound) | ||
| } |
There was a problem hiding this comment.
Flaky never-imported timeout test
Medium Severity
TestWaitForImageReady_NotFoundUntilContextExpires expects images.ErrNotFound when the context expires, but that only happens if the mock returns not-found before noticing cancellation. If ctx is already done when the next WaitForReady starts, the mock returns context.DeadlineExceeded and the outer loop exits immediately, so the assertion flakes. The real WaitForReady also returns ctx.Err() during its existence poll, so this test does not match production timeout behavior.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 24d9b52. Configure here.
| if !errors.Is(err, images.ErrNotFound) { | ||
| return err | ||
| } | ||
| m.logger.Info("build image not yet imported by registry, retrying", "image_ref", imageRef) |
There was a problem hiding this comment.
Retry matches conversion not-found errors
Low Severity
waitForImageReady retries on any error wrapping images.ErrNotFound, but WaitForReady can return that sentinel from a real conversion failure via the status-event path (image conversion failed with %w). Those failures are then misclassified as “import still in flight,” causing a spurious retry and misleading log before the next attempt sees failed status and exits.
Reviewed by Cursor Bugbot for commit 24d9b52. Configure here.
1 participant
Summary
Fixes spurious
image conversion failed: get image: image not foundbuild failures under concurrent deploy bursts.The registry imports pushed images asynchronously: after the manifest PUT returns 201,
triggerConversioncopies blobs into the OCI layout and only then callsImportLocalImage, which creates the image record. Under a burst of concurrent builds this import is disk-bound and can take well over 30 seconds — longer than the fixedmaxWaitForExistexistence deadline insideimages.WaitForReady. The build manager then fails the build even though the push succeeded and the image reachesreadyseconds later. Observed in production: a build declared failed at its 30s deadline whose image record wentready5 seconds after; ~15% of deploys failed this way during bursts, and retries succeeded.Since
waitForImageReadyonly runs after the builder has successfully pushed (buildctl pushes synchronously before reportingbuild_result), a not-found at this point means the import is still in flight — not that the push failed. This change makes the build flow keep retrying onimages.ErrNotFounduntil the build context (policy.TimeoutSeconds) expires, instead of giving up on the first existence timeout. Other errors (conversion failure, context cancellation) still fail immediately, and a genuinely-missing image now fails at the build timeout with the same error message.The 30s default inside
images.WaitForReadyis unchanged, so the instance-start pull path keeps its existing behavior.Changes
lib/builds/manager.go: retryWaitForReadyonErrNotFoundinwaitForImageReady, bounded by the build contextlib/builds/manager_test.go: mockWaitForReadynow returns not-found for missing images, mirroring the real managerlib/builds/race_test.go: regression tests for late import and never-imported casesTesting
go test ./lib/builds/passes locally (full package, including the two new regression tests)🤖 Generated with Claude Code
Note
Medium Risk
Changes post-build completion timing and failure semantics for the build→image handoff; bounded by existing build timeout and limited to ErrNotFound retries.
Overview
Fixes false failed builds when the push succeeded but the image record is not in the image manager yet because registry import is still running.
waitForImageReadyno longer treats a singleimages.ErrNotFoundfromWaitForReadyas terminal. After a successful builder push, not-found is interpreted as import in flight; the build manager loops with a 1simageImportRetryIntervaluntil the image is ready or the build context (policy.TimeoutSeconds) ends. Conversion failures and other non–not-found errors still fail immediately.Test support: the mock
WaitForReadyreturns not-found for missing images (matching production), plus regression tests for late import and never-imported timeout.Reviewed by Cursor Bugbot for commit 24d9b52. Bugbot is set up for automated code reviews on this repo. Configure here.