Skip to content

Fix macOS VZ default network egress - #368

Merged
rgarcia merged 3 commits into
mainfrom
issue-358
Aug 7, 2026
Merged

Fix macOS VZ default network egress#368
rgarcia merged 3 commits into
mainfrom
issue-358

Conversation

@rgarcia

@rgarcia rgarcia commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make the platform backend authoritative for the effective default network
  • always allocate macOS VZ guests from the default vmnet NAT (192.168.64.0/24, gateway 192.168.64.1), ignoring Linux-shaped subnet config
  • use the same effective network for allocation derivation and builder registry URL rewriting
  • add Darwin/Linux regression coverage and exercise guest TCP egress in the macOS install E2E test
  • preserve configured Linux bridge/subnet behavior and harden bridge-state querying

Closes #358.

Test plan

  • go test -count=1 -tags containers_image_openpgp ./lib/network ./lib/providers ./cmd/api/...
  • GOOS=linux GOARCH=arm64 go vet -tags containers_image_openpgp ./lib/network
  • bash -n scripts/e2e-install-test.sh
  • make sign-darwin and strict codesign verification
  • manual macOS VZ regression with NETWORK__SUBNET_CIDR=10.100.0.0/16:
    • API resolved 192.168.64.0/24, gateway 192.168.64.1
    • guest received 192.168.64.233/24
    • guest routed through 192.168.64.1
    • wget https://registry-1.docker.io/v2/ reached the registry and received the expected HTTP 401

Scope

Existing broken allocations are not migrated; affected VMs should be recreated. DHCP-based address discovery remains a follow-up.


Note

Medium Risk
Changes core guest networking, build startup readiness, and registry resolution on macOS; scope is broad but covered by new regression and E2E tests—existing VMs with wrong allocations are not migrated.

