Repository: kubernetes-sigs/cluster-api-provider-aws
Pull request #4273 (2023-05-17 15:11:34)

:sparkles: Add support to specify PlacementGroup Name in instances
<!-- Thanks for this PR! If this is your first PR please read the [contributing guide](../CONTRIBUTING.md) -->
<!-- If this PR is still work-in-progress and is being open for visibility please prefix the title with `WIP:` -->

**What type of PR is this?**

<!--
Add one of the following kinds:
/kind feature
/kind bug
/kind api-change
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind flake
/kind regression
/kind support
-->
/kind feature

**What this PR does / why we need it**:

Adds support for users to specify the placement group by name when running instances.

<!-- Enter a description of the change and why this change is needed -->

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Checklist**:
<!-- Put an "X" character inside the brackets of each completed task. Some may be optional depending on the PR in which case these can be deleted -->

- [x] squashed commits
- [ ] includes documentation
- [x] adds unit tests
- [ ] adds or updates e2e tests

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Add support to specify PlacementGroup Name in instances
```



Patch:
diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go
index d1be7abe5..680c3f5f6 100644
--- a/api/v1beta1/awscluster_conversion.go
+++ b/api/v1beta1/awscluster_conversion.go
@@ -46,6 +46,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.S3Bucket = restored.Spec.S3Bucket
 	if restored.Status.Bastion != nil {
 		dst.Status.Bastion.InstanceMetadataOptions = restored.Status.Bastion.InstanceMetadataOptions
+		dst.Status.Bastion.PlacementGroupName = restored.Status.Bastion.PlacementGroupName
 	}
 	dst.Spec.Partition = restored.Spec.Partition
 
diff --git a/api/v1beta1/awsmachine_conversion.go b/api/v1beta1/awsmachine_conversion.go
index 2fa13af81..503f6d37a 100644
--- a/api/v1beta1/awsmachine_conversion.go
+++ b/api/v1beta1/awsmachine_conversion.go
@@ -37,6 +37,7 @@ func (src *AWSMachine) ConvertTo(dstRaw conversion.Hub) error {
 
 	dst.Spec.Ignition = restored.Spec.Ignition
 	dst.Spec.InstanceMetadataOptions = restored.Spec.InstanceMetadataOptions
+	dst.Spec.PlacementGroupName = restored.Spec.PlacementGroupName
 
 	return nil
 }
@@ -83,6 +84,7 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.Template.ObjectMeta = restored.Spec.Template.ObjectMeta
 	dst.Spec.Template.Spec.Ignition = restored.Spec.Template.Spec.Ignition
 	dst.Spec.Template.Spec.InstanceMetadataOptions = restored.Spec.Template.Spec.InstanceMetadataOptions
+	dst.Spec.Template.Spec.PlacementGroupName = restored.Spec.Template.Spec.PlacementGroupName
 
 	return nil
 }
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
index 04d112f9b..f6dd46b9b 100644
--- a/api/v1beta1/zz_generated.conversion.go
+++ b/api/v1beta1/zz_generated.conversion.go
@@ -1391,6 +1391,7 @@ func autoConvert_v1beta2_AWSMachineSpec_To_v1beta1_AWSMachineSpec(in *v1beta2.AW
 	}
 	out.Ignition = (*Ignition)(unsafe.Pointer(in.Ignition))
 	out.SpotMarketOptions = (*SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions))
+	// WARNING: in.PlacementGroupName requires manual conversion: does not exist in peer-type
 	out.Tenancy = in.Tenancy
 	return nil
 }
@@ -1995,6 +1996,7 @@ func autoConvert_v1beta2_Instance_To_v1beta1_Instance(in *v1beta2.Instance, out
 	out.Tags = *(*map[string]string)(unsafe.Pointer(&in.Tags))
 	out.AvailabilityZone = in.AvailabilityZone
 	out.SpotMarketOptions = (*SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions))
+	// WARNING: in.PlacementGroupName requires manual conversion: does not exist in peer-type
 	out.Tenancy = in.Tenancy
 	out.VolumeIDs = *(*[]string)(unsafe.Pointer(&in.VolumeIDs))
 	// WARNING: in.InstanceMetadataOptions requires manual conversion: does not exist in peer-type
diff --git a/api/v1beta2/awsmachine_types.go b/api/v1beta2/awsmachine_types.go
index b995eb225..2e24f6105 100644
--- a/api/v1beta2/awsmachine_types.go
+++ b/api/v1beta2/awsmachine_types.go
@@ -152,6 +152,10 @@ type AWSMachineSpec struct {
 	// +optional
 	SpotMarketOptions *SpotMarketOptions `json:"spotMarketOptions,omitempty"`
 
+	// PlacementGroupName specifies the name of the placement group in which to launch the instance.
+	// +optional
+	PlacementGroupName string `json:"placementGroupName,omitempty"`
+
 	// Tenancy indicates if instance should run on shared or single-tenant hardware.
 	// +optional
 	// +kubebuilder:validation:Enum:=default;dedicated;host
diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go
index 839bfadab..040c30a6d 100644
--- a/api/v1beta2/types.go
+++ b/api/v1beta2/types.go
@@ -194,6 +194,10 @@ type Instance struct {
 	// SpotMarketOptions option for configuring instances to be run using AWS Spot instances.
 	SpotMarketOptions *SpotMarketOptions `json:"spotMarketOptions,omitempty"`
 
+	// PlacementGroupName specifies the name of the placement group in which to launch the instance.
+	// +optional
+	PlacementGroupName string `json:"placementGroupName,omitempty"`
+
 	// Tenancy indicates if instance should run on shared or single-tenant hardware.
 	// +optional
 	Tenancy string `json:"tenancy,omitempty"`
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 f92029cfe..43ca05c2d 100644
--- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
+++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -949,6 +949,10 @@ spec:
                       - size
                       type: object
                     type: array
+                  placementGroupName:
+                    description: PlacementGroupName specifies the name of the placement
+                      group in which to launch the instance.
+                    type: string
                   privateIp:
                     description: The private IPv4 address assigned to the instance.
                     type: string
@@ -2371,6 +2375,10 @@ spec:
                       - size
                       type: object
                     type: array
+                  placementGroupName:
+                    description: PlacementGroupName specifies the name of the placement
+                      group in which to launch the instance.
+                    type: string
                   privateIp:
                     description: The private IPv4 address assigned to the instance.
                     type: string
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 1bbb0fade..023b531db 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1441,6 +1441,10 @@ spec:
                       - size
                       type: object
                     type: array
+                  placementGroupName:
+                    description: PlacementGroupName specifies the name of the placement
+                      group in which to launch the instance.
+                    type: string
                   privateIp:
                     description: The private IPv4 address assigned to the instance.
                     type: string
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 7ce292bd3..58ed5e8f3 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml
@@ -772,6 +772,10 @@ spec:
                   - size
                   type: object
                 type: array
+              placementGroupName:
+                description: PlacementGroupName specifies the name of the placement
+                  group in which to launch the instance.
+                type: string
               providerID:
                 description: ProviderID is the unique identifier as specified by the
                   cloud provider.
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 7b7aef383..085358008 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml
@@ -726,6 +726,10 @@ spec:
                           - size
                           type: object
                         type: array
+                      placementGroupName:
+                        description: PlacementGroupName specifies the name of the
+                          placement group in which to launch the instance.
+                        type: string
                       providerID:
                         description: ProviderID is the unique identifier as specified
                           by the cloud provider.
diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go
index dbbabf412..118318c78 100644
--- a/pkg/cloud/services/ec2/instances.go
+++ b/pkg/cloud/services/ec2/instances.go
@@ -235,6 +235,8 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use
 
 	input.Tenancy = scope.AWSMachine.Spec.Tenancy
 
+	input.PlacementGroupName = scope.AWSMachine.Spec.PlacementGroupName
+
 	s.scope.Debug("Running instance", "machine-role", scope.Role())
 	out, err := s.runInstance(scope.Role(), input)
 	if err != nil {
@@ -587,6 +589,13 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan
 		}
 	}
 
+	if i.PlacementGroupName != "" {
+		if input.Placement == nil {
+			input.Placement = &ec2.Placement{}
+		}
+		input.Placement.GroupName = &i.PlacementGroupName
+	}
+
 	out, err := s.EC2Client.RunInstances(input)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to run instance")


Test patch:
diff --git a/pkg/cloud/services/ec2/instances_test.go b/pkg/cloud/services/ec2/instances_test.go
index d8094a16d..6470a0980 100644
--- a/pkg/cloud/services/ec2/instances_test.go
+++ b/pkg/cloud/services/ec2/instances_test.go
@@ -2500,7 +2500,163 @@ func TestCreateInstance(t *testing.T) {
 			},
 		},
 		{
-			name: "with dedicated tenancy ignition",
+			name: "with custom placement group cloud-config",
+			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: pointer.String("bootstrap-data"),
+					},
+				},
+			},
+			machineConfig: &infrav1.AWSMachineSpec{
+				AMI: infrav1.AMIReference{
+					ID: aws.String("abc"),
+				},
+				InstanceType:         "m5.large",
+				PlacementGroupName:   "placement-group1",
+				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. // TODO: Restore these parameters, but with the tags as well
+					RunInstances(gomock.Eq(&ec2.RunInstancesInput{
+						ImageId:      aws.String("abc"),
+						InstanceType: aws.String("m5.large"),
+						KeyName:      aws.String("default"),
+						MaxCount:     aws.Int64(1),
+						MinCount:     aws.Int64(1),
+						Placement: &ec2.Placement{
+							GroupName: aws.String("placement-group1"),
+						},
+						SecurityGroupIds: []*string{aws.String("2"), aws.String("3")},
+						SubnetId:         aws.String("subnet-1"),
+						TagSpecifications: []*ec2.TagSpecification{
+							{
+								ResourceType: aws.String("instance"),
+								Tags: []*ec2.Tag{
+									{
+										Key:   aws.String("MachineName"),
+										Value: aws.String("default/machine-aws-test1"),
+									},
+									{
+										Key:   aws.String("Name"),
+										Value: aws.String("aws-test1"),
+									},
+									{
+										Key:   aws.String("kubernetes.io/cluster/test1"),
+										Value: aws.String("owned"),
+									},
+									{
+										Key:   aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test1"),
+										Value: aws.String("owned"),
+									},
+									{
+										Key:   aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
+										Value: aws.String("node"),
+									},
+								},
+							},
+						},
+						UserData: aws.String(base64.StdEncoding.EncodeToString(userDataCompressed)),
+					})).
+					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,
+									GroupName:        aws.String("placement-group1"),
+								},
+							},
+						},
+					}, nil)
+				m.
+					DescribeInstanceTypes(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.
+					DescribeNetworkInterfaces(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: "with dedicated tenancy and placement group ignition",
 			machine: clusterv1.Machine{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:    map[string]string{"set": "node"},
@@ -2519,6 +2675,7 @@ func TestCreateInstance(t *testing.T) {
 				},
 				InstanceType:         "m5.large",
 				Tenancy:              "dedicated",
+				PlacementGroupName:   "placement-group1",
 				UncompressedUserData: &isUncompressedTrue,
 				Ignition:             &infrav1.Ignition{},
 			},
@@ -2582,7 +2739,8 @@ func TestCreateInstance(t *testing.T) {
 						MaxCount:     aws.Int64(1),
 						MinCount:     aws.Int64(1),
 						Placement: &ec2.Placement{
-							Tenancy: &tenancy,
+							Tenancy:   &tenancy,
+							GroupName: aws.String("placement-group1"),
 						},
 						SecurityGroupIds: []*string{aws.String("2"), aws.String("3")},
 						SubnetId:         aws.String("subnet-1"),