Skip to content

Commit 5ca2d55

Browse files
committed
fix config volume recreate fallback masking
1 parent 523d87a commit 5ca2d55

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

lib/builds/manager.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,14 +784,19 @@ func (m *manager) registerBuildConfigVolume(ctx context.Context, buildID, volID,
784784
SizeGb: 1,
785785
}
786786
_, err := m.volumeManager.CreateVolume(ctx, req)
787+
recreateAttempted := false
787788
if errors.Is(err, volumes.ErrAlreadyExists) {
788789
m.logger.Info("removing leftover config volume from crashed build attempt", "build_id", buildID, "volume", volID)
789790
if delErr := m.deleteLeftoverBuildVolume(ctx, buildID, volID); delErr != nil {
790791
return fmt.Errorf("remove leftover config volume: %w", delErr)
791792
}
793+
recreateAttempted = true
792794
_, err = m.volumeManager.CreateVolume(ctx, req)
793795
}
794796
if err != nil {
797+
if recreateAttempted {
798+
return fmt.Errorf("create config volume: %w", err)
799+
}
795800
// If volume creation fails, try to use the disk file directly
796801
// by copying it to the expected location
797802
volPath := m.paths.VolumeData(volID)

lib/builds/manager_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,43 @@ func TestRegisterBuildConfigVolume_AlreadyExists(t *testing.T) {
12481248
assert.Equal(t, configData, copied)
12491249
}
12501250

1251+
// TestRegisterBuildConfigVolume_RecreateFailure verifies that when recreating a
1252+
// deleted leftover config volume fails, the recreate error is surfaced rather
1253+
// than silently masked by the copy-over fallback.
1254+
func TestRegisterBuildConfigVolume_RecreateFailure(t *testing.T) {
1255+
mgr, _, volumeMgr, tempDir := setupTestManager(t)
1256+
defer os.RemoveAll(tempDir)
1257+
1258+
buildID := "build-crash-config-fail"
1259+
configVolID := "build-config-" + buildID
1260+
1261+
configDiskPath := filepath.Join(tempDir, "config.ext4")
1262+
require.NoError(t, os.WriteFile(configDiskPath, []byte("fake-ext4-config-disk"), 0644))
1263+
1264+
var createCalls int
1265+
recreateErr := fmt.Errorf("recreate failed")
1266+
volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) {
1267+
createCalls++
1268+
if createCalls == 1 {
1269+
return nil, volumes.ErrAlreadyExists
1270+
}
1271+
return nil, recreateErr
1272+
}
1273+
volumeMgr.deleteFunc = func(ctx context.Context, id string) error {
1274+
delete(volumeMgr.volumes, id)
1275+
return nil
1276+
}
1277+
1278+
err := mgr.registerBuildConfigVolume(context.Background(), buildID, configVolID, configDiskPath)
1279+
1280+
require.Error(t, err)
1281+
assert.ErrorIs(t, err, recreateErr)
1282+
assert.Contains(t, err.Error(), "create config volume")
1283+
assert.Equal(t, 2, createCalls, "expected delete + retry of config volume creation")
1284+
_, statErr := os.Stat(mgr.paths.VolumeData(configVolID))
1285+
assert.ErrorIs(t, statErr, os.ErrNotExist, "config volume data should not be copied when recreate fails")
1286+
}
1287+
12511288
func TestExtractInternalBaseImageRepos(t *testing.T) {
12521289
registryURL := "http://10.102.0.1:8085"
12531290

0 commit comments

Comments
 (0)