Overview
Fixes macOS VZ guest egress (#358) by treating the platform backend as the source of truth for the default network: VZ guests are allocated from 192.168.64.0/24 with gateway 192.168.64.1, and Linux bridge/subnet settings no longer override that. The same effective default network drives allocation derivation and loopback registry URL rewriting for builder VMs (via EffectiveDefaultNetwork and ProvideBuildManager wiring).

API: Adds timeoutNonStreamingRequests so the 60s request timeout does not apply to /logs and /events SSE streams—avoiding false CLI failures on long cold builds.

Builds: Tightens builder image readiness (builderReady only after success, retries on startup, failed images re-queued) and supports installer-built hypeman/builder:latest when the API is not started from a source checkout.

CI / E2E: Runs TestBuilderPersistentCacheReuse on Darwin (opt-in), ensures Docker/Colima on install E2E, includes lib/network in test-darwin, and extends install E2E with guest TCP egress and a full hypeman build smoke path. Linux bridge validation and gateway selection are slightly hardened without changing intended bridge behavior.

Reviewed by Cursor Bugbot for commit 76e0ae9. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Darwin builder test overruns CI
    • Added a Darwin-only deadline guard that skips the heavy builder cache integration test when the remaining go test timeout is below 20 minutes, preventing 600s CI timeouts.
  • ✅ Fixed: E2E parses missing build output
    • Changed the E2E build invocation to request structured --format json --transform id output and derive BUILD_ID from that value instead of a nonexistent Build started: prefix.
  • ✅ Fixed: E2E build lacks Docker setup
    • Added an ensure_docker_ready step before the build smoke test that verifies Docker is running and starts Colima on macOS when needed.

Create PR

Or push these changes by commenting:

@cursor push c5ee282536
Preview (c5ee282536)
diff --git a/integration/builder_cache_darwin_test.go b/integration/builder_cache_darwin_test.go
--- a/integration/builder_cache_darwin_test.go
+++ b/integration/builder_cache_darwin_test.go
@@ -9,6 +9,7 @@
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/kernel/hypeman/cmd/api/config"
 	"github.com/kernel/hypeman/lib/hypervisor"
@@ -19,6 +20,12 @@
 	if runtime.GOARCH != "arm64" {
 		t.Skip("VZ builder integration test requires Apple Silicon")
 	}
+	if deadline, ok := t.Deadline(); ok {
+		const minTimeout = 20 * time.Minute
+		if time.Until(deadline) < minTimeout {
+			t.Skipf("builder integration test requires go test timeout >= %s", minTimeout)
+		}
+	}
 	if _, err := exec.LookPath("docker"); err != nil {
 		t.Skip("builder integration test requires Docker")
 	}

diff --git a/scripts/e2e-install-test.sh b/scripts/e2e-install-test.sh
--- a/scripts/e2e-install-test.sh
+++ b/scripts/e2e-install-test.sh
@@ -20,6 +20,35 @@
 pass() { echo -e "${GREEN}[PASS]${NC} $1"; }
 fail() { echo -e "${RED}[FAIL]${NC} $1"; exit 1; }
 
+ensure_docker_ready() {
+    if docker info >/dev/null 2>&1; then
+        return 0
+    fi
+
+    if [ "$OS" = "darwin" ] && command -v colima >/dev/null 2>&1; then
+        warn "Docker daemon is not running; starting Colima..."
+        if colima status >/dev/null 2>&1; then
+            colima stop --force || true
+        fi
+        if ! colima start; then
+            colima stop --force || true
+            if ! colima start; then
+                colima delete --force
+                colima start
+            fi
+        fi
+    fi
+
+    for attempt in $(seq 1 30); do
+        if docker info >/dev/null 2>&1; then
+            return 0
+        fi
+        sleep 2
+    done
+
+    fail "Docker daemon is required for hypeman build E2E coverage"
+}
+
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
 REPO_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
 OS=$(uname -s | tr '[:upper:]' '[:lower:]')
@@ -215,6 +244,7 @@
 # Build a real image through the installed CLI. This exercises provider wiring,
 # the VZ builder VM, guest-to-host registry access, and the resulting image.
 E2E_BUILD_VM_NAME="e2e-build-test-vm"
+ensure_docker_ready
 BUILD_CONTEXT=$(mktemp -d)
 BUILD_OUTPUT_FILE=$(mktemp)
 trap 'rm -rf "${BUILD_CONTEXT:-}" "${BUILD_OUTPUT_FILE:-}"' EXIT
@@ -226,7 +256,7 @@
 
 BUILD_OK=false
 for i in $(seq 1 30); do
-    if $HYPEMAN_CMD build --file Dockerfile --timeout 600 \
+    if $HYPEMAN_CMD --format json --transform id build --file Dockerfile --timeout 600 \
         --image-name e2e/build-smoke:latest "$BUILD_CONTEXT" >"$BUILD_OUTPUT_FILE" 2>&1; then
         BUILD_OK=true
         break
@@ -240,7 +270,7 @@
 done
 [ "$BUILD_OK" = true ] || { cat "$BUILD_OUTPUT_FILE"; fail "hypeman build did not become ready"; }
 cat "$BUILD_OUTPUT_FILE"
-BUILD_ID=$(sed -n 's/^Build started: //p' "$BUILD_OUTPUT_FILE" | tail -1)
+BUILD_ID=$(tr -d '"[:space:]' < "$BUILD_OUTPUT_FILE")
 [ -n "$BUILD_ID" ] || fail "hypeman build output did not include a build ID"
 BUILD_IMAGE=$($HYPEMAN_CMD --format json --transform image_ref build get "$BUILD_ID") || fail "hypeman build get failed"
 BUILD_IMAGE=${BUILD_IMAGE#\"}

You can send follow-ups to the cloud agent here.

Comment thread integration/builder_cache_darwin_test.go
Comment thread scripts/e2e-install-test.sh
Comment thread scripts/e2e-install-test.sh
@rgarcia
rgarcia force-pushed the issue-358 branch 2 times, most recently from d71de71 to 1e54854 Compare August 7, 2026 15:30

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Kernel subnet mask diverges from iptables
    • Linux bridge initialization now rejects existing bridges whose gateway IP matches but subnet mask differs from the configured CIDR, preventing allocator/iptables divergence.

Create PR

Or push these changes by commenting:

@cursor push e0d7aa9235
Preview (e0d7aa9235)
diff --git a/lib/network/bridge_linux.go b/lib/network/bridge_linux.go
--- a/lib/network/bridge_linux.go
+++ b/lib/network/bridge_linux.go
@@ -51,6 +51,37 @@
 	return nil, err
 }
 
+func bridgeAddrMatchesGatewayAndMask(addrs []netlink.Addr, expectedGateway net.IP, expectedMask net.IPMask) (bool, bool, []string, []string) {
+	expectedOnes, expectedBits := expectedMask.Size()
+	hasGateway := false
+	hasGatewayWithMask := false
+	actualIPs := make([]string, 0, len(addrs))
+	gatewayCIDRs := make([]string, 0, 1)
+
+	for _, addr := range addrs {
+		actualCIDR := "<nil>"
+		if addr.IPNet != nil {
+			actualCIDR = addr.IPNet.String()
+		}
+		actualIPs = append(actualIPs, actualCIDR)
+		if !addr.IP.Equal(expectedGateway) {
+			continue
+		}
+		hasGateway = true
+		gatewayCIDRs = append(gatewayCIDRs, actualCIDR)
+		if addr.IPNet == nil {
+			continue
+		}
+
+		ones, bits := addr.IPNet.Mask.Size()
+		if ones == expectedOnes && bits == expectedBits {
+			hasGatewayWithMask = true
+		}
+	}
+
+	return hasGateway, hasGatewayWithMask, actualIPs, gatewayCIDRs
+}
+
 // checkSubnetConflicts checks if the configured subnet conflicts with existing routes.
 // Returns an error if a conflict is detected, with guidance on how to resolve it.
 func (m *manager) checkSubnetConflicts(ctx context.Context, subnet string) error {
@@ -128,14 +159,10 @@
 		}
 
 		expectedGW := net.ParseIP(gateway)
-		hasExpectedIP := false
-		var actualIPs []string
-		for _, addr := range addrs {
-			actualIPs = append(actualIPs, addr.IPNet.String())
-			if addr.IP.Equal(expectedGW) {
-				hasExpectedIP = true
-			}
+		if expectedGW == nil {
+			return fmt.Errorf("invalid gateway IP: %s", gateway)
 		}
+		hasExpectedIP, hasExpectedMask, actualIPs, gatewayCIDRs := bridgeAddrMatchesGatewayAndMask(addrs, expectedGW, ipNet.Mask)
 
 		if !hasExpectedIP {
 			ones, _ := ipNet.Mask.Size()
@@ -145,6 +172,14 @@
 				"or (3) delete the bridge with: sudo ip link delete %s",
 				name, actualIPs, gateway, ones, name)
 		}
+		if !hasExpectedMask {
+			ones, _ := ipNet.Mask.Size()
+			return fmt.Errorf("bridge %s exists with gateway %s but mask does not match expected /%d (gateway addresses: %v). "+
+				"Options: (1) update SUBNET_CIDR and SUBNET_GATEWAY to match the existing bridge, "+
+				"(2) use a different BRIDGE_NAME, "+
+				"or (3) delete the bridge with: sudo ip link delete %s",
+				name, gateway, ones, gatewayCIDRs, name)
+		}
 
 		// Bridge exists with correct IP, verify it's up
 		if err := netlink.LinkSetUp(existing); err != nil {

diff --git a/lib/network/bridge_linux_test.go b/lib/network/bridge_linux_test.go
--- a/lib/network/bridge_linux_test.go
+++ b/lib/network/bridge_linux_test.go
@@ -3,9 +3,12 @@
 package network
 
 import (
+	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/vishvananda/netlink"
 )
 
 func TestParseBridgeFilters(t *testing.T) {
@@ -66,3 +69,37 @@
 	assert.Nil(t, staleFilters)
 	assert.Nil(t, staleClasses)
 }
+
+func TestBridgeAddrMatchesGatewayAndMask(t *testing.T) {
+	addrs := []netlink.Addr{
+		{IPNet: &net.IPNet{IP: net.ParseIP("10.244.0.1"), Mask: net.CIDRMask(24, 32)}},
+		{IPNet: &net.IPNet{IP: net.ParseIP("10.244.0.2"), Mask: net.CIDRMask(24, 32)}},
+	}
+
+	hasGateway, hasMask, actualIPs, gatewayCIDRs := bridgeAddrMatchesGatewayAndMask(
+		addrs,
+		net.ParseIP("10.244.0.1"),
+		net.CIDRMask(24, 32),
+	)
+
+	require.True(t, hasGateway)
+	require.True(t, hasMask)
+	assert.Equal(t, []string{"10.244.0.1/24", "10.244.0.2/24"}, actualIPs)
+	assert.Equal(t, []string{"10.244.0.1/24"}, gatewayCIDRs)
+}
+
+func TestBridgeAddrMatchesGatewayAndMaskDetectsMaskMismatch(t *testing.T) {
+	addrs := []netlink.Addr{
+		{IPNet: &net.IPNet{IP: net.ParseIP("10.244.0.1"), Mask: net.CIDRMask(16, 32)}},
+	}
+
+	hasGateway, hasMask, _, gatewayCIDRs := bridgeAddrMatchesGatewayAndMask(
+		addrs,
+		net.ParseIP("10.244.0.1"),
+		net.CIDRMask(24, 32),
+	)
+
+	require.True(t, hasGateway)
+	require.False(t, hasMask)
+	assert.Equal(t, []string{"10.244.0.1/16"}, gatewayCIDRs)
+}

You can send follow-ups to the cloud agent here.

Comment thread lib/network/manager.go
@rgarcia
rgarcia force-pushed the issue-358 branch 4 times, most recently from 2504ec6 to 623176a Compare August 7, 2026 16:04

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Linux subnet loses canonical CIDR
    • Linux bridge state now canonicalizes the subnet to its network CIDR before caching EffectiveDefaultNetwork, with a regression test covering host-form gateway addresses.

Create PR

Or push these changes by commenting:

@cursor push 90c5447c30
Preview (90c5447c30)
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -264,6 +264,16 @@
         run: |
           export HYPEMAN_TEST_PREWARM_DIR="$HOME/.cache/hypeman-ci/darwin-arm64"
           make test
+      - name: Run VZ builder integration test
+        env:
+          HYPEMAN_RUN_BUILDER_INTEGRATION_TEST: "1"
+        run: |
+          # Self-hosted runners retain Docker layers across jobs. Reclaim them so
+          # the builder image and two VZ builds have predictable disk headroom.
+          docker system prune --all --force --volumes
+          PATH="/opt/homebrew/opt/e2fsprogs/sbin:$PATH" \
+            go test -count=1 -tags containers_image_openpgp \
+              -run='^TestBuilderPersistentCacheReuse$' -timeout=20m -v ./integration
       - name: Cleanup
         if: always()
         run: |
@@ -284,7 +294,25 @@
           go-version: '1.25.4'
           cache: false
       - name: Install dependencies
-        run: brew list caddy &>/dev/null || brew install caddy
+        run: |
+          brew list caddy &>/dev/null || brew install caddy
+          if ! docker info >/dev/null 2>&1; then
+            colima start || {
+              colima stop --force || true
+              colima start
+            }
+          fi
+          for attempt in {1..30}; do
+            if docker info >/dev/null 2>&1; then
+              break
+            fi
+            echo "waiting for Docker daemon (${attempt}/30)"
+            sleep 2
+          done
+          docker info >/dev/null
+          # Self-hosted runners retain Docker layers across jobs. Start the
+          # install E2E with deterministic headroom for its builder image.
+          docker system prune --all --force --volumes
       - name: Run E2E install test
         run: bash scripts/e2e-install-test.sh
       - name: Run E2E CLI-only install test

diff --git a/cmd/api/main.go b/cmd/api/main.go
--- a/cmd/api/main.go
+++ b/cmd/api/main.go
@@ -47,6 +47,19 @@
 	"golang.org/x/sync/errgroup"
 )
 
+func timeoutNonStreamingRequests(timeout time.Duration) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		timeoutHandler := middleware.Timeout(timeout)(next)
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if strings.HasSuffix(r.URL.Path, "/logs") || strings.HasSuffix(r.URL.Path, "/events") {
+				next.ServeHTTP(w, r)
+				return
+			}
+			timeoutHandler.ServeHTTP(w, r)
+		})
+	}
+}
+
 func main() {
 	if err := run(); err != nil {
 		slog.Error("application terminated", "error", err)
@@ -481,7 +494,11 @@
 			})
 		}
 
