kubernetes
/
autoscaler
mirror of https://github.com/kubernetes/autoscaler.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2714 from marwanad/fix-az-autodiscovery 

Fix azure autodiscovery when max tags aren't specified
This commit is contained in:
Kubernetes Prow Robot 2020-01-07 00:40:18 -08:00 committed by GitHub
parent f7843fb5d1 9d7738933a
commit 9831fc0404
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 273 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
cluster-autoscaler/cloudprovider/azure/azure_fakes.go
View File

@ -285,3 +285,18 @@ func (m *DeploymentsClientMock) Delete(ctx context.Context, resourceGroupName, d
return
}
func fakeVMSSWithTags(vmssName string, tags map[string]*string) compute.VirtualMachineScaleSet {
skuName := "Standard_D4_v2"
var vmssCapacity int64 = 3
return compute.VirtualMachineScaleSet{
Name: &vmssName,
Sku: &compute.Sku{
Capacity: &vmssCapacity,
Name: &skuName,
},
Tags: tags,
}
}

39
cluster-autoscaler/cloudprovider/azure/azure_manager.go
View File

@ -65,10 +65,6 @@ var validLabelAutoDiscovererKeys = strings.Join([]string{
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.
@ -456,7 +452,10 @@ func (m *AzureManager) listScaleSets(filter []labelAutoDiscoveryConfig) (asgs []
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)
return asgs, fmt.Errorf("no minimum size specified for vmss: %s", *scaleSet.Name)
}
if spec.MinSize < 0 {
return asgs, fmt.Errorf("minimum size must be a non-negative number of nodes")
}
if val, ok := scaleSet.Tags["max"]; ok {
if maxSize, err := strconv.Atoi(*val); err == nil {
@ -465,7 +464,7 @@ func (m *AzureManager) listScaleSets(filter []labelAutoDiscoveryConfig) (asgs []
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)
return asgs, fmt.Errorf("no maximum size specified for vmss: %s", *scaleSet.Name)
}
if spec.MaxSize < 1 {
return asgs, fmt.Errorf("maximum size must be greater than 1 node")
@ -530,8 +529,6 @@ func parseLabelAutoDiscoverySpecs(o cloudprovider.NodeGroupDiscoveryOptions) ([]
func parseLabelAutoDiscoverySpec(spec string) (labelAutoDiscoveryConfig, error) {
cfg := labelAutoDiscoveryConfig{
Selector: make(map[string]string),
MinSize: 1,
MaxSize: -1,
}
tokens := strings.Split(spec, ":")
@ -548,31 +545,11 @@ func parseLabelAutoDiscoverySpec(spec string) (labelAutoDiscoveryConfig, error)
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 k == "" || v == "" {
return cfg, fmt.Errorf("empty value not allowed in key=value tag pairs")
}
}
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")
cfg.Selector[k] = v
}
return cfg, nil
}

250
cluster-autoscaler/cloudprovider/azure/azure_manager_test.go
View File

@ -17,9 +17,11 @@ limitations under the License.
package azure
import (
"fmt"
"strings"
"testing"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
"github.com/stretchr/testify/assert"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
)
@ -65,3 +67,251 @@ func TestCreateAzureManagerInvalidConfig(t *testing.T) {
_, err := CreateAzureManager(strings.NewReader(invalidAzureCfg), cloudprovider.NodeGroupDiscoveryOptions{})
assert.Error(t, err, "failed to unmarshal config body")
}
func TestFetchExplicitAsgs(t *testing.T) {
min, max, name := 1, 15, "test-asg"
ngdo := cloudprovider.NodeGroupDiscoveryOptions{
NodeGroupSpecs: []string{
fmt.Sprintf("%d:%d:%s", min, max, name),
},
}
manager := newTestAzureManager(t)
manager.fetchExplicitAsgs(ngdo.NodeGroupSpecs)
asgs := manager.asgCache.get()
assert.Equal(t, 1, len(asgs))
assert.Equal(t, name, asgs[0].Id())
assert.Equal(t, min, asgs[0].MinSize())
assert.Equal(t, max, asgs[0].MaxSize())
}
func TestParseLabelAutoDiscoverySpecs(t *testing.T) {
testCases := []struct {
name string
specs []string
expected []labelAutoDiscoveryConfig
expectedErr bool
}{
{
name: "ValidSpec",
specs: []string{
"label:cluster-autoscaler-enabled=true,cluster-autoscaler-name=fake-cluster",
"label:test-tag=test-value,another-test-tag=another-test-value",
},
expected: []labelAutoDiscoveryConfig{
{Selector: map[string]string{"cluster-autoscaler-enabled": "true", "cluster-autoscaler-name": "fake-cluster"}},
{Selector: map[string]string{"test-tag": "test-value", "another-test-tag": "another-test-value"}},
},
},
{
name: "MissingAutoDiscoverLabel",
specs: []string{"test-tag=test-value,another-test-tag"},
expectedErr: true,
},
{
name: "InvalidAutoDiscoerLabel",
specs: []string{"invalid:test-tag=test-value,another-test-tag"},
expectedErr: true,
},
{
name: "MissingValue",
specs: []string{"label:test-tag="},
expectedErr: true,
},
{
name: "MissingKey",
specs: []string{"label:=test-val"},
expectedErr: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ngdo := cloudprovider.NodeGroupDiscoveryOptions{NodeGroupAutoDiscoverySpecs: tc.specs}
actual, err := parseLabelAutoDiscoverySpecs(ngdo)
if tc.expectedErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.True(t, assert.ObjectsAreEqualValues(tc.expected, actual), "expected %#v, but found: %#v", tc.expected, actual)
})
}
}
func TestListScalesets(t *testing.T) {
manager := newTestAzureManager(t)
vmssTag := "fake-tag"
vmssTagValue := "fake-value"
vmssName := "test-vmss"
ngdo := cloudprovider.NodeGroupDiscoveryOptions{
NodeGroupAutoDiscoverySpecs: []string{fmt.Sprintf("label:%s=%s", vmssTag, vmssTagValue)},
}
specs, err := parseLabelAutoDiscoverySpecs(ngdo)
assert.NoError(t, err)
testCases := []struct {
name string
specs map[string]string
expected []cloudprovider.NodeGroup
expectedErrString string
}{
{
name: "ValidMinMax",
specs: map[string]string{"min": "5", "max": "50"},
expected: []cloudprovider.NodeGroup{&ScaleSet{
azureRef: azureRef{
Name: vmssName,
},
minSize: 5,
maxSize: 50,
manager: manager,
curSize: -1,
}},
},
{
name: "InvalidMin",
specs: map[string]string{"min": "some-invalid-string"},
expectedErrString: "invalid minimum size specified for vmss:",
},
{
name: "NoMin",
specs: map[string]string{"max": "50"},
expectedErrString: fmt.Sprintf("no minimum size specified for vmss: %s", vmssName),
},
{
name: "InvalidMax",
specs: map[string]string{"min": "5", "max": "some-invalid-string"},
expectedErrString: "invalid maximum size specified for vmss:",
},
{
name: "NoMax",
specs: map[string]string{"min": "5"},
expectedErrString: fmt.Sprintf("no maximum size specified for vmss: %s", vmssName),
},
{
name: "MaxLessThanOne",
specs: map[string]string{"min": "5", "max": "-5"},
expectedErrString: "maximum size must be greater than 1 node",
},
{
name: "MinLessThanZero",
specs: map[string]string{"min": "-4", "max": "20"},
expectedErrString: fmt.Sprintf("minimum size must be a non-negative number of nodes"),
},
{
name: "MinGreaterThanMax",
specs: map[string]string{"min": "50", "max": "5"},
expectedErrString: "maximum size must be greater than minimum size",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tags := make(map[string]*string)
tags[vmssTag] = &vmssTagValue
if val, ok := tc.specs["min"]; ok {
tags["min"] = &val
}
if val, ok := tc.specs["max"]; ok {
tags["max"] = &val
}
scaleSetClient := &VirtualMachineScaleSetsClientMock{
FakeStore: map[string]map[string]compute.VirtualMachineScaleSet{
"test": {
vmssName: fakeVMSSWithTags(vmssName, tags),
},
},
}
manager.azClient.virtualMachineScaleSetsClient = scaleSetClient
asgs, err := manager.listScaleSets(specs)
if tc.expectedErrString != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErrString)
return
}
assert.NoError(t, err)
assert.True(t, assert.ObjectsAreEqualValues(tc.expected, asgs), "expected %#v, but found: %#v", tc.expected, asgs)
})
}
}
func TestGetFilteredAutoscalingGroupsVmss(t *testing.T) {
vmssName := "test-vmss"
vmssTag := "fake-tag"
vmssTagValue := "fake-value"
min := "1"
minVal := 1
max := "5"
maxVal := 5
ngdo := cloudprovider.NodeGroupDiscoveryOptions{
NodeGroupAutoDiscoverySpecs: []string{fmt.Sprintf("label:%s=%s", vmssTag, vmssTagValue)},
}
scaleSetClient := &VirtualMachineScaleSetsClientMock{
FakeStore: map[string]map[string]compute.VirtualMachineScaleSet{
"test": {
vmssName: fakeVMSSWithTags(vmssName, map[string]*string{vmssTag: &vmssTagValue, "min": &min, "max": &max}),
},
},
}
manager := newTestAzureManager(t)
manager.azClient.virtualMachineScaleSetsClient = scaleSetClient
specs, err := parseLabelAutoDiscoverySpecs(ngdo)
assert.NoError(t, err)
asgs, err := manager.getFilteredAutoscalingGroups(specs)
assert.NoError(t, err)
expectedAsgs := []cloudprovider.NodeGroup{&ScaleSet{
azureRef: azureRef{
Name: vmssName,
},
minSize: minVal,
maxSize: maxVal,
manager: manager,
curSize: -1,
}}
assert.True(t, assert.ObjectsAreEqualValues(expectedAsgs, asgs), "expected %#v, but found: %#v", expectedAsgs, asgs)
}
func TestFetchAutoAsgsVmss(t *testing.T) {
vmssName := "test-vmss"
vmssTag := "fake-tag"
vmssTagValue := "fake-value"
minString := "1"
minVal := 1
maxString := "5"
maxVal := 5
ngdo := cloudprovider.NodeGroupDiscoveryOptions{
NodeGroupAutoDiscoverySpecs: []string{fmt.Sprintf("label:%s=%s", vmssTag, vmssTagValue)},
}
scaleSetClient := &VirtualMachineScaleSetsClientMock{
FakeStore: map[string]map[string]compute.VirtualMachineScaleSet{
"test": {
vmssName: fakeVMSSWithTags(vmssName, map[string]*string{vmssTag: &vmssTagValue, "min": &minString, "max": &maxString}),
},
},
}
manager := newTestAzureManager(t)
manager.azClient.virtualMachineScaleSetsClient = scaleSetClient
specs, err := parseLabelAutoDiscoverySpecs(ngdo)
assert.NoError(t, err)
manager.asgAutoDiscoverySpecs = specs
// assert cache is empty before fetching auto asgs
asgs := manager.asgCache.get()
assert.Equal(t, 0, len(asgs))
manager.fetchAutoAsgs()
asgs = manager.asgCache.get()
assert.Equal(t, 1, len(asgs))
assert.Equal(t, vmssName, asgs[0].Id())
assert.Equal(t, minVal, asgs[0].MinSize())
assert.Equal(t, maxVal, asgs[0].MaxSize())
}