Skip to content

Commit 6f3953d

Browse files
fiunchinhofiunchinho
committed
🐛 Launchtemplate needs to be updated if spot options are changed
1 parent 7b09356 commit 6f3953d

File tree

2 files changed

+247
-6
lines changed

2 files changed

+247
-6
lines changed

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import (
3232
apimachinerytypes "k8s.io/apimachinery/pkg/types"
3333
"k8s.io/utils/ptr"
3434

35+
ctrl "sigs.k8s.io/controller-runtime"
36+
3537
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3638
expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2"
3739
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors"
@@ -680,6 +682,24 @@ func (s *Service) deleteLaunchTemplateVersion(id string, version *int64) error {
680682
return nil
681683
}
682684

685+
// SDKToSpotMarketOptions converts EC2 instance market options to SpotMarketOptions.
686+
func SDKToSpotMarketOptions(instanceMarketOptions *ec2.LaunchTemplateInstanceMarketOptions) *infrav1.SpotMarketOptions {
687+
if instanceMarketOptions == nil || aws.StringValue(instanceMarketOptions.MarketType) != ec2.MarketTypeSpot {
688+
return nil
689+
}
690+
691+
if instanceMarketOptions.SpotOptions == nil {
692+
return &infrav1.SpotMarketOptions{}
693+
}
694+
695+
result := &infrav1.SpotMarketOptions{}
696+
if instanceMarketOptions.SpotOptions.MaxPrice != nil {
697+
result.MaxPrice = instanceMarketOptions.SpotOptions.MaxPrice
698+
}
699+
700+
return result
701+
}
702+
683703
// SDKToLaunchTemplate converts an AWS EC2 SDK instance to the CAPA instance type.
684704
func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1.AWSLaunchTemplate, string, *apimachinerytypes.NamespacedName, error) {
685705
v := d.LaunchTemplateData
@@ -688,9 +708,10 @@ func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1
688708
AMI: infrav1.AMIReference{
689709
ID: v.ImageId,
690710
},
691-
InstanceType: aws.StringValue(v.InstanceType),
692-
SSHKeyName: v.KeyName,
693-
VersionNumber: d.VersionNumber,
711+
InstanceType: aws.StringValue(v.InstanceType),
712+
SSHKeyName: v.KeyName,
713+
SpotMarketOptions: SDKToSpotMarketOptions(v.InstanceMarketOptions),
714+
VersionNumber: d.VersionNumber,
694715
}
695716