-		r.Use(middleware.Timeout(60 * time.Second))
+		// Streaming endpoints can remain active for longer than the request timeout.
+		// In particular, cold builds routinely exceed 60 seconds while continuing
+		// to emit events; cancelling the SSE request makes the CLI report failure
+		// even though the build is still running.
+		r.Use(timeoutNonStreamingRequests(60 * time.Second))
 
 		// OpenAPI request validation with authentication
 		validatorOptions := &nethttpmiddleware.Options{

diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go
--- a/cmd/api/main_test.go
+++ b/cmd/api/main_test.go
@@ -21,6 +21,33 @@
 
 const testJWTSecret = "test-secret-key"
 
+func TestRequestTimeoutSkipsStreamingEndpoints(t *testing.T) {
+	for _, path := range []string{"/instances/test/logs", "/builds/test/events"} {
+		t.Run(path, func(t *testing.T) {
+			handler := timeoutNonStreamingRequests(10 * time.Millisecond)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				select {
+				case <-r.Context().Done():
+					return
+				case <-time.After(30 * time.Millisecond):
+					w.WriteHeader(http.StatusNoContent)
+				}
+			}))
+			recorder := httptest.NewRecorder()
+			handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, path, nil))
+			assert.Equal(t, http.StatusNoContent, recorder.Code)
+		})
+	}
+}
+
+func TestRequestTimeoutStillAppliesToRegularEndpoints(t *testing.T) {
+	handler := timeoutNonStreamingRequests(10 * time.Millisecond)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		<-r.Context().Done()
+	}))
+	recorder := httptest.NewRecorder()
+	handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/health", nil))
+	assert.Equal(t, http.StatusGatewayTimeout, recorder.Code)
+}
+
 func generateValidJWT(userID string) (string, error) {
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
 		"sub": userID,

diff --git a/integration/builder_cache_darwin_test.go b/integration/builder_cache_darwin_test.go
new file mode 100644
--- /dev/null
+++ b/integration/builder_cache_darwin_test.go
@@ -1,0 +1,93 @@
+//go:build darwin
+
+package integration
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/kernel/hypeman/cmd/api/config"
+	"github.com/kernel/hypeman/lib/hypervisor"
+)
+
+func requireBuilderIntegrationHost(t *testing.T) {
+	t.Helper()
+	if os.Getenv("HYPEMAN_RUN_BUILDER_INTEGRATION_TEST") != "1" {
+		t.Skip("set HYPEMAN_RUN_BUILDER_INTEGRATION_TEST=1 to run the VZ builder integration test")
+	}
+	if runtime.GOARCH != "arm64" {
+		t.Skip("VZ builder integration test requires Apple Silicon")
+	}
+	if _, err := exec.LookPath("docker"); err != nil {
+		t.Skip("builder integration test requires Docker")
+	}
+	if err := exec.Command("docker", "info").Run(); err != nil {
+		t.Skip("builder integration test requires a running Docker daemon")
+	}
+}
+
+func builderIntegrationDataDir(t *testing.T) string {
+	t.Helper()
+	dir, err := os.MkdirTemp("/tmp", "hb-")
+	if err != nil {
+		t.Fatalf("create short builder integration data directory: %v", err)
+	}
+	t.Cleanup(func() { _ = os.RemoveAll(dir) })
+	return dir
+}
+
+func builderIntegrationPlatformConfig(t *testing.T) (config.NetworkConfig, hypervisor.Type) {
+	t.Helper()
+	// Deliberately supply the Linux-shaped defaults from issue #358. VZ must
+	// still use its platform-effective shared NAT for the builder VM and registry.
+	return config.NetworkConfig{
+		BridgeName:    "vmbr0",
+		SubnetCIDR:    "10.100.0.0/16",
+		SubnetGateway: "10.100.0.1",
+		DNSServer:     "8.8.8.8",
+	}, hypervisor.TypeVZ
+}
+
+func builderIntegrationDockerSocket(t *testing.T) string {
+	t.Helper()
+	if socket := unixDockerSocket(os.Getenv("DOCKER_HOST")); socket != "" {
+		return socket
+	}
+	if output, err := exec.Command("docker", "context", "inspect", "--format", "{{.Endpoints.docker.Host}}").Output(); err == nil {
+		if socket := unixDockerSocket(strings.TrimSpace(string(output))); socket != "" {
+			return socket
+		}
+	}
+	home, _ := os.UserHomeDir()
+	for _, candidate := range []string{
+		"/var/run/docker.sock",
+		filepath.Join(home, ".colima", "default", "docker.sock"),
+		filepath.Join(home, ".docker", "run", "docker.sock"),
+	} {
+		if _, err := os.Stat(candidate); err == nil {
+			return candidate
+		}
+	}
+	t.Fatal("builder integration test requires a local Docker Unix socket")
+	return ""
+}
+
+func unixDockerSocket(host string) string {
+	if strings.HasPrefix(host, "unix://") {
+		return strings.TrimPrefix(host, "unix://")
+	}
+	if strings.HasPrefix(host, "/") {
+		return host
+	}
+	return ""
+}
+
+func prepareBuilderIntegrationRegistryAccess(t *testing.T, bridge string) {
+	t.Helper()
+	// VZ's shared NAT can reach host listeners through its gateway without a
+	// host firewall rule managed by Hypeman.
+}

