From 6c1c6bdde679b5926fa2b95aab5945f6e5d86d06 Mon Sep 17 00:00:00 2001 From: Thirumalesh Aaraveti Date: Thu, 7 Nov 2024 21:06:35 +0530 Subject: [PATCH 1/2] Added the support for capacity blocks --- api/v1beta1/awscluster_conversion.go | 1 + api/v1beta1/awsmachine_conversion.go | 2 + api/v1beta1/zz_generated.conversion.go | 2 + api/v1beta2/awsmachine_types.go | 5 + api/v1beta2/awsmachine_webhook.go | 9 + api/v1beta2/awsmachine_webhook_test.go | 21 + api/v1beta2/types.go | 5 + api/v1beta2/zz_generated.deepcopy.go | 10 + ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 10 + ...tructure.cluster.x-k8s.io_awsclusters.yaml | 5 + ...ture.cluster.x-k8s.io_awsmachinepools.yaml | 9 + ...tructure.cluster.x-k8s.io_awsmachines.yaml | 5 + ....cluster.x-k8s.io_awsmachinetemplates.yaml | 5 + ...uster.x-k8s.io_awsmanagedmachinepools.yaml | 9 + exp/api/v1beta1/conversion.go | 18 +- exp/api/v1beta1/zz_generated.conversion.go | 2 + exp/api/v1beta2/awsmachinepool_webhook.go | 9 + .../v1beta2/awsmachinepool_webhook_test.go | 23 + exp/api/v1beta2/types.go | 9 + exp/api/v1beta2/zz_generated.deepcopy.go | 10 + pkg/cloud/services/ec2/helper_test.go | 39 ++ pkg/cloud/services/ec2/instances.go | 71 +-- pkg/cloud/services/ec2/instances_test.go | 410 +++++++++++++++++- pkg/cloud/services/ec2/launchtemplate.go | 30 +- pkg/cloud/services/ec2/launchtemplate_test.go | 58 ++- 25 files changed, 728 insertions(+), 49 deletions(-) diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index e6d9982cbd..3fa974f07f 100644 --- a/api/v1beta1/awscluster_conversion.go +++ b/api/v1beta1/awscluster_conversion.go @@ -60,6 +60,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error { dst.Status.Bastion.PrivateDNSName = restored.Status.Bastion.PrivateDNSName dst.Status.Bastion.PublicIPOnLaunch = restored.Status.Bastion.PublicIPOnLaunch dst.Status.Bastion.CapacityReservationID = restored.Status.Bastion.CapacityReservationID + dst.Status.Bastion.UseCapacityBlock = restored.Status.Bastion.UseCapacityBlock } dst.Spec.Partition = restored.Spec.Partition diff --git a/api/v1beta1/awsmachine_conversion.go b/api/v1beta1/awsmachine_conversion.go index 6044416cdf..4a132dc5cb 100644 --- a/api/v1beta1/awsmachine_conversion.go +++ b/api/v1beta1/awsmachine_conversion.go @@ -42,6 +42,7 @@ func (src *AWSMachine) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.PrivateDNSName = restored.Spec.PrivateDNSName dst.Spec.SecurityGroupOverrides = restored.Spec.SecurityGroupOverrides dst.Spec.CapacityReservationID = restored.Spec.CapacityReservationID + dst.Spec.UseCapacityBlock = restored.Spec.UseCapacityBlock if restored.Spec.ElasticIPPool != nil { if dst.Spec.ElasticIPPool == nil { dst.Spec.ElasticIPPool = &infrav1.ElasticIPPool{} @@ -104,6 +105,7 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.PrivateDNSName = restored.Spec.Template.Spec.PrivateDNSName dst.Spec.Template.Spec.SecurityGroupOverrides = restored.Spec.Template.Spec.SecurityGroupOverrides dst.Spec.Template.Spec.CapacityReservationID = restored.Spec.Template.Spec.CapacityReservationID + dst.Spec.Template.Spec.UseCapacityBlock = restored.Spec.Template.Spec.UseCapacityBlock if restored.Spec.Template.Spec.ElasticIPPool != nil { if dst.Spec.Template.Spec.ElasticIPPool == nil { dst.Spec.Template.Spec.ElasticIPPool = &infrav1.ElasticIPPool{} diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 2d1641f424..56ea41f118 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -1434,6 +1434,7 @@ func autoConvert_v1beta2_AWSMachineSpec_To_v1beta1_AWSMachineSpec(in *v1beta2.AW out.Tenancy = in.Tenancy // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type + // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type return nil } @@ -2036,6 +2037,7 @@ func autoConvert_v1beta2_Instance_To_v1beta1_Instance(in *v1beta2.Instance, out // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type // WARNING: in.PublicIPOnLaunch requires manual conversion: does not exist in peer-type // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type + // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta2/awsmachine_types.go b/api/v1beta2/awsmachine_types.go index 39a649a0e5..4aed7dd1d1 100644 --- a/api/v1beta2/awsmachine_types.go +++ b/api/v1beta2/awsmachine_types.go @@ -197,6 +197,11 @@ type AWSMachineSpec struct { // CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. // +optional CapacityReservationID *string `json:"capacityReservationId,omitempty"` + + // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If enabled, CapacityReservationID must be specified to identify the target reservation. + // +optional + UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` } // CloudInit defines options related to the bootstrapping systems where diff --git a/api/v1beta2/awsmachine_webhook.go b/api/v1beta2/awsmachine_webhook.go index 50af4f2211..cf729948a4 100644 --- a/api/v1beta2/awsmachine_webhook.go +++ b/api/v1beta2/awsmachine_webhook.go @@ -66,6 +66,7 @@ func (r *AWSMachine) ValidateCreate() (admission.Warnings, error) { allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...) allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...) allErrs = append(allErrs, r.validateNetworkElasticIPPool()...) + allErrs = append(allErrs, r.validateInstanceMarketType()...) return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) } @@ -361,6 +362,14 @@ func (r *AWSMachine) validateNetworkElasticIPPool() field.ErrorList { return allErrs } +func (r *AWSMachine) validateInstanceMarketType() field.ErrorList { + var allErrs field.ErrorList + if r.Spec.UseCapacityBlock != nil && r.Spec.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "useCapacityBlock"), "useCapacityBlock and spotMarketOptions cannot be used together")) + } + return allErrs +} + func (r *AWSMachine) validateNonRootVolumes() field.ErrorList { var allErrs field.ErrorList diff --git a/api/v1beta2/awsmachine_webhook_test.go b/api/v1beta2/awsmachine_webhook_test.go index 80e7abc45a..bf31bff812 100644 --- a/api/v1beta2/awsmachine_webhook_test.go +++ b/api/v1beta2/awsmachine_webhook_test.go @@ -217,6 +217,27 @@ func TestAWSMachineCreate(t *testing.T) { }, wantErr: false, }, + { + name: "invalid useCapacityBlock and spotMarketOptions are specified", + machine: &AWSMachine{ + Spec: AWSMachineSpec{ + UseCapacityBlock: aws.Bool(true), + SpotMarketOptions: &SpotMarketOptions{}, + InstanceType: "test", + }, + }, + wantErr: true, + }, + { + name: "valid useCapacityBlock is specified", + machine: &AWSMachine{ + Spec: AWSMachineSpec{ + UseCapacityBlock: aws.Bool(true), + InstanceType: "test", + }, + }, + wantErr: false, + }, { name: "empty instance type not allowed", machine: &AWSMachine{ diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index 978d5310f2..0fe959f2bf 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -261,6 +261,11 @@ type Instance struct { // CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. // +optional CapacityReservationID *string `json:"capacityReservationId,omitempty"` + + // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If enabled, CapacityReservationID must be specified to identify the target reservation. + // +optional + UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` } // InstanceMetadataState describes the state of InstanceMetadataOptions.HttpEndpoint and InstanceMetadataOptions.InstanceMetadataTags diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index c727483f6b..81ff8d302d 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -772,6 +772,11 @@ func (in *AWSMachineSpec) DeepCopyInto(out *AWSMachineSpec) { *out = new(string) **out = **in } + if in.UseCapacityBlock != nil { + in, out := &in.UseCapacityBlock, &out.UseCapacityBlock + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSMachineSpec. @@ -1604,6 +1609,11 @@ func (in *Instance) DeepCopyInto(out *Instance) { *out = new(string) **out = **in } + if in.UseCapacityBlock != nil { + in, out := &in.UseCapacityBlock, &out.UseCapacityBlock + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Instance. diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 21be0fc920..95bf5b6bdb 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -1376,6 +1376,11 @@ spec: type: description: The instance type. type: string + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. @@ -3416,6 +3421,11 @@ spec: type: description: The instance type. type: string + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml index 3ccb164ec1..6bb89799c4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -2343,6 +2343,11 @@ spec: type: description: The instance type. type: string + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml index 6314fe7c62..135fcecfa4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml @@ -629,6 +629,10 @@ spec: description: ID of resource type: string type: object + capacityReservationId: + description: CapacityReservationID specifies the target Capacity + Reservation into which the instance should be launched. + type: string iamInstanceProfile: description: |- The name or the Amazon Resource Name (ARN) of the instance profile associated @@ -846,6 +850,11 @@ spec: SSHKeyName is the name of the ssh key to attach to the instance. Valid values are empty string (do not use SSH keys), a valid SSH key name, or omitted (use the default SSH key name) type: string + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean versionNumber: description: |- VersionNumber is the version of the launch template that is applied. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml index 70e7728369..77172ac074 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml @@ -1080,6 +1080,11 @@ spec: cloud-init has built-in support for gzip-compressed user data user data stored in aws secret manager is always gzip-compressed. type: boolean + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean required: - instanceType type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml index a679a68bc8..18f94fdb50 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml @@ -1021,6 +1021,11 @@ spec: cloud-init has built-in support for gzip-compressed user data user data stored in aws secret manager is always gzip-compressed. type: boolean + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean required: - instanceType type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml index 9ecac67715..119dfae34a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml @@ -625,6 +625,10 @@ spec: description: ID of resource type: string type: object + capacityReservationId: + description: CapacityReservationID specifies the target Capacity + Reservation into which the instance should be launched. + type: string iamInstanceProfile: description: |- The name or the Amazon Resource Name (ARN) of the instance profile associated @@ -842,6 +846,11 @@ spec: SSHKeyName is the name of the ssh key to attach to the instance. Valid values are empty string (do not use SSH keys), a valid SSH key name, or omitted (use the default SSH key name) type: string + useCapacityBlock: + description: |- + UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If enabled, CapacityReservationID must be specified to identify the target reservation. + type: boolean versionNumber: description: |- VersionNumber is the version of the launch template that is applied. diff --git a/exp/api/v1beta1/conversion.go b/exp/api/v1beta1/conversion.go index 7c39f1fcbd..8ad51f3839 100644 --- a/exp/api/v1beta1/conversion.go +++ b/exp/api/v1beta1/conversion.go @@ -57,9 +57,16 @@ func (src *AWSMachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.AWSLaunchTemplate.PrivateDNSName = restored.Spec.AWSLaunchTemplate.PrivateDNSName } + if restored.Spec.AWSLaunchTemplate.CapacityReservationID != nil { + dst.Spec.AWSLaunchTemplate.CapacityReservationID = restored.Spec.AWSLaunchTemplate.CapacityReservationID + } + + if restored.Spec.AWSLaunchTemplate.UseCapacityBlock != nil { + dst.Spec.AWSLaunchTemplate.UseCapacityBlock = restored.Spec.AWSLaunchTemplate.UseCapacityBlock + } + dst.Spec.DefaultInstanceWarmup = restored.Spec.DefaultInstanceWarmup dst.Spec.AWSLaunchTemplate.NonRootVolumes = restored.Spec.AWSLaunchTemplate.NonRootVolumes - return nil } @@ -109,6 +116,15 @@ func (src *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error { if restored.Spec.AWSLaunchTemplate.PrivateDNSName != nil { dst.Spec.AWSLaunchTemplate.PrivateDNSName = restored.Spec.AWSLaunchTemplate.PrivateDNSName } + + if restored.Spec.AWSLaunchTemplate.CapacityReservationID != nil { + dst.Spec.AWSLaunchTemplate.CapacityReservationID = restored.Spec.AWSLaunchTemplate.CapacityReservationID + } + + if restored.Spec.AWSLaunchTemplate.UseCapacityBlock != nil { + dst.Spec.AWSLaunchTemplate.UseCapacityBlock = restored.Spec.AWSLaunchTemplate.UseCapacityBlock + } + } if restored.Spec.AvailabilityZoneSubnetType != nil { dst.Spec.AvailabilityZoneSubnetType = restored.Spec.AvailabilityZoneSubnetType diff --git a/exp/api/v1beta1/zz_generated.conversion.go b/exp/api/v1beta1/zz_generated.conversion.go index 585cbd1504..da119ccc0f 100644 --- a/exp/api/v1beta1/zz_generated.conversion.go +++ b/exp/api/v1beta1/zz_generated.conversion.go @@ -414,6 +414,8 @@ func autoConvert_v1beta2_AWSLaunchTemplate_To_v1beta1_AWSLaunchTemplate(in *v1be out.SpotMarketOptions = (*apiv1beta2.SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions)) // WARNING: in.InstanceMetadataOptions requires manual conversion: does not exist in peer-type // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type + // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type + // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type return nil } diff --git a/exp/api/v1beta2/awsmachinepool_webhook.go b/exp/api/v1beta2/awsmachinepool_webhook.go index a4f6a44d41..fce1a61031 100644 --- a/exp/api/v1beta2/awsmachinepool_webhook.go +++ b/exp/api/v1beta2/awsmachinepool_webhook.go @@ -176,6 +176,7 @@ func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) { allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...) allErrs = append(allErrs, r.validateSpotInstances()...) allErrs = append(allErrs, r.validateRefreshPreferences()...) + allErrs = append(allErrs, r.validateInstanceMarketType()...) if len(allErrs) == 0 { return nil, nil @@ -188,6 +189,14 @@ func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) { ) } +func (r *AWSMachinePool) validateInstanceMarketType() field.ErrorList { + var allErrs field.ErrorList + if r.Spec.AWSLaunchTemplate.UseCapacityBlock != nil && r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "useCapacityBlock"), "useCapacityBlock and spotMarketOptions cannot be used together")) + } + return allErrs +} + // ValidateUpdate will do any extra validation when updating a AWSMachinePool. func (r *AWSMachinePool) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) { var allErrs field.ErrorList diff --git a/exp/api/v1beta2/awsmachinepool_webhook_test.go b/exp/api/v1beta2/awsmachinepool_webhook_test.go index 0f14ad1c0a..65ecfb21dd 100644 --- a/exp/api/v1beta2/awsmachinepool_webhook_test.go +++ b/exp/api/v1beta2/awsmachinepool_webhook_test.go @@ -174,6 +174,29 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) { }, wantErr: true, }, + { + name: "invalid useCapacityBlock and spotMarketOptions are specified", + pool: &AWSMachinePool{ + Spec: AWSMachinePoolSpec{ + AWSLaunchTemplate: AWSLaunchTemplate{ + UseCapacityBlock: aws.Bool(true), + SpotMarketOptions: &infrav1.SpotMarketOptions{}, + }, + }, + }, + wantErr: true, + }, + { + name: "valid useCapacityBlock is specified", + pool: &AWSMachinePool{ + Spec: AWSMachinePoolSpec{ + AWSLaunchTemplate: AWSLaunchTemplate{ + UseCapacityBlock: aws.Bool(true), + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/exp/api/v1beta2/types.go b/exp/api/v1beta2/types.go index 0bc4009a2e..9622786073 100644 --- a/exp/api/v1beta2/types.go +++ b/exp/api/v1beta2/types.go @@ -128,6 +128,15 @@ type AWSLaunchTemplate struct { // PrivateDNSName is the options for the instance hostname. // +optional PrivateDNSName *infrav1.PrivateDNSName `json:"privateDnsName,omitempty"` + + // CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. + // +optional + CapacityReservationID *string `json:"capacityReservationId,omitempty"` + + // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If enabled, CapacityReservationID must be specified to identify the target reservation. + // +optional + UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` } // Overrides are used to override the instance type specified by the launch template with multiple diff --git a/exp/api/v1beta2/zz_generated.deepcopy.go b/exp/api/v1beta2/zz_generated.deepcopy.go index f34e8f9d9b..046361f5c1 100644 --- a/exp/api/v1beta2/zz_generated.deepcopy.go +++ b/exp/api/v1beta2/zz_generated.deepcopy.go @@ -136,6 +136,16 @@ func (in *AWSLaunchTemplate) DeepCopyInto(out *AWSLaunchTemplate) { *out = new(apiv1beta2.PrivateDNSName) (*in).DeepCopyInto(*out) } + if in.CapacityReservationID != nil { + in, out := &in.CapacityReservationID, &out.CapacityReservationID + *out = new(string) + **out = **in + } + if in.UseCapacityBlock != nil { + in, out := &in.UseCapacityBlock, &out.UseCapacityBlock + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSLaunchTemplate. diff --git a/pkg/cloud/services/ec2/helper_test.go b/pkg/cloud/services/ec2/helper_test.go index 0a0b67ab0a..dffb5180e4 100644 --- a/pkg/cloud/services/ec2/helper_test.go +++ b/pkg/cloud/services/ec2/helper_test.go @@ -61,6 +61,45 @@ func setupMachinePoolScope(cl client.Client, ec2Scope scope.EC2Scope) (*scope.Ma }) } +func setupCapacityBlocksMachinePoolScope(cl client.Client, ec2Scope scope.EC2Scope) (*scope.MachinePoolScope, error) { + return scope.NewMachinePoolScope(scope.MachinePoolScopeParams{ + Client: cl, + InfraCluster: ec2Scope, + Cluster: newCluster(), + MachinePool: newMachinePool(), + AWSMachinePool: newAWSCapacityBlockMachinePool(), + }) +} + +func newAWSCapacityBlockMachinePool() *expinfrav1.AWSMachinePool { + return &expinfrav1.AWSMachinePool{ + TypeMeta: metav1.TypeMeta{ + Kind: "AWSMachinePool", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "aws-mp-name", + Namespace: "aws-mp-ns", + }, + Spec: expinfrav1.AWSMachinePoolSpec{ + AvailabilityZones: []string{"us-east-1"}, + AdditionalTags: infrav1.Tags{}, + AWSLaunchTemplate: expinfrav1.AWSLaunchTemplate{ + Name: "aws-launch-template", + IamInstanceProfile: "instance-profile", + AMI: infrav1.AMIReference{}, + InstanceType: "t3.large", + SSHKeyName: aws.String("default"), + UseCapacityBlock: aws.Bool(true), + CapacityReservationID: aws.String("cr-12345678901234567"), + }, + }, + Status: expinfrav1.AWSMachinePoolStatus{ + LaunchTemplateID: "launch-template-id", + }, + } +} + func defaultEC2Tags(name, clusterName string) []*ec2.Tag { return []*ec2.Tag{ { diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go index 1da1893ff7..f0044d06a7 100644 --- a/pkg/cloud/services/ec2/instances.go +++ b/pkg/cloud/services/ec2/instances.go @@ -253,6 +253,8 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use input.CapacityReservationID = scope.AWSMachine.Spec.CapacityReservationID + input.UseCapacityBlock = scope.AWSMachine.Spec.UseCapacityBlock + s.scope.Debug("Running instance", "machine-role", scope.Role()) s.scope.Debug("Running instance with instance metadata options", "metadata options", input.InstanceMetadataOptions) out, err := s.runInstance(scope.Role(), input) @@ -637,8 +639,13 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan input.TagSpecifications = append(input.TagSpecifications, spec) } } - - input.InstanceMarketOptions = getInstanceMarketOptionsRequest(i.SpotMarketOptions) + marketOptions, err := getInstanceMarketOptionsRequest(i) + if err != nil { + return nil, err + } + if marketOptions != nil { + input.InstanceMarketOptions = marketOptions + } input.MetadataOptions = getInstanceMetadataOptionsRequest(i.InstanceMetadataOptions) input.PrivateDnsNameOptions = getPrivateDNSNameOptionsRequest(i.PrivateDNSName) input.CapacityReservationSpecification = getCapacityReservationSpecification(i.CapacityReservationID) @@ -1138,34 +1145,48 @@ func getCapacityReservationSpecification(capacityReservationID *string) *ec2.Cap } } -func getInstanceMarketOptionsRequest(spotMarketOptions *infrav1.SpotMarketOptions) *ec2.InstanceMarketOptionsRequest { - if spotMarketOptions == nil { - // Instance is not a Spot instance - return nil +func getInstanceMarketOptionsRequest(i *infrav1.Instance) (*ec2.InstanceMarketOptionsRequest, error) { + if i.UseCapacityBlock != nil && i.SpotMarketOptions != nil { + return nil, errors.New("can't create spot capacity-blocks, remove spot market request") } - // Set required values for Spot instances - spotOptions := &ec2.SpotMarketOptions{} - - // The following two options ensure that: - // - If an instance is interrupted, it is terminated rather than hibernating or stopping - // - No replacement instance will be created if the instance is interrupted - // - If the spot request cannot immediately be fulfilled, it will not be created - // This behaviour should satisfy the 1:1 mapping of Machines to Instances as - // assumed by the Cluster API. - spotOptions.SetInstanceInterruptionBehavior(ec2.InstanceInterruptionBehaviorTerminate) - spotOptions.SetSpotInstanceType(ec2.SpotInstanceTypeOneTime) - - maxPrice := spotMarketOptions.MaxPrice - if maxPrice != nil && *maxPrice != "" { - spotOptions.SetMaxPrice(*maxPrice) + // Handle Capacity Block case. + if ptr.Deref(i.UseCapacityBlock, false) { + if i.CapacityReservationID == nil { + return nil, errors.Errorf("capacityReservationID is required when CapacityBlock is enabled") + } + return &ec2.InstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeCapacityBlock), + }, nil } - instanceMarketOptionsRequest := &ec2.InstanceMarketOptionsRequest{} - instanceMarketOptionsRequest.SetMarketType(ec2.MarketTypeSpot) - instanceMarketOptionsRequest.SetSpotOptions(spotOptions) + // Handle Spot instance case. + if i.SpotMarketOptions == nil { + // Instance is not a Spot instance + return nil, nil + } - return instanceMarketOptionsRequest + // Set required values for Spot instances + spotOpts := &ec2.SpotMarketOptions{ + // The following two options ensure that: + // - If an instance is interrupted, it is terminated rather than hibernating or stopping + // - No replacement instance will be created if the instance is interrupted + // - If the spot request cannot immediately be fulfilled, it will not be created + // This behaviour should satisfy the 1:1 mapping of Machines to Instances as + // assumed by the Cluster API. + InstanceInterruptionBehavior: aws.String(ec2.InstanceInterruptionBehaviorTerminate), + SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime), + } + + maxPrice := ptr.Deref(i.SpotMarketOptions.MaxPrice, "") + if maxPrice != "" { + spotOpts.MaxPrice = aws.String(maxPrice) + } + + return &ec2.InstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeSpot), + SpotOptions: spotOpts, + }, nil } func getInstanceMetadataOptionsRequest(metadataOptions *infrav1.InstanceMetadataOptions) *ec2.InstanceMetadataOptionsRequest { diff --git a/pkg/cloud/services/ec2/instances_test.go b/pkg/cloud/services/ec2/instances_test.go index d8e48602ee..9c33f3bf85 100644 --- a/pkg/cloud/services/ec2/instances_test.go +++ b/pkg/cloud/services/ec2/instances_test.go @@ -5197,6 +5197,332 @@ func TestCreateInstance(t *testing.T) { } }, }, + { + name: "Simple, setting CapacityBlock and providing CapacityReservationID", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"set": "node"}, + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To[string]("bootstrap-data"), + }, + }, + }, + machineConfig: &infrav1.AWSMachineSpec{ + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.large", + UseCapacityBlock: aws.Bool(true), + CapacityReservationID: aws.String("cr-12345678901234567"), + }, + awsCluster: &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: infrav1.AWSClusterSpec{ + + NetworkSpec: infrav1.NetworkSpec{ + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + ID: "subnet-1", + IsPublic: false, + }, + infrav1.SubnetSpec{ + IsPublic: false, + }, + }, + VPC: infrav1.VPCSpec{ + ID: "vpc-test", + }, + }, + }, + Status: infrav1.AWSClusterStatus{ + Network: infrav1.NetworkStatus{ + SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{ + infrav1.SecurityGroupControlPlane: { + ID: "1", + }, + infrav1.SecurityGroupNode: { + ID: "2", + }, + infrav1.SecurityGroupLB: { + ID: "3", + }, + }, + APIServerELB: infrav1.LoadBalancer{ + DNSName: "test-apiserver.us-east-1.aws", + }, + }, + }, + }, + expect: func(m *mocks.MockEC2APIMockRecorder) { + m. + DescribeInstanceTypesWithContext(context.TODO(), gomock.Eq(&ec2.DescribeInstanceTypesInput{ + InstanceTypes: []*string{ + aws.String("m5.large"), + }, + })). + Return(&ec2.DescribeInstanceTypesOutput{ + InstanceTypes: []*ec2.InstanceTypeInfo{ + { + ProcessorInfo: &ec2.ProcessorInfo{ + SupportedArchitectures: []*string{ + aws.String("x86_64"), + }, + }, + }, + }, + }, nil) + m. // TODO: Restore these parameters, but with the tags as well + RunInstancesWithContext(context.TODO(), gomock.Any()). + Return(&ec2.Reservation{ + Instances: []*ec2.Instance{ + { + State: &ec2.InstanceState{ + Name: aws.String(ec2.InstanceStateNamePending), + }, + IamInstanceProfile: &ec2.IamInstanceProfile{ + Arn: aws.String("arn:aws:iam::123456789012:instance-profile/foo"), + }, + InstanceId: aws.String("two"), + InstanceType: aws.String("m5.large"), + SubnetId: aws.String("subnet-1"), + ImageId: aws.String("ami-1"), + RootDeviceName: aws.String("device-1"), + BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{ + { + DeviceName: aws.String("device-1"), + Ebs: &ec2.EbsInstanceBlockDevice{ + VolumeId: aws.String("volume-1"), + }, + }, + }, + Placement: &ec2.Placement{ + AvailabilityZone: &az, + }, + InstanceLifecycle: aws.String(ec2.MarketTypeCapacityBlock), + CapacityReservationId: aws.String("cr-12345678901234567"), + }, + }, + }, nil) + m. + DescribeNetworkInterfacesWithContext(context.TODO(), gomock.Any()). + Return(&ec2.DescribeNetworkInterfacesOutput{ + NetworkInterfaces: []*ec2.NetworkInterface{}, + NextToken: nil, + }, nil) + }, + check: func(instance *infrav1.Instance, err error) { + if err != nil { + t.Fatalf("did not expect error: %v", err) + } + }, + }, + { + name: "expect error when CapacityBlock set but not providing CapacityReservationID", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"set": "node"}, + Namespace: "default", + Name: "machine-aws-test1", + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To[string]("bootstrap-data"), + }, + }, + }, + machineConfig: &infrav1.AWSMachineSpec{ + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + UseCapacityBlock: aws.Bool(true), + InstanceType: "m5.large", + PlacementGroupPartition: 2, + UncompressedUserData: &isUncompressedFalse, + }, + awsCluster: &infrav1.AWSCluster{ + Spec: infrav1.AWSClusterSpec{ + NetworkSpec: infrav1.NetworkSpec{ + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + ID: "subnet-1", + IsPublic: false, + }, + infrav1.SubnetSpec{ + IsPublic: false, + }, + }, + }, + }, + Status: infrav1.AWSClusterStatus{ + Network: infrav1.NetworkStatus{ + SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{ + infrav1.SecurityGroupControlPlane: { + ID: "1", + }, + infrav1.SecurityGroupNode: { + ID: "2", + }, + infrav1.SecurityGroupLB: { + ID: "3", + }, + }, + APIServerELB: infrav1.LoadBalancer{ + DNSName: "test-apiserver.us-east-1.aws", + }, + }, + }, + }, + expect: func(m *mocks.MockEC2APIMockRecorder) { + m. + DescribeInstanceTypesWithContext(context.TODO(), gomock.Eq(&ec2.DescribeInstanceTypesInput{ + InstanceTypes: []*string{ + aws.String("m5.large"), + }, + })). + Return(&ec2.DescribeInstanceTypesOutput{ + InstanceTypes: []*ec2.InstanceTypeInfo{ + { + ProcessorInfo: &ec2.ProcessorInfo{ + SupportedArchitectures: []*string{ + aws.String("x86_64"), + }, + }, + }, + }, + }, nil) + }, + check: func(instance *infrav1.Instance, err error) { + expectedErrMsg := "capacityReservationID is required when CapacityBlock is enabled" + if err == nil { + t.Fatalf("Expected error, but got nil") + } + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Fatalf("Expected error: %s\nInstead got: `%s", expectedErrMsg, err.Error()) + } + }, + }, + { + name: "Simple, setting CapacityBlock to false and proving CapacityReservationID", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"set": "node"}, + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To[string]("bootstrap-data"), + }, + }, + }, + machineConfig: &infrav1.AWSMachineSpec{ + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.large", + UseCapacityBlock: aws.Bool(false), + CapacityReservationID: aws.String("cr-12345678901234567"), + }, + awsCluster: &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: infrav1.AWSClusterSpec{ + + NetworkSpec: infrav1.NetworkSpec{ + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + ID: "subnet-1", + IsPublic: false, + }, + infrav1.SubnetSpec{ + IsPublic: false, + }, + }, + VPC: infrav1.VPCSpec{ + ID: "vpc-test", + }, + }, + }, + Status: infrav1.AWSClusterStatus{ + Network: infrav1.NetworkStatus{ + SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{ + infrav1.SecurityGroupControlPlane: { + ID: "1", + }, + infrav1.SecurityGroupNode: { + ID: "2", + }, + infrav1.SecurityGroupLB: { + ID: "3", + }, + }, + APIServerELB: infrav1.LoadBalancer{ + DNSName: "test-apiserver.us-east-1.aws", + }, + }, + }, + }, + expect: func(m *mocks.MockEC2APIMockRecorder) { + m. + DescribeInstanceTypesWithContext(context.TODO(), gomock.Eq(&ec2.DescribeInstanceTypesInput{ + InstanceTypes: []*string{ + aws.String("m5.large"), + }, + })). + Return(&ec2.DescribeInstanceTypesOutput{ + InstanceTypes: []*ec2.InstanceTypeInfo{ + { + ProcessorInfo: &ec2.ProcessorInfo{ + SupportedArchitectures: []*string{ + aws.String("x86_64"), + }, + }, + }, + }, + }, nil) + m. // TODO: Restore these parameters, but with the tags as well + RunInstancesWithContext(context.TODO(), gomock.Any()). + Return(&ec2.Reservation{ + Instances: []*ec2.Instance{ + { + State: &ec2.InstanceState{ + Name: aws.String(ec2.InstanceStateNamePending), + }, + IamInstanceProfile: &ec2.IamInstanceProfile{ + Arn: aws.String("arn:aws:iam::123456789012:instance-profile/foo"), + }, + InstanceId: aws.String("two"), + InstanceType: aws.String("m5.large"), + SubnetId: aws.String("subnet-1"), + ImageId: aws.String("ami-1"), + RootDeviceName: aws.String("device-1"), + BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{ + { + DeviceName: aws.String("device-1"), + Ebs: &ec2.EbsInstanceBlockDevice{ + VolumeId: aws.String("volume-1"), + }, + }, + }, + Placement: &ec2.Placement{ + AvailabilityZone: &az, + }, + InstanceLifecycle: aws.String("scheduled"), + }, + }, + }, nil) + m. + DescribeNetworkInterfacesWithContext(context.TODO(), gomock.Any()). + Return(&ec2.DescribeNetworkInterfacesOutput{ + NetworkInterfaces: []*ec2.NetworkInterface{}, + NextToken: nil, + }, nil) + }, + check: func(instance *infrav1.Instance, err error) { + if err != nil { + t.Fatalf("did not expect error: %v", err) + } + }, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { @@ -5273,19 +5599,26 @@ func TestCreateInstance(t *testing.T) { } func TestGetInstanceMarketOptionsRequest(t *testing.T) { + mockCapacityReservationID := ptr.To[string]("cr-123") testCases := []struct { - name string - spotMarketOptions *infrav1.SpotMarketOptions - expectedRequest *ec2.InstanceMarketOptionsRequest + name string + instance *infrav1.Instance + expectedRequest *ec2.InstanceMarketOptionsRequest + expectedError error }{ { - name: "with no Spot options specified", - spotMarketOptions: nil, - expectedRequest: nil, + name: "with no Spot options specified", + expectedRequest: nil, + instance: &infrav1.Instance{ + SpotMarketOptions: nil, + }, + expectedError: nil, }, { - name: "with an empty Spot options specified", - spotMarketOptions: &infrav1.SpotMarketOptions{}, + name: "with an empty Spot options specified", + instance: &infrav1.Instance{ + SpotMarketOptions: &infrav1.SpotMarketOptions{}, + }, expectedRequest: &ec2.InstanceMarketOptionsRequest{ MarketType: aws.String(ec2.MarketTypeSpot), SpotOptions: &ec2.SpotMarketOptions{ @@ -5293,11 +5626,14 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime), }, }, + expectedError: nil, }, { name: "with an empty MaxPrice specified", - spotMarketOptions: &infrav1.SpotMarketOptions{ - MaxPrice: aws.String(""), + instance: &infrav1.Instance{ + SpotMarketOptions: &infrav1.SpotMarketOptions{ + MaxPrice: aws.String(""), + }, }, expectedRequest: &ec2.InstanceMarketOptionsRequest{ MarketType: aws.String(ec2.MarketTypeSpot), @@ -5306,11 +5642,14 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime), }, }, + expectedError: nil, }, { name: "with a valid MaxPrice specified", - spotMarketOptions: &infrav1.SpotMarketOptions{ - MaxPrice: aws.String("0.01"), + instance: &infrav1.Instance{ + SpotMarketOptions: &infrav1.SpotMarketOptions{ + MaxPrice: aws.String("0.01"), + }, }, expectedRequest: &ec2.InstanceMarketOptionsRequest{ MarketType: aws.String(ec2.MarketTypeSpot), @@ -5320,15 +5659,56 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { MaxPrice: aws.String("0.01"), }, }, + expectedError: nil, + }, + { + name: "with no MarketTypeCapacityBlock options specified", + instance: &infrav1.Instance{}, + expectedRequest: nil, + expectedError: nil, + }, + { + name: "with a CapacityBlock specified with capacityReservationID set to nil", + instance: &infrav1.Instance{ + UseCapacityBlock: aws.Bool(true), + CapacityReservationID: nil, + }, + expectedRequest: nil, + expectedError: errors.Errorf("capacityReservationID is required when CapacityBlock is enabled"), + }, + { + name: "with a CapacityBlock set to false with capacityReservationID set to nil", + instance: &infrav1.Instance{ + UseCapacityBlock: aws.Bool(true), + CapacityReservationID: mockCapacityReservationID, + }, + expectedRequest: &ec2.InstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeCapacityBlock), + }, + expectedError: nil, + }, + { + name: "with a CapacityBlock set to false with capacityReservationID set and empty Spot options specified", + instance: &infrav1.Instance{ + UseCapacityBlock: aws.Bool(true), + SpotMarketOptions: &infrav1.SpotMarketOptions{}, + CapacityReservationID: mockCapacityReservationID, + }, + expectedRequest: nil, + expectedError: errors.New("can't create spot capacity-blocks, remove spot market request"), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - request := getInstanceMarketOptionsRequest(tc.spotMarketOptions) - if !cmp.Equal(request, tc.expectedRequest) { - t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, request, tc.expectedRequest) + request, err := getInstanceMarketOptionsRequest(tc.instance) + g := NewWithT(t) + if tc.expectedError != nil { + g.Expect(err.Error()).To(Equal(tc.expectedError.Error())) + } else { + g.Expect(err).To(BeNil()) } + g.Expect(request).To(Equal(tc.expectedRequest)) }) } } diff --git a/pkg/cloud/services/ec2/launchtemplate.go b/pkg/cloud/services/ec2/launchtemplate.go index 5da57f2521..2383521925 100644 --- a/pkg/cloud/services/ec2/launchtemplate.go +++ b/pkg/cloud/services/ec2/launchtemplate.go @@ -519,7 +519,11 @@ func (s *Service) createLaunchTemplateData(scope scope.LaunchTemplateScope, imag // set the AMI ID data.ImageId = imageID - data.InstanceMarketOptions = getLaunchTemplateInstanceMarketOptionsRequest(scope.GetLaunchTemplate().SpotMarketOptions) + instanceMarketOptions, err := getLaunchTemplateInstanceMarketOptionsRequest(scope.GetLaunchTemplate()) + if err != nil { + return nil, err + } + data.InstanceMarketOptions = instanceMarketOptions data.PrivateDnsNameOptions = getLaunchTemplatePrivateDNSNameOptionsRequest(scope.GetLaunchTemplate().PrivateDNSName) blockDeviceMappings := []*ec2.LaunchTemplateBlockDeviceMappingRequest{} @@ -962,10 +966,24 @@ func (s *Service) getFilteredSecurityGroupIDs(securityGroup infrav1.AWSResourceR return ids, nil } -func getLaunchTemplateInstanceMarketOptionsRequest(spotMarketOptions *infrav1.SpotMarketOptions) *ec2.LaunchTemplateInstanceMarketOptionsRequest { - if spotMarketOptions == nil { +func getLaunchTemplateInstanceMarketOptionsRequest(i *expinfrav1.AWSLaunchTemplate) (*ec2.LaunchTemplateInstanceMarketOptionsRequest, error) { + if i.UseCapacityBlock != nil && i.SpotMarketOptions != nil { + return nil, errors.New("can't create spot capacity-blocks, remove spot market request") + } + + // Handle Capacity Block case. + if ptr.Deref(i.UseCapacityBlock, false) { + if i.CapacityReservationID == nil { + return nil, errors.Errorf("capacityReservationID is required when CapacityBlock is enabled") + } + return &ec2.LaunchTemplateInstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeCapacityBlock), + }, nil + } + + if i.SpotMarketOptions == nil { // Instance is not a Spot instance - return nil + return nil, nil } // Set required values for Spot instances @@ -974,7 +992,7 @@ func getLaunchTemplateInstanceMarketOptionsRequest(spotMarketOptions *infrav1.Sp // Persistent option is not available for EC2 autoscaling, EC2 makes a one-time request by default and setting request type should not be allowed. // For one-time requests, only terminate option is available as interruption behavior, and default for spotOptions.SetInstanceInterruptionBehavior() is terminate, so it is not set here explicitly. - if maxPrice := aws.StringValue(spotMarketOptions.MaxPrice); maxPrice != "" { + if maxPrice := aws.StringValue(i.SpotMarketOptions.MaxPrice); maxPrice != "" { spotOptions.SetMaxPrice(maxPrice) } @@ -982,7 +1000,7 @@ func getLaunchTemplateInstanceMarketOptionsRequest(spotMarketOptions *infrav1.Sp launchTemplateInstanceMarketOptionsRequest.SetMarketType(ec2.MarketTypeSpot) launchTemplateInstanceMarketOptionsRequest.SetSpotOptions(spotOptions) - return launchTemplateInstanceMarketOptionsRequest + return launchTemplateInstanceMarketOptionsRequest, nil } func getLaunchTemplatePrivateDNSNameOptionsRequest(privateDNSName *infrav1.PrivateDNSName) *ec2.LaunchTemplatePrivateDnsNameOptionsRequest { diff --git a/pkg/cloud/services/ec2/launchtemplate_test.go b/pkg/cloud/services/ec2/launchtemplate_test.go index 4553ad4546..80b390b8b3 100644 --- a/pkg/cloud/services/ec2/launchtemplate_test.go +++ b/pkg/cloud/services/ec2/launchtemplate_test.go @@ -1076,6 +1076,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { awsResourceReference []infrav1.AWSResourceReference expect func(m *mocks.MockEC2APIMockRecorder) wantErr bool + useCapacityBlocks bool }{ { name: "Should successfully creates launch template version", @@ -1128,6 +1129,55 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { }) }, }, + { + name: "Should successfully create launch template version with capacity-block", + awsResourceReference: []infrav1.AWSResourceReference{{ID: aws.String("1")}}, + useCapacityBlocks: true, + expect: func(m *mocks.MockEC2APIMockRecorder) { + sgMap := make(map[infrav1.SecurityGroupRole]infrav1.SecurityGroup) + sgMap[infrav1.SecurityGroupNode] = infrav1.SecurityGroup{ID: "1"} + sgMap[infrav1.SecurityGroupLB] = infrav1.SecurityGroup{ID: "2"} + + expectedInput := &ec2.CreateLaunchTemplateVersionInput{ + LaunchTemplateData: &ec2.RequestLaunchTemplateData{ + InstanceType: aws.String("t3.large"), + IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecificationRequest{ + Name: aws.String("instance-profile"), + }, + KeyName: aws.String("default"), + UserData: ptr.To[string](base64.StdEncoding.EncodeToString(userData)), + SecurityGroupIds: aws.StringSlice([]string{"nodeSG", "lbSG", "1"}), + ImageId: aws.String("imageID"), + InstanceMarketOptions: &ec2.LaunchTemplateInstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeCapacityBlock), + }, + TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ + { + ResourceType: aws.String(ec2.ResourceTypeInstance), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), + }, + { + ResourceType: aws.String(ec2.ResourceTypeVolume), + Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + }, + }, + }, + LaunchTemplateId: aws.String("launch-template-id"), + } + m.CreateLaunchTemplateVersionWithContext(context.TODO(), gomock.AssignableToTypeOf(expectedInput)).Return(&ec2.CreateLaunchTemplateVersionOutput{ + LaunchTemplateVersion: &ec2.LaunchTemplateVersion{ + LaunchTemplateId: aws.String("launch-template-id"), + }, + }, nil).Do( + func(ctx context.Context, arg *ec2.CreateLaunchTemplateVersionInput, requestOptions ...request.Option) { + // formatting added to match tags slice during cmp.Equal() + formatTagsInput(arg) + if !cmp.Equal(expectedInput, arg) { + t.Fatalf("mismatch in input expected: %+v, but got %+v, diff: %s", expectedInput, arg, cmp.Diff(expectedInput, arg)) + } + }) + }, + }, { name: "Should return error if AWS failed during launch template version creation", awsResourceReference: []infrav1.AWSResourceReference{{ID: aws.String("1")}}, @@ -1181,7 +1231,6 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - scheme, err := setupScheme() g.Expect(err).NotTo(HaveOccurred()) client := fake.NewClientBuilder().WithScheme(scheme).Build() @@ -1189,7 +1238,12 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { cs, err := setupClusterScope(client) g.Expect(err).NotTo(HaveOccurred()) - ms, err := setupMachinePoolScope(client, cs) + var ms *scope.MachinePoolScope + if tc.useCapacityBlocks { + ms, err = setupCapacityBlocksMachinePoolScope(client, cs) + } else { + ms, err = setupMachinePoolScope(client, cs) + } g.Expect(err).NotTo(HaveOccurred()) ms.AWSMachinePool.Spec.AWSLaunchTemplate.AdditionalSecurityGroups = tc.awsResourceReference From c1d94dc12cc3ff7dbb82a32a6d0ded926e988e26 Mon Sep 17 00:00:00 2001 From: Thirumalesh Aaraveti Date: Thu, 12 Dec 2024 17:09:47 +0530 Subject: [PATCH 2/2] new API field marketType --- api/v1beta1/awscluster_conversion.go | 2 +- api/v1beta1/awsmachine_conversion.go | 4 +- api/v1beta1/zz_generated.conversion.go | 4 +- api/v1beta2/awsmachine_types.go | 12 ++-- api/v1beta2/awsmachine_webhook.go | 7 +- api/v1beta2/awsmachine_webhook_test.go | 21 ++++-- api/v1beta2/types.go | 25 ++++++- api/v1beta2/zz_generated.deepcopy.go | 10 --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 36 +++++++--- ...tructure.cluster.x-k8s.io_awsclusters.yaml | 18 +++-- ...ture.cluster.x-k8s.io_awsmachinepools.yaml | 18 +++-- ...tructure.cluster.x-k8s.io_awsmachines.yaml | 18 +++-- ....cluster.x-k8s.io_awsmachinetemplates.yaml | 18 +++-- ...uster.x-k8s.io_awsmanagedmachinepools.yaml | 18 +++-- exp/api/v1beta1/conversion.go | 8 +-- exp/api/v1beta1/zz_generated.conversion.go | 2 +- exp/api/v1beta2/awsmachinepool_webhook.go | 7 +- .../v1beta2/awsmachinepool_webhook_test.go | 20 ++++-- exp/api/v1beta2/types.go | 10 ++- exp/api/v1beta2/zz_generated.deepcopy.go | 5 -- pkg/cloud/services/ec2/helper_test.go | 2 +- pkg/cloud/services/ec2/instances.go | 67 +++++++++++-------- pkg/cloud/services/ec2/instances_test.go | 40 +++++++---- pkg/cloud/services/ec2/launchtemplate.go | 53 +++++++++------ pkg/cloud/services/ec2/launchtemplate_test.go | 6 +- 25 files changed, 282 insertions(+), 149 deletions(-) diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index 3fa974f07f..94f8078f96 100644 --- a/api/v1beta1/awscluster_conversion.go +++ b/api/v1beta1/awscluster_conversion.go @@ -60,7 +60,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error { dst.Status.Bastion.PrivateDNSName = restored.Status.Bastion.PrivateDNSName dst.Status.Bastion.PublicIPOnLaunch = restored.Status.Bastion.PublicIPOnLaunch dst.Status.Bastion.CapacityReservationID = restored.Status.Bastion.CapacityReservationID - dst.Status.Bastion.UseCapacityBlock = restored.Status.Bastion.UseCapacityBlock + dst.Status.Bastion.MarketType = restored.Status.Bastion.MarketType } dst.Spec.Partition = restored.Spec.Partition diff --git a/api/v1beta1/awsmachine_conversion.go b/api/v1beta1/awsmachine_conversion.go index 4a132dc5cb..829bdc912b 100644 --- a/api/v1beta1/awsmachine_conversion.go +++ b/api/v1beta1/awsmachine_conversion.go @@ -42,7 +42,7 @@ func (src *AWSMachine) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.PrivateDNSName = restored.Spec.PrivateDNSName dst.Spec.SecurityGroupOverrides = restored.Spec.SecurityGroupOverrides dst.Spec.CapacityReservationID = restored.Spec.CapacityReservationID - dst.Spec.UseCapacityBlock = restored.Spec.UseCapacityBlock + dst.Spec.MarketType = restored.Spec.MarketType if restored.Spec.ElasticIPPool != nil { if dst.Spec.ElasticIPPool == nil { dst.Spec.ElasticIPPool = &infrav1.ElasticIPPool{} @@ -105,7 +105,7 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.PrivateDNSName = restored.Spec.Template.Spec.PrivateDNSName dst.Spec.Template.Spec.SecurityGroupOverrides = restored.Spec.Template.Spec.SecurityGroupOverrides dst.Spec.Template.Spec.CapacityReservationID = restored.Spec.Template.Spec.CapacityReservationID - dst.Spec.Template.Spec.UseCapacityBlock = restored.Spec.Template.Spec.UseCapacityBlock + dst.Spec.Template.Spec.MarketType = restored.Spec.Template.Spec.MarketType if restored.Spec.Template.Spec.ElasticIPPool != nil { if dst.Spec.Template.Spec.ElasticIPPool == nil { dst.Spec.Template.Spec.ElasticIPPool = &infrav1.ElasticIPPool{} diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 56ea41f118..286b6e7530 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -1434,7 +1434,7 @@ func autoConvert_v1beta2_AWSMachineSpec_To_v1beta1_AWSMachineSpec(in *v1beta2.AW out.Tenancy = in.Tenancy // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type - // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type + // WARNING: in.MarketType requires manual conversion: does not exist in peer-type return nil } @@ -2037,7 +2037,7 @@ func autoConvert_v1beta2_Instance_To_v1beta1_Instance(in *v1beta2.Instance, out // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type // WARNING: in.PublicIPOnLaunch requires manual conversion: does not exist in peer-type // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type - // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type + // WARNING: in.MarketType requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta2/awsmachine_types.go b/api/v1beta2/awsmachine_types.go index 4aed7dd1d1..4ba87c72ef 100644 --- a/api/v1beta2/awsmachine_types.go +++ b/api/v1beta2/awsmachine_types.go @@ -198,10 +198,14 @@ type AWSMachineSpec struct { // +optional CapacityReservationID *string `json:"capacityReservationId,omitempty"` - // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - // If enabled, CapacityReservationID must be specified to identify the target reservation. - // +optional - UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` + // marketType specifies the type of market for the EC2 instance. Valid values include: + // "on-demand" (default): The instance runs as a standard On-Demand instance. + // "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + // "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If this value is selected, CapacityReservationID must be specified to identify the target reservation. + // If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + // +optional + MarketType MarketType `json:"marketType,omitempty"` } // CloudInit defines options related to the bootstrapping systems where diff --git a/api/v1beta2/awsmachine_webhook.go b/api/v1beta2/awsmachine_webhook.go index cf729948a4..5fcd2750ee 100644 --- a/api/v1beta2/awsmachine_webhook.go +++ b/api/v1beta2/awsmachine_webhook.go @@ -364,8 +364,11 @@ func (r *AWSMachine) validateNetworkElasticIPPool() field.ErrorList { func (r *AWSMachine) validateInstanceMarketType() field.ErrorList { var allErrs field.ErrorList - if r.Spec.UseCapacityBlock != nil && r.Spec.SpotMarketOptions != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "useCapacityBlock"), "useCapacityBlock and spotMarketOptions cannot be used together")) + if r.Spec.MarketType == MarketTypeCapacityBlock && r.Spec.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "MarketType"), "MarketType set to CapacityBlock and spotMarketOptions cannot be used together")) + } + if r.Spec.MarketType == MarketTypeOnDemand && r.Spec.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "MarketType"), "setting MarketType to OnDemand and spotMarketOptions cannot be used together")) } return allErrs } diff --git a/api/v1beta2/awsmachine_webhook_test.go b/api/v1beta2/awsmachine_webhook_test.go index bf31bff812..eeb067abe0 100644 --- a/api/v1beta2/awsmachine_webhook_test.go +++ b/api/v1beta2/awsmachine_webhook_test.go @@ -218,10 +218,10 @@ func TestAWSMachineCreate(t *testing.T) { wantErr: false, }, { - name: "invalid useCapacityBlock and spotMarketOptions are specified", + name: "invalid case, MarketType set to MarketTypeCapacityBlock and spotMarketOptions are specified", machine: &AWSMachine{ Spec: AWSMachineSpec{ - UseCapacityBlock: aws.Bool(true), + MarketType: MarketTypeCapacityBlock, SpotMarketOptions: &SpotMarketOptions{}, InstanceType: "test", }, @@ -229,11 +229,22 @@ func TestAWSMachineCreate(t *testing.T) { wantErr: true, }, { - name: "valid useCapacityBlock is specified", + name: "invalid case, MarketType set to MarketTypeOnDemand and spotMarketOptions are specified", machine: &AWSMachine{ Spec: AWSMachineSpec{ - UseCapacityBlock: aws.Bool(true), - InstanceType: "test", + MarketType: MarketTypeOnDemand, + SpotMarketOptions: &SpotMarketOptions{}, + InstanceType: "test", + }, + }, + wantErr: true, + }, + { + name: "valid MarketType set to MarketTypeCapacityBlock is specified", + machine: &AWSMachine{ + Spec: AWSMachineSpec{ + MarketType: MarketTypeCapacityBlock, + InstanceType: "test", }, }, wantErr: false, diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index 0fe959f2bf..ae2e6c42c7 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -262,12 +262,31 @@ type Instance struct { // +optional CapacityReservationID *string `json:"capacityReservationId,omitempty"` - // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - // If enabled, CapacityReservationID must be specified to identify the target reservation. + // marketType specifies the type of market for the EC2 instance. Valid values include: + // "on-demand" (default): The instance runs as a standard On-Demand instance. + // "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + // "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If this value is selected, CapacityReservationID must be specified to identify the target reservation. + // If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". // +optional - UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` + MarketType MarketType `json:"marketType,omitempty"` } +// MarketType describes the market type of an Instance +// +kubebuilder:validation:Enum:=OnDemand;Spot;CapacityBlock +type MarketType string + +const ( + // MarketTypeOnDemand is a MarketType enum value + MarketTypeOnDemand MarketType = "OnDemand" + + // MarketTypeSpot is a MarketType enum value + MarketTypeSpot MarketType = "Spot" + + // MarketTypeCapacityBlock is a MarketType enum value + MarketTypeCapacityBlock MarketType = "CapacityBlock" +) + // InstanceMetadataState describes the state of InstanceMetadataOptions.HttpEndpoint and InstanceMetadataOptions.InstanceMetadataTags type InstanceMetadataState string diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 81ff8d302d..c727483f6b 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -772,11 +772,6 @@ func (in *AWSMachineSpec) DeepCopyInto(out *AWSMachineSpec) { *out = new(string) **out = **in } - if in.UseCapacityBlock != nil { - in, out := &in.UseCapacityBlock, &out.UseCapacityBlock - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSMachineSpec. @@ -1609,11 +1604,6 @@ func (in *Instance) DeepCopyInto(out *Instance) { *out = new(string) **out = **in } - if in.UseCapacityBlock != nil { - in, out := &in.UseCapacityBlock, &out.UseCapacityBlock - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Instance. diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 95bf5b6bdb..c8a828d0ef 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -1210,6 +1210,19 @@ spec: instanceState: description: The current state of the instance. type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string networkInterfaces: description: Specifies ENIs attached to instance items: @@ -1376,11 +1389,6 @@ spec: type: description: The instance type. type: string - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. @@ -3255,6 +3263,19 @@ spec: instanceState: description: The current state of the instance. type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string networkInterfaces: description: Specifies ENIs attached to instance items: @@ -3421,11 +3442,6 @@ spec: type: description: The instance type. type: string - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml index 6bb89799c4..80c58fea4f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -2177,6 +2177,19 @@ spec: instanceState: description: The current state of the instance. type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string networkInterfaces: description: Specifies ENIs attached to instance items: @@ -2343,11 +2356,6 @@ spec: type: description: The instance type. type: string - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean userData: description: |- UserData is the raw data script passed to the instance which is run upon bootstrap. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml index 135fcecfa4..33de122a97 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml @@ -728,6 +728,19 @@ spec: description: 'InstanceType is the type of instance to create. Example: m4.xlarge' type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string name: description: The name of the launch template. type: string @@ -850,11 +863,6 @@ spec: SSHKeyName is the name of the ssh key to attach to the instance. Valid values are empty string (do not use SSH keys), a valid SSH key name, or omitted (use the default SSH key name) type: string - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean versionNumber: description: |- VersionNumber is the version of the launch template that is applied. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml index 77172ac074..95f008e0c8 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml @@ -878,6 +878,19 @@ spec: m4.xlarge' minLength: 2 type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string networkInterfaces: description: |- NetworkInterfaces is a list of ENIs to associate with the instance. @@ -1080,11 +1093,6 @@ spec: cloud-init has built-in support for gzip-compressed user data user data stored in aws secret manager is always gzip-compressed. type: boolean - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean required: - instanceType type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml index 18f94fdb50..5ec4fd6e22 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml @@ -812,6 +812,19 @@ spec: Example: m4.xlarge' minLength: 2 type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string networkInterfaces: description: |- NetworkInterfaces is a list of ENIs to associate with the instance. @@ -1021,11 +1034,6 @@ spec: cloud-init has built-in support for gzip-compressed user data user data stored in aws secret manager is always gzip-compressed. type: boolean - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean required: - instanceType type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml index 119dfae34a..8ad5b83996 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml @@ -724,6 +724,19 @@ spec: description: 'InstanceType is the type of instance to create. Example: m4.xlarge' type: string + marketType: + description: |- + marketType specifies the type of market for the EC2 instance. Valid values include: + "on-demand" (default): The instance runs as a standard On-Demand instance. + "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + If this value is selected, CapacityReservationID must be specified to identify the target reservation. + If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". + enum: + - OnDemand + - Spot + - CapacityBlock + type: string name: description: The name of the launch template. type: string @@ -846,11 +859,6 @@ spec: SSHKeyName is the name of the ssh key to attach to the instance. Valid values are empty string (do not use SSH keys), a valid SSH key name, or omitted (use the default SSH key name) type: string - useCapacityBlock: - description: |- - UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - If enabled, CapacityReservationID must be specified to identify the target reservation. - type: boolean versionNumber: description: |- VersionNumber is the version of the launch template that is applied. diff --git a/exp/api/v1beta1/conversion.go b/exp/api/v1beta1/conversion.go index 8ad51f3839..16af878660 100644 --- a/exp/api/v1beta1/conversion.go +++ b/exp/api/v1beta1/conversion.go @@ -61,8 +61,8 @@ func (src *AWSMachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.AWSLaunchTemplate.CapacityReservationID = restored.Spec.AWSLaunchTemplate.CapacityReservationID } - if restored.Spec.AWSLaunchTemplate.UseCapacityBlock != nil { - dst.Spec.AWSLaunchTemplate.UseCapacityBlock = restored.Spec.AWSLaunchTemplate.UseCapacityBlock + if restored.Spec.AWSLaunchTemplate.MarketType != "" { + dst.Spec.AWSLaunchTemplate.MarketType = restored.Spec.AWSLaunchTemplate.MarketType } dst.Spec.DefaultInstanceWarmup = restored.Spec.DefaultInstanceWarmup @@ -121,8 +121,8 @@ func (src *AWSManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.AWSLaunchTemplate.CapacityReservationID = restored.Spec.AWSLaunchTemplate.CapacityReservationID } - if restored.Spec.AWSLaunchTemplate.UseCapacityBlock != nil { - dst.Spec.AWSLaunchTemplate.UseCapacityBlock = restored.Spec.AWSLaunchTemplate.UseCapacityBlock + if restored.Spec.AWSLaunchTemplate.MarketType != "" { + dst.Spec.AWSLaunchTemplate.MarketType = restored.Spec.AWSLaunchTemplate.MarketType } } diff --git a/exp/api/v1beta1/zz_generated.conversion.go b/exp/api/v1beta1/zz_generated.conversion.go index da119ccc0f..3cb29cd7b1 100644 --- a/exp/api/v1beta1/zz_generated.conversion.go +++ b/exp/api/v1beta1/zz_generated.conversion.go @@ -415,7 +415,7 @@ func autoConvert_v1beta2_AWSLaunchTemplate_To_v1beta1_AWSLaunchTemplate(in *v1be // WARNING: in.InstanceMetadataOptions requires manual conversion: does not exist in peer-type // WARNING: in.PrivateDNSName requires manual conversion: does not exist in peer-type // WARNING: in.CapacityReservationID requires manual conversion: does not exist in peer-type - // WARNING: in.UseCapacityBlock requires manual conversion: does not exist in peer-type + // WARNING: in.MarketType requires manual conversion: does not exist in peer-type return nil } diff --git a/exp/api/v1beta2/awsmachinepool_webhook.go b/exp/api/v1beta2/awsmachinepool_webhook.go index fce1a61031..49e2be4e51 100644 --- a/exp/api/v1beta2/awsmachinepool_webhook.go +++ b/exp/api/v1beta2/awsmachinepool_webhook.go @@ -191,8 +191,11 @@ func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) { func (r *AWSMachinePool) validateInstanceMarketType() field.ErrorList { var allErrs field.ErrorList - if r.Spec.AWSLaunchTemplate.UseCapacityBlock != nil && r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "useCapacityBlock"), "useCapacityBlock and spotMarketOptions cannot be used together")) + if r.Spec.AWSLaunchTemplate.MarketType == v1beta2.MarketTypeCapacityBlock && r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "MarketType"), "setting MarketType to CapacityBlock and spotMarketOptions cannot be used together")) + } + if r.Spec.AWSLaunchTemplate.MarketType == v1beta2.MarketTypeOnDemand && r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "MarketType"), "setting MarketType to OnDemand and spotMarketOptions cannot be used together")) } return allErrs } diff --git a/exp/api/v1beta2/awsmachinepool_webhook_test.go b/exp/api/v1beta2/awsmachinepool_webhook_test.go index 65ecfb21dd..74d3704385 100644 --- a/exp/api/v1beta2/awsmachinepool_webhook_test.go +++ b/exp/api/v1beta2/awsmachinepool_webhook_test.go @@ -175,11 +175,11 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) { wantErr: true, }, { - name: "invalid useCapacityBlock and spotMarketOptions are specified", + name: "invalid, MarketType set to MarketTypeCapacityBlock and spotMarketOptions are specified", pool: &AWSMachinePool{ Spec: AWSMachinePoolSpec{ AWSLaunchTemplate: AWSLaunchTemplate{ - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, SpotMarketOptions: &infrav1.SpotMarketOptions{}, }, }, @@ -187,11 +187,23 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) { wantErr: true, }, { - name: "valid useCapacityBlock is specified", + name: "invalid, MarketType set to MarketTypeOnDemand and spotMarketOptions are specified", pool: &AWSMachinePool{ Spec: AWSMachinePoolSpec{ AWSLaunchTemplate: AWSLaunchTemplate{ - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeOnDemand, + SpotMarketOptions: &infrav1.SpotMarketOptions{}, + }, + }, + }, + wantErr: true, + }, + { + name: "valid MarketType set to MarketTypeCapacityBlock is specified", + pool: &AWSMachinePool{ + Spec: AWSMachinePoolSpec{ + AWSLaunchTemplate: AWSLaunchTemplate{ + MarketType: infrav1.MarketTypeCapacityBlock, }, }, }, diff --git a/exp/api/v1beta2/types.go b/exp/api/v1beta2/types.go index 9622786073..530a581bb0 100644 --- a/exp/api/v1beta2/types.go +++ b/exp/api/v1beta2/types.go @@ -133,10 +133,14 @@ type AWSLaunchTemplate struct { // +optional CapacityReservationID *string `json:"capacityReservationId,omitempty"` - // UseCapacityBlock enables usage of pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. - // If enabled, CapacityReservationID must be specified to identify the target reservation. + // marketType specifies the type of market for the EC2 instance. Valid values include: + // "on-demand" (default): The instance runs as a standard On-Demand instance. + // "spot": The instance runs as a Spot instance. When SpotMarketOptions is provided, the MarketType defaults to "spot". + // "capacity-block": The instance utilizes pre-purchased compute capacity (capacity blocks) with AWS Capacity Reservations. + // If this value is selected, CapacityReservationID must be specified to identify the target reservation. + // If MarketType is not specified and SpotMarketOptions is provided, the MarketType defaults to "spot". // +optional - UseCapacityBlock *bool `json:"useCapacityBlock,omitempty"` + MarketType infrav1.MarketType `json:"marketType,omitempty"` } // Overrides are used to override the instance type specified by the launch template with multiple diff --git a/exp/api/v1beta2/zz_generated.deepcopy.go b/exp/api/v1beta2/zz_generated.deepcopy.go index 046361f5c1..8c016a39ea 100644 --- a/exp/api/v1beta2/zz_generated.deepcopy.go +++ b/exp/api/v1beta2/zz_generated.deepcopy.go @@ -141,11 +141,6 @@ func (in *AWSLaunchTemplate) DeepCopyInto(out *AWSLaunchTemplate) { *out = new(string) **out = **in } - if in.UseCapacityBlock != nil { - in, out := &in.UseCapacityBlock, &out.UseCapacityBlock - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSLaunchTemplate. diff --git a/pkg/cloud/services/ec2/helper_test.go b/pkg/cloud/services/ec2/helper_test.go index dffb5180e4..eb296f85b7 100644 --- a/pkg/cloud/services/ec2/helper_test.go +++ b/pkg/cloud/services/ec2/helper_test.go @@ -90,7 +90,7 @@ func newAWSCapacityBlockMachinePool() *expinfrav1.AWSMachinePool { AMI: infrav1.AMIReference{}, InstanceType: "t3.large", SSHKeyName: aws.String("default"), - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, CapacityReservationID: aws.String("cr-12345678901234567"), }, }, diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go index f0044d06a7..408e0ab44b 100644 --- a/pkg/cloud/services/ec2/instances.go +++ b/pkg/cloud/services/ec2/instances.go @@ -253,7 +253,7 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use input.CapacityReservationID = scope.AWSMachine.Spec.CapacityReservationID - input.UseCapacityBlock = scope.AWSMachine.Spec.UseCapacityBlock + input.MarketType = scope.AWSMachine.Spec.MarketType s.scope.Debug("Running instance", "machine-role", scope.Role()) s.scope.Debug("Running instance with instance metadata options", "metadata options", input.InstanceMetadataOptions) @@ -1146,47 +1146,56 @@ func getCapacityReservationSpecification(capacityReservationID *string) *ec2.Cap } func getInstanceMarketOptionsRequest(i *infrav1.Instance) (*ec2.InstanceMarketOptionsRequest, error) { - if i.UseCapacityBlock != nil && i.SpotMarketOptions != nil { + if i.MarketType != "" && i.MarketType == infrav1.MarketTypeCapacityBlock && i.SpotMarketOptions != nil { return nil, errors.New("can't create spot capacity-blocks, remove spot market request") } - // Handle Capacity Block case. - if ptr.Deref(i.UseCapacityBlock, false) { + // Infer MarketType if not explicitly set + if i.SpotMarketOptions != nil && i.MarketType == "" { + i.MarketType = infrav1.MarketTypeSpot + } + + if i.MarketType == "" { + i.MarketType = infrav1.MarketTypeOnDemand + } + + switch i.MarketType { + case infrav1.MarketTypeCapacityBlock: if i.CapacityReservationID == nil { return nil, errors.Errorf("capacityReservationID is required when CapacityBlock is enabled") } return &ec2.InstanceMarketOptionsRequest{ MarketType: aws.String(ec2.MarketTypeCapacityBlock), }, nil - } - // Handle Spot instance case. - if i.SpotMarketOptions == nil { - // Instance is not a Spot instance - return nil, nil - } + case infrav1.MarketTypeSpot: + // Set required values for Spot instances + spotOpts := &ec2.SpotMarketOptions{ + // The following two options ensure that: + // - If an instance is interrupted, it is terminated rather than hibernating or stopping + // - No replacement instance will be created if the instance is interrupted + // - If the spot request cannot immediately be fulfilled, it will not be created + // This behaviour should satisfy the 1:1 mapping of Machines to Instances as + // assumed by the Cluster API. + InstanceInterruptionBehavior: aws.String(ec2.InstanceInterruptionBehaviorTerminate), + SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime), + } - // Set required values for Spot instances - spotOpts := &ec2.SpotMarketOptions{ - // The following two options ensure that: - // - If an instance is interrupted, it is terminated rather than hibernating or stopping - // - No replacement instance will be created if the instance is interrupted - // - If the spot request cannot immediately be fulfilled, it will not be created - // This behaviour should satisfy the 1:1 mapping of Machines to Instances as - // assumed by the Cluster API. - InstanceInterruptionBehavior: aws.String(ec2.InstanceInterruptionBehaviorTerminate), - SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime), - } + if maxPrice := aws.StringValue(i.SpotMarketOptions.MaxPrice); maxPrice != "" { + spotOpts.MaxPrice = aws.String(maxPrice) + } - maxPrice := ptr.Deref(i.SpotMarketOptions.MaxPrice, "") - if maxPrice != "" { - spotOpts.MaxPrice = aws.String(maxPrice) + return &ec2.InstanceMarketOptionsRequest{ + MarketType: aws.String(ec2.MarketTypeSpot), + SpotOptions: spotOpts, + }, nil + case infrav1.MarketTypeOnDemand: + // Instance is on-demand or empty + return nil, nil + default: + // Invalid MarketType provided + return nil, errors.Errorf("invalid MarketType %s, must be spot/capacity-block or empty", i.MarketType) } - - return &ec2.InstanceMarketOptionsRequest{ - MarketType: aws.String(ec2.MarketTypeSpot), - SpotOptions: spotOpts, - }, nil } func getInstanceMetadataOptionsRequest(metadataOptions *infrav1.InstanceMetadataOptions) *ec2.InstanceMetadataOptionsRequest { diff --git a/pkg/cloud/services/ec2/instances_test.go b/pkg/cloud/services/ec2/instances_test.go index 9c33f3bf85..3d9fcf5a31 100644 --- a/pkg/cloud/services/ec2/instances_test.go +++ b/pkg/cloud/services/ec2/instances_test.go @@ -5198,7 +5198,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "Simple, setting CapacityBlock and providing CapacityReservationID", + name: "Simple, setting MarketType to MarketTypeCapacityBlock and providing CapacityReservationID", machine: &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -5214,7 +5214,7 @@ func TestCreateInstance(t *testing.T) { ID: aws.String("abc"), }, InstanceType: "m5.large", - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, CapacityReservationID: aws.String("cr-12345678901234567"), }, awsCluster: &infrav1.AWSCluster{ @@ -5319,7 +5319,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "expect error when CapacityBlock set but not providing CapacityReservationID", + name: "expect error when MarketType to MarketTypeCapacityBlock set but not providing CapacityReservationID", machine: &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -5336,7 +5336,7 @@ func TestCreateInstance(t *testing.T) { AMI: infrav1.AMIReference{ ID: aws.String("abc"), }, - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, InstanceType: "m5.large", PlacementGroupPartition: 2, UncompressedUserData: &isUncompressedFalse, @@ -5404,7 +5404,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "Simple, setting CapacityBlock to false and proving CapacityReservationID", + name: "Simple, setting not setting MarketType and proving CapacityReservationID", machine: &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -5420,7 +5420,6 @@ func TestCreateInstance(t *testing.T) { ID: aws.String("abc"), }, InstanceType: "m5.large", - UseCapacityBlock: aws.Bool(false), CapacityReservationID: aws.String("cr-12345678901234567"), }, awsCluster: &infrav1.AWSCluster{ @@ -5506,7 +5505,8 @@ func TestCreateInstance(t *testing.T) { Placement: &ec2.Placement{ AvailabilityZone: &az, }, - InstanceLifecycle: aws.String("scheduled"), + CapacityReservationId: aws.String("cr-12345678901234567"), + InstanceLifecycle: aws.String("scheduled"), }, }, }, nil) @@ -5614,6 +5614,20 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { }, expectedError: nil, }, + { + name: "with no MarketType", + expectedRequest: nil, + instance: &infrav1.Instance{}, + expectedError: nil, + }, + { + name: "invalid MarketType specified", + expectedRequest: nil, + instance: &infrav1.Instance{ + MarketType: infrav1.MarketType("inValid"), + }, + expectedError: errors.New("invalid MarketType inValid, must be spot/capacity-block or empty"), + }, { name: "with an empty Spot options specified", instance: &infrav1.Instance{ @@ -5668,18 +5682,18 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { expectedError: nil, }, { - name: "with a CapacityBlock specified with capacityReservationID set to nil", + name: "with a MarketType to MarketTypeCapacityBlock specified with capacityReservationID set to nil", instance: &infrav1.Instance{ - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, CapacityReservationID: nil, }, expectedRequest: nil, expectedError: errors.Errorf("capacityReservationID is required when CapacityBlock is enabled"), }, { - name: "with a CapacityBlock set to false with capacityReservationID set to nil", + name: "with a MarketType to MarketTypeCapacityBlock with capacityReservationID set to nil", instance: &infrav1.Instance{ - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, CapacityReservationID: mockCapacityReservationID, }, expectedRequest: &ec2.InstanceMarketOptionsRequest{ @@ -5688,9 +5702,9 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) { expectedError: nil, }, { - name: "with a CapacityBlock set to false with capacityReservationID set and empty Spot options specified", + name: "with a MarketType to MarketTypeCapacityBlock set with capacityReservationID set and empty Spot options specified", instance: &infrav1.Instance{ - UseCapacityBlock: aws.Bool(true), + MarketType: infrav1.MarketTypeCapacityBlock, SpotMarketOptions: &infrav1.SpotMarketOptions{}, CapacityReservationID: mockCapacityReservationID, }, diff --git a/pkg/cloud/services/ec2/launchtemplate.go b/pkg/cloud/services/ec2/launchtemplate.go index 2383521925..0fa8d762e6 100644 --- a/pkg/cloud/services/ec2/launchtemplate.go +++ b/pkg/cloud/services/ec2/launchtemplate.go @@ -967,40 +967,53 @@ func (s *Service) getFilteredSecurityGroupIDs(securityGroup infrav1.AWSResourceR } func getLaunchTemplateInstanceMarketOptionsRequest(i *expinfrav1.AWSLaunchTemplate) (*ec2.LaunchTemplateInstanceMarketOptionsRequest, error) { - if i.UseCapacityBlock != nil && i.SpotMarketOptions != nil { + if i.MarketType != "" && i.MarketType != infrav1.MarketTypeCapacityBlock && i.SpotMarketOptions != nil { return nil, errors.New("can't create spot capacity-blocks, remove spot market request") } - // Handle Capacity Block case. - if ptr.Deref(i.UseCapacityBlock, false) { + // Infer MarketType if not explicitly set and SpotMarketOptions specified + if i.SpotMarketOptions != nil && i.MarketType == "" { + i.MarketType = infrav1.MarketTypeSpot + } + + // Infer MarketType if not explicitly set + if i.MarketType == "" { + i.MarketType = infrav1.MarketTypeOnDemand + } + + switch i.MarketType { + case infrav1.MarketTypeCapacityBlock: + // Handle Capacity Block case. if i.CapacityReservationID == nil { return nil, errors.Errorf("capacityReservationID is required when CapacityBlock is enabled") } return &ec2.LaunchTemplateInstanceMarketOptionsRequest{ MarketType: aws.String(ec2.MarketTypeCapacityBlock), }, nil - } - - if i.SpotMarketOptions == nil { - // Instance is not a Spot instance - return nil, nil - } - // Set required values for Spot instances - spotOptions := &ec2.LaunchTemplateSpotMarketOptionsRequest{} + case infrav1.MarketTypeSpot: + // Set required values for Spot instances + spotOptions := &ec2.LaunchTemplateSpotMarketOptionsRequest{} - // Persistent option is not available for EC2 autoscaling, EC2 makes a one-time request by default and setting request type should not be allowed. - // For one-time requests, only terminate option is available as interruption behavior, and default for spotOptions.SetInstanceInterruptionBehavior() is terminate, so it is not set here explicitly. + // Persistent option is not available for EC2 autoscaling, EC2 makes a one-time request by default and setting request type should not be allowed. + // For one-time requests, only terminate option is available as interruption behavior, and default for spotOptions.SetInstanceInterruptionBehavior() is terminate, so it is not set here explicitly. - if maxPrice := aws.StringValue(i.SpotMarketOptions.MaxPrice); maxPrice != "" { - spotOptions.SetMaxPrice(maxPrice) - } + if maxPrice := aws.StringValue(i.SpotMarketOptions.MaxPrice); maxPrice != "" { + spotOptions.SetMaxPrice(maxPrice) + } - launchTemplateInstanceMarketOptionsRequest := &ec2.LaunchTemplateInstanceMarketOptionsRequest{} - launchTemplateInstanceMarketOptionsRequest.SetMarketType(ec2.MarketTypeSpot) - launchTemplateInstanceMarketOptionsRequest.SetSpotOptions(spotOptions) + launchTemplateInstanceMarketOptionsRequest := &ec2.LaunchTemplateInstanceMarketOptionsRequest{} + launchTemplateInstanceMarketOptionsRequest.SetMarketType(ec2.MarketTypeSpot) + launchTemplateInstanceMarketOptionsRequest.SetSpotOptions(spotOptions) - return launchTemplateInstanceMarketOptionsRequest, nil + return launchTemplateInstanceMarketOptionsRequest, nil + case infrav1.MarketTypeOnDemand: + // Instance is on-demand + return nil, nil + default: + // Invalid MarketType provided + return nil, errors.Errorf("invalid MarketType %s, must be spot/capacity-block or empty", i.MarketType) + } } func getLaunchTemplatePrivateDNSNameOptionsRequest(privateDNSName *infrav1.PrivateDNSName) *ec2.LaunchTemplatePrivateDnsNameOptionsRequest { diff --git a/pkg/cloud/services/ec2/launchtemplate_test.go b/pkg/cloud/services/ec2/launchtemplate_test.go index 80b390b8b3..ff3a337f22 100644 --- a/pkg/cloud/services/ec2/launchtemplate_test.go +++ b/pkg/cloud/services/ec2/launchtemplate_test.go @@ -1076,7 +1076,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { awsResourceReference []infrav1.AWSResourceReference expect func(m *mocks.MockEC2APIMockRecorder) wantErr bool - useCapacityBlocks bool + MarketType *string }{ { name: "Should successfully creates launch template version", @@ -1132,7 +1132,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { { name: "Should successfully create launch template version with capacity-block", awsResourceReference: []infrav1.AWSResourceReference{{ID: aws.String("1")}}, - useCapacityBlocks: true, + MarketType: aws.String(ec2.MarketTypeCapacityBlock), expect: func(m *mocks.MockEC2APIMockRecorder) { sgMap := make(map[infrav1.SecurityGroupRole]infrav1.SecurityGroup) sgMap[infrav1.SecurityGroupNode] = infrav1.SecurityGroup{ID: "1"} @@ -1239,7 +1239,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) var ms *scope.MachinePoolScope - if tc.useCapacityBlocks { + if aws.StringValue(tc.MarketType) == ec2.MarketTypeCapacityBlock { ms, err = setupCapacityBlocksMachinePoolScope(client, cs) } else { ms, err = setupMachinePoolScope(client, cs)