Merge pull request #7661 from yuxiang-zhang/7644

Include MixedInstancesPolicy LaunchTemplate for validation
eksctl-io · Mar 22, 2024 · cfec0d9 · cfec0d9
2 parents 78e6711 + 42cab35
commit cfec0d9
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 4 deletions.
diff --git a/pkg/actions/nodegroup/scale.go b/pkg/actions/nodegroup/scale.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/autoscaling"
+	autoscalingtypes "github.com/aws/aws-sdk-go-v2/service/autoscaling/types"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	awseks "github.com/aws/aws-sdk-go-v2/service/eks"
 	ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
@@ -162,10 +163,20 @@ func validateNodeGroupAMI(ctx context.Context, awsProvider api.ClusterProvider,
 	if len(asg.AutoScalingGroups) != 1 {
 		return fmt.Errorf("expected to find exactly one Auto Scaling group for nodegroup; got %d", len(asg.AutoScalingGroups))
 	}
-	lt := asg.AutoScalingGroups[0].LaunchTemplate
-	if lt == nil {
-		logger.Warning("nodegroup with Auto Scaling group %q does not have a launch template", asgName)
-		return nil
+
+	var lt *autoscalingtypes.LaunchTemplateSpecification
+	if asg.AutoScalingGroups[0].MixedInstancesPolicy == nil {
+		lt = asg.AutoScalingGroups[0].LaunchTemplate
+		if lt == nil {
+			logger.Warning("nodegroup with Auto Scaling group %q does not have a launch template", asgName)
+			return nil
+		}
+	} else {
+		// ASG only use the LT in MixedInstancesPolicy if the ASG has a MixedInstancesPolicy
+		if asg.AutoScalingGroups[0].MixedInstancesPolicy.LaunchTemplate == nil {
+			return fmt.Errorf("expected the MixedInstancesPolicy in Auto Scaling group %q to include a launch template", asgName)
+		}
+		lt = asg.AutoScalingGroups[0].MixedInstancesPolicy.LaunchTemplate.LaunchTemplateSpecification
 	}
 
 	ltData, err := awsProvider.EC2().DescribeLaunchTemplateVersions(ctx, &ec2.DescribeLaunchTemplateVersionsInput{

diff --git a/pkg/actions/nodegroup/scale_test.go b/pkg/actions/nodegroup/scale_test.go
@@ -198,6 +198,75 @@ var _ = Describe("Scale", func() {
 			}
 		}
 
+		mockMixedInstanceNodeGroupAMI := func(missingLaunchTemplate bool, asgName string) {
+			asgOutputWithMixedInstancesPolicy := &autoscaling.DescribeAutoScalingGroupsOutput{
+				AutoScalingGroups: []autoscalingtypes.AutoScalingGroup{
+					{
+						MixedInstancesPolicy: &autoscalingtypes.MixedInstancesPolicy{},
+					},
+				},
+			}
+			if !missingLaunchTemplate {
+				asgOutputWithMixedInstancesPolicy.AutoScalingGroups[0].MixedInstancesPolicy.LaunchTemplate = &autoscalingtypes.LaunchTemplate{
+					LaunchTemplateSpecification: &autoscalingtypes.LaunchTemplateSpecification{
+						LaunchTemplateId:   aws.String("lt-1234"),
+						LaunchTemplateName: aws.String("eksctl-test-ng"),
+						Version:            aws.String("1"),
+					},
+				}
+			}
+			p.MockASG().On("DescribeAutoScalingGroups", mock.Anything, mock.MatchedBy(func(input *autoscaling.DescribeAutoScalingGroupsInput) bool {
+				return len(input.AutoScalingGroupNames) == 1 && input.AutoScalingGroupNames[0] == asgName
+			})).Return(asgOutputWithMixedInstancesPolicy, nil)
+			p.MockEC2().On("DescribeLaunchTemplateVersions", mock.Anything, mock.MatchedBy(func(input *ec2.DescribeLaunchTemplateVersionsInput) bool {
+				return len(input.Versions) == 1 && input.LaunchTemplateId != nil && *input.LaunchTemplateId == "lt-1234"
+			})).Return(&ec2.DescribeLaunchTemplateVersionsOutput{
+				LaunchTemplateVersions: []ec2types.LaunchTemplateVersion{
+					{
+						LaunchTemplateData: &ec2types.ResponseLaunchTemplateData{
+							ImageId: aws.String("ami-1234"),
+						},
+					},
+				},
+			}, nil)
+			describeImagesOutput := &ec2.DescribeImagesOutput{
+				Images: []ec2types.Image{
+					{
+						ImageId: aws.String("ami-1234"),
+					},
+				},
+			}
+			p.MockEC2().On("DescribeImages", mock.Anything, mock.MatchedBy(func(input *ec2.DescribeImagesInput) bool {
+				return len(input.ImageIds) == 1 && input.ImageIds[0] == "ami-1234"
+			})).Return(describeImagesOutput, nil)
+
+			p.MockASG().On("UpdateAutoScalingGroup", mock.Anything, &autoscaling.UpdateAutoScalingGroupInput{
+				AutoScalingGroupName: aws.String(asgName),
+				MinSize:              aws.Int32(1),
+				DesiredCapacity:      aws.Int32(3),
+			}).Return(nil, nil)
+		}
+
+		When("the ASG exists with a MixedInstancesPolicy", func() {
+			asgName := "asg-name"
+
+			BeforeEach(func() {
+				mockNodeGroupStack(ngName, asgName)
+			})
+
+			It("fails for missing launch template", func() {
+				mockMixedInstanceNodeGroupAMI(true, asgName)
+				err := m.Scale(context.Background(), ng, false)
+				Expect(err).To(MatchError(fmt.Sprintf("failed to scale nodegroup %q for cluster %q, error: expected the MixedInstancesPolicy in Auto Scaling group %q to include a launch template", ng.Name, clusterName, asgName)))
+			})
+
+			It("scales the nodegroup", func() {
+				mockMixedInstanceNodeGroupAMI(false, asgName)
+				err := m.Scale(context.Background(), ng, false)
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+
 		When("the ASG exists", func() {
 			BeforeEach(func() {
 				mockNodeGroupStack(ngName, "asg-name")