Merge pull request #3302 from nilo19/fix/cherry-pick-2950-1.26

fix: remove vmss vm from cache when invalidate the cache #3299

pkg/provider/azure_utils.go

@@ -56,35 +56,25 @@ func newLockMap() *lockMap {
 func (lm *lockMap) LockEntry(entry string) {
 	lm.Lock()
 	// check if entry does not exists, then add entry
-	if _, exists := lm.mutexMap[entry]; !exists {
-		lm.addEntry(entry)
+	mutex, exists := lm.mutexMap[entry]
+	if !exists {
+		mutex = &sync.Mutex{}
+		lm.mutexMap[entry] = mutex
 	}
-
 	lm.Unlock()
-	lm.lockEntry(entry)
+	mutex.Lock()
 }
 
 // UnlockEntry release the lock associated with the specific entry
 func (lm *lockMap) UnlockEntry(entry string) {
 	lm.Lock()
 	defer lm.Unlock()
 
-	if _, exists := lm.mutexMap[entry]; !exists {
+	mutex, exists := lm.mutexMap[entry]
+	if !exists {
 		return
 	}
-	lm.unlockEntry(entry)
-}
-
-func (lm *lockMap) addEntry(entry string) {
-	lm.mutexMap[entry] = &sync.Mutex{}
-}
-
-func (lm *lockMap) lockEntry(entry string) {
-	lm.mutexMap[entry].Lock()
-}
-
-func (lm *lockMap) unlockEntry(entry string) {
-	lm.mutexMap[entry].Unlock()
+	mutex.Unlock()
 }
 
 func getContextWithCancel() (context.Context, context.CancelFunc) {

pkg/provider/azure_vmss_cache.go

@@ -152,11 +152,6 @@ func (ss *ScaleSet) getVMSSVMsFromCache(resourceGroup, vmssName string, crt azca
 	return virtualMachines, nil
 }
 
-// gcVMSSVMCache delete stale VMSS VMs caches from deleted VMSSes.
-func (ss *ScaleSet) gcVMSSVMCache() error {
-	return ss.vmssCache.Delete(consts.VMSSKey)
-}
-
 // newVMSSVirtualMachinesCache instantiates a new VMs cache for VMs belonging to the provided VMSS.
 func (ss *ScaleSet) newVMSSVirtualMachinesCache() (*azcache.TimedCache, error) {
 	vmssVirtualMachinesCacheTTL := time.Duration(ss.Config.VmssVirtualMachinesCacheTTLInSeconds) * time.Second
@@ -280,16 +275,20 @@ func (ss *ScaleSet) DeleteCacheForNode(nodeName string) error {
 		return err
 	}
 
-	err = ss.vmssVMCache.Delete(getVMSSVMCacheKey(node.resourceGroup, node.vmssName))
+	// get sync.Map cache and remove the node from the cache
+	cacheKey := getVMSSVMCacheKey(node.resourceGroup, node.vmssName)
+	ss.lockMap.LockEntry(cacheKey)
+	defer ss.lockMap.UnlockEntry(cacheKey)
+
+	virtualMachines, err := ss.getVMSSVMsFromCache(node.resourceGroup, node.vmssName, azcache.CacheReadTypeUnsafe)
 	if err != nil {
-		klog.Errorf("DeleteCacheForNode(%s) failed to remove from vmssVMCache with error: %v", nodeName, err)
+		klog.Errorf("DeleteCacheForNode(%s, %s) failed to getVMSSVMsFromCache: %v", node.resourceGroup, node.vmssName, err)
 		return err
 	}
 
-	if err := ss.gcVMSSVMCache(); err != nil {
-		klog.Errorf("DeleteCacheForNode(%s) failed to gc stale vmss caches: %v", nodeName, err)
-	}
-
+	virtualMachines.Delete(nodeName)
+	ss.vmssVMCache.Update(cacheKey, virtualMachines)
+	klog.V(2).Infof("DeleteCacheForNode(%s, %s, %s) successfully", node.resourceGroup, node.vmssName, nodeName)
 	return nil
 }
 

pkg/provider/azure_vmss_test.go

@@ -2636,6 +2636,13 @@ func TestEnsureBackendPoolDeleted(t *testing.T) {
 	}
 }
 
+func TestEnsureBackendPoolDeletedConcurrentlyLoop(t *testing.T) {
+	// run TestEnsureBackendPoolDeletedConcurrently 20 times to detect race conditions
+	for i := 0; i < 20; i++ {
+		TestEnsureBackendPoolDeletedConcurrently(t)
+	}
+}
+
 func TestEnsureBackendPoolDeletedConcurrently(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()