Unify vGPU retention wrapping and dedupe test fixtures

lib/instances/create.go

@@ -280,7 +280,8 @@ func (m *manager) createInstance(
 	var gpuAssignedAt *time.Time
 	retention := vgpuRetention{instanceID: id}
 
-	defer retention.deferWrapPending(&retErr)
+	// Deferred before cu.Clean so rollback records retention before this wraps the error.
+	defer func() { retErr = retention.wrapPending(retErr) }()
 	cu := cleanup.Make(func() {
 		log.DebugContext(ctx, "cleaning up instance on error", "instance_id", id)
 		m.persistVGPURetention(ctx, &retention)

lib/instances/start.go

@@ -121,7 +121,8 @@ func (m *manager) startInstance(
 
 	// Setup cleanup stack for automatic rollback on errors
 	retention := vgpuRetention{instanceID: id}
-	defer retention.deferWrapPending(&retErr)
+	// Deferred before cu.Clean so rollback records retention before this wraps the error.
+	defer func() { retErr = retention.wrapPending(retErr) }()
 	cu := cleanup.Make(func() {})
 	defer cu.Clean()
 
@@ -177,20 +178,20 @@ func (m *manager) startInstance(
 		device, err := m.createVGPUDevice(ctx, stored.GPUProfile, id)
 		if err != nil {
 			log.ErrorContext(ctx, "failed to create vGPU", "instance_id", id, "profile", stored.GPUProfile, "error", err)
+			wrapped := fmt.Errorf("create vGPU for profile %s: %w", stored.GPUProfile, err)
 			if pendingDevice, ok := vgpuDevicePendingCleanup(err); ok {
-				assignedAt := m.nowUTC()
 				retentionMeta := rollbackMeta
-				setStoredVGPUDevice(&retentionMeta.StoredMetadata, pendingDevice, assignedAt)
-				wrapped := fmt.Errorf("create vGPU for profile %s: %w", stored.GPUProfile, err)
+				setStoredVGPUDevice(&retentionMeta.StoredMetadata, pendingDevice, m.nowUTC())
+				persisted := true
 				if saveErr := m.saveMetadata(&retentionMeta); saveErr != nil {
-					m.recordVGPURetainedAssignment(ctx, vgpuRetentionOperationStart, false)
 					log.ErrorContext(ctx, "failed to retain vGPU assignment after create rollback failure", "instance_id", id, "error", saveErr)
-					return nil, &VGPUCleanupPendingError{InstanceID: id, Err: fmt.Errorf("%w; retain assignment: %v", wrapped, saveErr)}
+					wrapped = fmt.Errorf("%w; retain assignment: %v", wrapped, saveErr)
+					persisted = false
 				}
-				m.recordVGPURetainedAssignment(ctx, vgpuRetentionOperationStart, true)
-				return nil, &VGPUCleanupPendingError{InstanceID: id, Retained: true, Err: wrapped}
+				retention.markRetained(persisted)
+				m.recordVGPURetainedAssignment(ctx, vgpuRetentionOperationStart, persisted)
 			}
-			return nil, fmt.Errorf("create vGPU for profile %s: %w", stored.GPUProfile, err)
+			return nil, wrapped
 		}
 		assignedAt := m.nowUTC()
 		setStoredVGPUDevice(stored, device, assignedAt)

lib/instances/vgpu_retention.go

@@ -42,11 +42,6 @@ func (r *vgpuRetention) wrapPending(err error) error {
 	return &VGPUCleanupPendingError{InstanceID: r.instanceID, Retained: r.persisted, Err: err}
 }
 
-// Defer before cleanup so rollback records retention before this wraps the error.
-func (r *vgpuRetention) deferWrapPending(retErr *error) {
-	*retErr = r.wrapPending(*retErr)
-}
-
 func (m *manager) persistVGPURetention(ctx context.Context, retention *vgpuRetention) {
 	if retention.stub == nil {
 		m.deleteInstanceData(retention.instanceID)

lib/instances/vgpu_test.go

@@ -18,6 +18,16 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func testVendorVFIODevice(profileName string) devices.VGPUDevice {
+	return devices.VGPUDevice{
+		Framework:   devices.VGPUFrameworkVendorVFIO,
+		VFAddress:   "0000:82:00.4",
+		ProfileType: "1148",
+		ProfileName: profileName,
+		SysfsPath:   "/sys/bus/pci/devices/0000:82:00.4",
+	}
+}
+
 func persistTestVGPURetention(m *manager, ctx context.Context, id string, stub *StoredMetadata) bool {
 	retention := vgpuRetention{instanceID: id, stub: stub, retained: stub != nil}
 	m.persistVGPURetention(ctx, &retention)
@@ -131,13 +141,8 @@ func newStartRollbackVGPUManager(t *testing.T, destroy func(context.Context, dev
 		instanceLocks:   sync.Map{},
 		bootMarkerScans: sync.Map{},
 		createVGPU: func(_ context.Context, profileName, _ string) (*devices.VGPUDevice, error) {
-			return &devices.VGPUDevice{
-				Framework:   devices.VGPUFrameworkVendorVFIO,
-				VFAddress:   "0000:82:00.4",
-				ProfileType: "1148",
-				ProfileName: profileName,
-				SysfsPath:   "/sys/bus/pci/devices/0000:82:00.4",
-			}, nil
+			device := testVendorVFIODevice(profileName)
+			return &device, nil
 		},
 		destroyVGPU: destroy,
 	}
@@ -180,13 +185,7 @@ func TestStartRetainsVGPUWhenCreateRollbackFails(t *testing.T) {
 	meta.ExitMessage = "previous exit"
 	require.NoError(t, m.saveMetadata(meta))
 
-	device := devices.VGPUDevice{
-		Framework:   devices.VGPUFrameworkVendorVFIO,
-		VFAddress:   "0000:82:00.4",
-		ProfileType: "1148",
-		ProfileName: "NVIDIA L40S-2Q",
-		SysfsPath:   "/sys/bus/pci/devices/0000:82:00.4",
-	}
+	device := testVendorVFIODevice("NVIDIA L40S-2Q")
 	cause := errors.New("create verification and rollback failed")
 	m.createVGPU = func(context.Context, string, string) (*devices.VGPUDevice, error) {
 		return nil, &devices.VGPUCreateCleanupPendingError{Device: device, Err: cause}
@@ -225,13 +224,7 @@ func TestStartReportsUnretainedVGPUWhenRetentionSaveFails(t *testing.T) {
 	m, id := newStartRollbackVGPUManager(t, func(context.Context, devices.VGPUAssignment) error {
 		return nil
 	})
-	device := devices.VGPUDevice{
-		Framework:   devices.VGPUFrameworkVendorVFIO,
-		VFAddress:   "0000:82:00.4",
-		ProfileType: "1148",
-		ProfileName: "NVIDIA L40S-2Q",
-		SysfsPath:   "/sys/bus/pci/devices/0000:82:00.4",
-	}
+	device := testVendorVFIODevice("NVIDIA L40S-2Q")
 	cause := errors.New("create verification and rollback failed")
 	m.createVGPU = func(context.Context, string, string) (*devices.VGPUDevice, error) {
 		instanceDir := filepath.Dir(m.paths.InstanceMetadata(id))