Skip to content

Commit e5ceb8e

Browse files
fiunchinhofiunchinho
committed
🐛 Launchtemplate needs to be updated if spot options are changed
1 parent f509a70 commit e5ceb8e

File tree

2 files changed

+246
-6
lines changed

2 files changed

+246
-6
lines changed

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,25 @@ func (s *Service) deleteLaunchTemplateVersion(id string, version *int64) error {
826826
return nil
827827
}
828828

829+
// convertInstanceMarketOptionsToSpotMarketOptions converts EC2 instance market options to SpotMarketOptions
830+
func convertInstanceMarketOptionsToSpotMarketOptions(instanceMarketOptions *ec2.LaunchTemplateInstanceMarketOptions) *infrav1.SpotMarketOptions {
831+
if instanceMarketOptions == nil || aws.StringValue(instanceMarketOptions.MarketType) != ec2.MarketTypeSpot {
832+
return nil
833+
}
834+
835+
spotOptions := instanceMarketOptions.SpotOptions
836+
if spotOptions == nil {
837+
return &infrav1.SpotMarketOptions{}
838+
}
839+
840+
result := &infrav1.SpotMarketOptions{}
841+
if spotOptions.MaxPrice != nil {
842+
result.MaxPrice = spotOptions.MaxPrice
843+
}
844+
845+
return result
846+
}
847+
829848
// SDKToLaunchTemplate converts an AWS EC2 SDK instance to the CAPA instance type.
830849
func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1.AWSLaunchTemplate, string, *apimachinerytypes.NamespacedName, *string, error) {
831850
v := d.LaunchTemplateData
@@ -834,9 +853,10 @@ func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1
834853
AMI: infrav1.AMIReference{
835854
ID: v.ImageId,
836855
},
837-
InstanceType: aws.StringValue(v.InstanceType),
838-
SSHKeyName: v.KeyName,
839-
VersionNumber: d.VersionNumber,
856+
InstanceType: aws.StringValue(v.InstanceType),
857+
SSHKeyName: v.KeyName,
858+
SpotMarketOptions: convertInstanceMarketOptionsToSpotMarketOptions(v.InstanceMarketOptions),
859+
VersionNumber: d.VersionNumber,
840860
}
841861

