Merge pull request #2121 from alexeldeib/ace/labelVmss

feat: enable min/max on Azure VMSS autodiscovery

cluster-autoscaler/FAQ.md

@@ -657,7 +657,7 @@ The following startup parameters are supported for cluster autoscaler:
 | `ok-total-unready-count` | Number of allowed unready nodes, irrespective of max-total-unready-percentage  | 3
 | `max-node-provision-time` | Maximum time CA waits for node to be provisioned | 15 minutes
 | `nodes` | sets min,max size and other configuration data for a node group in a format accepted by cloud provider. Can be used multiple times. Format: <min>:<max>:<other...> | ""
-| `node-group-auto-discovery` | One or more definition(s) of node group auto-discovery.<br>A definition is expressed `<name of discoverer>:[<key>[=<value>]]`<br>The `aws` and `gce` cloud providers are currently supported. AWS matches by ASG tags, e.g. `asg:tag=tagKey,anotherTagKey`<br>GCE matches by IG name prefix, and requires you to specify min and max nodes per IG, e.g. `mig:namePrefix=pfx,min=0,max=10`<br>Can be used multiple times | ""
+| `node-group-auto-discovery` | One or more definition(s) of node group auto-discovery.<br>A definition is expressed `<name of discoverer>:[<key>[=<value>]]`<br>The `aws`, `gce`, and `azure` cloud providers are currently supported. AWS matches by ASG tags, e.g. `asg:tag=tagKey,anotherTagKey`<br>GCE matches by IG name prefix, and requires you to specify min and max nodes per IG, e.g. `mig:namePrefix=pfx,min=0,max=10`<br> Azure matches by tags on VMSS, e.g. `label:foo=bar`, and will auto-detect `min` and `max` tags on the VMSS to set scaling limits.<br>Can be used multiple times | ""
 | `estimator` | Type of resource estimator to be used in scale up | binpacking
 | `expander` | Type of node group expander to be used in scale up.  | random
 | `write-status-configmap` | Should CA write status information to a configmap  | true

cluster-autoscaler/cloudprovider/aws/auto_scaling_groups.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 	"sync"
 
-	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
 	"k8s.io/autoscaler/cluster-autoscaler/config/dynamic"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -44,7 +43,7 @@ type asgCache struct {
 	service        autoScalingWrapper
 	interrupt      chan struct{}
 
-	asgAutoDiscoverySpecs []cloudprovider.ASGAutoDiscoveryConfig
+	asgAutoDiscoverySpecs []asgAutoDiscoveryConfig
 	explicitlyConfigured  map[AwsRef]bool
 }
 
@@ -72,7 +71,7 @@ type asg struct {
 	Tags                    []*autoscaling.TagDescription
 }
 
