Add MixedInstancesPolicy struct to better handle instance type

cluster-autoscaler/cloudprovider/aws/README.md

@@ -179,6 +179,8 @@ If you'd like to scale node groups from 0, an `autoscaling:DescribeLaunchConfigu
 
 ## Using AutoScalingGroup MixedInstancesPolicy
 
+> Note: The minimum version of cluster autoscaler to support MixedInstancePolicy is v1.14.x.
+
 It is possible to use Cluster Autoscaler with a [mixed instances policy](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-autoscaling-autoscalinggroup-mixedinstancespolicy.html), to enable diversification across on-demand and spot instances, of multiple instance types in a single ASG. When using spot instances, this increases the likelihood of successfully launching a spot instance to add the desired capacity to the cluster versus a single instance type, which may be in short supply.
 
 Note that the instance types should have the same amount of RAM and number of CPU cores, since this is fundamental to CA's scaling calculations. Using mismatched instances types can produce unintended results.

cluster-autoscaler/cloudprovider/aws/auto_scaling_groups.go

@@ -44,6 +44,16 @@ type asgCache struct {
 	explicitlyConfigured  map[AwsRef]bool
 }
 
+type launchTemplate struct {
+	name    string
+	version string
+}
+
+type mixedInstancesPolicy struct {
+	launchTemplate         *launchTemplate
+	instanceTypesOverrides []string
+}
+
 type asg struct {
 	AwsRef
 
@@ -52,9 +62,9 @@ type asg struct {
 	curSize int
 
 	AvailabilityZones       []string
-	LaunchTemplateName      string
-	LaunchTemplateVersion   string
 	LaunchConfigurationName string
+	LaunchTemplate          *launchTemplate
+	MixedInstancesPolicy    *mixedInstancesPolicy
 	Tags                    []*autoscaling.TagDescription
 }
 
@@ -114,8 +124,8 @@ func (m *asgCache) register(asg *asg) *asg {
 			// from zero
 			existing.AvailabilityZones = asg.AvailabilityZones
 			existing.LaunchConfigurationName = asg.LaunchConfigurationName
-			existing.LaunchTemplateName = asg.LaunchTemplateName
-			existing.LaunchTemplateVersion = asg.LaunchTemplateVersion
+			existing.LaunchTemplate = asg.LaunchTemplate
+			existing.MixedInstancesPolicy = asg.MixedInstancesPolicy
 			existing.Tags = asg.Tags
 
 			return existing
@@ -367,8 +377,6 @@ func (m *asgCache) buildAsgFromAWS(g *autoscaling.Group) (*asg, error) {
 		return nil, fmt.Errorf("failed to create node group spec: %v", verr)
 	}
 
-	launchTemplateName, launchTemplateVersion := m.buildLaunchTemplateParams(g)
-
 	asg := &asg{
 		AwsRef:  AwsRef{Name: spec.Name},
 		minSize: spec.MinSize,
@@ -377,23 +385,36 @@ func (m *asgCache) buildAsgFromAWS(g *autoscaling.Group) (*asg, error) {
 		curSize:                 int(aws.Int64Value(g.DesiredCapacity)),
 		AvailabilityZones:       aws.StringValueSlice(g.AvailabilityZones),
 		LaunchConfigurationName: aws.StringValue(g.LaunchConfigurationName),
-		LaunchTemplateName:      launchTemplateName,
-		LaunchTemplateVersion:   launchTemplateVersion,
 		Tags:                    g.Tags,
 	}
 
+	if g.LaunchTemplate != nil {
+		asg.LaunchTemplate = m.buildLaunchTemplateFromSpec(g.LaunchTemplate)
+	}
+
+	if g.MixedInstancesPolicy != nil {
+		getInstanceTypes := func(data []*autoscaling.LaunchTemplateOverrides) []string {
+			res := make([]string, len(data))
+			for i := 0; i < len(data); i++ {
+				res[i] = aws.StringValue(data[i].InstanceType)
+			}
+			return res
+		}
+
+		asg.MixedInstancesPolicy = &mixedInstancesPolicy{
+			launchTemplate:         m.buildLaunchTemplateFromSpec(g.MixedInstancesPolicy.LaunchTemplate.LaunchTemplateSpecification),
+			instanceTypesOverrides: getInstanceTypes(g.MixedInstancesPolicy.LaunchTemplate.Overrides),
+		}
+	}
+
 	return asg, nil
 }
 
-func (m *asgCache) buildLaunchTemplateParams(g *autoscaling.Group) (string, string) {
-	if g.LaunchTemplate != nil {
-		return aws.StringValue(g.LaunchTemplate.LaunchTemplateName), aws.StringValue(g.LaunchTemplate.Version)
-	} else if g.MixedInstancesPolicy != nil && g.MixedInstancesPolicy.LaunchTemplate != nil {
-		return aws.StringValue(g.MixedInstancesPolicy.LaunchTemplate.LaunchTemplateSpecification.LaunchTemplateName),
-			aws.StringValue(g.MixedInstancesPolicy.LaunchTemplate.LaunchTemplateSpecification.Version)
+func (m *asgCache) buildLaunchTemplateFromSpec(ltSpec *autoscaling.LaunchTemplateSpecification) *launchTemplate {
+	return &launchTemplate{
+		name:    aws.StringValue(ltSpec.LaunchTemplateName),
+		version: aws.StringValue(ltSpec.Version),
 	}
-
-	return "", ""
 }
 
 func (m *asgCache) buildInstanceRefFromAWS(instance *autoscaling.Instance) AwsInstanceRef {

cluster-autoscaler/cloudprovider/aws/aws_manager.go

@@ -316,8 +316,15 @@ func (m *AwsManager) getAsgTemplate(asg *asg) (*asgTemplate, error) {
 func (m *AwsManager) buildInstanceType(asg *asg) (string, error) {
 	if asg.LaunchConfigurationName != "" {
 		return m.autoScalingService.getInstanceTypeByLCName(asg.LaunchConfigurationName)
-	} else if asg.LaunchTemplateName != "" && asg.LaunchTemplateVersion != "" {
-		return m.ec2Service.getInstanceTypeByLT(asg.LaunchTemplateName, asg.LaunchTemplateVersion)
+	} 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
+		}
+
+		return m.ec2Service.getInstanceTypeByLT(asg.MixedInstancesPolicy.launchTemplate)
 	}
 
 	return "", errors.New("Unable to get instance type from launch config or launch template")

cluster-autoscaler/cloudprovider/aws/aws_manager_test.go

@@ -258,8 +258,7 @@ func TestBuildInstanceType(t *testing.T) {
 	assert.NoError(t, err)
 
 	asg := asg{
-		LaunchTemplateName:    ltName,
-		LaunchTemplateVersion: ltVersion,
+		LaunchTemplate: &launchTemplate{name: ltName, version: ltVersion},
 	}
 
 	builtInstanceType, err := m.buildInstanceType(&asg)
@@ -268,6 +267,66 @@ func TestBuildInstanceType(t *testing.T) {
 	assert.Equal(t, instanceType, builtInstanceType)
 }
 
+func TestBuildInstanceTypeMixedInstancePolicyOverride(t *testing.T) {
+	ltName, ltVersion, instanceType := "launcher", "1", "t2.large"
+	instanceTypes := []string{}
+
+	s := &EC2Mock{}
+	s.On("DescribeLaunchTemplateVersions", &ec2.DescribeLaunchTemplateVersionsInput{
+		LaunchTemplateName: aws.String(ltName),
+		Versions:           []*string{aws.String(ltVersion)},
+	}).Return(&ec2.DescribeLaunchTemplateVersionsOutput{
+		LaunchTemplateVersions: []*ec2.LaunchTemplateVersion{
+			{
+				LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
+					InstanceType: aws.String(instanceType),
+				},
+			},
+		},
+	})
+
+	defer resetAWSRegion(os.LookupEnv("AWS_REGION"))
+	os.Setenv("AWS_REGION", "fanghorn")
+	m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{s})
+	assert.NoError(t, err)
+
+	lt := &launchTemplate{name: ltName, version: ltVersion}
+	asg := asg{
+		MixedInstancesPolicy: &mixedInstancesPolicy{
+			launchTemplate:         lt,
+			instanceTypesOverrides: instanceTypes,
+		},
+	}
+
+	builtInstanceType, err := m.buildInstanceType(&asg)
+
+	assert.NoError(t, err)
+	assert.Equal(t, instanceType, builtInstanceType)
+}
+
+func TestBuildInstanceTypeMixedInstancePolicyNoOverride(t *testing.T) {
+	ltName, ltVersion := "launcher", "1"
+	instanceTypes := []string{"m4.xlarge", "m5.xlarge"}
+
+	defer resetAWSRegion(os.LookupEnv("AWS_REGION"))
+	os.Setenv("AWS_REGION", "fanghorn")
+	m, err := createAWSManagerInternal(nil, cloudprovider.NodeGroupDiscoveryOptions{}, nil, &ec2Wrapper{})
+	assert.NoError(t, err)
+
+	lt := &launchTemplate{name: ltName, version: ltVersion}
+	asg := asg{
+		MixedInstancesPolicy: &mixedInstancesPolicy{
+			launchTemplate:         lt,
+			instanceTypesOverrides: instanceTypes,
+		},
+	}
+
+	builtInstanceType, err := m.buildInstanceType(&asg)
+
+	assert.NoError(t, err)
+	assert.Equal(t, instanceTypes[0], builtInstanceType)
+}
+
 func TestGetASGTemplate(t *testing.T) {
 	const (
 		knownInstanceType = "t3.micro"
@@ -323,11 +382,12 @@ func TestGetASGTemplate(t *testing.T) {
 			assert.NoError(t, err)
 
 			asg := &asg{
-				AwsRef:                AwsRef{Name: "sample"},
-				AvailabilityZones:     test.availabilityZones,
-				LaunchTemplateName:    ltName,
-				LaunchTemplateVersion: ltVersion,
-				Tags:                  tags,
+				AwsRef:            AwsRef{Name: "sample"},
+				AvailabilityZones: test.availabilityZones,
+				LaunchTemplate: &launchTemplate{
+					name:    ltName,
+					version: ltVersion},
+				Tags: tags,
 			}
 
 			template, err := m.getAsgTemplate(asg)

cluster-autoscaler/cloudprovider/aws/ec2.go

@@ -31,10 +31,10 @@ type ec2Wrapper struct {
 	ec2I
 }
 
-func (m ec2Wrapper) getInstanceTypeByLT(name string, version string) (string, error) {
+func (m ec2Wrapper) getInstanceTypeByLT(launchTemplate *launchTemplate) (string, error) {
 	params := &ec2.DescribeLaunchTemplateVersionsInput{
-		LaunchTemplateName: aws.String(name),
-		Versions:           []*string{aws.String(version)},
+		LaunchTemplateName: aws.String(launchTemplate.name),
+		Versions:           []*string{aws.String(launchTemplate.version)},
 	}
 
 	describeData, err := m.DescribeLaunchTemplateVersions(params)