Merge pull request #2771 from andyzhangx/cache-nil-check

fix: panic in vmss cache conversion
kubernetes-sigs · Nov 16, 2022 · f22ac73 · f22ac73
2 parents 0d23a51 + faa26f7
commit f22ac73
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 11 deletions.
diff --git a/pkg/provider/azure_controller_vmss.go b/pkg/provider/azure_controller_vmss.go
@@ -223,7 +223,8 @@ func (ss *ScaleSet) DetachDisk(ctx context.Context, nodeName types.NodeName, dis
 		// Update the cache with the updated result only if its not nil
 		// and contains the VirtualMachineScaleSetVMProperties
 		if updateResult != nil && updateResult.VirtualMachineScaleSetVMProperties != nil {
-			if updErr := ss.updateCache(vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, updateResult); updErr != nil {
+			if err := ss.updateCache(vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, updateResult); err != nil {
+				klog.Errorf("updateCache(%s, %s, %s) failed with error: %v", vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, err)
 				// if err faced during updating cache, invalidate the cache
 				_ = ss.DeleteCacheForNode(vmName)
 			}
@@ -277,7 +278,8 @@ func (ss *ScaleSet) UpdateVM(ctx context.Context, nodeName types.NodeName) error
 		// Update the cache with the updated result only if its not nil
 		// and contains the VirtualMachineScaleSetVMProperties
 		if updateResult != nil && updateResult.VirtualMachineScaleSetVMProperties != nil {
-			if updErr := ss.updateCache(vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, updateResult); updErr != nil {
+			if err := ss.updateCache(vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, updateResult); err != nil {
+				klog.Errorf("updateCache(%s, %s, %s) failed with error: %v", vmName, nodeResourceGroup, vm.VMSSName, vm.InstanceID, err)
 				// if err faced during updating cache, invalidate the cache
 				_ = ss.DeleteCacheForNode(vmName)
 			}

diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
@@ -141,7 +141,6 @@ func (ss *ScaleSet) getVMSS(vmssName string, crt azcache.AzureCacheReadType) (*c
 		if err != nil {
 			return nil, err
 		}
-
 		vmsses := cached.(*sync.Map)
 		if vmss, ok := vmsses.Load(vmssName); ok {
 			result := vmss.(*VMSSEntry)
@@ -186,7 +185,6 @@ func (ss *ScaleSet) getVmssVMByNodeIdentity(node *nodeIdentity, crt azcache.Azur
 		if err != nil {
 			return nil, found, err
 		}
-
 		virtualMachines := cached.(*sync.Map)
 		if entry, ok := virtualMachines.Load(nodeName); ok {
 			result := entry.(*VMSSVirtualMachinesEntry)

diff --git a/pkg/provider/azure_vmss_cache.go b/pkg/provider/azure_vmss_cache.go
@@ -328,9 +328,11 @@ func (ss *ScaleSet) updateCache(nodeName, resourceGroupName, vmssName, instanceI
 
 	vmCache, err := timedCache.Get(cacheKey, azcache.CacheReadTypeUnsafe)
 	if err != nil {
-		klog.Errorf("updateCache(%s, %s, %s) failed getting vmCache with error: %v", vmssName, resourceGroupName, nodeName, err)
+		return err
+	}
+	if vmCache == nil {
+		return fmt.Errorf("nil vmCache")
 	}
-
 	virtualMachines := vmCache.(*sync.Map)
 	virtualMachines.Range(func(key, value interface{}) bool {
 		if key.(string) != nodeName {

diff --git a/pkg/provider/azure_vmssflex_cache.go b/pkg/provider/azure_vmssflex_cache.go
@@ -218,7 +218,6 @@ func (fs *FlexScaleSet) getVmssFlexVM(nodeName string, crt azcache.AzureCacheRea
 	if err != nil {
 		return vm, err
 	}
-
 	vmMap := cached.(*sync.Map)
 	cachedVM, ok := vmMap.Load(nodeName)
 	if !ok {
@@ -234,7 +233,6 @@ func (fs *FlexScaleSet) getVmssFlexByVmssFlexID(vmssFlexID string, crt azcache.A
 	if err != nil {
 		return nil, err
 	}
-
 	vmssFlexes := cached.(*sync.Map)
 	if vmssFlex, ok := vmssFlexes.Load(vmssFlexID); ok {
 		result := vmssFlex.(*compute.VirtualMachineScaleSet)
@@ -246,7 +244,6 @@ func (fs *FlexScaleSet) getVmssFlexByVmssFlexID(vmssFlexID string, crt azcache.A
 	if err != nil {
 		return nil, err
 	}
-
 	vmssFlexes = cached.(*sync.Map)
 	if vmssFlex, ok := vmssFlexes.Load(vmssFlexID); ok {
 		result := vmssFlex.(*compute.VirtualMachineScaleSet)
@@ -272,7 +269,6 @@ func (fs *FlexScaleSet) getVmssFlexIDByName(vmssFlexName string) (string, error)
 	if err != nil {
 		return "", err
 	}
-
 	var targetVmssFlexID string
 	vmssFlexes := cached.(*sync.Map)
 	vmssFlexes.Range(func(key, value interface{}) bool {
@@ -330,7 +326,6 @@ func (fs *FlexScaleSet) DeleteCacheForNode(nodeName string) error {
 	if err != nil {
 		return err
 	}
-
 	vmMap := cached.(*sync.Map)
 	vmMap.Delete(nodeName)