From 809330765f53e9a63624c3c7a2b17cc46aceebaf Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:56:07 +0000 Subject: [PATCH] Unify vGPU retention wrapping and dedupe test fixtures --- lib/instances/create.go | 3 ++- lib/instances/start.go | 19 +++++++++--------- lib/instances/vgpu_retention.go | 5 ----- lib/instances/vgpu_test.go | 35 +++++++++++++-------------------- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/lib/instances/create.go b/lib/instances/create.go index 525f2c262..cd50deb5c 100644 --- a/lib/instances/create.go +++ b/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) diff --git a/lib/instances/start.go b/lib/instances/start.go index ea8c2a6f4..37b836427 100644 --- a/lib/instances/start.go +++ b/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) diff --git a/lib/instances/vgpu_retention.go b/lib/instances/vgpu_retention.go index a0e048bdc..1bcba2c42 100644 --- a/lib/instances/vgpu_retention.go +++ b/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) diff --git a/lib/instances/vgpu_test.go b/lib/instances/vgpu_test.go index e82773115..07cdcb8e5 100644 --- a/lib/instances/vgpu_test.go +++ b/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))