Skip to content

Fix macOS VZ default network egress - #368

Open
rgarcia wants to merge 2 commits into
mainfrom
issue-358
Open

Fix macOS VZ default network egress#368
rgarcia wants to merge 2 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
Touches core networking, build startup, and registry URL resolution on both platforms; macOS static NAT allocation can still collide with other vmnet DHCP clients, and existing VMs with wrong allocations are not migrated.

Overview
Fixes macOS VZ guest egress by treating the platform backend as the source of truth for the default network: VZ guests always use the shared NAT 192.168.64.0/24 (gateway 192.168.64.1) even when config still has Linux-style bridge/subnet values. EffectiveDefaultNetwork() drives allocations, metadata derivation, and loopback → guest gateway registry URL rewriting for builder VMs.

Linux bridge behavior is tightened (gateway/prefix validation, preferred gateway when multiple bridge IPs exist) without changing configured bridge/subnet semantics.

Build/API reliability: the global 60s HTTP timeout no longer applies to /logs and /events SSE streams; builder image prep only marks builds ready after success and retries on failure; installed macOS services can reuse hypeman/builder:latest when there is no source checkout.

Testing/CI: shared Linux/Darwin builder cache integration test; optional VZ builder integration in macOS CI; install E2E adds guest TCP egress and a full hypeman build path; Darwin make test no longer excludes lib/network.

Reviewed by Cursor Bugbot for commit 0571a84. 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

@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
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

1 participant