fix: wait for image conversion before reporting build ready (KERNEL-863) (#65)

* fix: wait for image conversion before reporting build ready (KERNEL-863)

Fixes a race condition where build status would transition to "ready"
before the image conversion completed, causing instance creation to fail.

The registry's triggerConversion() runs asynchronously after returning
201 to the builder. This meant the builder could report success and
the build manager would set status="ready" while image conversion was
still in progress.

Changes:
- Add imageManager dependency to build manager
- Add waitForImageReady() that polls until image status is "ready"
- Call waitForImageReady() before setting build to "ready" status
- If image conversion fails/times out, mark build as failed

* fix: use buildCtx for image wait and recalculate duration

Addresses PR review feedback:
1. Use buildCtx instead of ctx for waitForImageReady to respect
   build timeout during image conversion wait
2. Recalculate duration after waitForImageReady completes to
   accurately report total build time including image conversion

Author: hiroTamada

cmd/api/wire_gen.go

@@ -88,6 +88,7 @@ type manager struct {
 	queue           *BuildQueue
 	instanceManager instances.Manager
 	volumeManager   volumes.Manager
+	imageManager    images.Manager
 	secretProvider  SecretProvider
 	tokenGenerator  *RegistryTokenGenerator
 	logger          *slog.Logger
@@ -105,6 +106,7 @@ func NewManager(
 	config Config,
 	instanceMgr instances.Manager,
 	volumeMgr volumes.Manager,
+	imageMgr images.Manager,
 	secretProvider SecretProvider,
 	logger *slog.Logger,
 	meter metric.Meter,
@@ -119,6 +121,7 @@ func NewManager(
 		queue:             NewBuildQueue(config.MaxConcurrentBuilds),
 		instanceManager:   instanceMgr,
 		volumeManager:     volumeMgr,
+		imageManager:      imageMgr,
 		secretProvider:    secretProvider,
 		tokenGenerator:    NewRegistryTokenGenerator(config.RegistrySecret),
 		logger:            logger,
@@ -294,6 +297,28 @@ func (m *manager) runBuild(ctx context.Context, id string, req CreateBuildReques
 
 	m.logger.Info("build succeeded", "id", id, "digest", result.ImageDigest, "duration", duration)
 	imageRef := fmt.Sprintf("%s/builds/%s", m.config.RegistryURL, id)
+
+	// Wait for image to be ready before reporting build as complete.
+	// This fixes the race condition (KERNEL-863) where build reports "ready"
+	// but image conversion hasn't finished yet.
+	// Use buildCtx to respect the build timeout during image wait.
+	if err := m.waitForImageReady(buildCtx, id); err != nil {
+		// Recalculate duration to include image wait time
+		duration = time.Since(start)
+		durationMS = duration.Milliseconds()
+		m.logger.Error("image conversion failed after build", "id", id, "error", err, "duration", duration)
+		errMsg := fmt.Sprintf("image conversion failed: %v", err)
+		m.updateBuildComplete(id, StatusFailed, nil, &errMsg, &result.Provenance, &durationMS)
+		if m.metrics != nil {
+			m.metrics.RecordBuild(buildCtx, "failed", duration)
+		}
+		return
+	}
+
+	// Recalculate duration to include image wait time for accurate reporting
+	duration = time.Since(start)
+	durationMS = duration.Milliseconds()
+
 	m.updateBuildComplete(id, StatusReady, &result.ImageDigest, nil, &result.Provenance, &durationMS)
 
 	// Update with image ref
@@ -303,7 +328,7 @@ func (m *manager) runBuild(ctx context.Context, id string, req CreateBuildReques
 	}
 
 	if m.metrics != nil {
-		m.metrics.RecordBuild(ctx, "success", duration)
+		m.metrics.RecordBuild(buildCtx, "success", duration)
 	}
 }
 
@@ -640,6 +665,44 @@ func (m *manager) updateBuildComplete(id string, status string, digest *string,
 	m.notifyStatusChange(id, status)
 }
 
+// waitForImageReady polls the image manager until the build's image is ready.
+// This ensures that when a build reports "ready", the image is actually usable
+// for instance creation (fixes KERNEL-863 race condition).
+func (m *manager) waitForImageReady(ctx context.Context, id string) error {
+	imageRef := fmt.Sprintf("%s/builds/%s", m.config.RegistryURL, id)
+
+	// Poll for up to 60 seconds (image conversion is typically fast)
+	const maxAttempts = 120
+	const pollInterval = 500 * time.Millisecond
+
+	m.logger.Debug("waiting for image to be ready", "id", id, "image_ref", imageRef)
+
+	for attempt := 0; attempt < maxAttempts; attempt++ {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		default:
+		}
+
+		img, err := m.imageManager.GetImage(ctx, imageRef)
+		if err == nil {
+			switch img.Status {
+			case images.StatusReady:
+				m.logger.Debug("image is ready", "id", id, "image_ref", imageRef, "attempts", attempt+1)
+				return nil
+			case images.StatusFailed:
+				return fmt.Errorf("image conversion failed")
+			case images.StatusPending, images.StatusPulling, images.StatusConverting:
+				// Still processing, continue polling
+			}
+		}
+		// Image not found or still processing, wait and retry
+		time.Sleep(pollInterval)
+	}
+
+	return fmt.Errorf("timeout waiting for image to be ready after %v", time.Duration(maxAttempts)*pollInterval)
+}
+
 // subscribeToStatus adds a subscriber channel for status updates on a build
 func (m *manager) subscribeToStatus(buildID string, ch chan BuildEvent) {
 	m.subscriberMu.Lock()

lib/builds/manager.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/kernel/hypeman/lib/images"
 	"github.com/kernel/hypeman/lib/instances"
 	"github.com/kernel/hypeman/lib/paths"
 	"github.com/kernel/hypeman/lib/resources"
@@ -219,8 +220,85 @@ func (m *mockSecretProvider) GetSecrets(ctx context.Context, secretIDs []string)
 	return make(map[string]string), nil
 }
 
+// mockImageManager implements images.Manager for testing
+type mockImageManager struct {
+	images      map[string]*images.Image
+	getImageErr error
+}
+
+func newMockImageManager() *mockImageManager {
+	return &mockImageManager{
+		images: make(map[string]*images.Image),
+	}
+}
+
+func (m *mockImageManager) ListImages(ctx context.Context) ([]images.Image, error) {
+	var result []images.Image
+	for _, img := range m.images {
+		result = append(result, *img)
+	}
+	return result, nil
+}
+
+func (m *mockImageManager) CreateImage(ctx context.Context, req images.CreateImageRequest) (*images.Image, error) {
+	img := &images.Image{
+		Name:   req.Name,
+		Status: images.StatusPending,
+	}
+	m.images[req.Name] = img
+	return img, nil
+}
+
+func (m *mockImageManager) ImportLocalImage(ctx context.Context, repo, reference, digest string) (*images.Image, error) {
+	img := &images.Image{
+		Name:   repo + ":" + reference,
+		Status: images.StatusReady,
+	}
+	m.images[img.Name] = img
+	return img, nil
+}
+
+func (m *mockImageManager) GetImage(ctx context.Context, name string) (*images.Image, error) {
+	if m.getImageErr != nil {
+		return nil, m.getImageErr
+	}
+	if img, ok := m.images[name]; ok {
+		return img, nil
+	}
+	return nil, images.ErrNotFound
+}
+
+func (m *mockImageManager) DeleteImage(ctx context.Context, name string) error {
+	delete(m.images, name)
+	return nil
+}
+
+func (m *mockImageManager) RecoverInterruptedBuilds() {}
+
+func (m *mockImageManager) TotalImageBytes(ctx context.Context) (int64, error) {
+	return 0, nil
+}
+
+func (m *mockImageManager) TotalOCICacheBytes(ctx context.Context) (int64, error) {
+	return 0, nil
+}
+
+// SetImageReady sets an image to ready status for testing
+func (m *mockImageManager) SetImageReady(name string) {
+	m.images[name] = &images.Image{
+		Name:   name,
+		Status: images.StatusReady,
+	}
+}
+
 // Test helper to create a manager with test paths and mocks
 func setupTestManager(t *testing.T) (*manager, *mockInstanceManager, *mockVolumeManager, string) {
+	mgr, instanceMgr, volumeMgr, _, tempDir := setupTestManagerWithImageMgr(t)
+	return mgr, instanceMgr, volumeMgr, tempDir
+}
+
+// setupTestManagerWithImageMgr returns the image manager for tests that need it
+func setupTestManagerWithImageMgr(t *testing.T) (*manager, *mockInstanceManager, *mockVolumeManager, *mockImageManager, string) {
 	t.Helper()
 
 	// Create temp directory for test data
@@ -236,6 +314,7 @@ func setupTestManager(t *testing.T) (*manager, *mockInstanceManager, *mockVolume
 	// Create mocks
 	instanceMgr := newMockInstanceManager()
 	volumeMgr := newMockVolumeManager()
+	imageMgr := newMockImageManager()
 	secretProvider := &mockSecretProvider{}
 
 	// Create config
@@ -257,13 +336,14 @@ func setupTestManager(t *testing.T) (*manager, *mockInstanceManager, *mockVolume
 		queue:             NewBuildQueue(config.MaxConcurrentBuilds),
 		instanceManager:   instanceMgr,
 		volumeManager:     volumeMgr,
+		imageManager:      imageMgr,
 		secretProvider:    secretProvider,
 		tokenGenerator:    NewRegistryTokenGenerator(config.RegistrySecret),
 		logger:            logger,
 		statusSubscribers: make(map[string][]chan BuildEvent),
 	}
 
-	return mgr, instanceMgr, volumeMgr, tempDir
+	return mgr, instanceMgr, volumeMgr, imageMgr, tempDir
 }
 
 func TestCreateBuild_Success(t *testing.T) {