diff --git a/integration/builder_cache_linux_test.go b/integration/builder_cache_linux_test.go
--- a/integration/builder_cache_linux_test.go
+++ b/integration/builder_cache_linux_test.go
@@ -1,189 +1,44 @@
+//go:build linux
+
 package integration
 
 import (
-	"archive/tar"
-	"bytes"
-	"compress/gzip"
-	"context"
-	"crypto/rand"
-	"crypto/rsa"
-	"crypto/tls"
-	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
-	"math/big"
-	"net"
-	"net/http"
 	"os"
 	"os/exec"
-	"strconv"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/kernel/hypeman/cmd/api/config"
-	"github.com/kernel/hypeman/lib/builders"
-	"github.com/kernel/hypeman/lib/builds"
-	"github.com/kernel/hypeman/lib/devices"
-	"github.com/kernel/hypeman/lib/images"
-	"github.com/kernel/hypeman/lib/instances"
-	"github.com/kernel/hypeman/lib/network"
-	"github.com/kernel/hypeman/lib/paths"
-	"github.com/kernel/hypeman/lib/registry"
-	"github.com/kernel/hypeman/lib/system"
-	"github.com/kernel/hypeman/lib/volumes"
-	"github.com/stretchr/testify/assert"
+	"github.com/kernel/hypeman/lib/hypervisor"
 	"github.com/stretchr/testify/require"
 )
 
-func TestBuilderPersistentCacheReuse(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping integration test in short mode")
-	}
+func requireBuilderIntegrationHost(t *testing.T) {
+	t.Helper()
 	if os.Geteuid() != 0 {
-		t.Skip("builder integration test requires root")
+		t.Skip("builder integration test requires root on Linux")
 	}
 	if _, err := os.Stat("/dev/kvm"); err != nil {
 		t.Skip("builder integration test requires /dev/kvm")
 	}
+}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute)
-	defer cancel()
-	t.Chdir("..")
+func builderIntegrationDataDir(t *testing.T) string {
+	t.Helper()
+	return t.TempDir()
+}
 
