From f70f753acd77282e1a0e654e20b233b23eaa5a8e Mon Sep 17 00:00:00 2001 From: "Michael S. Fischer" Date: Sun, 7 Jun 2020 01:38:32 +0000 Subject: [PATCH] AWS: Use smallest instance type found in MixedInstancesPolicy --- .../cloudprovider/aws/aws_manager.go | 8 +- cluster-autoscaler/cloudprovider/aws/ec2.go | 27 ++++++ .../cloudprovider/aws/ec2_test.go | 83 +++++++++++++++++++ cluster-autoscaler/go.mod-extra | 1 - cluster-autoscaler/hack/update-vendor.sh | 2 +- 5 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 cluster-autoscaler/cloudprovider/aws/ec2_test.go diff --git a/cluster-autoscaler/cloudprovider/aws/aws_manager.go b/cluster-autoscaler/cloudprovider/aws/aws_manager.go index fa33bc9a7324..2948143ef8df 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_manager.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_manager.go @@ -331,9 +331,11 @@ func (m *AwsManager) buildInstanceType(asg *asg) (string, error) { } else if asg.LaunchTemplate != nil { return m.ec2Service.getInstanceTypeByLT(asg.LaunchTemplate) } else if asg.MixedInstancesPolicy != nil { - // always use first instance - if len(asg.MixedInstancesPolicy.instanceTypesOverrides) != 0 { - return asg.MixedInstancesPolicy.instanceTypesOverrides[0], nil + // Use smallest instance of all the instance types. Otherwise, we run + // the risk of launching an instance that is too small for the pending + // pod to actually fit in. + if len(asg.MixedInstancesPolicy.instanceTypesOverrides) > 0 { + return smallestInstanceType(asg.MixedInstancesPolicy.instanceTypesOverrides), nil } return m.ec2Service.getInstanceTypeByLT(asg.MixedInstancesPolicy.launchTemplate) diff --git a/cluster-autoscaler/cloudprovider/aws/ec2.go b/cluster-autoscaler/cloudprovider/aws/ec2.go index dee33760ee1a..6d027893a04b 100644 --- a/cluster-autoscaler/cloudprovider/aws/ec2.go +++ b/cluster-autoscaler/cloudprovider/aws/ec2.go @@ -18,6 +18,7 @@ package aws import ( "fmt" + "math" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -55,3 +56,29 @@ func (m ec2Wrapper) getInstanceTypeByLT(launchTemplate *launchTemplate) (string, return aws.StringValue(instanceType), nil } + +// smallestInstanceType returns the smallest EC2 instance type +// from the list of provided instance types. +func smallestInstanceType(instanceTypes []string) (smallest string) { + var ( + leastCPU int64 = math.MaxInt64 + leastMem int64 = math.MaxInt64 + leastGPU int64 = math.MaxInt64 + ) + for _, instanceType := range instanceTypes { + candidate := InstanceTypes[instanceType] + if candidate.VCPU < leastCPU { + smallest = candidate.InstanceType + leastCPU = candidate.VCPU + } + if candidate.MemoryMb < leastMem { + smallest = candidate.InstanceType + leastMem = candidate.MemoryMb + } + if candidate.GPU < leastGPU { + smallest = candidate.InstanceType + leastGPU = candidate.GPU + } + } + return smallest +} diff --git a/cluster-autoscaler/cloudprovider/aws/ec2_test.go b/cluster-autoscaler/cloudprovider/aws/ec2_test.go new file mode 100644 index 000000000000..2c7a880282fc --- /dev/null +++ b/cluster-autoscaler/cloudprovider/aws/ec2_test.go @@ -0,0 +1,83 @@ +package aws + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSmallestInstanceTypeSameFamily(t *testing.T) { + instanceTypes := []string{ + "c5.large", + "c5.xlarge", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, "c5.large") +} + +func TestSmallestInstanceTypeSameCPUDifferentMem(t *testing.T) { + instanceTypes := []string{ + "c4.large", + "c5.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, "c4.large") +} + +func TestSmallestInstanceTypeSameMemDifferentCPU(t *testing.T) { + instanceTypes := []string{ + "c5.xlarge", + "m4.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, "m4.large") +} + +func TestSmallestInstanceTypeSameFamilyDifferentProcessor(t *testing.T) { + { + instanceTypes := []string{ + "m5a.large", + "m5.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, instanceTypes[0]) + } + { + + instanceTypes := []string{ + "m5.large", + "m5a.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, instanceTypes[0]) + } +} + +func TestSmallestInstanceEqualButHasStorage(t *testing.T) { + { + instanceTypes := []string{ + "c5d.large", + "c5.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, instanceTypes[0]) + } + + { + instanceTypes := []string{ + "c5.large", + "c5d.large", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, instanceTypes[0]) + } +} + +func TestSmallestInstanceTypeCompareGPU(t *testing.T) { + instanceTypes := []string{ + "p3.2xlarge", + "r3.2xlarge", + } + smallest := smallestInstanceType(instanceTypes) + assert.Equal(t, smallest, "r3.2xlarge") +} diff --git a/cluster-autoscaler/go.mod-extra b/cluster-autoscaler/go.mod-extra index b36973eeed4e..4800ed5cdfb3 100644 --- a/cluster-autoscaler/go.mod-extra +++ b/cluster-autoscaler/go.mod-extra @@ -3,7 +3,6 @@ go 1.12 require ( github.com/rancher/go-rancher v0.1.0 - github.com/google/go-querystring v1.0.0 github.com/aws/aws-sdk-go v1.23.12 ) diff --git a/cluster-autoscaler/hack/update-vendor.sh b/cluster-autoscaler/hack/update-vendor.sh index 12f4b925980b..3f8bcfb9ff49 100755 --- a/cluster-autoscaler/hack/update-vendor.sh +++ b/cluster-autoscaler/hack/update-vendor.sh @@ -17,7 +17,7 @@ fi SCRIPT_NAME=$(basename "$0") K8S_FORK="git@github.com:kubernetes/kubernetes.git" -K8S_REV="master" +K8S_REV="release-1.16" BATCH_MODE="false" TARGET_MODULE=${TARGET_MODULE:-k8s.io/autoscaler/cluster-autoscaler}