From 24d9b5207b74edfecce52c336629d996bf57e132 Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:33:01 +0000 Subject: [PATCH] Keep waiting for build image while registry import is in flight 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 --- lib/builds/manager.go | 31 +++++++++++++++++---- lib/builds/manager_test.go | 6 ++++ lib/builds/race_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/lib/builds/manager.go b/lib/builds/manager.go index 87f21dfe..6bcf7b84 100644 --- a/lib/builds/manager.go +++ b/lib/builds/manager.go @@ -5,6 +5,7 @@ import ( "context" _ "embed" "encoding/json" + "errors" "fmt" "log/slog" "net" @@ -958,20 +959,40 @@ func (m *manager) updateBuildComplete(id string, status string, digest *string, m.notifyStatusChange(id, status) } +// imageImportRetryInterval is the delay between retries when the build's +// image has not yet been imported by the registry. Var so tests can shorten it. +var imageImportRetryInterval = time.Second + // waitForImageReady blocks until the build's image reaches a terminal state. // imageRef should be the short repo name (e.g., "builds/abc123" or "myapp") // matching what triggerConversion stores in the image manager. // This ensures that when a build reports "ready", the image is actually usable // for instance creation (fixes KERNEL-863 race condition). +// +// The registry imports pushed images asynchronously, and under concurrent +// build bursts the import (OCI layout copy) can outlast the image manager's +// internal existence deadline. By the time this is called the builder has +// already pushed the image, so "not found" means "import still in flight", +// not "push failed" — keep retrying until the build context expires. func (m *manager) waitForImageReady(ctx context.Context, imageRef string) error { m.logger.Debug("waiting for image to be ready", "image_ref", imageRef) - if err := m.imageManager.WaitForReady(ctx, imageRef); err != nil { - return err + for { + err := m.imageManager.WaitForReady(ctx, imageRef) + if err == nil { + m.logger.Debug("image is ready", "image_ref", imageRef) + return nil + } + if !errors.Is(err, images.ErrNotFound) { + return err + } + m.logger.Info("build image not yet imported by registry, retrying", "image_ref", imageRef) + select { + case <-ctx.Done(): + return err + case <-time.After(imageImportRetryInterval): + } } - - m.logger.Debug("image is ready", "image_ref", imageRef) - return nil } // subscribeToStatus adds a subscriber channel for status updates on a build diff --git a/lib/builds/manager_test.go b/lib/builds/manager_test.go index c26295ea..db819942 100644 --- a/lib/builds/manager_test.go +++ b/lib/builds/manager_test.go @@ -360,6 +360,12 @@ func (m *mockImageManager) WaitForReady(ctx context.Context, name string) error status = img.Status } m.mu.RUnlock() + // Mirror the real manager: a missing image is reported as not found + // (the real implementation polls for 30s first; return immediately + // to keep tests fast). + if !ok { + return fmt.Errorf("get image: %w", images.ErrNotFound) + } switch status { case images.StatusReady: return nil diff --git a/lib/builds/race_test.go b/lib/builds/race_test.go index d8103b81..d192a43c 100644 --- a/lib/builds/race_test.go +++ b/lib/builds/race_test.go @@ -121,6 +121,63 @@ func TestWaitForImageReady_WaitsForConversion(t *testing.T) { assert.GreaterOrEqual(t, elapsed, 200*time.Millisecond, "should have waited for conversion") } +// TestWaitForImageReady_RetriesWhileImportInFlight tests that waitForImageReady +// keeps waiting when the image doesn't exist yet. The registry imports pushed +// images asynchronously, so under concurrent build bursts the image record can +// appear well after the builder reports success — "not found" at this point +// means the import is still in flight, not that the push failed. +func TestWaitForImageReady_RetriesWhileImportInFlight(t *testing.T) { + mgr, _, _, imageMgr, tempDir := setupTestManagerWithImageMgr(t) + defer os.RemoveAll(tempDir) + + prevInterval := imageImportRetryInterval + imageImportRetryInterval = 20 * time.Millisecond + defer func() { imageImportRetryInterval = prevInterval }() + + ctx := context.Background() + buildID := "test-build-late-import" + imageRef := "builds/" + buildID + + // Image does not exist at all yet. Simulate the registry's async import + // registering it (pending) and conversion completing shortly after. + go func() { + time.Sleep(100 * time.Millisecond) + imageMgr.mu.Lock() + imageMgr.images[imageRef] = &images.Image{ + Name: imageRef, + Status: images.StatusPending, + } + imageMgr.mu.Unlock() + time.Sleep(100 * time.Millisecond) + imageMgr.SetImageStatus(imageRef, images.StatusReady) + }() + + start := time.Now() + err := mgr.waitForImageReady(ctx, imageRef) + elapsed := time.Since(start) + + require.NoError(t, err) + assert.GreaterOrEqual(t, elapsed, 200*time.Millisecond, "should have waited for the import to land") +} + +// TestWaitForImageReady_NotFoundUntilContextExpires tests that a build whose +// image never gets imported still fails once the build context expires. +func TestWaitForImageReady_NotFoundUntilContextExpires(t *testing.T) { + mgr, _, _, _, tempDir := setupTestManagerWithImageMgr(t) + defer os.RemoveAll(tempDir) + + prevInterval := imageImportRetryInterval + imageImportRetryInterval = 20 * time.Millisecond + defer func() { imageImportRetryInterval = prevInterval }() + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + err := mgr.waitForImageReady(ctx, "builds/test-build-never-imported") + require.Error(t, err) + assert.ErrorIs(t, err, images.ErrNotFound) +} + // TestWaitForImageReady_Timeout tests that waitForImageReady times out if image never becomes ready func TestWaitForImageReady_ContextCancelled(t *testing.T) { mgr, _, _, imageMgr, tempDir := setupTestManagerWithImageMgr(t)