Skip to content

Commit f8ec0c6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5496 from giantswarm/fix-launchtemplate-needs-update
🐛 Launchtemplate needs to be updated if spot options are changed
2 parents 7b09356 + 99c1047 commit f8ec0c6

File tree

2 files changed

+241
-6
lines changed

2 files changed

+241
-6
lines changed

‎pkg/cloud/services/ec2/launchtemplate.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,24 @@ func (s *Service) deleteLaunchTemplateVersion(id string, version *int64) error {
680680
return nil
681681
}
682682

683+
// SDKToSpotMarketOptions converts EC2 instance market options to SpotMarketOptions.
684+
func SDKToSpotMarketOptions(instanceMarketOptions *ec2.LaunchTemplateInstanceMarketOptions) *infrav1.SpotMarketOptions {
685+
if instanceMarketOptions == nil || aws.StringValue(instanceMarketOptions.MarketType) != ec2.MarketTypeSpot {
686+
return nil
687+
}
688+
689+
if instanceMarketOptions.SpotOptions == nil {
690+
return &infrav1.SpotMarketOptions{}
691+
}
692+
693+
result := &infrav1.SpotMarketOptions{}
694+
if instanceMarketOptions.SpotOptions.MaxPrice != nil {
695+
result.MaxPrice = instanceMarketOptions.SpotOptions.MaxPrice
696+
}
697+
698+
return result
699+
}
700+
683701
// SDKToLaunchTemplate converts an AWS EC2 SDK instance to the CAPA instance type.
684702
func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1.AWSLaunchTemplate, string, *apimachinerytypes.NamespacedName, error) {
685703
v := d.LaunchTemplateData
@@ -688,9 +706,10 @@ func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1
688706
AMI: infrav1.AMIReference{
689707
ID: v.ImageId,
690708
},
691-
InstanceType: aws.StringValue(v.InstanceType),
692-
SSHKeyName: v.KeyName,
693-
VersionNumber: d.VersionNumber,
709+
InstanceType: aws.StringValue(v.InstanceType),
710+
SSHKeyName: v.KeyName,
711+
SpotMarketOptions: SDKToSpotMarketOptions(v.InstanceMarketOptions),
712+
VersionNumber: d.VersionNumber,
694713
}
695714