-	p := paths.New(t.TempDir())
-	cfg := &config.Config{
-		DataDir: p.DataDir(),
-		Network: newParallelTestNetworkConfig(t),
-	}
-	gateway, err := network.DeriveGateway(cfg.Network.SubnetCIDR)
-	require.NoError(t, err)
-	cfg.Network.SubnetGateway = gateway
-
-	imageManager, err := images.NewManager(p, 1, nil)
-	require.NoError(t, err)
-	volumeManager := volumes.NewManager(p, 0, nil)
-	networkManager := network.NewManager(p, cfg, nil)
-	require.NoError(t, networkManager.Initialize(ctx, nil))
-	allowHostRegistryTraffic(t, cfg.Network.BridgeName)
-	systemManager := system.NewManager(p)
-	require.NoError(t, systemManager.EnsureSystemFiles(ctx))
-	instanceManager := instances.NewManager(
-		p,
-		imageManager,
-		systemManager,
-		networkManager,
-		devices.NewManager(p),
-		volumeManager,
-		instances.ResourceLimits{MaxOverlaySize: 100 << 30},
-		"",
-		instances.SnapshotPolicy{},
-		nil,
-		nil,
-	)
-	t.Cleanup(func() {
-		all, listErr := instanceManager.ListInstances(context.Background(), nil)
-		if listErr != nil {
-			t.Logf("list instances during cleanup: %v", listErr)
-			return
-		}
-		for _, instance := range all {
-			if deleteErr := instanceManager.DeleteInstance(context.Background(), instance.Id); deleteErr != nil {
-				t.Logf("delete instance %s during cleanup: %v", instance.Id, deleteErr)
-			}
-		}
-	})
-
-	registryURL, registryCA := startBuildRegistry(t, gateway, p, imageManager)
-
-	builderManager, err := builders.NewManager(
-		p,
-		builders.Config{DefaultDiskSizeGb: 4},
-		volumeManager,
-		instanceManager,
-		nil,
-		nil,
-	)
-	require.NoError(t, err)
-	require.NoError(t, builderManager.Start(ctx))
-
-	buildManager, err := builds.NewManager(
-		p,
-		builds.Config{
-			MaxConcurrentBuilds: 1,
-			RegistryURL:         registryURL,
-			RegistryCACert:      registryCA,
-			RegistrySecret:      "builder-cache-integration-test",
-			DefaultTimeout:      600,
-		},
-		instanceManager,
-		volumeManager,
-		builderManager,
-		imageManager,
-		nil,
-		nil,
-		nil,
-	)
-	require.NoError(t, err)
-	builderManager.SetBuildActivityChecker(buildManager.BuilderHasBuilds)
-	require.NoError(t, buildManager.Start(ctx))
-	require.EventuallyWithT(t, func(collect *assert.CollectT) {
-		all, listErr := imageManager.ListImages(ctx)
-		require.NoError(collect, listErr)
-		ready := false
-		for _, image := range all {
-			if strings.Contains(image.Name, "/internal/builder") {
-				ready = image.Status == images.StatusReady
-			}
-		}
-		require.True(collect, ready)
-	}, 5*time.Minute, time.Second)
-	require.Eventually(t, buildManager.ReadyForBuilds, time.Second, 10*time.Millisecond)
-
-	builder, err := builderManager.CreateBuilder(ctx, builders.CreateBuilderRequest{DiskSizeGb: 4})
-	require.NoError(t, err)
-
-	dockerfile := `FROM alpine:3.18
-ARG CACHE_BUSTER
-RUN --mount=type=cache,target=/cache sh -c 'if [ -f /cache/sentinel ]; then echo BUILDER_CACHE_HIT; else echo BUILDER_CACHE_MISS; touch /cache/sentinel; fi; echo "$CACHE_BUSTER" > /cache-buster'
-`
-	source := sourceArchive(t, dockerfile)
-	first := runBuilderBuild(t, ctx, buildManager, builder.ID, dockerfile, source, "first")
-	firstLogs, err := buildManager.GetBuildLogs(ctx, first.ID)
-	require.NoError(t, err)
-	require.Contains(t, string(firstLogs), "BUILDER_CACHE_MISS")
-
-	second := runBuilderBuild(t, ctx, buildManager, builder.ID, dockerfile, source, "second")
-	secondLogs, err := buildManager.GetBuildLogs(ctx, second.ID)
-	require.NoError(t, err)
-	require.Contains(t, string(secondLogs), "BUILDER_CACHE_HIT")
-	require.NotNil(t, first.BuilderInstanceID)
-	require.NotNil(t, second.BuilderInstanceID)
-	require.NotEqual(t, *first.BuilderInstanceID, *second.BuilderInstanceID)
+func builderIntegrationPlatformConfig(t *testing.T) (config.NetworkConfig, hypervisor.Type) {
+	t.Helper()
+	return newParallelTestNetworkConfig(t), hypervisor.TypeCloudHypervisor
 }
 
-func startBuildRegistry(t *testing.T, gateway string, p *paths.Paths, imageManager images.Manager) (string, string) {
+func builderIntegrationDockerSocket(t *testing.T) string {
 	t.Helper()
-	reg, err := registry.New(p, imageManager)
-	require.NoError(t, err)
-	certPEM, keyPEM := registryCertificate(t, net.ParseIP(gateway))
-	certificate, err := tls.X509KeyPair(certPEM, keyPEM)
-	require.NoError(t, err)
-	listener, err := net.Listen("tcp", "0.0.0.0:0")
-	require.NoError(t, err)
-	tlsListener := tls.NewListener(listener, &tls.Config{Certificates: []tls.Certificate{certificate}, MinVersion: tls.VersionTLS12})
-	server := &http.Server{Handler: reg.Handler()}
-	go func() {
-		if serveErr := server.Serve(tlsListener); serveErr != nil && serveErr != http.ErrServerClosed {
-			t.Logf("registry server: %v", serveErr)
-		}
-	}()
-	t.Cleanup(func() { _ = server.Close() })
-	port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port)
-	return net.JoinHostPort(gateway, port), string(certPEM)
+	return "/var/run/docker.sock"
 }
 
-func allowHostRegistryTraffic(t *testing.T, bridge string) {
+func prepareBuilderIntegrationRegistryAccess(t *testing.T, bridge string) {
 	t.Helper()
 	if exec.Command("nft", "list", "table", "inet", "kernel_firewall").Run() != nil {
 		return
@@ -206,66 +61,3 @@
 		}
 	})
 }
