Skip to content

Commit

Permalink
Merge pull request #990 from Danil-Grigorev/handle-no-id-gracefully
Browse files Browse the repository at this point in the history
Prevent panic on no launch template ID
  • Loading branch information
mjura authored Dec 9, 2024
2 parents bac40d9 + 8c4bed5 commit 14cfacc
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/eks/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func CreateNodeGroup(ctx context.Context, opts *CreateNodeGroupOptions) (string,
}

_, err = opts.EKSService.CreateNodegroup(ctx, nodeGroupCreateInput)
if err != nil {
if err != nil && lt.ID != nil {
// If there was an error creating the node group, then the template version should be deleted
// to prevent many launch template versions from being created before the issue is fixed.
DeleteLaunchTemplateVersions(ctx, opts.EC2Service, *lt.ID, []*string{launchTemplateVersion})
Expand Down
52 changes: 52 additions & 0 deletions pkg/eks/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,58 @@ var _ = Describe("CreateNodeGroup", func() {
Expect(launchTemplateVersion).To(Equal("1"))
Expect(generatedNodeRole).To(Equal("test"))
})

It("handles no id case gracefully", func() {
createNodeGroupOpts.NodeGroup.ImageID = nil
ec2ServiceMock.EXPECT().CreateLaunchTemplateVersion(ctx, gomock.Any()).Return(&ec2.CreateLaunchTemplateVersionOutput{
LaunchTemplateVersion: &ec2types.LaunchTemplateVersion{
LaunchTemplateName: aws.String("test"),
LaunchTemplateId: nil,
VersionNumber: aws.Int64(1),
},
}, nil)

cloudFormationServiceMock.EXPECT().CreateStack(ctx, gomock.Any()).Return(nil, nil)

cloudFormationServiceMock.EXPECT().DescribeStacks(ctx, gomock.Any()).Return(
&cloudformation.DescribeStacksOutput{
Stacks: []cftypes.Stack{
{
StackStatus: createCompleteStatus,
Outputs: []cftypes.Output{
{
OutputKey: aws.String("NodeInstanceRole"),
OutputValue: aws.String("test"),
},
},
},
},
}, nil)

eksServiceMock.EXPECT().CreateNodegroup(ctx, &eks.CreateNodegroupInput{
ClusterName: aws.String(createNodeGroupOpts.Config.Spec.DisplayName),
NodegroupName: createNodeGroupOpts.NodeGroup.NodegroupName,
Labels: aws.ToStringMap(createNodeGroupOpts.NodeGroup.Labels),
ScalingConfig: &ekstypes.NodegroupScalingConfig{
DesiredSize: createNodeGroupOpts.NodeGroup.DesiredSize,
MaxSize: createNodeGroupOpts.NodeGroup.MaxSize,
MinSize: createNodeGroupOpts.NodeGroup.MinSize,
},
CapacityType: ekstypes.CapacityTypesSpot,
LaunchTemplate: &ekstypes.LaunchTemplateSpecification{
Id: nil,
Version: aws.String("1"),
},
InstanceTypes: createNodeGroupOpts.NodeGroup.SpotInstanceTypes,
Subnets: createNodeGroupOpts.NodeGroup.Subnets,
NodeRole: aws.String("test"),
AmiType: ekstypes.AMITypesAl2023X8664Standard,
}).Return(nil, errors.New("error"))

_, _, err := CreateNodeGroup(ctx, createNodeGroupOpts)
Expect(err).To(HaveOccurred())
})

It("set resource tags", func() {
createNodeGroupOpts.NodeGroup.ResourceTags = map[string]string{
"tag1": "val1",
Expand Down

0 comments on commit 14cfacc

Please sign in to comment.