Merge pull request #3520 from marwanad/cherry-pick-3437-1.19

Cherry pick #3437 onto 1.19 - Avoid unwanted VMSS VMs caches invalidations
kubernetes · Sep 17, 2020 · bba156c · bba156c
2 parents 0971e3c + c8d9d01
commit bba156c
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 15 deletions.
diff --git a/cluster-autoscaler/cloudprovider/azure/azure_cache.go b/cluster-autoscaler/cloudprovider/azure/azure_cache.go
@@ -17,7 +17,6 @@ limitations under the License.
 package azure
 
 import (
-	"reflect"
 	"regexp"
 	"strings"
 	"sync"
@@ -56,25 +55,25 @@ func newAsgCache() (*asgCache, error) {
 }
 
 // Register registers a node group if it hasn't been registered.
-func (m *asgCache) Register(asg cloudprovider.NodeGroup) bool {
+func (m *asgCache) Register(newAsg cloudprovider.NodeGroup) bool {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
 	for i := range m.registeredAsgs {
-		if existing := m.registeredAsgs[i]; strings.EqualFold(existing.Id(), asg.Id()) {
-			if reflect.DeepEqual(existing, asg) {
+		if existing := m.registeredAsgs[i]; strings.EqualFold(existing.Id(), newAsg.Id()) {
+			if existing.MinSize() == newAsg.MinSize() && existing.MaxSize() == newAsg.MaxSize() {
 				return false
 			}
 
-			m.registeredAsgs[i] = asg
-			klog.V(4).Infof("ASG %q updated", asg.Id())
+			m.registeredAsgs[i] = newAsg
+			klog.V(4).Infof("ASG %q updated", newAsg.Id())
 			m.invalidateUnownedInstanceCache()
 			return true
 		}
 	}
 
-	klog.V(4).Infof("Registering ASG %q", asg.Id())
-	m.registeredAsgs = append(m.registeredAsgs, asg)
+	klog.V(4).Infof("Registering ASG %q", newAsg.Id())
+	m.registeredAsgs = append(m.registeredAsgs, newAsg)
 	m.invalidateUnownedInstanceCache()
 	return true
 }
@@ -181,7 +180,7 @@ func (m *asgCache) regenerate() error {
 
 	m.instanceToAsg = newCache
 
-	// Incalidating unowned instance cache.
+	// Invalidating unowned instance cache.
 	m.invalidateUnownedInstanceCache()
 
 	return nil

diff --git a/cluster-autoscaler/cloudprovider/azure/azure_manager.go b/cluster-autoscaler/cloudprovider/azure/azure_manager.go
@@ -511,7 +511,7 @@ func (m *AzureManager) buildAsgFromSpec(spec string) (cloudprovider.NodeGroup, e
 	case vmTypeStandard:
 		return NewAgentPool(s, m)
 	case vmTypeVMSS:
-		return NewScaleSet(s, m)
+		return NewScaleSet(s, m, -1)
 	case vmTypeAKS:
 		return NewAKSAgentPool(s, m)
 	default:
@@ -698,7 +698,12 @@ func (m *AzureManager) listScaleSets(filter []labelAutoDiscoveryConfig) ([]cloud
 			continue
 		}
 
-		asg, err := NewScaleSet(spec, m)
+		curSize := int64(-1)
+		if scaleSet.Sku != nil && scaleSet.Sku.Capacity != nil {
+			curSize = *scaleSet.Sku.Capacity
+		}
+
+		asg, err := NewScaleSet(spec, m, curSize)
 		if err != nil {
 			klog.Warningf("ignoring nodegroup %q %s", *scaleSet.Name, err)
 			continue

diff --git a/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go b/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go
@@ -730,7 +730,7 @@ func TestListScalesets(t *testing.T) {
 				minSize:           5,
 				maxSize:           50,
 				manager:           manager,
-				curSize:           -1,
+				curSize:           3,
 				sizeRefreshPeriod: defaultVmssSizeRefreshPeriod,
 			}},
 		},
@@ -836,7 +836,7 @@ func TestGetFilteredAutoscalingGroupsVmss(t *testing.T) {
 		minSize:           minVal,
 		maxSize:           maxVal,
 		manager:           manager,
-		curSize:           -1,
+		curSize:           3,
 		sizeRefreshPeriod: defaultVmssSizeRefreshPeriod,
 	}}
 	assert.True(t, assert.ObjectsAreEqualValues(expectedAsgs, asgs), "expected %#v, but found: %#v", expectedAsgs, asgs)

diff --git a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go
@@ -89,15 +89,15 @@ type ScaleSet struct {
 }
 
 // NewScaleSet creates a new NewScaleSet.
-func NewScaleSet(spec *dynamic.NodeGroupSpec, az *AzureManager) (*ScaleSet, error) {
+func NewScaleSet(spec *dynamic.NodeGroupSpec, az *AzureManager, curSize int64) (*ScaleSet, error) {
 	scaleSet := &ScaleSet{
 		azureRef: azureRef{
 			Name: spec.Name,
 		},
 		minSize: spec.MinSize,
 		maxSize: spec.MaxSize,
 		manager: az,
-		curSize: -1,
+		curSize: curSize,
 	}
 
 	if az.config.VmssCacheTTL != 0 {