Deslop wedged-VF detection: dedupe sentinel report guard, trim redundant tests

lib/devices/vf_health_test.go

@@ -3,10 +3,8 @@ package devices
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
-	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -328,7 +326,7 @@ func TestReportVFInitSuccessWithoutMatchingFailureClearsNothing(t *testing.T) {
 	assert.False(t, result.Rescinded)
 }
 
-func TestReportVFInitSuccessNoopFanoutDoesNotRetryFailedPersist(t *testing.T) {
+func TestReportVFInitSuccessNoopDoesNotRetryFailedPersist(t *testing.T) {
 	resetVFHealthStore(t)
 	var syncCalls int
 	vfHealth.mu.Lock()
@@ -339,28 +337,12 @@ func TestReportVFInitSuccessNoopFanoutDoesNotRetryFailedPersist(t *testing.T) {
 	}
 	vfHealth.mu.Unlock()
 
-	var wg sync.WaitGroup
-	errs := make(chan error, 64)
-	for i := 0; i < 64; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			result, err := ReportVFInitSuccess(VFInitSuccessReport{
-				VFAddress:  "0000:e3:00.4",
-				InstanceID: "healthy-instance",
-			})
-			if result != (VFSuccessResult{}) {
-				errs <- fmt.Errorf("unexpected result: %+v", result)
-				return
-			}
-			errs <- err
-		}()
-	}
-	wg.Wait()
-	close(errs)
-	for err := range errs {
-		require.NoError(t, err)
-	}
+	result, err := ReportVFInitSuccess(VFInitSuccessReport{
+		VFAddress:  "0000:e3:00.4",
+		InstanceID: "healthy-instance",
+	})
+	require.NoError(t, err)
+	assert.Equal(t, VFSuccessResult{}, result)
 	assert.Zero(t, syncCalls)
 	assert.True(t, VFHealthStoreUnavailable())
 

lib/instances/process_identity.go

@@ -202,7 +202,6 @@ func classifyResolvedHypervisorOwner(socketPath string, stored, resolved int, er
 }
 
 // Ambiguous ownership is treated as live; this must not authorize teardown.