696717
if v.MetadataOptions != nil {
@@ -777,6 +798,9 @@ func (s *Service) LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, inc
777798
if !cmp.Equal(incoming.InstanceMetadataOptions, existing.InstanceMetadataOptions) {
778799
return true, nil
779800
}
801+
if !cmp.Equal(incoming.SpotMarketOptions, existing.SpotMarketOptions) {
802+
return true, nil
803+
}
780804

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

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

Lines changed: 220 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,143 @@ 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+
wantUserDataHash: testUserDataHash,
390+
wantDataSecretKey: nil,
391+
wantBootstrapDataHash: nil,
392+
},
393+
{
394+
name: "spot market options with no max price",
395+
input: &ec2.LaunchTemplateVersion{
396+
LaunchTemplateId: aws.String("lt-12345"),
397+
LaunchTemplateName: aws.String("foo"),
398+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
399+
ImageId: aws.String("foo-image"),
400+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
401+
Arn: aws.String("instance-profile/foo-profile"),
402+
},
403+
KeyName: aws.String("foo-keyname"),
404+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
405+
MarketType: aws.String(ec2.MarketTypeSpot),
406+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{},
407+
},
408+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
409+
},
410+
VersionNumber: aws.Int64(1),
411+
},
412+
wantLT: &expinfrav1.AWSLaunchTemplate{
413+
Name: "foo",
414+
AMI: infrav1.AMIReference{
415+
ID: aws.String("foo-image"),
416+
},
417+
IamInstanceProfile: "foo-profile",
418+
SSHKeyName: aws.String("foo-keyname"),
419+
VersionNumber: aws.Int64(1),
420+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
421+
},
422+
wantUserDataHash: testUserDataHash,
423+
wantDataSecretKey: nil,
424+
wantBootstrapDataHash: nil,
425+
},
426+
{
427+
name: "spot market options without SpotOptions",
428+
input: &ec2.LaunchTemplateVersion{
429+
LaunchTemplateId: aws.String("lt-12345"),
430+
LaunchTemplateName: aws.String("foo"),
431+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
432+
ImageId: aws.String("foo-image"),
433+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
434+
Arn: aws.String("instance-profile/foo-profile"),
435+
},
436+
KeyName: aws.String("foo-keyname"),
437+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
438+
MarketType: aws.String(ec2.MarketTypeSpot),
439+
},
440+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
441+
},
442+
VersionNumber: aws.Int64(1),
443+
},
444+
wantLT: &expinfrav1.AWSLaunchTemplate{
445+
Name: "foo",
446+
AMI: infrav1.AMIReference{
447+
ID: aws.String("foo-image"),
448+
},
449+
IamInstanceProfile: "foo-profile",
450+
SSHKeyName: aws.String("foo-keyname"),
451+
VersionNumber: aws.Int64(1),
452+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
453+
},
454+
wantUserDataHash: testUserDataHash,
455+
wantDataSecretKey: nil,
456+
wantBootstrapDataHash: nil,
457+
},
458+
{
459+
name: "non-spot market type",
460+
input: &ec2.LaunchTemplateVersion{
461+
LaunchTemplateId: aws.String("lt-12345"),
462+
LaunchTemplateName: aws.String("foo"),
463+
LaunchTemplateData: &ec2.ResponseLaunchTemplateData{
464+
ImageId: aws.String("foo-image"),
465+
IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{
466+
Arn: aws.String("instance-profile/foo-profile"),
467+
},
468+
KeyName: aws.String("foo-keyname"),
469+
InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptions{
470+
MarketType: aws.String("different-market-type"),
471+
SpotOptions: &ec2.LaunchTemplateSpotMarketOptions{
472+
MaxPrice: aws.String("0.05"),
473+
},
474+
},
475+
UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))),
476+
},
477+
VersionNumber: aws.Int64(1),
478+
},
479+
wantLT: &expinfrav1.AWSLaunchTemplate{
480+
Name: "foo",
481+
AMI: infrav1.AMIReference{
482+
ID: aws.String("foo-image"),
483+
},
484+
IamInstanceProfile: "foo-profile",
485+
SSHKeyName: aws.String("foo-keyname"),
486+
VersionNumber: aws.Int64(1),
487+
SpotMarketOptions: nil, // Should be nil since market type is not "spot"
488+
},
489+
wantUserDataHash: testUserDataHash,
490+
wantDataSecretKey: nil,
491+
wantBootstrapDataHash: nil,
492+
},
356493
{
357494
name: "tag of bootstrap secret",
358495
input: &ec2.LaunchTemplateVersion{
@@ -440,6 +577,20 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
440577
want bool
441578
wantErr bool
442579
}{
580+
{
581+
name: "only core security groups, order shouldn't matter",
582+
incoming: &expinfrav1.AWSLaunchTemplate{
583+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{},
584+
},
585+
existing: &expinfrav1.AWSLaunchTemplate{
586+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
587+
{ID: aws.String("sg-222")},
588+
{ID: aws.String("sg-111")},
589+
},
590+
},
591+
want: false,
592+
wantErr: false,
593+
},
443594
{
444595
name: "the same security groups",
445596
incoming: &expinfrav1.AWSLaunchTemplate{
@@ -497,6 +648,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
497648
IamInstanceProfile: DefaultAmiNameFormat,
498649
},
499650
existing: &expinfrav1.AWSLaunchTemplate{
651+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
652+
{ID: aws.String("sg-111")},
653+
{ID: aws.String("sg-222")},
654+
},
500655
IamInstanceProfile: "some-other-profile",
501656
},
502657
want: true,
@@ -507,6 +662,10 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
507662
InstanceType: "t3.micro",
508663
},
509664
existing: &expinfrav1.AWSLaunchTemplate{
665+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
666+
{ID: aws.String("sg-111")},
667+
{ID: aws.String("sg-222")},
668+
},
510669
InstanceType: "t3.large",
511670
},
512671
want: true,
@@ -540,9 +699,14 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
540699
HTTPTokens: infrav1.HTTPTokensStateRequired,
541700
},
542701
},
543-
existing: &expinfrav1.AWSLaunchTemplate{},
544-
want: true,
545-
wantErr: false,
702+
existing: &expinfrav1.AWSLaunchTemplate{
703+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
704+
{ID: aws.String("sg-111")},
705+
{ID: aws.String("sg-222")},
706+
},
707+
},
708+
want: true,
709+
wantErr: false,
546710
},
547711
{
548712
name: "new launch template instance metadata options, removing IMDSv2 requirement",
@@ -556,6 +720,59 @@ func TestServiceLaunchTemplateNeedsUpdate(t *testing.T) {
556720
want: true,
557721
wantErr: false,
558722
},
723+
{
724+
name: "Should return true if incoming SpotMarketOptions is different from existing SpotMarketOptions",
725+
incoming: &expinfrav1.AWSLaunchTemplate{
726+
SpotMarketOptions: &infrav1.SpotMarketOptions{
727+
MaxPrice: aws.String("0.10"),
728+
},
729+
},
730+
existing: &expinfrav1.AWSLaunchTemplate{
731+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
732+
{ID: aws.String("sg-111")},
733+
{ID: aws.String("sg-222")},
734+
},
735+
SpotMarketOptions: &infrav1.SpotMarketOptions{
736+
MaxPrice: aws.String("0.05"),
737+
},
738+
},
739+
want: true,
740+
wantErr: false,
741+
},
742+
{
743+
name: "Should return true if incoming adds SpotMarketOptions and existing has none",
744+
incoming: &expinfrav1.AWSLaunchTemplate{
745+
SpotMarketOptions: &infrav1.SpotMarketOptions{
746+
MaxPrice: aws.String("0.10"),
747+
},
748+
},
749+
existing: &expinfrav1.AWSLaunchTemplate{
750+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
751+
{ID: aws.String("sg-111")},
752+
{ID: aws.String("sg-222")},
753+
},
754+
SpotMarketOptions: nil,
755+
},
756+
want: true,
757+
wantErr: false,
758+
},
759+
{
760+
name: "Should return true if incoming removes SpotMarketOptions and existing has some",
761+
incoming: &expinfrav1.AWSLaunchTemplate{
762+
SpotMarketOptions: nil,
763+
},
764+
existing: &expinfrav1.AWSLaunchTemplate{
765+
AdditionalSecurityGroups: []infrav1.AWSResourceReference{
766+
{ID: aws.String("sg-111")},
767+
{ID: aws.String("sg-222")},
768+
},
769+
SpotMarketOptions: &infrav1.SpotMarketOptions{
770+
MaxPrice: aws.String("0.05"),
771+
},
772+
},
773+
want: true,
774+
wantErr: false,
775+
},
559776
}
560777
for _, tt := range tests {
561778
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)