-func newASGCache(service autoScalingWrapper, explicitSpecs []string, autoDiscoverySpecs []cloudprovider.ASGAutoDiscoveryConfig) (*asgCache, error) {
+func newASGCache(service autoScalingWrapper, explicitSpecs []string, autoDiscoverySpecs []asgAutoDiscoveryConfig) (*asgCache, error) {
 	registry := &asgCache{
 		registeredAsgs:        make([]*asg, 0),
 		service:               service,

cluster-autoscaler/cloudprovider/aws/aws_cloud_provider_test.go

@@ -79,7 +79,7 @@ var testAwsManager = &AwsManager{
 	autoScalingService: testService,
 }
 
-func newTestAwsManagerWithService(service autoScaling, autoDiscoverySpecs []cloudprovider.ASGAutoDiscoveryConfig) *AwsManager {
+func newTestAwsManagerWithService(service autoScaling, autoDiscoverySpecs []asgAutoDiscoveryConfig) *AwsManager {
 	wrapper := autoScalingWrapper{service, map[string]string{}}
 	return &AwsManager{
 		autoScalingService: wrapper,
@@ -101,7 +101,7 @@ func newTestAwsManagerWithAsgs(t *testing.T, service autoScaling, specs []string
 	return m
 }
 
-func newTestAwsManagerWithAutoAsgs(t *testing.T, service autoScaling, specs []string, autoDiscoverySpecs []cloudprovider.ASGAutoDiscoveryConfig) *AwsManager {
+func newTestAwsManagerWithAutoAsgs(t *testing.T, service autoScaling, specs []string, autoDiscoverySpecs []asgAutoDiscoveryConfig) *AwsManager {
 	m := newTestAwsManagerWithService(service, autoDiscoverySpecs)
 	m.asgCache.parseExplicitAsgs(specs)
 	return m
@@ -167,7 +167,7 @@ func TestNodeGroups(t *testing.T) {
 
 func TestAutoDiscoveredNodeGroups(t *testing.T) {
 	service := &AutoScalingMock{}
-	provider := testProvider(t, newTestAwsManagerWithAutoAsgs(t, service, []string{}, []cloudprovider.ASGAutoDiscoveryConfig{
+	provider := testProvider(t, newTestAwsManagerWithAutoAsgs(t, service, []string{}, []asgAutoDiscoveryConfig{
 		{
 			Tags: map[string]string{"test": ""},
 		},

cluster-autoscaler/cloudprovider/aws/aws_manager.go

@@ -51,6 +51,8 @@ const (
 	maxRecordsReturnedByAPI = 100
 	maxAsgNamesPerDescribe  = 50
 	refreshInterval         = 1 * time.Minute
+	autoDiscovererTypeASG   = "asg"
+	asgAutoDiscovererKeyTag = "tag"
 )
 
 // AwsManager is handles aws communication and data caching.
@@ -202,7 +204,7 @@ func createAWSManagerInternal(
 		}
 	}
 
-	specs, err := discoveryOpts.ParseASGAutoDiscoverySpecs()
+	specs, err := parseASGAutoDiscoverySpecs(discoveryOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -459,3 +461,63 @@ func extractTaintsFromAsg(tags []*autoscaling.TagDescription) []apiv1.Taint {
 	}
 	return taints
 }
+
+// An asgAutoDiscoveryConfig specifies how to autodiscover AWS ASGs.
+type asgAutoDiscoveryConfig struct {
+	// Tags to match on.
+	// Any ASG with all of the provided tag keys will be autoscaled.
+	Tags map[string]string
+}
+
+// ParseASGAutoDiscoverySpecs returns any provided NodeGroupAutoDiscoverySpecs
+// parsed into configuration appropriate for ASG autodiscovery.
+func parseASGAutoDiscoverySpecs(o cloudprovider.NodeGroupDiscoveryOptions) ([]asgAutoDiscoveryConfig, error) {
+	cfgs := make([]asgAutoDiscoveryConfig, len(o.NodeGroupAutoDiscoverySpecs))
+	var err error
+	for i, spec := range o.NodeGroupAutoDiscoverySpecs {
+		cfgs[i], err = parseASGAutoDiscoverySpec(spec)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return cfgs, nil
+}
+
+func parseASGAutoDiscoverySpec(spec string) (asgAutoDiscoveryConfig, error) {
+	cfg := asgAutoDiscoveryConfig{}
+
+	tokens := strings.Split(spec, ":")
+	if len(tokens) != 2 {
+		return cfg, fmt.Errorf("invalid node group auto discovery spec specified via --node-group-auto-discovery: %s", spec)
+	}
+	discoverer := tokens[0]
+	if discoverer != autoDiscovererTypeASG {
+		return cfg, fmt.Errorf("unsupported discoverer specified: %s", discoverer)
+	}
+	param := tokens[1]
+	kv := strings.SplitN(param, "=", 2)
+	if len(kv) != 2 {
+		return cfg, fmt.Errorf("invalid key=value pair %s", kv)
+	}
+	k, v := kv[0], kv[1]
+	if k != asgAutoDiscovererKeyTag {
+		return cfg, fmt.Errorf("unsupported parameter key \"%s\" is specified for discoverer \"%s\". The only supported key is \"%s\"", k, discoverer, asgAutoDiscovererKeyTag)
+	}
+	if v == "" {
+		return cfg, errors.New("tag value not supplied")
+	}
+	p := strings.Split(v, ",")
+	if len(p) == 0 {
+		return cfg, fmt.Errorf("invalid ASG tag for auto discovery specified: ASG tag must not be empty")
+	}
+	cfg.Tags = make(map[string]string, len(p))
+	for _, label := range p {
+		lp := strings.SplitN(label, "=", 2)
+		if len(lp) > 1 {
+			cfg.Tags[lp[0]] = lp[1]
+			continue
+		}
+		cfg.Tags[lp[0]] = ""
+	}
+	return cfg, nil
+}

cluster-autoscaler/cloudprovider/aws/aws_manager_test.go

@@ -777,3 +777,64 @@ func flatTagSlice(filters []*autoscaling.Filter) []string {
 	sort.Strings(tags)
 	return tags
 }
+
+func TestParseASGAutoDiscoverySpecs(t *testing.T) {
+	cases := []struct {
+		name    string
+		specs   []string
+		want    []asgAutoDiscoveryConfig
+		wantErr bool
+	}{
+		{
+			name: "GoodSpecs",
+			specs: []string{
+				"asg:tag=tag,anothertag",
+				"asg:tag=cooltag,anothertag",
+				"asg:tag=label=value,anothertag",
+			},
+			want: []asgAutoDiscoveryConfig{
+				{Tags: map[string]string{"tag": "", "anothertag": ""}},
+				{Tags: map[string]string{"cooltag": "", "anothertag": ""}},
+				{Tags: map[string]string{"label": "value", "anothertag": ""}},
+			},
+		},
+		{
+			name:    "MissingASGType",
+			specs:   []string{"tag=tag,anothertag"},
+			wantErr: true,
+		},
+		{
+			name:    "WrongType",
+			specs:   []string{"mig:tag=tag,anothertag"},
+			wantErr: true,
+		},
+		{
+			name:    "KeyMissingValue",
+			specs:   []string{"asg:tag="},
+			wantErr: true,
+		},
+		{
+			name:    "ValueMissingKey",
+			specs:   []string{"asg:=tag"},
+			wantErr: true,
+		},
+		{
+			name:    "KeyMissingSeparator",
+			specs:   []string{"asg:tag"},
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			do := cloudprovider.NodeGroupDiscoveryOptions{NodeGroupAutoDiscoverySpecs: tc.specs}
+			got, err := parseASGAutoDiscoverySpecs(do)
+			if tc.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			assert.NoError(t, err)
+			assert.True(t, assert.ObjectsAreEqualValues(tc.want, got), "\ngot: %#v\nwant: %#v", got, tc.want)
+		})
+	}
+}

cluster-autoscaler/cloudprovider/azure/azure_manager.go

@@ -43,8 +43,29 @@ const (
 
 	// The path of deployment parameters for standard vm.
 	deploymentParametersPath = "/var/lib/azure/azuredeploy.parameters.json"
+
+	vmssTagMin                     = "min"
+	vmssTagMax                     = "max"
+	autoDiscovererTypeLabel        = "label"
+	labelAutoDiscovererKeyMinNodes = "min"
+	labelAutoDiscovererKeyMaxNodes = "max"
 )
 
+var validLabelAutoDiscovererKeys = strings.Join([]string{
+	labelAutoDiscovererKeyMinNodes,
+	labelAutoDiscovererKeyMaxNodes,
+}, ", ")
+
+// A labelAutoDiscoveryConfig specifies how to autodiscover Azure scale sets.
+type labelAutoDiscoveryConfig struct {
+	// Key-values to match on.
+	Selector map[string]string
+	// MinSize specifies the minimum size for all MIGs that match Re.
+	MinSize int
+	// MaxSize specifies the maximum size for all MIGs that match Re.
+	MaxSize int
+}
+
 // AzureManager handles Azure communication and data caching.
 type AzureManager struct {
 	config   *Config
@@ -53,7 +74,7 @@ type AzureManager struct {
 
 	asgCache              *asgCache
 	lastRefresh           time.Time
-	asgAutoDiscoverySpecs []cloudprovider.LabelAutoDiscoveryConfig
+	asgAutoDiscoverySpecs []labelAutoDiscoveryConfig
 	explicitlyConfigured  map[string]bool
 }
 
@@ -181,7 +202,7 @@ func CreateAzureManager(configReader io.Reader, discoveryOpts cloudprovider.Node
 	}
 	manager.asgCache = cache
 
-	specs, err := discoveryOpts.ParseLabelAutoDiscoverySpecs()
+	specs, err := parseLabelAutoDiscoverySpecs(discoveryOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -335,7 +356,7 @@ func (m *AzureManager) Cleanup() {
 	m.asgCache.Cleanup()
 }
 
-func (m *AzureManager) getFilteredAutoscalingGroups(filter []cloudprovider.LabelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
+func (m *AzureManager) getFilteredAutoscalingGroups(filter []labelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
 	if len(filter) == 0 {
 		return nil, nil
 	}
@@ -359,7 +380,7 @@ func (m *AzureManager) getFilteredAutoscalingGroups(filter []cloudprovider.Label
 }
 
 // listScaleSets gets a list of scale sets and instanceIDs.
-func (m *AzureManager) listScaleSets(filter []cloudprovider.LabelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
+func (m *AzureManager) listScaleSets(filter []labelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
 	ctx, cancel := getContextWithCancel()
 	defer cancel()
 
@@ -379,13 +400,38 @@ func (m *AzureManager) listScaleSets(filter []cloudprovider.LabelAutoDiscoveryCo
 				continue
 			}
 		}
-
 		spec := &dynamic.NodeGroupSpec{
 			Name:               *scaleSet.Name,
 			MinSize:            1,
 			MaxSize:            -1,
 			SupportScaleToZero: scaleToZeroSupportedVMSS,
 		}
+
+		if val, ok := scaleSet.Tags["min"]; ok {
+			if minSize, err := strconv.Atoi(*val); err == nil {
+				spec.MinSize = minSize
+			} else {
+				return asgs, fmt.Errorf("invalid minimum size specified for vmss: %s", err)
+			}
+		} else {
+			return asgs, fmt.Errorf("no minimum size specified for vmss: %s", err)
+		}
+		if val, ok := scaleSet.Tags["max"]; ok {
+			if maxSize, err := strconv.Atoi(*val); err == nil {
+				spec.MaxSize = maxSize
+			} else {
+				return asgs, fmt.Errorf("invalid maximum size specified for vmss: %s", err)
+			}
+		} else {
+			return asgs, fmt.Errorf("no maximum size specified for vmss: %s", err)
+		}
+		if spec.MaxSize < 1 {
+			return asgs, fmt.Errorf("maximum size must be greater than 1 node")
+		}
+		if spec.MaxSize < spec.MinSize {
+			return asgs, fmt.Errorf("maximum size must be greater than minimum size")
+		}
+
 		asg, _ := NewScaleSet(spec, m)
 		asgs = append(asgs, asg)
 	}
@@ -395,7 +441,7 @@ func (m *AzureManager) listScaleSets(filter []cloudprovider.LabelAutoDiscoveryCo
 
 // listAgentPools gets a list of agent pools and instanceIDs.
 // Note: filter won't take effect for agent pools.
-func (m *AzureManager) listAgentPools(filter []cloudprovider.LabelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
+func (m *AzureManager) listAgentPools(filter []labelAutoDiscoveryConfig) (asgs []cloudprovider.NodeGroup, err error) {
 	ctx, cancel := getContextWithCancel()
 	defer cancel()
 	deploy, err := m.azClient.deploymentsClient.Get(ctx, m.config.ResourceGroup, m.config.Deployment)
@@ -423,3 +469,68 @@ func (m *AzureManager) listAgentPools(filter []cloudprovider.LabelAutoDiscoveryC
 
 	return asgs, nil
 }
+
+// ParseLabelAutoDiscoverySpecs returns any provided NodeGroupAutoDiscoverySpecs
+// parsed into configuration appropriate for ASG autodiscovery.
+func parseLabelAutoDiscoverySpecs(o cloudprovider.NodeGroupDiscoveryOptions) ([]labelAutoDiscoveryConfig, error) {
+	cfgs := make([]labelAutoDiscoveryConfig, len(o.NodeGroupAutoDiscoverySpecs))
+	var err error
+	for i, spec := range o.NodeGroupAutoDiscoverySpecs {
+		cfgs[i], err = parseLabelAutoDiscoverySpec(spec)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return cfgs, nil
+}
+
+// parseLabelAutoDiscoverySpec parses a single spec and returns the corredponding node group spec.
+func parseLabelAutoDiscoverySpec(spec string) (labelAutoDiscoveryConfig, error) {
+	cfg := labelAutoDiscoveryConfig{
+		Selector: make(map[string]string),
+		MinSize:  1,
+		MaxSize:  -1,
+	}
+
+	tokens := strings.Split(spec, ":")
+	if len(tokens) != 2 {
+		return cfg, fmt.Errorf("spec \"%s\" should be discoverer:key=value,key=value", spec)
+	}
+	discoverer := tokens[0]
+	if discoverer != autoDiscovererTypeLabel {
+		return cfg, fmt.Errorf("unsupported discoverer specified: %s", discoverer)
+	}
+
+	for _, arg := range strings.Split(tokens[1], ",") {
+		kv := strings.Split(arg, "=")
+		if len(kv) != 2 {
+			return cfg, fmt.Errorf("invalid key=value pair %s", kv)
+		}
+
+		k, v := kv[0], kv[1]
+
+		switch k {
+		case labelAutoDiscovererKeyMinNodes:
+			if minSize, err := strconv.Atoi(v); err == nil {
+				cfg.MinSize = minSize
+			} else {
+				return cfg, fmt.Errorf("invalid minimum nodes: %s", v)
+			}
+		case labelAutoDiscovererKeyMaxNodes:
+			if maxSize, err := strconv.Atoi(v); err == nil {
+				cfg.MaxSize = maxSize
+			} else {
+				return cfg, fmt.Errorf("invalid maximum nodes: %s", v)
+			}
+		default:
+			cfg.Selector[k] = v
+		}
+	}
+	if cfg.MaxSize < 1 {
+		return cfg, fmt.Errorf("maximum size must be greater than 1 node")
+	}
+	if cfg.MaxSize < cfg.MinSize {
+		return cfg, fmt.Errorf("maximum size must be greater than minimum size")
+	}
+	return cfg, nil
+}