Improve SR-IOV device assignment to ensure it's idempotent (#7322) (#7327)

If the SR-IOV device configuration fails after the device has been
assigned to the Pod, subsequent retries will also fail due to the device
being marked as unavailable. Improve the device assignment logic to
ensure the Pod can consistently get the correct device.

Signed-off-by: Lan Luo <[email protected]>

Author: luolanzone

pkg/agent/secondarynetwork/podwatch/controller_test.go

@@ -335,8 +335,7 @@ func TestPodControllerRun(t *testing.T) {
 		containerNetNS(containerID),
 		interfaceName,
 		defaultMTU,
-		// We haven't updated the vfDeviceIDUsageMap, so a different device will be allocated.
-		sriovDeviceID12,
+		sriovDeviceID11,
 		&ipamResult.Result,
 	).Do(func(string, string, string, string, string, int, string, *current.Result) {
 		atomic.AddInt32(&interfaceConfigured, 1)
@@ -1144,15 +1143,26 @@ func TestPodControllerAddPod(t *testing.T) {
 	t.Run("updating deviceID cache per Pod", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		podController, _, _, _ := testPodController(ctrl)
-		_, err := podController.assignUnusedSriovVFDeviceID(podName, testNamespace, sriovResourceName1, interfaceName)
-		_, exists := podController.vfDeviceIDUsageMap.Load(podKey)
+		tmpPodName := "poda"
+		tmpPodNamespace := "testns"
+		tmpPodKey := podKeyGet(tmpPodName, tmpPodNamespace)
+		_, err := podController.assignSriovVFDeviceID(tmpPodName, tmpPodNamespace, sriovResourceName1, interfaceName)
+		vfDeviceIDInfoCache, exists := podController.vfDeviceIDUsageMap.Load(tmpPodKey)
 		assert.True(t, exists)
+		vfDeviceIDsInfo := vfDeviceIDInfoCache.([]podSriovVFDeviceIDInfo)
+		assert.Equal(t, interfaceName, vfDeviceIDsInfo[0].ifName, "incorrect interface name")
 		require.NoError(t, err, "error while assigning unused VfDevice ID")
-		podController.releaseSriovVFDeviceID(podName, testNamespace, interfaceName)
-		_, exists = podController.vfDeviceIDUsageMap.Load(podKey)
+		// The second call would get the assigned SR-IOV device ID directly.
+		deviceID, err := podController.assignSriovVFDeviceID(tmpPodName, tmpPodNamespace, sriovResourceName1, interfaceName)
+		_, exists = podController.vfDeviceIDUsageMap.Load(tmpPodKey)
+		assert.True(t, exists)
+		assert.Equal(t, sriovDeviceID11, deviceID, "incorrect device ID")
+		require.NoError(t, err, "error while getting assigned VfDevice ID")
+		podController.releaseSriovVFDeviceID(tmpPodName, tmpPodNamespace, interfaceName)
+		_, exists = podController.vfDeviceIDUsageMap.Load(tmpPodKey)
 		assert.True(t, exists)
-		podController.deleteVFDeviceIDListPerPod(podName, testNamespace)
-		_, exists = podController.vfDeviceIDUsageMap.Load(podKey)
+		podController.deleteVFDeviceIDListPerPod(tmpPodName, tmpPodNamespace)
+		_, exists = podController.vfDeviceIDUsageMap.Load(tmpPodKey)
 		assert.False(t, exists)
 	})
 }

pkg/agent/secondarynetwork/podwatch/sriov.go

@@ -147,19 +147,31 @@ func (pc *PodController) releaseSriovVFDeviceID(podName, podNamespace, interface
 	}
 }
 
-func (pc *PodController) assignUnusedSriovVFDeviceID(podName, podNamespace, resourceName, interfaceName string) (string, error) {
+func (pc *PodController) assignSriovVFDeviceID(podName, podNamespace, resourceName, interfaceName string) (string, error) {
 	var cache []podSriovVFDeviceIDInfo
 	cache, err := pc.buildVFDeviceIDListPerPod(podName, podNamespace)
 	if err != nil {
 		return "", err
 	}
-	for idx := range cache {
-		if cache[idx].resourceName == resourceName && cache[idx].ifName == "" {
-			// Unused PCI address found. Associate PCI address to the interface.
-			cache[idx].ifName = interfaceName
-			return cache[idx].vfDeviceID, nil
+
+	var unusedCacheEntry *podSriovVFDeviceIDInfo
+	for i := range cache {
+		entry := &cache[i]
+		if entry.resourceName == resourceName {
+			if entry.ifName == interfaceName {
+				return entry.vfDeviceID, nil
+			}
+			if entry.ifName == "" && unusedCacheEntry == nil {
+				unusedCacheEntry = entry // remember the first match of unused PCI address
+			}
 		}
 	}
+
+	if unusedCacheEntry != nil {
+		// Update the cache entry
+		unusedCacheEntry.ifName = interfaceName
+		return unusedCacheEntry.vfDeviceID, nil
+	}
 	return "", fmt.Errorf("no available device")
 }
 
@@ -172,7 +184,7 @@ func (pc *PodController) configureSriovAsSecondaryInterface(
 	mtu int,
 	result *current.Result,
 ) error {
-	podSriovVFDeviceID, err := pc.assignUnusedSriovVFDeviceID(pod.Name, pod.Namespace, resourceName, network.InterfaceRequest)
+	podSriovVFDeviceID, err := pc.assignSriovVFDeviceID(pod.Name, pod.Namespace, resourceName, network.InterfaceRequest)
 	if err != nil {
 		return err
 	}