Don't autodiscover explicitly configured MIGs/ASGs

If a MIG/ASG is both explicitly configured and a candidate for autodetection we want the explicitly configured min/max size to win.
kubernetes · Dec 4, 2017 · 27c97fd · 27c97fd
1 parent c6c9941
commit 27c97fd
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 15 deletions.
diff --git a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go
@@ -70,8 +70,8 @@ var testAwsManager = &AwsManager{
 		interrupt:      make(chan struct{}),
 		service:        testService,
 	},
-	neverUnregister: make(map[AwsRef]bool),
-	service:         testService,
+	explicitlyConfigured: make(map[AwsRef]bool),
+	service:              testService,
 }
 
 func newTestAwsManagerWithService(service autoScaling) *AwsManager {
@@ -84,7 +84,7 @@ func newTestAwsManagerWithService(service autoScaling) *AwsManager {
 			interrupt:      make(chan struct{}),
 			service:        wrapper,
 		},
-		neverUnregister: make(map[AwsRef]bool),
+		explicitlyConfigured: make(map[AwsRef]bool),
 	}
 }
 

diff --git a/cluster-autoscaler/cloudprovider/aws/aws_manager.go b/cluster-autoscaler/cloudprovider/aws/aws_manager.go
@@ -57,7 +57,7 @@ type AwsManager struct {
 	asgCache              *asgCache
 	lastRefresh           time.Time
 	asgAutoDiscoverySpecs []cloudprovider.ASGAutoDiscoveryConfig
-	neverUnregister       map[AwsRef]bool
+	explicitlyConfigured  map[AwsRef]bool
 }
 
 type asgTemplate struct {
@@ -101,7 +101,7 @@ func createAWSManagerInternal(
 		service:               *service,
 		asgCache:              cache,
 		asgAutoDiscoverySpecs: specs,
-		neverUnregister:       make(map[AwsRef]bool),
+		explicitlyConfigured:  make(map[AwsRef]bool),
 	}
 
 	if err := manager.fetchExplicitAsgs(discoveryOpts.NodeGroupSpecs); err != nil {
@@ -132,7 +132,7 @@ func (m *AwsManager) fetchExplicitAsgs(specs []string) error {
 		if m.RegisterAsg(asg) {
 			changed = true
 		}
-		m.neverUnregister[asg.AwsRef] = true
+		m.explicitlyConfigured[asg.AwsRef] = true
 	}
 
 	if changed {
@@ -172,16 +172,23 @@ func (m *AwsManager) fetchAutoAsgs() error {
 			if err != nil {
 				return err
 			}
+			exists[asg.AwsRef] = true
+			if m.explicitlyConfigured[asg.AwsRef] {
+				// This ASG was explicitly configured, but would also be
+				// autodiscovered. We want the explicitly configured min and max
+				// nodes to take precedence.
+				glog.V(3).Infof("Ignoring explicitly configured ASG %s for autodiscovery.", asg.AwsRef.Name)
+				continue
+			}
 			if m.RegisterAsg(asg) {
 				glog.V(3).Infof("Autodiscovered ASG %s using tags %v", asg.AwsRef.Name, spec.TagKeys)
 				changed = true
 			}
-			exists[asg.AwsRef] = true
 		}
 	}
 
 	for _, asg := range m.getAsgs() {
-		if !exists[asg.config.AwsRef] && !m.neverUnregister[asg.config.AwsRef] {
+		if !exists[asg.config.AwsRef] && !m.explicitlyConfigured[asg.config.AwsRef] {
 			m.UnregisterAsg(asg.config)
 			changed = true
 		}

diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager.go b/cluster-autoscaler/cloudprovider/gce/gce_manager.go
@@ -142,7 +142,7 @@ type gceManagerImpl struct {
 	templates             *templateBuilder
 	interrupt             chan struct{}
 	isRegional            bool
-	neverUnregister       map[GceRef]bool
+	explicitlyConfigured  map[GceRef]bool
 	migAutoDiscoverySpecs []cloudprovider.MIGAutoDiscoveryConfig
 	resourceLimiter       *cloudprovider.ResourceLimiter
 	lastRefresh           time.Time
@@ -217,8 +217,8 @@ func CreateGceManager(configReader io.Reader, mode GcpCloudProviderMode, cluster
 			projectId: projectId,
 			service:   gceService,
 		},
-		interrupt:       make(chan struct{}),
-		neverUnregister: make(map[GceRef]bool),
+		interrupt:            make(chan struct{}),
+		explicitlyConfigured: make(map[GceRef]bool),
 	}
 
 	switch mode {
@@ -862,7 +862,7 @@ func (m *gceManagerImpl) fetchExplicitMigs(specs []string) error {
 		if m.RegisterMig(mig) {
 			changed = true
 		}
-		m.neverUnregister[mig.GceRef] = true
+		m.explicitlyConfigured[mig.GceRef] = true
 	}
 
 	if changed {
@@ -905,16 +905,23 @@ func (m *gceManagerImpl) fetchAutoMigs() error {
 			if err != nil {
 				return err
 			}
+			exists[mig.GceRef] = true
+			if m.explicitlyConfigured[mig.GceRef] {
+				// This MIG was explicitly configured, but would also be
+				// autodiscovered. We want the explicitly configured min and max
+				// nodes to take precedence.
+				glog.V(3).Infof("Ignoring explicitly configured MIG %s for autodiscovery.", mig.GceRef.Name)
+				continue
+			}
 			if m.RegisterMig(mig) {
 				glog.V(3).Infof("Autodiscovered MIG %s using regexp %s", mig.GceRef.Name, cfg.Re.String())
 				changed = true
 			}
-			exists[mig.GceRef] = true
 		}
 	}
 
 	for _, mig := range m.getMigs() {
-		if !exists[mig.config.GceRef] && !m.neverUnregister[mig.config.GceRef] {
+		if !exists[mig.config.GceRef] && !m.explicitlyConfigured[mig.config.GceRef] {
 			m.UnregisterMig(mig.config)
 			changed = true
 		}

diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go
@@ -543,7 +543,7 @@ func newTestGceManager(t *testing.T, testServerURL string, mode GcpCloudProvider
 			projectId: projectId,
 			service:   gceService,
 		},
-		neverUnregister: make(map[GceRef]bool),
+		explicitlyConfigured: make(map[GceRef]bool),
 	}
 
 	if isRegional {