-
-func registryCertificate(t *testing.T, ip net.IP) ([]byte, []byte) {
-	t.Helper()
-	key, err := rsa.GenerateKey(rand.Reader, 2048)
-	require.NoError(t, err)
-	now := time.Now()
-	template := &x509.Certificate{
-		SerialNumber: big.NewInt(1),
-		Subject:      pkix.Name{CommonName: ip.String()},
-		NotBefore:    now.Add(-time.Minute),
-		NotAfter:     now.Add(time.Hour),
-		KeyUsage:     x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
-		ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		IPAddresses:  []net.IP{ip},
-	}
-	der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
-	require.NoError(t, err)
-	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
-	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)})
-	return certPEM, keyPEM
-}
-
-func runBuilderBuild(t *testing.T, ctx context.Context, manager builds.Manager, builderID, dockerfile string, source []byte, cacheBuster string) *builds.Build {
-	t.Helper()
-	build, err := manager.CreateBuild(ctx, builds.CreateBuildRequest{
-		Dockerfile: dockerfile,
-		BuilderID:  builderID,
-		BuildArgs:  map[string]string{"CACHE_BUSTER": cacheBuster},
-	}, source)
-	require.NoError(t, err)
-
-	require.EventuallyWithT(t, func(collect *assert.CollectT) {
-		current, getErr := manager.GetBuild(ctx, build.ID)
-		require.NoError(collect, getErr)
-		require.Contains(collect, []string{builds.StatusReady, builds.StatusFailed}, current.Status)
-	}, 10*time.Minute, time.Second)
-
-	result, err := manager.GetBuild(ctx, build.ID)
-	require.NoError(t, err)
-	if result.Status != builds.StatusReady {
-		logs, _ := manager.GetBuildLogs(ctx, build.ID)
-		t.Fatalf("build %s failed: %v\n%s", build.ID, result.Error, logs)
-	}
-	return result
-}
-
-func sourceArchive(t *testing.T, dockerfile string) []byte {
-	t.Helper()
-	var out bytes.Buffer
-	gz := gzip.NewWriter(&out)
-	tw := tar.NewWriter(gz)
-	contents := []byte(dockerfile)
-	require.NoError(t, tw.WriteHeader(&tar.Header{
-		Name: "Dockerfile",
-		Mode: 0644,
-		Size: int64(len(contents)),
-	}))
-	_, err := tw.Write(contents)
-	require.NoError(t, err)
-	require.NoError(t, tw.Close())
-	require.NoError(t, gz.Close())
-	return out.Bytes()
-}

