builds: recover crashed builds by cleaning up leftover source/config volumes

[rgarcia]

Problem

If the hypeman process dies mid-build (host crash, kill -9, OOM), RecoverPendingBuilds re-executes the interrupted build on restart — and the re-run always fails:

create source volume: volume already exists

executeBuild creates deterministically-named volumes (build-source-<id>, build-config-<id>) that survive the crash, typically still attached to the crashed attempt's surviving builder-<id> instance record. So crash recovery is guaranteed-broken for any in-flight build, and each occurrence strands two volumes plus a stale instance. Reproduced in live QA on macOS (kill -9 during a build → restart → recovered build fails immediately with the error above).

Fix

executeBuild now tolerates leftovers from a crashed prior attempt of the same build via a shared helper (deleteLeftoverBuildVolume):

• ErrAlreadyExists on source/config volume creation → delete the leftover and retry the create once
• Delete returns ErrInUse → delete the stale instance named exactly builder-<buildID> (detaching its volumes), then delete + retry; if no such instance exists, fail with a clear error — never force-delete a volume attached to an unknown holder
• The config volume's copy-over fallback no longer masks ErrAlreadyExists; explicit handling runs first
• Any order/combination of attached/detached leftovers is handled (deleting the stale builder during source handling leaves config detached)

Safety: only volumes/instances named for this exact build ID are ever touched; the new attempt's builder is created only after both volume setups complete, so stale-builder deletion cannot race the current attempt. CreateVolumeFromArchive returns ErrAlreadyExists before consuming the archive reader, so the retry is safe.

Tests

New tests in lib/builds/manager_test.go (mock managers): source ErrAlreadyExists → recreate; ErrInUse + stale builder → builder deleted, volume recreated; ErrInUse + unknown holder → build fails, nothing force-deleted; config ErrAlreadyExists → explicit recreate with data copy. go test ./lib/builds/..., go vet, gofmt clean.

Note: PR #332's branch contains a related stale-builder helper for its disk-root volume; a merge conflict between the two is expected and will be resolved in whichever lands second.

---

Note

Medium Risk
Changes instance and volume lifecycle during build execution; scope is limited to build-ID-named resources with conservative refusal when a volume is in use by an unknown holder.

Overview
Fixes build recovery after hypeman dies mid-build, where RecoverPendingBuilds re-runs the same job but executeBuild failed with volume already exists on deterministic build-source-<id> / build-config-<id> volumes.

Source and config volume setup are moved into createBuildSourceVolume and registerBuildConfigVolume. On ErrAlreadyExists, the code deletes the leftover and retries create once. deleteLeftoverBuildVolume handles ErrInUse by deleting only the matching stale builder-<buildID> instance (to detach volumes), then deleting the volume; if that builder is missing, it fails with an explicit “refusing to force-delete” error instead of touching an unknown holder. Config volume handling no longer falls through to copy-over when the volume already exists.

Tests cover already-exists recreate, in-use + stale builder, in-use without stale builder, and config volume recreate with data copy.

^{Reviewed by Cursor Bugbot for commit 523d87a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

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

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

• ✅ Fixed: Config fallback masks recreate failure
  • registerBuildConfigVolume now returns the recreate error after deleting a leftover config volume instead of falling back to copyFile, and a regression test covers this path.

Or push these changes by commenting:

@cursor push 5ca2d55675

Preview (5ca2d55675)

diff --git a/lib/builds/manager.go b/lib/builds/manager.go
--- a/lib/builds/manager.go
+++ b/lib/builds/manager.go
@@ -784,14 +784,19 @@
 		SizeGb: 1,
 	}
 	_, err := m.volumeManager.CreateVolume(ctx, req)
+	recreateAttempted := false
 	if errors.Is(err, volumes.ErrAlreadyExists) {
 		m.logger.Info("removing leftover config volume from crashed build attempt", "build_id", buildID, "volume", volID)
 		if delErr := m.deleteLeftoverBuildVolume(ctx, buildID, volID); delErr != nil {
 			return fmt.Errorf("remove leftover config volume: %w", delErr)
 		}
+		recreateAttempted = true
 		_, err = m.volumeManager.CreateVolume(ctx, req)
 	}
 	if err != nil {
+		if recreateAttempted {
+			return fmt.Errorf("create config volume: %w", err)
+		}
 		// If volume creation fails, try to use the disk file directly
 		// by copying it to the expected location
 		volPath := m.paths.VolumeData(volID)

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
@@ -1248,6 +1248,43 @@
 	assert.Equal(t, configData, copied)
 }
 
+// TestRegisterBuildConfigVolume_RecreateFailure verifies that when recreating a
+// deleted leftover config volume fails, the recreate error is surfaced rather
+// than silently masked by the copy-over fallback.
+func TestRegisterBuildConfigVolume_RecreateFailure(t *testing.T) {
+	mgr, _, volumeMgr, tempDir := setupTestManager(t)
+	defer os.RemoveAll(tempDir)
+
+	buildID := "build-crash-config-fail"
+	configVolID := "build-config-" + buildID
+
+	configDiskPath := filepath.Join(tempDir, "config.ext4")
+	require.NoError(t, os.WriteFile(configDiskPath, []byte("fake-ext4-config-disk"), 0644))
+
+	var createCalls int
+	recreateErr := fmt.Errorf("recreate failed")
+	volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) {
+		createCalls++
+		if createCalls == 1 {
+			return nil, volumes.ErrAlreadyExists
+		}
+		return nil, recreateErr
+	}
+	volumeMgr.deleteFunc = func(ctx context.Context, id string) error {
+		delete(volumeMgr.volumes, id)
+		return nil
+	}
+
+	err := mgr.registerBuildConfigVolume(context.Background(), buildID, configVolID, configDiskPath)
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, recreateErr)
+	assert.Contains(t, err.Error(), "create config volume")
+	assert.Equal(t, 2, createCalls, "expected delete + retry of config volume creation")
+	_, statErr := os.Stat(mgr.paths.VolumeData(configVolID))
+	assert.ErrorIs(t, statErr, os.ErrNotExist, "config volume data should not be copied when recreate fails")
+}
+
 func TestExtractInternalBaseImageRepos(t *testing.T) {
 	registryURL := "http://10.102.0.1:8085"

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

^{Reviewed by Cursor Bugbot for commit 523d87a. Configure here.}

[cursor]

Config fallback masks recreate failure

Medium Severity

After ErrAlreadyExists, registerBuildConfigVolume deletes the leftover and retries CreateVolume. If that retry fails, the legacy copy-over path still runs and returns success. Because the leftover was already removed, copyFile only writes data.raw and does not restore volume metadata, so later builder attach fails with a confusing volume-not-found error.

 

^{Reviewed by Cursor Bugbot for commit 523d87a. Configure here.}