-// The error is returned so callers can surface why teardown was suppressed.
 func hypervisorMayBeAlive(id HypervisorProcessIdentity, socketPath string) (bool, error) {
 	pid, err := resolveLiveHypervisorPID(id, socketPath)
 	return err != nil || pid > 0, err

lib/instances/storage.go

@@ -193,9 +193,8 @@ func (m *manager) listMetadataFiles() ([]string, error) {
 }
 
 // listMetadataFilesStrict returns readable metadata paths and joins any stat
-// errors other than absence. Fail-closed callers (the vGPU release claim scan
-// and startup reconcile protection) use it so an unreadable instance is an
-// error instead of silently missing.
+// errors other than absence, so fail-closed callers treat an unreadable
+// instance as an error instead of silently missing.
 func (m *manager) listMetadataFilesStrict() ([]string, error) {
 	files, statErr, err := m.walkMetadataFiles()
 	return files, errors.Join(statErr, err)

lib/instances/vgpu_sentinel.go

@@ -238,24 +238,23 @@ func (c *VGPUSentinelController) recordCheck(ctx context.Context, result string)
 // confirmAssignment rejects a report only when the instance now holds a
 // different VF assignment. Released assignments remain attributable to the
 // assignment captured in the poll target.
-func (c *VGPUSentinelController) confirmAssignment(ctx context.Context, target vgpuSentinelTarget) (bool, error) {
+func (c *VGPUSentinelController) confirmAssignment(ctx context.Context, target vgpuSentinelTarget, action string) bool {
 	current, ok, err := c.store.getVGPUSentinelTarget(ctx, target.instanceID)
 	if err != nil {
-		return false, err
+		c.log.WarnContext(ctx, "vGPU sentinel could not confirm assignment; dropping report",
+			"action", action, "vf", target.vfAddress, "instance_id", target.instanceID, "error", err)
+		return false
+	}
+	if ok && (current.vfAddress != target.vfAddress || current.assignedAt != target.assignedAt) {
+		c.log.InfoContext(ctx, "vGPU sentinel skipping report: assignment changed during poll",
+			"action", action, "vf", target.vfAddress, "instance_id", target.instanceID)
+		return false
 	}
-	return !ok || (current.vfAddress == target.vfAddress && current.assignedAt == target.assignedAt), nil
+	return true
 }
 
 func (c *VGPUSentinelController) handleFailure(ctx context.Context, target vgpuSentinelTarget, nvrm string) {
-	unchanged, err := c.confirmAssignment(ctx, target)
-	if err != nil {
-		c.log.WarnContext(ctx, "vGPU sentinel could not confirm assignment before recording an init failure",
-			"vf", target.vfAddress, "instance_id", target.instanceID, "error", err)
-		return
-	}
-	if !unchanged {
-		c.log.InfoContext(ctx, "vGPU sentinel skipping init failure: assignment changed during poll",
-			"vf", target.vfAddress, "instance_id", target.instanceID)
+	if !c.confirmAssignment(ctx, target, "init_failure") {
 		return
 	}
 	result, err := c.reportFailure(devices.VFInitFailureReport{
@@ -292,13 +291,7 @@ func (c *VGPUSentinelController) handleFailure(ctx context.Context, target vgpuS
 }
 
 func (c *VGPUSentinelController) handleSuccess(ctx context.Context, target vgpuSentinelTarget) {
-	unchanged, err := c.confirmAssignment(ctx, target)
-	if err != nil {
-		c.log.WarnContext(ctx, "vGPU sentinel could not confirm assignment before clearing init failures",
-			"vf", target.vfAddress, "instance_id", target.instanceID, "error", err)
-		return
-	}
-	if !unchanged {
+	if !c.confirmAssignment(ctx, target, "init_success") {
 		return
 	}
 	result, err := c.reportSuccess(devices.VFInitSuccessReport{

lib/instances/vgpu_sentinel_test.go

@@ -68,14 +68,7 @@ func newTestSentinelController(t *testing.T, store *fakeSentinelStore) (*VGPUSen
 		log:               slog.New(slog.DiscardHandler),
 		interval:          time.Hour,
 		repairHealthStore: func() error { return nil },
-		// Mirrors the real store: repeated reports for the same assignment are
-		// deduplicated.
 		reportFailure: func(report devices.VFInitFailureReport) (devices.VFReportResult, error) {
-			for _, previous := range reported {
-				if previous == report {
-					return devices.VFReportResult{Outcome: devices.VFReportUnchanged, Failures: 1, Threshold: 1}, nil
-				}
-			}
 			reported = append(reported, report)
 			return devices.VFReportResult{Outcome: devices.VFReportQuarantined, Failures: 1, Threshold: 1}, nil
 		},
@@ -90,7 +83,7 @@ func newTestSentinelController(t *testing.T, store *fakeSentinelStore) (*VGPUSen
 	return c, &reported
 }
 
-func TestVGPUSentinelControllerReportsFailureOnce(t *testing.T) {
+func TestVGPUSentinelControllerReportsFailure(t *testing.T) {
 	t.Parallel()
 
 	store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{
@@ -99,6 +92,8 @@ func TestVGPUSentinelControllerReportsFailureOnce(t *testing.T) {
 		assignedAt: "2026-08-20T15:00:00Z",
 	}}}
 	c, reported := newTestSentinelController(t, store)
+	var logs bytes.Buffer
+	c.log = slog.New(slog.NewTextHandler(&logs, nil))
 	ctx := context.Background()
 
 	c.guestGPUInitStatus = func(context.Context, string) (guest.GPUInitState, string, error) {
@@ -112,9 +107,8 @@ func TestVGPUSentinelControllerReportsFailureOnce(t *testing.T) {
 	require.Len(t, *reported, 1)
 	assert.Equal(t, "0000:e3:00.4", (*reported)[0].VFAddress)
 	assert.Equal(t, "instance-1", (*reported)[0].InstanceID)
-
-	c.pollOnce(ctx)
-	assert.Len(t, *reported, 1, "repeated polls of the same failed assignment must deduplicate")
+	assert.Contains(t, logs.String(), "quarantined wedged vGPU VF")
+	assert.Contains(t, logs.String(), "RmInitAdapter failed!")
 }
 
 func TestVGPUSentinelControllerSkipsUnreachableGuest(t *testing.T) {
@@ -143,10 +137,7 @@ func TestVGPUSentinelControllerSkipsUnreachableGuest(t *testing.T) {
 func TestVGPUSentinelControllerRepairsHealthStoreOncePerPoll(t *testing.T) {
 	t.Parallel()
 
-	targets := make([]vgpuSentinelTarget, vgpuSentinelMaxConcurrentPolls)
-	for i := range targets {
-		targets[i] = vgpuSentinelTarget{instanceID: fmt.Sprintf("instance-%d", i)}
-	}
+	targets := []vgpuSentinelTarget{{instanceID: "instance-1"}, {instanceID: "instance-2"}}
 	c, _ := newTestSentinelController(t, &fakeSentinelStore{targets: targets})
 	c.guestGPUInitStatus = guestReportsOK
 	var repairs int
@@ -159,49 +150,6 @@ func TestVGPUSentinelControllerRepairsHealthStoreOncePerPoll(t *testing.T) {
 	assert.Equal(t, 1, repairs)
 }
 
-func TestVGPUSentinelControllerPollsTargetsConcurrently(t *testing.T) {
-	t.Parallel()
-
-	store := &fakeSentinelStore{targets: []vgpuSentinelTarget{
-		{instanceID: "instance-1"},
-		{instanceID: "instance-2"},
-	}}
-	c, _ := newTestSentinelController(t, store)
-	started := make(chan struct{}, len(store.targets))
-	release := make(chan struct{})
-	released := false
-	defer func() {
-		if !released {
-			close(release)
-		}
-	}()
-	c.guestGPUInitStatus = func(context.Context, string) (guest.GPUInitState, string, error) {
-		started <- struct{}{}
-		<-release
-		return guest.GPUInitState_GPU_INIT_STATE_UNKNOWN, "", nil
-	}
-
-	done := make(chan struct{})
-	go func() {
-		c.pollOnce(context.Background())
-		close(done)
-	}()
-	for range store.targets {
-		select {
-		case <-started:
-		case <-time.After(time.Second):
-			t.Fatal("targets were not polled concurrently")
-		}
-	}
-	close(release)
-	released = true
-	select {
-	case <-done:
-	case <-time.After(time.Second):
-		t.Fatal("poll did not finish")
-	}
-}
-
 func TestVGPUSentinelControllerRecordsCheckResults(t *testing.T) {
 	t.Parallel()
 
@@ -302,8 +250,6 @@ func TestVGPUSentinelControllerRetriesFailedTallyClear(t *testing.T) {
 	}}}
 	c, _ := newTestSentinelController(t, store)
 	c.guestGPUInitStatus = guestReportsOK
-	var logs bytes.Buffer
-	c.log = slog.New(slog.NewTextHandler(&logs, nil))
 	var reports []devices.VFInitSuccessReport
 	c.reportSuccess = func(report devices.VFInitSuccessReport) (devices.VFSuccessResult, error) {
 		reports = append(reports, report)
@@ -315,9 +261,7 @@ func TestVGPUSentinelControllerRetriesFailedTallyClear(t *testing.T) {
 
 	c.pollOnce(context.Background())
 	require.Len(t, reports, 1)
-	assert.Contains(t, logs.String(), "persist failed")
 
-	// The guest keeps reporting OK, so the next poll retries the clear.
 	c.pollOnce(context.Background())
 	require.Len(t, reports, 2)
 	assert.Equal(t, "0000:e3:00.4", reports[1].VFAddress)
@@ -345,90 +289,59 @@ func TestVGPUSentinelControllerRetriesFailedQuarantine(t *testing.T) {
 	assert.Len(t, *reported, 1)
 }
 
-func TestVGPUSentinelControllerLogsNVRMMessageOnQuarantine(t *testing.T) {
-	t.Parallel()
-
-	store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{
-		instanceID: "instance-1",
-		vfAddress:  "0000:e3:00.4",
-		assignedAt: "2026-08-20T15:00:00Z",
-	}}}
-	c, _ := newTestSentinelController(t, store)
-	var logs bytes.Buffer
-	c.log = slog.New(slog.NewTextHandler(&logs, nil))
-
-	c.pollOnce(context.Background())
-	assert.Contains(t, logs.String(), "quarantined wedged vGPU VF")
-	assert.Contains(t, logs.String(), "RmInitAdapter failed!")
-}
-
-func TestVGPUSentinelControllerSkipsFailureOnChangedAssignment(t *testing.T) {
+func TestVGPUSentinelControllerConfirmsAssignmentBeforeReporting(t *testing.T) {
 	t.Parallel()
 
-	store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{
-		instanceID: "instance-1",
-		vfAddress:  "0000:e3:00.5",
-		assignedAt: "2026-08-21T00:00:10Z",
-	}}}
-	c, reported := newTestSentinelController(t, store)
-
 	stale := vgpuSentinelTarget{
 		instanceID: "instance-1",
 		vfAddress:  "0000:e3:00.4",
 		assignedAt: "2026-08-21T00:00:00Z",
 	}
-	c.pollTarget(context.Background(), stale)
-	assert.Empty(t, *reported)
-
-	// A released assignment (instance gone) remains attributable.
-	store.targets = nil
-	c.pollTarget(context.Background(), stale)
-	require.Len(t, *reported, 1)
-	assert.Equal(t, "0000:e3:00.4", (*reported)[0].VFAddress)
-}
-
-func TestVGPUSentinelControllerSkipsInitOKOnChangedAssignment(t *testing.T) {
-	t.Parallel()
-
-	store := &fakeSentinelStore{targets: []vgpuSentinelTarget{{
+	changed := []vgpuSentinelTarget{{
 		instanceID: "instance-1",
 		vfAddress:  "0000:e3:00.5",
 		assignedAt: "2026-08-21T00:00:10Z",
-	}}}
-	c, _ := newTestSentinelController(t, store)
-	c.guestGPUInitStatus = guestReportsOK
-	var cleared []devices.VFInitSuccessReport
-	c.reportSuccess = func(report devices.VFInitSuccessReport) (devices.VFSuccessResult, error) {
-		cleared = append(cleared, report)
-		return devices.VFSuccessResult{Cleared: 1}, nil
+	}}
+	tests := []struct {
+		name        string
+		guestState  func(context.Context, string) (guest.GPUInitState, string, error)
+		targets     []vgpuSentinelTarget
+		wantApplied bool
+	}{
+		{"failure skipped when the assignment changed", guestReportsFailed, changed, false},
+		// A released assignment (instance gone) remains attributable.
+		{"failure from a released assignment is reported", guestReportsFailed, nil, true},
+		{"init OK skipped when the assignment changed", guestReportsOK, changed, false},
+		{"init OK from a released assignment clears", guestReportsOK, nil, true},
 	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c, reported := newTestSentinelController(t, &fakeSentinelStore{targets: tt.targets})
+			c.guestGPUInitStatus = tt.guestState
+			var cleared []devices.VFInitSuccessReport
+			c.reportSuccess = func(report devices.VFInitSuccessReport) (devices.VFSuccessResult, error) {
+				cleared = append(cleared, report)
+				return devices.VFSuccessResult{Cleared: 1}, nil
+			}
 
-	c.pollTarget(context.Background(), vgpuSentinelTarget{
-		instanceID: "instance-1",
-		vfAddress:  "0000:e3:00.4",
-		assignedAt: "2026-08-21T00:00:00Z",
-	})
-	assert.Empty(t, cleared)
-}
-
-func TestVGPUSentinelControllerAppliesInitOKFromReleasedAssignment(t *testing.T) {
-	t.Parallel()
+			c.pollTarget(context.Background(), stale)
 
-	c, _ := newTestSentinelController(t, &fakeSentinelStore{})
-	c.guestGPUInitStatus = guestReportsOK
-	var cleared []devices.VFInitSuccessReport
-	c.reportSuccess = func(report devices.VFInitSuccessReport) (devices.VFSuccessResult, error) {
-		cleared = append(cleared, report)
-		return devices.VFSuccessResult{Cleared: 1}, nil
+			if !tt.wantApplied {
+				assert.Empty(t, *reported)
+				assert.Empty(t, cleared)
+				return
+			}
+			if len(*reported) == 1 {
+				assert.Equal(t, stale.vfAddress, (*reported)[0].VFAddress)
+				assert.Equal(t, stale.assignedAt, (*reported)[0].AssignedAt)
+				assert.Empty(t, cleared)
+				return
+			}
+			require.Len(t, cleared, 1)
+			assert.Equal(t, stale.vfAddress, cleared[0].VFAddress)
+			assert.Equal(t, stale.assignedAt, cleared[0].AssignedAt)
+		})
 	}
-
-	c.pollTarget(context.Background(), vgpuSentinelTarget{
-		instanceID: "instance-1",
-		vfAddress:  "0000:e3:00.4",
-		assignedAt: "2026-08-21T00:00:00Z",
-	})
-	require.Len(t, cleared, 1)
-	assert.Equal(t, "2026-08-21T00:00:00Z", cleared[0].AssignedAt)
 }
 
 func TestGetVGPUSentinelTargetSkipsRetentionStub(t *testing.T) {