842862
if v.MetadataOptions != nil {
@@ -928,6 +948,9 @@ func (s *Service) LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, inc
928948
if !cmp.Equal(incoming.InstanceMetadataOptions, existing.InstanceMetadataOptions) {
929949
return true, nil
930950
}
951+
if !cmp.Equal(incoming.SpotMarketOptions, existing.SpotMarketOptions) {
952+
return true, nil
953+
}
931954

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

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

Lines changed: 220 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,143 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) {
364364
wantDataSecretKey: nil, // respective tag is not given
365365
wantBootstrapDataHash: nil, // respective tag is not given
366366
},
367+
{
368+
name: "spot market options",
369+
input: &ec2.LaunchTemplateVersion{
370+
LaunchTemplateId: aws.String("lt-12345"),
371+
LaunchTemplateName: aws.String("foo"),
372+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
373+
ImageId: aws.String("foo-image"),
374+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
375+
Arn: aws.String("instance-profile/foo-profile"),
376+
},
377+
KeyName: aws.String("foo-keyname"),
378+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
379+
MarketType: aws.String(ec2.MarketTypeSpot),
380+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{
381+
MaxPrice: aws.String("0.05"),
382+
},
383+
},
384+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
385+
},
386+
VersionNumber: aws.Int64(1),
387+
},
388+
wantLT: &expinfrav1.AWSLaunchTemplate{
389+
Name: "foo",
390+
AMI: infrav1.AMIReference{
391+
ID: aws.String("foo-image"),
392+
},
393+
IamInstanceProfile: "foo-profile",
394+
SSHKeyName: aws.String("foo-keyname"),
395+
VersionNumber: aws.Int64(1),
396+
SpotMarketOptions: &infrav1.SpotMarketOptions{
397+
MaxPrice: aws.String("0.05"),
398+
},
399+
},
400+
wantUserDataHash: testUserDataHash,
401+
wantDataSecretKey: nil,
402+
wantBootstrapDataHash: nil,
403+
},
404+
{
405+
name: "spot market options with no max price",
406+
input: &ec2.LaunchTemplateVersion{
407+
LaunchTemplateId: aws.String("lt-12345"),
408+
LaunchTemplateName: aws.String("foo"),
409+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
410+
ImageId: aws.String("foo-image"),
411+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
412+
Arn: aws.String("instance-profile/foo-profile"),
413+
},
414+
KeyName: aws.String("foo-keyname"),
415+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
416+
MarketType: aws.String(ec2.MarketTypeSpot),
417+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{},
418+
},
419+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
420+
},
421+
VersionNumber: aws.Int64(1),
422+
},
423+
wantLT: &expinfrav1.AWSLaunchTemplate{
424+
Name: "foo",
425+
AMI: infrav1.AMIReference{
426+
ID: aws.String("foo-image"),
427+
},
428+
IamInstanceProfile: "foo-profile",
429+
SSHKeyName: aws.String("foo-keyname"),
430+
VersionNumber: aws.Int64(1),
431+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
432+
},
433+
wantUserDataHash: testUserDataHash,
434+
wantDataSecretKey: nil,
435+
wantBootstrapDataHash: nil,
436+
},
437+
{
438+
name: "spot market options without SpotOptions",
439+
input: &ec2.LaunchTemplateVersion{
440+
LaunchTemplateId: aws.String("lt-12345"),
441+
LaunchTemplateName: aws.String("foo"),
442+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
443+
ImageId: aws.String("foo-image"),
444+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
445+
Arn: aws.String("instance-profile/foo-profile"),
446+
},
447+
KeyName: aws.String("foo-keyname"),
448+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
449+
MarketType: aws.String(ec2.MarketTypeSpot),
450+
},
451+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
452+
},
453+
VersionNumber: aws.Int64(1),
454+
},
455+
wantLT: &expinfrav1.AWSLaunchTemplate{
456+
Name: "foo",
457+
AMI: infrav1.AMIReference{
458+
ID: aws.String("foo-image"),
459+
},
460+
IamInstanceProfile: "foo-profile",
461+
SSHKeyName: aws.String("foo-keyname"),
462+
VersionNumber: aws.Int64(1),
463+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
464+
},
465+
wantUserDataHash: testUserDataHash,
466+
wantDataSecretKey: nil,
467+
wantBootstrapDataHash: nil,
468+
},
469+
{
470+
name: "non-spot market type",
471+
input: &ec2.LaunchTemplateVersion{
472+
LaunchTemplateId: aws.String("lt-12345"),
473+
LaunchTemplateName: aws.String("foo"),
474+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
475+
ImageId: aws.String("foo-image"),
476+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
477+
Arn: aws.String("instance-profile/foo-profile"),
478+
},
479+
KeyName: aws.String("foo-keyname"),
480+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
481+
MarketType: aws.String("different-market-type"),
482+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{
483+
MaxPrice: aws.String("0.05"),
484+
},
485+
},
486+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
487+
},
488+
VersionNumber: aws.Int64(1),
489+
},
490+
wantLT: &expinfrav1.AWSLaunchTemplate{
491+
Name: "foo",
492+
AMI: infrav1.AMIReference{
493+
ID: aws.String("foo-image"),
494+
},
495+
IamInstanceProfile: "foo-profile",
496+
SSHKeyName: aws.String("foo-keyname"),
497+
VersionNumber: aws.Int64(1),
498+
SpotMarketOptions: nil, // Should be nil since market type is not "spot"
499+
},
500+
wantUserDataHash: testUserDataHash,
501+
wantDataSecretKey: nil,
502+
wantBootstrapDataHash: nil,
503+
},
367504
{
368505
name: "tag of bootstrap secret",
369506
input: &ec2.LaunchTemplateVersion{
@@ -458,6 +595,20 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
458595
want bool
459596
wantErr bool
460597
}{
598+
{
599+
name: "only core security groups, order shouldn't matter",
600+
incoming: &expinfrav1.AWSLaunchTemplate{
601+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{},
602+
},
603+
existing: &expinfrav1.AWSLaunchTemplate{
604+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
605+
{ID: aws.String("sg-222")},
606+
{ID: aws.String("sg-111")},
607+
},
608+
},
609+
want: false,
610+
wantErr: false,
611+
},
461612
{
462613
name: "the same security groups",
463614
incoming: &expinfrav1.AWSLaunchTemplate{
@@ -515,6 +666,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
515666
IamInstanceProfile: DefaultAmiNameFormat,
516667
},
517668
existing: &expinfrav1.AWSLaunchTemplate{
669+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
670+
{ID: aws.String("sg-111")},
671+
{ID: aws.String("sg-222")},
672+
},
518673
IamInstanceProfile: "some-other-profile",
519674
},
520675
want: true,
@@ -525,6 +680,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
525680
InstanceType: "t3.micro",
526681
},
527682
existing: &expinfrav1.AWSLaunchTemplate{
683+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
684+
{ID: aws.String("sg-111")},
685+
{ID: aws.String("sg-222")},
686+
},
528687
InstanceType: "t3.large",
529688
},
530689
want: true,
@@ -558,9 +717,14 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
558717
HTTPTokens: infrav1.HTTPTokensStateRequired,
559718
},
560719
},
561-
existing: &expinfrav1.AWSLaunchTemplate{},
562-
want: true,
563-
wantErr: false,
720+
existing: &expinfrav1.AWSLaunchTemplate{
721+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
722+
{ID: aws.String("sg-111")},
723+
{ID: aws.String("sg-222")},
724+
},
725+
},
726+
want: true,
727+
wantErr: false,
564728
},
565729
{
566730
name: "new launch template instance metadata options, removing IMDSv2 requirement",
@@ -574,6 +738,59 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
574738
want: true,
575739
wantErr: false,
576740
},
741+
{
742+
name: "Should return true if incoming SpotMarketOptions is different from existing SpotMarketOptions",
743+
incoming: &expinfrav1.AWSLaunchTemplate{
744+
SpotMarketOptions: &infrav1.SpotMarketOptions{
745+
MaxPrice: aws.String("0.10"),
746+
},
747+
},
748+
existing: &expinfrav1.AWSLaunchTemplate{
749+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
750+
{ID: aws.String("sg-111")},
751+
{ID: aws.String("sg-222")},
752+
},
753+
SpotMarketOptions: &infrav1.SpotMarketOptions{
754+
MaxPrice: aws.String("0.05"),
755+
},
756+
},
757+
want: true,
758+
wantErr: false,
759+
},
760+
{
761+
name: "Should return true if incoming adds SpotMarketOptions and existing has none",
762+
incoming: &expinfrav1.AWSLaunchTemplate{
763+
SpotMarketOptions: &infrav1.SpotMarketOptions{
764+
MaxPrice: aws.String("0.10"),
765+
},
766+
},
767+
existing: &expinfrav1.AWSLaunchTemplate{
768+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
769+
{ID: aws.String("sg-111")},
770+
{ID: aws.String("sg-222")},
771+
},
772+
SpotMarketOptions: nil,
773+
},
774+
want: true,
775+
wantErr: false,
776+
},
777+
{
778+
name: "Should return true if incoming removes SpotMarketOptions and existing has some",
779+
incoming: &expinfrav1.AWSLaunchTemplate{
780+
SpotMarketOptions: nil,
781+
},
782+
existing: &expinfrav1.AWSLaunchTemplate{
783+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
784+
{ID: aws.String("sg-111")},
785+
{ID: aws.String("sg-222")},
786+
},
787+
SpotMarketOptions: &infrav1.SpotMarketOptions{
788+
MaxPrice: aws.String("0.05"),
789+
},
790+
},
791+
want: true,
792+
wantErr: false,
793+
},
577794
}
578795
for _, tt := range tests {
579796
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)