From f21fca7893a972d8fae9235c11d95fac7d5981b7 Mon Sep 17 00:00:00 2001 From: Thirumalesh Aaraveti Date: Thu, 7 Nov 2024 21:06:35 +0530 Subject: [PATCH] Added the suport 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/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/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 | 412 +++++++++++++++++- pkg/cloud/services/ec2/launchtemplate.go | 30 +- pkg/cloud/services/ec2/launchtemplate_test.go | 53 +++ 21 files changed, 665 insertions(+), 47 deletions(-) diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index e6d9982cbd..052bc425a4 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.CapacityBlock = restored.Status.Bastion.CapacityBlock } dst.Spec.Partition = restored.Spec.Partition diff --git a/api/v1beta1/awsmachine_conversion.go b/api/v1beta1/awsmachine_conversion.go index 6044416cdf..d5859ecf6f 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.CapacityBlock = restored.Spec.CapacityBlock 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.CapacityBlock = restored.Spec.Template.Spec.CapacityBlock 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..b300cfab2f 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.CapacityBlock 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.CapacityBlock 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..e7669c81cd 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"` + + // 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 + CapacityBlock *bool `json:"capacityBlock,omitempty"` } // CloudInit defines options related to the bootstrapping systems where diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index 978d5310f2..88e51f90b4 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"` + + // 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 + CapacityBlock *bool `json:"capacityBlock,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..ca79f3ba11 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.CapacityBlock != nil { + in, out := &in.CapacityBlock, &out.CapacityBlock + *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.CapacityBlock != nil { + in, out := &in.CapacityBlock, &out.CapacityBlock + *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 af854b225e..5acfaeb274 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -1138,6 +1138,11 @@ spec: availabilityZone: description: Availability zone of instance type: string + capacityBlock: + description: |- + 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 capacityReservationId: description: CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. @@ -3199,6 +3204,11 @@ spec: availabilityZone: description: Availability zone of instance type: string + capacityBlock: + description: |- + 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 capacityReservationId: description: CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. 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 7d6cc0a025..b29a74e561 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -2113,6 +2113,11 @@ spec: availabilityZone: description: Availability zone of instance type: string + capacityBlock: + description: |- + 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 capacityReservationId: description: CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. 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 e70f544535..abd56b0442 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml @@ -633,6 +633,15 @@ spec: description: ID of resource type: string type: object + capacityBlock: + description: |- + 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 + 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 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 c02466fa59..d405fa5363 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml @@ -624,6 +624,11 @@ spec: description: ID of resource type: string type: object + capacityBlock: + description: |- + 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 capacityReservationId: description: CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. 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 501a837555..66e11e0e40 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml @@ -554,6 +554,11 @@ spec: description: ID of resource type: string type: object + capacityBlock: + description: |- + 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 capacityReservationId: description: CapacityReservationID specifies the target Capacity Reservation into which the instance should be launched. 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 008bfd9d2e..a85de013f6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml @@ -629,6 +629,15 @@ spec: description: ID of resource type: string type: object + capacityBlock: + description: |- + 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 + 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 diff --git a/exp/api/v1beta1/conversion.go b/exp/api/v1beta1/conversion.go index 7c39f1fcbd..9a40a98213 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.CapacityBlock != nil { + dst.Spec.AWSLaunchTemplate.CapacityBlock = restored.Spec.AWSLaunchTemplate.CapacityBlock + } + 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.CapacityBlock != nil { + dst.Spec.AWSLaunchTemplate.CapacityBlock = restored.Spec.AWSLaunchTemplate.CapacityBlock + } + } 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..506bb676ec 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.CapacityBlock requires manual conversion: does not exist in peer-type return nil } diff --git a/exp/api/v1beta2/types.go b/exp/api/v1beta2/types.go index 0bc4009a2e..4b7bab27c6 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"` + + // 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 + CapacityBlock *bool `json:"capacityBlock,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..e6896c87f2 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.CapacityBlock != nil { + in, out := &in.CapacityBlock, &out.CapacityBlock + *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..430ef23ee0 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: newAWSCapacityMachinePool(), + }) +} + +func newAWSCapacityMachinePool() *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"), + CapacityBlock: 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..b14cedc3ab 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.CapacityBlock = scope.AWSMachine.Spec.CapacityBlock + 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.CapacityBlock != 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.CapacityBlock, 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..e0f4378954 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 provding 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", + CapacityBlock: 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"), + }, + CapacityBlock: 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", + CapacityBlock: 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,14 +5659,57 @@ 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{ + CapacityBlock: 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{ + CapacityBlock: 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{ + CapacityBlock: 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) + if err != nil { + if !cmp.Equal(err.Error(), tc.expectedError.Error()) { + t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, err, tc.expectedError) + } + } else { + if !cmp.Equal(request, tc.expectedRequest) { + t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, request, tc.expectedRequest) + } } }) } diff --git a/pkg/cloud/services/ec2/launchtemplate.go b/pkg/cloud/services/ec2/launchtemplate.go index 5da57f2521..49589dfd35 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.CapacityBlock != 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.CapacityBlock, 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..01609dcb72 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 creates 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")}}, @@ -1190,6 +1240,9 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) ms, err := setupMachinePoolScope(client, cs) + if tc.useCapacityBlocks { + ms, err = setupCapacityBlocksMachinePoolScope(client, cs) + } g.Expect(err).NotTo(HaveOccurred()) ms.AWSMachinePool.Spec.AWSLaunchTemplate.AdditionalSecurityGroups = tc.awsResourceReference