diff --git a/integration/builder_cache_test.go b/integration/builder_cache_test.go
new file mode 100644
--- /dev/null
+++ b/integration/builder_cache_test.go
@@ -1,0 +1,282 @@
+//go:build linux || darwin
+
+package integration
+
+import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
+	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"net"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/kernel/hypeman/cmd/api/config"
+	"github.com/kernel/hypeman/lib/builders"
+	"github.com/kernel/hypeman/lib/builds"
+	"github.com/kernel/hypeman/lib/devices"
+	"github.com/kernel/hypeman/lib/images"
+	"github.com/kernel/hypeman/lib/instances"
+	"github.com/kernel/hypeman/lib/network"
+	"github.com/kernel/hypeman/lib/paths"
+	"github.com/kernel/hypeman/lib/registry"
+	"github.com/kernel/hypeman/lib/system"
+	"github.com/kernel/hypeman/lib/volumes"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuilderPersistentCacheReuse(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test in short mode")
+	}
+	requireBuilderIntegrationHost(t)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute)
+	defer cancel()
+	repoRoot, err := filepath.Abs("..")
+	require.NoError(t, err)
+	t.Chdir(builderIntegrationDockerContext(t, repoRoot))
+
+	p := paths.New(builderIntegrationDataDir(t))
+	networkConfig, defaultHypervisor := builderIntegrationPlatformConfig(t)
+	cfg := &config.Config{
+		DataDir: p.DataDir(),
+		Network: networkConfig,
+	}
+
+	imageManager, err := images.NewManager(p, 1, nil)
+	require.NoError(t, err)
+	volumeManager := volumes.NewManager(p, 0, nil)
+	networkManager := network.NewManager(p, cfg, nil)
+	require.NoError(t, networkManager.Initialize(ctx, nil))
+	effectiveNetwork, err := networkManager.EffectiveDefaultNetwork()
+	require.NoError(t, err)
+	prepareBuilderIntegrationRegistryAccess(t, effectiveNetwork.Bridge)
+	systemManager := system.NewManager(p)
+	require.NoError(t, systemManager.EnsureSystemFiles(ctx))
+	instanceManager := instances.NewManager(
+		p,
+		imageManager,
+		systemManager,
+		networkManager,
+		devices.NewManager(p),
+		volumeManager,
+		instances.ResourceLimits{MaxOverlaySize: 100 << 30},
+		defaultHypervisor,
+		instances.SnapshotPolicy{},
+		nil,
+		nil,
+	)
+	t.Cleanup(func() {
+		all, listErr := instanceManager.ListInstances(context.Background(), nil)
+		if listErr != nil {
+			t.Logf("list instances during cleanup: %v", listErr)
+			return
+		}
+		for _, instance := range all {
+			if deleteErr := instanceManager.DeleteInstance(context.Background(), instance.Id); deleteErr != nil {
+				t.Logf("delete instance %s during cleanup: %v", instance.Id, deleteErr)
+			}
+		}
+	})
+
+	registryURL, registryCA := startBuildRegistry(t, effectiveNetwork.Gateway, p, imageManager)
+
+	builderManager, err := builders.NewManager(
+		p,
+		builders.Config{DefaultDiskSizeGb: 4},
+		volumeManager,
+		instanceManager,
+		nil,
+		nil,
+	)
+	require.NoError(t, err)
+	require.NoError(t, builderManager.Start(ctx))
+
+	buildManager, err := builds.NewManager(
+		p,
+		builds.Config{
+			MaxConcurrentBuilds: 1,
+			DockerSocket:        builderIntegrationDockerSocket(t),
+			RegistryURL:         registryURL,
+			RegistryCACert:      registryCA,
+			RegistrySecret:      "builder-cache-integration-test",
+			DefaultTimeout:      600,
+		},
+		instanceManager,
+		volumeManager,
+		builderManager,
+		imageManager,
+		nil,
+		nil,
+		nil,
+	)
+	require.NoError(t, err)
+	builderManager.SetBuildActivityChecker(buildManager.BuilderHasBuilds)
+	require.NoError(t, buildManager.Start(ctx))
+	require.EventuallyWithT(t, func(collect *assert.CollectT) {
+		all, listErr := imageManager.ListImages(ctx)
+		require.NoError(collect, listErr)
+		ready := false
+		for _, image := range all {
+			if strings.Contains(image.Name, "/internal/builder") {
+				ready = image.Status == images.StatusReady
+			}
+		}
+		require.True(collect, ready)
+	}, 5*time.Minute, time.Second)
+	require.Eventually(t, buildManager.ReadyForBuilds, 30*time.Second, 100*time.Millisecond)
+
+	builder, err := builderManager.CreateBuilder(ctx, builders.CreateBuilderRequest{DiskSizeGb: 4})
+	require.NoError(t, err)
+
+	dockerfile := `FROM alpine:3.18
+ARG CACHE_BUSTER
+RUN --mount=type=cache,target=/cache sh -c 'if [ -f /cache/sentinel ]; then echo BUILDER_CACHE_HIT; else echo BUILDER_CACHE_MISS; touch /cache/sentinel; fi; echo "$CACHE_BUSTER" > /cache-buster'
+`
+	source := sourceArchive(t, dockerfile)
+	first := runBuilderBuild(t, ctx, buildManager, builder.ID, dockerfile, source, "first")
+	firstLogs, err := buildManager.GetBuildLogs(ctx, first.ID)
+	require.NoError(t, err)
+	require.Contains(t, string(firstLogs), "BUILDER_CACHE_MISS")
+
+	second := runBuilderBuild(t, ctx, buildManager, builder.ID, dockerfile, source, "second")
+	secondLogs, err := buildManager.GetBuildLogs(ctx, second.ID)
+	require.NoError(t, err)
+	require.Contains(t, string(secondLogs), "BUILDER_CACHE_HIT")
+	require.NotNil(t, first.BuilderInstanceID)
+	require.NotNil(t, second.BuilderInstanceID)
+	require.NotEqual(t, *first.BuilderInstanceID, *second.BuilderInstanceID)
+}
+
+func builderIntegrationDockerContext(t *testing.T, repoRoot string) string {
+	t.Helper()
+	contextDir := t.TempDir()
+	for _, path := range []string{"go.mod", "go.sum", "lib/guest/guest.pb.go", "lib/guest/guest_grpc.pb.go"} {
+		copyBuilderIntegrationFile(t, repoRoot, contextDir, path)
+	}
+	for _, dir := range []string{"lib/builds/builder_agent", "lib/system/guest_agent"} {
+		err := filepath.WalkDir(filepath.Join(repoRoot, dir), func(path string, entry os.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
+			if entry.IsDir() || filepath.Ext(path) != ".go" || strings.HasSuffix(path, "_test.go") {
+				return nil
+			}
+			rel, err := filepath.Rel(repoRoot, path)
+			if err != nil {
+				return err
+			}
+			copyBuilderIntegrationFile(t, repoRoot, contextDir, rel)
+			return nil
+		})
+		require.NoError(t, err)
+	}
+	return contextDir
+}
+
+func copyBuilderIntegrationFile(t *testing.T, sourceRoot, destinationRoot, path string) {
+	t.Helper()
+	contents, err := os.ReadFile(filepath.Join(sourceRoot, path))
+	require.NoError(t, err)
+	destination := filepath.Join(destinationRoot, path)
+	require.NoError(t, os.MkdirAll(filepath.Dir(destination), 0o755))
+	require.NoError(t, os.WriteFile(destination, contents, 0o644))
+}
+
+func startBuildRegistry(t *testing.T, gateway string, p *paths.Paths, imageManager images.Manager) (string, string) {
+	t.Helper()
+	reg, err := registry.New(p, imageManager)
+	require.NoError(t, err)
+	certPEM, keyPEM := registryCertificate(t, net.ParseIP(gateway))
+	certificate, err := tls.X509KeyPair(certPEM, keyPEM)
+	require.NoError(t, err)
+	listener, err := net.Listen("tcp", "0.0.0.0:0")
+	require.NoError(t, err)
+	tlsListener := tls.NewListener(listener, &tls.Config{Certificates: []tls.Certificate{certificate}, MinVersion: tls.VersionTLS12})
+	server := &http.Server{Handler: reg.Handler()}
+	go func() {
+		if serveErr := server.Serve(tlsListener); serveErr != nil && serveErr != http.ErrServerClosed {
+			t.Logf("registry server: %v", serveErr)
+		}
+	}()
+	t.Cleanup(func() { _ = server.Close() })
+	port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port)
+	return net.JoinHostPort(gateway, port), string(certPEM)
+}
+
+func registryCertificate(t *testing.T, ip net.IP) ([]byte, []byte) {
+	t.Helper()
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NoError(t, err)
+	now := time.Now()
+	template := &x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject:      pkix.Name{CommonName: ip.String()},
+		NotBefore:    now.Add(-time.Minute),
+		NotAfter:     now.Add(time.Hour),
+		KeyUsage:     x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
+		ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		IPAddresses:  []net.IP{ip},
+	}
+	der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
+	require.NoError(t, err)
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)})
+	return certPEM, keyPEM
+}
+
+func runBuilderBuild(t *testing.T, ctx context.Context, manager builds.Manager, builderID, dockerfile string, source []byte, cacheBuster string) *builds.Build {
+	t.Helper()
+	build, err := manager.CreateBuild(ctx, builds.CreateBuildRequest{
+		Dockerfile: dockerfile,
+		BuilderID:  builderID,
+		BuildArgs:  map[string]string{"CACHE_BUSTER": cacheBuster},
+	}, source)
+	require.NoError(t, err)
+
+	require.EventuallyWithT(t, func(collect *assert.CollectT) {
+		current, getErr := manager.GetBuild(ctx, build.ID)
+		require.NoError(collect, getErr)
+		require.Contains(collect, []string{builds.StatusReady, builds.StatusFailed}, current.Status)
+	}, 10*time.Minute, time.Second)
+
+	result, err := manager.GetBuild(ctx, build.ID)
+	require.NoError(t, err)
+	if result.Status != builds.StatusReady {
+		logs, _ := manager.GetBuildLogs(ctx, build.ID)
+		t.Fatalf("build %s failed: %v\n%s", build.ID, result.Error, logs)
+	}
+	return result
+}
+
+func sourceArchive(t *testing.T, dockerfile string) []byte {
+	t.Helper()
+	var out bytes.Buffer
+	gz := gzip.NewWriter(&out)
+	tw := tar.NewWriter(gz)
+	contents := []byte(dockerfile)
+	require.NoError(t, tw.WriteHeader(&tar.Header{
+		Name: "Dockerfile",
+		Mode: 0644,
+		Size: int64(len(contents)),
+	}))
+	_, err := tw.Write(contents)
+	require.NoError(t, err)
+	require.NoError(t, tw.Close())
+	require.NoError(t, gz.Close())
+	return out.Bytes()
+}