696715
if v.MetadataOptions != nil {
@@ -777,6 +796,9 @@ func (s *Service) LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, inc
777796
if !cmp.Equal(incoming.InstanceMetadataOptions, existing.InstanceMetadataOptions) {
778797
return true, nil
779798
}
799+
if !cmp.Equal(incoming.SpotMarketOptions, existing.SpotMarketOptions) {
800+
return true, nil
801+
}
780802

781803
incomingIDs, err := s.GetAdditionalSecurityGroupsIDs(incoming.AdditionalSecurityGroups)
782804
if err != nil {

‎pkg/cloud/services/ec2/launchtemplate_test.go

Lines changed: 216 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,139 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) {
353353
wantHash: testUserDataHash,
354354
wantDataSecretKey: nil, // respective tag is not given
355355
},
356+
{
357+
name: "spot market options",
358+
input: &ec2.LaunchTemplateVersion{
359+
LaunchTemplateId: aws.String("lt-12345"),
360+
LaunchTemplateName: aws.String("foo"),
361+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
362+
ImageId: aws.String("foo-image"),
363+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
364+
Arn: aws.String("instance-profile/foo-profile"),
365+
},
366+
KeyName: aws.String("foo-keyname"),
367+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
368+
MarketType: aws.String(ec2.MarketTypeSpot),
369+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{
370+
MaxPrice: aws.String("0.05"),
371+
},
372+
},
373+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
374+
},
375+
VersionNumber: aws.Int64(1),
376+
},
377+
wantLT: &expinfrav1.AWSLaunchTemplate{
378+
Name: "foo",
379+
AMI: infrav1.AMIReference{
380+
ID: aws.String("foo-image"),
381+
},
382+
IamInstanceProfile: "foo-profile",
383+
SSHKeyName: aws.String("foo-keyname"),
384+
VersionNumber: aws.Int64(1),
385+
SpotMarketOptions: &infrav1.SpotMarketOptions{
386+
MaxPrice: aws.String("0.05"),
387+
},
388+
},
389+
wantHash: testUserDataHash,
390+
wantDataSecretKey: nil,
391+
},
392+
{
393+
name: "spot market options with no max price",
394+
input: &ec2.LaunchTemplateVersion{
395+
LaunchTemplateId: aws.String("lt-12345"),
396+
LaunchTemplateName: aws.String("foo"),
397+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
398+
ImageId: aws.String("foo-image"),
399+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
400+
Arn: aws.String("instance-profile/foo-profile"),
401+
},
402+
KeyName: aws.String("foo-keyname"),
403+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
404+
MarketType: aws.String(ec2.MarketTypeSpot),
405+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{},
406+
},
407+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
408+
},
409+
VersionNumber: aws.Int64(1),
410+
},
411+
wantLT: &expinfrav1.AWSLaunchTemplate{
412+
Name: "foo",
413+
AMI: infrav1.AMIReference{
414+
ID: aws.String("foo-image"),
415+
},
416+
IamInstanceProfile: "foo-profile",
417+
SSHKeyName: aws.String("foo-keyname"),
418+
VersionNumber: aws.Int64(1),
419+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
420+
},
421+
wantHash: testUserDataHash,
422+
wantDataSecretKey: nil,
423+
},
424+
{
425+
name: "spot market options without SpotOptions",
426+
input: &ec2.LaunchTemplateVersion{
427+
LaunchTemplateId: aws.String("lt-12345"),
428+
LaunchTemplateName: aws.String("foo"),
429+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
430+
ImageId: aws.String("foo-image"),
431+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
432+
Arn: aws.String("instance-profile/foo-profile"),
433+
},
434+
KeyName: aws.String("foo-keyname"),
435+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
436+
MarketType: aws.String(ec2.MarketTypeSpot),
437+
},
438+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
439+
},
440+
VersionNumber: aws.Int64(1),
441+
},
442+
wantLT: &expinfrav1.AWSLaunchTemplate{
443+
Name: "foo",
444+
AMI: infrav1.AMIReference{
445+
ID: aws.String("foo-image"),
446+
},
447+
IamInstanceProfile: "foo-profile",
448+
SSHKeyName: aws.String("foo-keyname"),
449+
VersionNumber: aws.Int64(1),
450+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
451+
},
452+
wantHash: testUserDataHash,
453+
wantDataSecretKey: nil,
454+
},
455+
{
456+
name: "non-spot market type",
457+
input: &ec2.LaunchTemplateVersion{
458+
LaunchTemplateId: aws.String("lt-12345"),
459+
LaunchTemplateName: aws.String("foo"),
460+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
461+
ImageId: aws.String("foo-image"),
462+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
463+
Arn: aws.String("instance-profile/foo-profile"),
464+
},
465+
KeyName: aws.String("foo-keyname"),
466+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
467+
MarketType: aws.String("different-market-type"),
468+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{
469+
MaxPrice: aws.String("0.05"),
470+
},
471+
},
472+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
473+
},
474+
VersionNumber: aws.Int64(1),
475+
},
476+
wantLT: &expinfrav1.AWSLaunchTemplate{
477+
Name: "foo",
478+
AMI: infrav1.AMIReference{
479+
ID: aws.String("foo-image"),
480+
},
481+
IamInstanceProfile: "foo-profile",
482+
SSHKeyName: aws.String("foo-keyname"),
483+
VersionNumber: aws.Int64(1),
484+
SpotMarketOptions: nil, // Should be nil since market type is not "spot"
485+
},
486+
wantHash: testUserDataHash,
487+
wantDataSecretKey: nil,
488+
},
356489
{
357490
name: "tag of bootstrap secret",
358491
input: &ec2.LaunchTemplateVersion{
@@ -440,6 +573,20 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
440573
want bool
441574
wantErr bool
442575
}{
576+
{
577+
name: "only core security groups, order shouldn't matter",
578+
incoming: &expinfrav1.AWSLaunchTemplate{
579+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{},
580+
},
581+
existing: &expinfrav1.AWSLaunchTemplate{
582+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
583+
{ID: aws.String("sg-222")},
584+
{ID: aws.String("sg-111")},
585+
},
586+
},
587+
want: false,
588+
wantErr: false,
589+
},
443590
{
444591
name: "the same security groups",
445592
incoming: &expinfrav1.AWSLaunchTemplate{
@@ -497,6 +644,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
497644
IamInstanceProfile: DefaultAmiNameFormat,
498645
},
499646
existing: &expinfrav1.AWSLaunchTemplate{
647+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
648+
{ID: aws.String("sg-111")},
649+
{ID: aws.String("sg-222")},
650+
},
500651
IamInstanceProfile: "some-other-profile",
501652
},
502653
want: true,
@@ -507,6 +658,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
507658
InstanceType: "t3.micro",
508659
},
509660
existing: &expinfrav1.AWSLaunchTemplate{
661+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
662+
{ID: aws.String("sg-111")},
663+
{ID: aws.String("sg-222")},
664+
},
510665
InstanceType: "t3.large",
511666
},
512667
want: true,
@@ -540,9 +695,14 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
540695
HTTPTokens: infrav1.HTTPTokensStateRequired,
541696
},
542697
},
543-
existing: &expinfrav1.AWSLaunchTemplate{},
544-
want: true,
545-
wantErr: false,
698+
existing: &expinfrav1.AWSLaunchTemplate{
699+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
700+
{ID: aws.String("sg-111")},
701+
{ID: aws.String("sg-222")},
702+
},
703+
},
704+
want: true,
705+
wantErr: false,
546706
},
547707
{
548708
name: "new launch template instance metadata options, removing IMDSv2 requirement",
@@ -556,6 +716,59 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
556716
want: true,
557717
wantErr: false,
558718
},
719+
{
720+
name: "Should return true if incoming SpotMarketOptions is different from existing SpotMarketOptions",
721+
incoming: &expinfrav1.AWSLaunchTemplate{
722+
SpotMarketOptions: &infrav1.SpotMarketOptions{
723+
MaxPrice: aws.String("0.10"),
724+
},
725+
},
726+
existing: &expinfrav1.AWSLaunchTemplate{
727+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
728+
{ID: aws.String("sg-111")},
729+
{ID: aws.String("sg-222")},
730+
},
731+
SpotMarketOptions: &infrav1.SpotMarketOptions{
732+
MaxPrice: aws.String("0.05"),
733+
},
734+
},
735+
want: true,
736+
wantErr: false,
737+
},
738+
{
739+
name: "Should return true if incoming adds SpotMarketOptions and existing has none",
740+
incoming: &expinfrav1.AWSLaunchTemplate{
741+
SpotMarketOptions: &infrav1.SpotMarketOptions{
742+
MaxPrice: aws.String("0.10"),
743+
},
744+
},
745+
existing: &expinfrav1.AWSLaunchTemplate{
746+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
747+
{ID: aws.String("sg-111")},
748+
{ID: aws.String("sg-222")},
749+
},
750+
SpotMarketOptions: nil,
751+
},
752+
want: true,
753+
wantErr: false,
754+
},
755+
{
756+
name: "Should return true if incoming removes SpotMarketOptions and existing has some",
757+
incoming: &expinfrav1.AWSLaunchTemplate{
758+
SpotMarketOptions: nil,
759+
},
760+
existing: &expinfrav1.AWSLaunchTemplate{
761+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
762+
{ID: aws.String("sg-111")},
763+
{ID: aws.String("sg-222")},
764+
},
765+
SpotMarketOptions: &infrav1.SpotMarketOptions{
766+
MaxPrice: aws.String("0.05"),
767+
},
768+
},
769+
want: true,
770+
wantErr: false,
771+
},
559772
}
560773
for _, tt := range tests {
561774
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)