Merge pull request #2417 from nilo19/chore/cherry-pick-2402-1.23

[release-1.23] chore: remove node from VMSS VM cache when it is deleted
kubernetes-sigs · Sep 29, 2022 · 6f84607 · 6f84607
2 parents 804f629 + 805661c
commit 6f84607
Show file tree

Hide file tree

Showing 10 changed files with 322 additions and 220 deletions.
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
@@ -1042,6 +1042,9 @@ func (az *Cloud) SetInformers(informerFactory informers.SharedInformerFactory) {
 				}
 			}
 			az.updateNodeCaches(node, nil)
+
+			klog.V(4).Infof("Removing node %s from VMSet cache.", node.Name)
+			_ = az.VMSet.DeleteCacheForNode(node.Name)
 		},
 	})
 	az.nodeInformerSynced = nodeInformer.HasSynced

diff --git a/pkg/provider/azure_controller_standard.go b/pkg/provider/azure_controller_standard.go
@@ -100,7 +100,7 @@ func (as *availabilitySet) AttachDisk(nodeName types.NodeName, diskMap map[strin
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = as.cloud.vmCache.Delete(vmName)
+		_ = as.DeleteCacheForNode(vmName)
 	}()
 
 	future, rerr := as.VirtualMachinesClient.UpdateAsync(ctx, nodeResourceGroup, vmName, newVM, "attach_disk")
@@ -121,6 +121,11 @@ func (as *availabilitySet) AttachDisk(nodeName types.NodeName, diskMap map[strin
 	return future, nil
 }
 
+func (as *availabilitySet) DeleteCacheForNode(nodeName string) error {
+	_ = as.cloud.vmCache.Delete(nodeName)
+	return nil
+}
+
 // WaitForUpdateResult waits for the response of the update request
 func (as *availabilitySet) WaitForUpdateResult(ctx context.Context, future *azure.Future, resourceGroupName, source string) error {
 	if rerr := as.VirtualMachinesClient.WaitForUpdateResult(ctx, future, resourceGroupName, source); rerr != nil {
@@ -190,7 +195,7 @@ func (as *availabilitySet) DetachDisk(nodeName types.NodeName, diskMap map[strin
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = as.cloud.vmCache.Delete(vmName)
+		_ = as.DeleteCacheForNode(vmName)
 	}()
 
 	rerr := as.VirtualMachinesClient.Update(ctx, nodeResourceGroup, vmName, newVM, "detach_disk")
@@ -224,7 +229,7 @@ func (as *availabilitySet) UpdateVM(nodeName types.NodeName) error {
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = as.cloud.vmCache.Delete(vmName)
+		_ = as.DeleteCacheForNode(vmName)
 	}()
 
 	rerr := as.VirtualMachinesClient.Update(ctx, nodeResourceGroup, vmName, compute.VirtualMachineUpdate{}, "update_vm")

diff --git a/pkg/provider/azure_controller_vmss.go b/pkg/provider/azure_controller_vmss.go
@@ -103,7 +103,7 @@ func (ss *ScaleSet) AttachDisk(nodeName types.NodeName, diskMap map[string]*Atta
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = ss.deleteCacheForNode(vmName)
+		_ = ss.DeleteCacheForNode(vmName)
 	}()
 
 	klog.V(2).Infof("azureDisk - update(%s): vm(%s) - attach disk list(%s)", nodeResourceGroup, nodeName, diskMap)
@@ -194,7 +194,7 @@ func (ss *ScaleSet) DetachDisk(nodeName types.NodeName, diskMap map[string]strin
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = ss.deleteCacheForNode(vmName)
+		_ = ss.DeleteCacheForNode(vmName)
 	}()
 
 	klog.V(2).Infof("azureDisk - update(%s): vm(%s) - detach disk list(%s)", nodeResourceGroup, nodeName, diskMap)
@@ -234,7 +234,7 @@ func (ss *ScaleSet) UpdateVM(nodeName types.NodeName) error {
 
 	// Invalidate the cache right after updating
 	defer func() {
-		_ = ss.deleteCacheForNode(vmName)
+		_ = ss.DeleteCacheForNode(vmName)
 	}()
 
 	klog.V(2).Infof("azureDisk - update(%s): vm(%s)", nodeResourceGroup, nodeName)

diff --git a/pkg/provider/azure_controller_vmss_test.go b/pkg/provider/azure_controller_vmss_test.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	cloudprovider "k8s.io/cloud-provider"
 
+	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmclient/mockvmclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssclient/mockvmssclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssvmclient/mockvmssvmclient"
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
@@ -87,6 +88,8 @@ func TestAttachDiskWithVMSS(t *testing.T) {
 		mockVMSSClient.EXPECT().List(gomock.Any(), testCloud.ResourceGroup).Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil).AnyTimes()
 		mockVMSSClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, scaleSetName).Return(expectedVMSS, nil).MaxTimes(1)
 		mockVMSSClient.EXPECT().CreateOrUpdate(gomock.Any(), testCloud.ResourceGroup, scaleSetName, gomock.Any()).Return(nil).MaxTimes(1)
+		mockVMClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
+		mockVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachine{}, nil).AnyTimes()
 
 		expectedVMSSVMs, _, _ := buildTestVirtualMachineEnv(testCloud, scaleSetName, "", 0, test.vmssVMList, "succeeded", false)
 		for _, vmssvm := range expectedVMSSVMs {
@@ -241,6 +244,9 @@ func TestDetachDiskWithVMSS(t *testing.T) {
 			mockVMSSVMClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, scaleSetName, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
 		}
 
+		mockVMClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
+		mockVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachine{}, nil).AnyTimes()
+
 		diskMap := map[string]string{}
 		for _, diskName := range test.disks {
 			diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s",
@@ -350,6 +356,9 @@ func TestUpdateVMWithVMSS(t *testing.T) {
 			mockVMSSVMClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, scaleSetName, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
 		}
 
+		mockVMClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
+		mockVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachine{}, nil).AnyTimes()
+
 		err = ss.UpdateVM(test.vmssvmName)
 		assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s, err: %v", i, test.desc, err)
 		if test.expectedErr {