diff --git a/lib/network/bridge_linux.go b/lib/network/bridge_linux.go
--- a/lib/network/bridge_linux.go
+++ b/lib/network/bridge_linux.go
@@ -1094,13 +1094,25 @@
 
 	// Bridge existence plus an IPv4 address is sufficient. OperState may be
 	// OperUp or OperUnknown; both are functional for this bridge.
+	subnet := canonicalSubnetCIDR(gatewayAddr.IPNet)
 	return &Network{
 		Bridge:  bridgeName,
 		Gateway: gatewayAddr.IP.String(),
-		Subnet:  gatewayAddr.IPNet.String(),
... diff truncated: showing 800 of 912 lines

You can send follow-ups to the cloud agent here.

Comment thread lib/network/bridge_linux.go Outdated
@rgarcia
rgarcia force-pushed the issue-358 branch 2 times, most recently from 5cd33c8 to eab5670 Compare August 7, 2026 16:28
Comment thread lib/builds/manager.go Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Builder readiness fails permanently
    • Start now retries builder-image preparation until success or context cancellation, so readiness can recover after transient failures and pending builds are eventually recovered.

Create PR

Or push these changes by commenting:

@cursor push e58c08c685
Preview (e58c08c685)
diff --git a/lib/builds/manager.go b/lib/builds/manager.go
--- a/lib/builds/manager.go
+++ b/lib/builds/manager.go
@@ -34,6 +34,10 @@
 	// of a build; releaseBuildRetryDelay spaces the attempts.
 	releaseBuildMaxAttempts = 5
 	releaseBuildRetryDelay  = time.Second
+
+	// builderImageRetryDelay controls how often startup retries builder image
+	// preparation after a transient failure.
+	builderImageRetryDelay = time.Second
 )
 
 //go:embed images/generic/Dockerfile
@@ -205,11 +209,19 @@
 // Start starts the build manager's background services
 func (m *manager) Start(ctx context.Context) error {
 	go func() {
-		m.ensureBuilderImage(ctx)
-		// Recover pending builds only after the builder image is ready,
-		// otherwise recovered builds fail with "builder image is being prepared".
-		if m.ReadyForBuilds() {
-			m.RecoverPendingBuilds()
+		for {
+			m.ensureBuilderImage(ctx)
+			// Recover pending builds only after the builder image is ready,
+			// otherwise recovered builds fail with "builder image is being prepared".
+			if m.ReadyForBuilds() {
+				m.RecoverPendingBuilds()
+				return
+			}
+			select {
+			case <-ctx.Done():
+				return
+			case <-time.After(builderImageRetryDelay):
+			}
 		}
 	}()
 	m.logger.Info("build manager started")

diff --git a/lib/builds/manager_test.go b/lib/builds/manager_test.go
--- a/lib/builds/manager_test.go
+++ b/lib/builds/manager_test.go
@@ -309,9 +309,10 @@
 
 // mockImageManager implements images.Manager for testing
 type mockImageManager struct {
-	mu          sync.RWMutex
-	images      map[string]*images.Image
-	getImageErr error
+	mu              sync.RWMutex
+	images          map[string]*images.Image
+	createImageFunc func(ctx context.Context, req images.CreateImageRequest) (*images.Image, error)
+	getImageErr     error
 }
 
 func newMockImageManager() *mockImageManager {
@@ -329,6 +330,9 @@
 }
 
 func (m *mockImageManager) CreateImage(ctx context.Context, req images.CreateImageRequest) (*images.Image, error) {
+	if m.createImageFunc != nil {
+		return m.createImageFunc(ctx, req)
+	}
 	img := &images.Image{
 		Name:   req.Name,
 		Status: images.StatusPending,
@@ -898,6 +902,34 @@
 	assert.NoError(t, err)
 }
 
+func TestStart_RetriesBuilderPreparationAfterFailure(t *testing.T) {
+	mgr, _, _, imageMgr, tempDir := setupTestManagerWithImageMgr(t)
+	defer os.RemoveAll(tempDir)
+	mgr.builderReady.Store(false)
+
+	attempts := 0
+	imageMgr.createImageFunc = func(ctx context.Context, req images.CreateImageRequest) (*images.Image, error) {
+		attempts++
+		if attempts == 1 {
+			return nil, fmt.Errorf("transient pull failure")
+		}
+		imageMgr.mu.Lock()
+		defer imageMgr.mu.Unlock()
+		img := &images.Image{
+			Name:   req.Name,
+			Status: images.StatusReady,
+		}
+		imageMgr.images[req.Name] = img
+		return img, nil
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	require.NoError(t, mgr.Start(ctx))
+	require.Eventually(t, mgr.ReadyForBuilds, 5*time.Second, 50*time.Millisecond)
+}
+
 func TestCreateBuild_MultipleConcurrent(t *testing.T) {
 	mgr, _, _, tempDir := setupTestManager(t)
 	defer os.RemoveAll(tempDir)

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 7a60b21. Configure here.

Comment thread lib/builds/manager.go

@sjmiller609 sjmiller609 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/builds/manager.go:246-255 — failed explicit builder images do not actually retry. Every iteration sees the existing StatusFailed image and exits before calling CreateImage, which contains the cleanup and requeue logic.

@rgarcia

rgarcia commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Steven’s review in 76e0ae9: StatusFailed explicit builder images now fall through to CreateImage, which removes failed metadata and requeues the pull/conversion. Added regression coverage proving a failed image invokes CreateImage and only then marks builder readiness.

@rgarcia
rgarcia merged commit be07127 into main Aug 7, 2026
11 of 12 checks passed
@rgarcia
rgarcia deleted the issue-358 branch August 7, 2026 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS: guests are assigned a 10.100.0.0/16 address that vz never NATs, so they have zero egress

2 participants