diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index a95efaa113..820bfdd410 100644 --- a/api/v1beta1/awscluster_conversion.go +++ b/api/v1beta1/awscluster_conversion.go @@ -47,6 +47,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error { if restored.Status.Bastion != nil { dst.Status.Bastion.InstanceMetadataOptions = restored.Status.Bastion.InstanceMetadataOptions dst.Status.Bastion.PlacementGroupName = restored.Status.Bastion.PlacementGroupName + dst.Status.Bastion.PlacementGroupPartition = restored.Status.Bastion.PlacementGroupPartition } dst.Spec.Partition = restored.Spec.Partition diff --git a/api/v1beta1/awsmachine_conversion.go b/api/v1beta1/awsmachine_conversion.go index 503f6d37a5..a2ae61eb8d 100644 --- a/api/v1beta1/awsmachine_conversion.go +++ b/api/v1beta1/awsmachine_conversion.go @@ -38,6 +38,8 @@ 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 + dst.Spec.PlacementGroupPartition = restored.Spec.PlacementGroupPartition + dst.Spec.SecurityGroupOverrides = restored.Spec.SecurityGroupOverrides return nil } @@ -85,6 +87,8 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { 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 + dst.Spec.Template.Spec.PlacementGroupPartition = restored.Spec.Template.Spec.PlacementGroupPartition + dst.Spec.Template.Spec.SecurityGroupOverrides = restored.Spec.Template.Spec.SecurityGroupOverrides return nil } diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 66b4adbf3f..38686de99e 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -1382,6 +1382,7 @@ func autoConvert_v1beta2_AWSMachineSpec_To_v1beta1_AWSMachineSpec(in *v1beta2.AW } else { out.Subnet = nil } + // WARNING: in.SecurityGroupOverrides requires manual conversion: does not exist in peer-type out.SSHKeyName = (*string)(unsafe.Pointer(in.SSHKeyName)) out.RootVolume = (*Volume)(unsafe.Pointer(in.RootVolume)) out.NonRootVolumes = *(*[]Volume)(unsafe.Pointer(&in.NonRootVolumes)) @@ -1393,6 +1394,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 + // WARNING: in.PlacementGroupPartition requires manual conversion: does not exist in peer-type out.Tenancy = in.Tenancy return nil } @@ -1994,6 +1996,7 @@ func autoConvert_v1beta2_Instance_To_v1beta1_Instance(in *v1beta2.Instance, out out.AvailabilityZone = in.AvailabilityZone out.SpotMarketOptions = (*SpotMarketOptions)(unsafe.Pointer(in.SpotMarketOptions)) // WARNING: in.PlacementGroupName requires manual conversion: does not exist in peer-type + // WARNING: in.PlacementGroupPartition 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 2e24f6105e..30a5e20647 100644 --- a/api/v1beta2/awsmachine_types.go +++ b/api/v1beta2/awsmachine_types.go @@ -114,6 +114,11 @@ type AWSMachineSpec struct { // +optional Subnet *AWSResourceReference `json:"subnet,omitempty"` + // SecurityGroupOverrides is an optional set of security groups to use for the node. + // This is optional - if not provided security groups from the cluster will be used. + // +optional + SecurityGroupOverrides map[SecurityGroupRole]string `json:"securityGroupOverrides,omitempty"` + // 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) // +optional SSHKeyName *string `json:"sshKeyName,omitempty"` @@ -156,6 +161,14 @@ type AWSMachineSpec struct { // +optional PlacementGroupName string `json:"placementGroupName,omitempty"` + // PlacementGroupPartition is the partition number within the placement group in which to launch the instance. + // This value is only valid if the placement group, referred in `PlacementGroupName`, was created with + // strategy set to partition. + // +kubebuilder:validation:Minimum:=1 + // +kubebuilder:validation:Maximum:=7 + // +optional + PlacementGroupPartition int64 `json:"placementGroupPartition,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 66dcb93966..bb61326a3d 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -198,6 +198,14 @@ type Instance struct { // +optional PlacementGroupName string `json:"placementGroupName,omitempty"` + // PlacementGroupPartition is the partition number within the placement group in which to launch the instance. + // This value is only valid if the placement group, referred in `PlacementGroupName`, was created with + // strategy set to partition. + // +kubebuilder:validation:Minimum:=1 + // +kubebuilder:validation:Maximum:=7 + // +optional + PlacementGroupPartition int64 `json:"placementGroupPartition,omitempty"` + // Tenancy indicates if instance should run on shared or single-tenant hardware. // +optional Tenancy string `json:"tenancy,omitempty"` diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 7041557d04..26c00d3fc6 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -695,6 +695,13 @@ func (in *AWSMachineSpec) DeepCopyInto(out *AWSMachineSpec) { *out = new(AWSResourceReference) (*in).DeepCopyInto(*out) } + if in.SecurityGroupOverrides != nil { + in, out := &in.SecurityGroupOverrides, &out.SecurityGroupOverrides + *out = make(map[SecurityGroupRole]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.SSHKeyName != nil { in, out := &in.SSHKeyName, &out.SSHKeyName *out = new(string) 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 e43e9a117c..16b88f9b32 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -1031,6 +1031,15 @@ spec: description: PlacementGroupName specifies the name of the placement group in which to launch the instance. type: string + placementGroupPartition: + description: PlacementGroupPartition is the partition number within + the placement group in which to launch the instance. This value + is only valid if the placement group, referred in `PlacementGroupName`, + was created with strategy set to partition. + format: int64 + maximum: 7 + minimum: 1 + type: integer privateIp: description: The private IPv4 address assigned to the instance. type: string @@ -2574,6 +2583,15 @@ spec: description: PlacementGroupName specifies the name of the placement group in which to launch the instance. type: string + placementGroupPartition: + description: PlacementGroupPartition is the partition number within + the placement group in which to launch the instance. This value + is only valid if the placement group, referred in `PlacementGroupName`, + was created with strategy set to partition. + format: int64 + maximum: 7 + minimum: 1 + type: integer 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 82c96b098f..841761744f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -1588,6 +1588,15 @@ spec: description: PlacementGroupName specifies the name of the placement group in which to launch the instance. type: string + placementGroupPartition: + description: PlacementGroupPartition is the partition number within + the placement group in which to launch the instance. This value + is only valid if the placement group, referred in `PlacementGroupName`, + was created with strategy set to partition. + format: int64 + maximum: 7 + minimum: 1 + type: integer 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 bf219b95a0..590737372f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml @@ -775,6 +775,15 @@ spec: description: PlacementGroupName specifies the name of the placement group in which to launch the instance. type: string + placementGroupPartition: + description: PlacementGroupPartition is the partition number within + the placement group in which to launch the instance. This value + is only valid if the placement group, referred in `PlacementGroupName`, + was created with strategy set to partition. + format: int64 + maximum: 7 + minimum: 1 + type: integer providerID: description: ProviderID is the unique identifier as specified by the cloud provider. @@ -824,6 +833,13 @@ spec: required: - size type: object + securityGroupOverrides: + additionalProperties: + type: string + description: SecurityGroupOverrides is an optional set of security + groups to use for the node. This is optional - if not provided security + groups from the cluster will be used. + type: object spotMarketOptions: description: SpotMarketOptions allows users to configure instances to be run using AWS Spot instances. 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 b2cdf7375f..57e15ea981 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml @@ -729,6 +729,16 @@ spec: description: PlacementGroupName specifies the name of the placement group in which to launch the instance. type: string + placementGroupPartition: + description: PlacementGroupPartition is the partition number + within the placement group in which to launch the instance. + This value is only valid if the placement group, referred + in `PlacementGroupName`, was created with strategy set to + partition. + format: int64 + maximum: 7 + minimum: 1 + type: integer providerID: description: ProviderID is the unique identifier as specified by the cloud provider. @@ -781,6 +791,14 @@ spec: required: - size type: object + securityGroupOverrides: + additionalProperties: + type: string + description: SecurityGroupOverrides is an optional set of + security groups to use for the node. This is optional - + if not provided security groups from the cluster will be + used. + type: object spotMarketOptions: description: SpotMarketOptions allows users to configure instances to be run using AWS Spot instances. diff --git a/controllers/awsmachine_controller_test.go b/controllers/awsmachine_controller_test.go index 2ec6b37a29..0f29cf7c33 100644 --- a/controllers/awsmachine_controller_test.go +++ b/controllers/awsmachine_controller_test.go @@ -537,10 +537,6 @@ func mockedCreateSecretCall(s *mock_services.MockSecretInterfaceMockRecorder) { func mockedCreateInstanceCalls(m *mocks.MockEC2APIMockRecorder) { m.DescribeInstances(gomock.Eq(&ec2.DescribeInstancesInput{ Filters: []*ec2.Filter{ - { - Name: aws.String("vpc-id"), - Values: aws.StringSlice([]string{"vpc-exists"}), - }, { Name: aws.String("tag:sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"), Values: aws.StringSlice([]string{"owned"}), @@ -649,10 +645,6 @@ func mockedCreateInstanceCalls(m *mocks.MockEC2APIMockRecorder) { Name: aws.String("state"), Values: aws.StringSlice([]string{"pending", "available"}), }, - { - Name: aws.String("vpc-id"), - Values: aws.StringSlice([]string{"vpc-exists"}), - }, { Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"subnet-1"}), diff --git a/docs/book/src/topics/bring-your-own-aws-infrastructure.md b/docs/book/src/topics/bring-your-own-aws-infrastructure.md index 4841425158..bd157fa28f 100644 --- a/docs/book/src/topics/bring-your-own-aws-infrastructure.md +++ b/docs/book/src/topics/bring-your-own-aws-infrastructure.md @@ -120,6 +120,30 @@ spec: Users may either specify `failureDomain` on the Machine or MachineDeployment objects, _or_ users may explicitly specify subnet IDs on the AWSMachine or AWSMachineTemplate objects. If both are specified, the subnet ID is used and the `failureDomain` is ignored. +### Placing EC2 Instances in Specific External VPCs + +CAPA clusters are deployed within a single VPC, but it's possible to place machines that live in external VPCs. For this kind of configuration, we assume that all the VPCs have the ability to communicate, either through external peering, a transit gateway, or some other mechanism already established outside of CAPA. CAPA will not create a tunnel or manage the network configuration for any secondary VPCs. + +The AWSMachineTemplate `subnet` field allows specifying filters or specific subnet ids for worker machine placement. If the filters or subnet id is specified in a secondary VPC, CAPA will place the machine in that VPC and subnet. + +```yaml +spec: + template: + spec: + subnet: + filters: + name: "vpc-id" + values: + - "secondary-vpc-id" + securityGroupOverrides: + node: sg-04e870a3507a5ad2c5c8c2 + node-eks-additional: sg-04e870a3507a5ad2c5c8c1 +``` + +#### Caveats/Notes + +CAPA helpfully creates security groups for various roles in the cluster and automatically attaches them to workers. However, security groups are tied to a specific VPC, so workers placed in a VPC outside of the cluster will need to have these security groups created by some external process first and set in the `securityGroupOverrides` field, otherwise the ec2 creation will fail. + ### Security Groups To use existing security groups for instances for a cluster, add this to the AWSCluster specification: @@ -147,6 +171,15 @@ spec: - ... ``` +It's also possible to override the cluster security groups for an individual AWSMachine or AWSMachineTemplate: + +```yaml +spec: + SecurityGroupOverrides: + node: sg-04e870a3507a5ad2c5c8c2 + node-eks-additional: sg-04e870a3507a5ad2c5c8c1 +``` + ### Control Plane Load Balancer The cluster control plane is accessed through a Classic ELB. By default, Cluster API creates the Classic ELB. To use an existing Classic ELB, add its name to the AWSCluster specification: diff --git a/pkg/cloud/services/ec2/instances.go b/pkg/cloud/services/ec2/instances.go index fc4b7d4202..aa1dec68a4 100644 --- a/pkg/cloud/services/ec2/instances.go +++ b/pkg/cloud/services/ec2/instances.go @@ -45,7 +45,6 @@ func (s *Service) GetRunningInstanceByTags(scope *scope.MachineScope) (*infrav1. input := &ec2.DescribeInstancesInput{ Filters: []*ec2.Filter{ - filter.EC2.VPC(s.scope.VPC().ID), filter.EC2.ClusterOwned(s.scope.Name()), filter.EC2.Name(scope.Name()), filter.EC2.InstanceStates(ec2.InstanceStateNamePending, ec2.InstanceStateNameRunning), @@ -238,6 +237,8 @@ func (s *Service) CreateInstance(scope *scope.MachineScope, userData []byte, use input.PlacementGroupName = scope.AWSMachine.Spec.PlacementGroupName + input.PlacementGroupPartition = scope.AWSMachine.Spec.PlacementGroupPartition + 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) @@ -306,9 +307,6 @@ func (s *Service) findSubnet(scope *scope.MachineScope) (string, error) { criteria := []*ec2.Filter{ filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), } - if !scope.IsExternallyManaged() { - criteria = append(criteria, filter.EC2.VPC(s.scope.VPC().ID)) - } if scope.AWSMachine.Spec.Subnet.ID != nil { criteria = append(criteria, &ec2.Filter{Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{*scope.AWSMachine.Spec.Subnet.ID})}) } @@ -343,6 +341,11 @@ func (s *Service) findSubnet(scope *scope.MachineScope) (string, error) { } filtered = append(filtered, subnet) } + // prefer a subnet in the cluster VPC if multiple match + clusterVPC := s.scope.VPC().ID + sort.SliceStable(filtered, func(i, j int) bool { + return strings.Compare(*filtered[i].VpcId, clusterVPC) > strings.Compare(*filtered[j].VpcId, clusterVPC) + }) if len(filtered) == 0 { errMessage = fmt.Sprintf("failed to run machine %q, found %d subnets matching criteria but post-filtering failed.", scope.Name(), len(subnets)) + errMessage @@ -438,10 +441,15 @@ func (s *Service) GetCoreSecurityGroups(scope *scope.MachineScope) ([]string, er } ids := make([]string, 0, len(sgRoles)) for _, sg := range sgRoles { - if _, ok := s.scope.SecurityGroups()[sg]; !ok { - return nil, awserrors.NewFailedDependency(fmt.Sprintf("%s security group not available", sg)) + if _, ok := scope.AWSMachine.Spec.SecurityGroupOverrides[sg]; ok { + ids = append(ids, scope.AWSMachine.Spec.SecurityGroupOverrides[sg]) + continue } - ids = append(ids, s.scope.SecurityGroups()[sg].ID) + if _, ok := s.scope.SecurityGroups()[sg]; ok { + ids = append(ids, s.scope.SecurityGroups()[sg].ID) + continue + } + return nil, awserrors.NewFailedDependency(fmt.Sprintf("%s security group not available", sg)) } return ids, nil } @@ -576,21 +584,25 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan } if len(i.Tags) > 0 { - spec := &ec2.TagSpecification{ResourceType: aws.String(ec2.ResourceTypeInstance)} - // We need to sort keys for tests to work - keys := make([]string, 0, len(i.Tags)) - for k := range i.Tags { - keys = append(keys, k) - } - sort.Strings(keys) - for _, key := range keys { - spec.Tags = append(spec.Tags, &ec2.Tag{ - Key: aws.String(key), - Value: aws.String(i.Tags[key]), - }) - } + resources := []string{ec2.ResourceTypeInstance, ec2.ResourceTypeVolume, ec2.ResourceTypeNetworkInterface} + for _, r := range resources { + spec := &ec2.TagSpecification{ResourceType: aws.String(r)} + + // We need to sort keys for tests to work + keys := make([]string, 0, len(i.Tags)) + for k := range i.Tags { + keys = append(keys, k) + } + sort.Strings(keys) + for _, key := range keys { + spec.Tags = append(spec.Tags, &ec2.Tag{ + Key: aws.String(key), + Value: aws.String(i.Tags[key]), + }) + } - input.TagSpecifications = append(input.TagSpecifications, spec) + input.TagSpecifications = append(input.TagSpecifications, spec) + } } input.InstanceMarketOptions = getInstanceMarketOptionsRequest(i.SpotMarketOptions) @@ -602,11 +614,18 @@ func (s *Service) runInstance(role string, i *infrav1.Instance) (*infrav1.Instan } } + if i.PlacementGroupName == "" && i.PlacementGroupPartition != 0 { + return nil, errors.Errorf("placementGroupPartition is set but placementGroupName is empty") + } + if i.PlacementGroupName != "" { if input.Placement == nil { input.Placement = &ec2.Placement{} } input.Placement.GroupName = &i.PlacementGroupName + if i.PlacementGroupPartition != 0 { + input.Placement.PartitionNumber = &i.PlacementGroupPartition + } } out, err := s.EC2Client.RunInstances(input) diff --git a/pkg/cloud/services/ec2/instances_test.go b/pkg/cloud/services/ec2/instances_test.go index 0042055a88..54e71e1056 100644 --- a/pkg/cloud/services/ec2/instances_test.go +++ b/pkg/cloud/services/ec2/instances_test.go @@ -554,37 +554,56 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with ImageLookupOrg specified at the machine level", + name: "when multiple subnets match filters, subnets in the cluster vpc are preferred", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, }, Spec: clusterv1.MachineSpec{ Bootstrap: clusterv1.Bootstrap{ - DataSecretName: pointer.String("bootstrap-data"), + DataSecretName: aws.String("bootstrap-data"), }, - Version: pointer.String("v1.16.1"), + FailureDomain: aws.String("us-east-1c"), }, }, machineConfig: &infrav1.AWSMachineSpec{ - ImageLookupOrg: "test-org-123", - InstanceType: "m6g.large", + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.2xlarge", + Subnet: &infrav1.AWSResourceReference{ + Filters: []infrav1.Filter{ + { + Name: "availability-zone", + Values: []string{"us-east-1c"}, + }, + }, + }, + UncompressedUserData: &isUncompressedFalse, }, awsCluster: &infrav1.AWSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: infrav1.AWSClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: "vpc-foo", + }, Subnets: infrav1.Subnets{ infrav1.SubnetSpec{ - ID: "subnet-1", - IsPublic: false, + ID: "subnet-1", + AvailabilityZone: "us-east-1a", + IsPublic: false, }, infrav1.SubnetSpec{ - IsPublic: false, + ID: "subnet-2", + AvailabilityZone: "us-east-1b", + IsPublic: false, + }, + infrav1.SubnetSpec{ + ID: "subnet-3", + AvailabilityZone: "us-east-1c", + IsPublic: false, }, - }, - VPC: infrav1.VPCSpec{ - ID: "vpc-test", }, }, }, @@ -608,14 +627,10 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { - amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1") - if err != nil { - t.Fatalf("Failed to process ami format: %v", err) - } m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ - aws.String("m6g.large"), + aws.String("m5.2xlarge"), }, })). Return(&ec2.DescribeInstanceTypesOutput{ @@ -623,76 +638,151 @@ func TestCreateInstance(t *testing.T) { { ProcessorInfo: &ec2.ProcessorInfo{ SupportedArchitectures: []*string{ - aws.String("arm64"), + aws.String("x86_64"), }, }, }, }, }, nil) - // verify that the ImageLookupOrg is used when finding AMIs + m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + { + Name: aws.String("availability-zone"), + Values: aws.StringSlice([]string{"us-east-1c"}), + }, + }})).Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{ + { + VpcId: aws.String("vpc-bar"), + SubnetId: aws.String("subnet-4"), + AvailabilityZone: aws.String("us-east-1c"), + CidrBlock: aws.String("10.0.10.0/24"), + MapPublicIpOnLaunch: aws.Bool(false), + }, + { + VpcId: aws.String("vpc-foo"), + SubnetId: aws.String("subnet-3"), + AvailabilityZone: aws.String("us-east-1c"), + CidrBlock: aws.String("10.0.11.0/24"), + }, + }, + }, nil) m. - DescribeImages(gomock.Eq(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("owner-id"), - Values: []*string{aws.String("test-org-123")}, - }, - { - Name: aws.String("name"), - Values: []*string{aws.String(amiName)}, - }, - { - Name: aws.String("architecture"), - Values: []*string{aws.String("arm64")}, - }, - { - Name: aws.String("state"), - Values: []*string{aws.String("available")}, - }, + RunInstances(&ec2.RunInstancesInput{ + ImageId: aws.String("abc"), + InstanceType: aws.String("m5.2xlarge"), + KeyName: aws.String("default"), + SecurityGroupIds: aws.StringSlice([]string{"2", "3"}), + SubnetId: aws.String("subnet-3"), + TagSpecifications: []*ec2.TagSpecification{ { - Name: aws.String("virtualization-type"), - Values: []*string{aws.String("hvm")}, + ResourceType: aws.String("instance"), + Tags: []*ec2.Tag{ + { + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), + }, + }, }, - }, - })). - Return(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ { - Name: aws.String("ami-1"), - CreationDate: aws.String("2006-01-02T15:04:05.000Z"), + ResourceType: aws.String("volume"), + Tags: []*ec2.Tag{ + { + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), + }, + }, }, - }, - }, nil) - m. // TODO: Restore these parameters, but with the tags as well - RunInstances(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{ + ResourceType: aws.String("network-interface"), + Tags: []*ec2.Tag{ { - DeviceName: aws.String("device-1"), - Ebs: &ec2.EbsInstanceBlockDevice{ - VolumeId: aws.String("volume-1"), - }, + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), }, }, - Placement: &ec2.Placement{ - AvailabilityZone: &az, + }, + }, + UserData: aws.String(base64.StdEncoding.EncodeToString(userDataCompressed)), + MaxCount: aws.Int64(1), + MinCount: aws.Int64(1), + }).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-3"), + 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, + }, }, - }, nil) + }, + }, nil) m. DescribeNetworkInterfaces(gomock.Any()). Return(&ec2.DescribeNetworkInterfacesOutput{ @@ -704,42 +794,72 @@ func TestCreateInstance(t *testing.T) { if err != nil { t.Fatalf("did not expect error: %v", err) } + + if instance.SubnetID != "subnet-3" { + t.Fatalf("expected subnet-3 from availability zone us-east-1c, got %q", instance.SubnetID) + } }, }, { - name: "with ImageLookupOrg specified at the cluster-level", + name: "with a subnet outside the cluster vpc", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, }, Spec: clusterv1.MachineSpec{ Bootstrap: clusterv1.Bootstrap{ - DataSecretName: pointer.String("bootstrap-data"), + DataSecretName: aws.String("bootstrap-data"), }, - Version: pointer.String("v1.16.1"), + FailureDomain: aws.String("us-east-1c"), }, }, machineConfig: &infrav1.AWSMachineSpec{ - InstanceType: "m5.large", + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.2xlarge", + Subnet: &infrav1.AWSResourceReference{ + Filters: []infrav1.Filter{ + { + Name: "vpc-id", + Values: []string{"vpc-bar"}, + }, + { + Name: "availability-zone", + Values: []string{"us-east-1c"}, + }, + }, + }, + SecurityGroupOverrides: map[infrav1.SecurityGroupRole]string{ + infrav1.SecurityGroupNode: "4", + }, + UncompressedUserData: &isUncompressedFalse, }, awsCluster: &infrav1.AWSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: infrav1.AWSClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: "vpc-foo", + }, Subnets: infrav1.Subnets{ infrav1.SubnetSpec{ - ID: "subnet-1", - IsPublic: false, + ID: "subnet-1", + AvailabilityZone: "us-east-1a", + IsPublic: false, }, infrav1.SubnetSpec{ - IsPublic: false, + ID: "subnet-2", + AvailabilityZone: "us-east-1b", + IsPublic: false, + }, + infrav1.SubnetSpec{ + ID: "subnet-3", + AvailabilityZone: "us-east-1c", + IsPublic: false, }, - }, - VPC: infrav1.VPCSpec{ - ID: "vpc-test", }, }, - ImageLookupOrg: "cluster-level-image-lookup-org", }, Status: infrav1.AWSClusterStatus{ Network: infrav1.NetworkStatus{ @@ -761,14 +881,10 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { - amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1") - if err != nil { - t.Fatalf("Failed to process ami format: %v", err) - } m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ - aws.String("m5.large"), + aws.String("m5.2xlarge"), }, })). Return(&ec2.DescribeInstanceTypesOutput{ @@ -782,70 +898,139 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) - // verify that the ImageLookupOrg is used when finding AMIs + m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + filter.EC2.VPC("vpc-bar"), + { + Name: aws.String("availability-zone"), + Values: aws.StringSlice([]string{"us-east-1c"}), + }, + }})).Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{ + { + VpcId: aws.String("vpc-bar"), + SubnetId: aws.String("subnet-5"), + AvailabilityZone: aws.String("us-east-1c"), + CidrBlock: aws.String("10.0.11.0/24"), + }, + }, + }, nil) m. - DescribeImages(gomock.Eq(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("owner-id"), - Values: []*string{aws.String("cluster-level-image-lookup-org")}, - }, - { - Name: aws.String("name"), - Values: []*string{aws.String(amiName)}, - }, - { - Name: aws.String("architecture"), - Values: []*string{aws.String("x86_64")}, - }, - { - Name: aws.String("state"), - Values: []*string{aws.String("available")}, - }, + RunInstances(&ec2.RunInstancesInput{ + ImageId: aws.String("abc"), + InstanceType: aws.String("m5.2xlarge"), + KeyName: aws.String("default"), + SecurityGroupIds: aws.StringSlice([]string{"4", "3"}), + SubnetId: aws.String("subnet-5"), + TagSpecifications: []*ec2.TagSpecification{ { - Name: aws.String("virtualization-type"), - Values: []*string{aws.String("hvm")}, - }, - }, - })). - Return(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ - { - Name: aws.String("ami-1"), - CreationDate: aws.String("2006-01-02T15:04:05.000Z"), + ResourceType: aws.String("instance"), + Tags: []*ec2.Tag{ + { + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), + }, + }, }, - }, - }, nil) - m. // TODO: Restore these parameters, but with the tags as well - RunInstances(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"), + ResourceType: aws.String("volume"), + Tags: []*ec2.Tag{ + { + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), + }, }, - 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{ + }, + { + ResourceType: aws.String("network-interface"), + Tags: []*ec2.Tag{ { - DeviceName: aws.String("device-1"), - Ebs: &ec2.EbsInstanceBlockDevice{ - VolumeId: aws.String("volume-1"), - }, + Key: aws.String("MachineName"), + Value: aws.String("/"), + }, + { + 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"), }, }, - Placement: &ec2.Placement{ - AvailabilityZone: &az, + }, + }, + UserData: aws.String(base64.StdEncoding.EncodeToString(userDataCompressed)), + MaxCount: aws.Int64(1), + MinCount: aws.Int64(1), + }).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-5"), + 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, + }, }, - }, nil) + }, + }, nil) m. DescribeNetworkInterfaces(gomock.Any()). Return(&ec2.DescribeNetworkInterfacesOutput{ @@ -857,10 +1042,14 @@ func TestCreateInstance(t *testing.T) { if err != nil { t.Fatalf("did not expect error: %v", err) } + + if instance.SubnetID != "subnet-5" { + t.Fatalf("expected subnet-5 from availability zone us-east-1c, got %q", instance.SubnetID) + } }, }, { - name: "AWSMachine ImageLookupOrg overrides AWSCluster ImageLookupOrg", + name: "with ImageLookupOrg specified at the machine level", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -873,8 +1062,8 @@ func TestCreateInstance(t *testing.T) { }, }, machineConfig: &infrav1.AWSMachineSpec{ - InstanceType: "m5.large", - ImageLookupOrg: "machine-level-image-lookup-org", + ImageLookupOrg: "test-org-123", + InstanceType: "m6g.large", }, awsCluster: &infrav1.AWSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, @@ -893,7 +1082,6 @@ func TestCreateInstance(t *testing.T) { ID: "vpc-test", }, }, - ImageLookupOrg: "cluster-level-image-lookup-org", }, Status: infrav1.AWSClusterStatus{ Network: infrav1.NetworkStatus{ @@ -922,7 +1110,7 @@ func TestCreateInstance(t *testing.T) { m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ - aws.String("m5.large"), + aws.String("m6g.large"), }, })). Return(&ec2.DescribeInstanceTypesOutput{ @@ -930,7 +1118,7 @@ func TestCreateInstance(t *testing.T) { { ProcessorInfo: &ec2.ProcessorInfo{ SupportedArchitectures: []*string{ - aws.String("x86_64"), + aws.String("arm64"), }, }, }, @@ -942,7 +1130,7 @@ func TestCreateInstance(t *testing.T) { Filters: []*ec2.Filter{ { Name: aws.String("owner-id"), - Values: []*string{aws.String("machine-level-image-lookup-org")}, + Values: []*string{aws.String("test-org-123")}, }, { Name: aws.String("name"), @@ -950,7 +1138,7 @@ func TestCreateInstance(t *testing.T) { }, { Name: aws.String("architecture"), - Values: []*string{aws.String("x86_64")}, + Values: []*string{aws.String("arm64")}, }, { Name: aws.String("state"), @@ -1014,7 +1202,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "subnet filter and failureDomain defined", + name: "with ImageLookupOrg specified at the cluster-level", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1023,29 +1211,30 @@ func TestCreateInstance(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: pointer.String("bootstrap-data"), }, - FailureDomain: aws.String("us-east-1b"), + Version: pointer.String("v1.16.1"), }, }, machineConfig: &infrav1.AWSMachineSpec{ - AMI: infrav1.AMIReference{ - ID: aws.String("abc"), - }, InstanceType: "m5.large", - Subnet: &infrav1.AWSResourceReference{ - Filters: []infrav1.Filter{{ - Name: "tag:some-tag", - Values: []string{"some-value"}, - }}, - }, }, 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-id", + ID: "vpc-test", }, }, + ImageLookupOrg: "cluster-level-image-lookup-org", }, Status: infrav1.AWSClusterStatus{ Network: infrav1.NetworkStatus{ @@ -1067,6 +1256,10 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { + amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1") + if err != nil { + t.Fatalf("Failed to process ami format: %v", err) + } m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ @@ -1084,21 +1277,41 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) + // verify that the ImageLookupOrg is used when finding AMIs m. - DescribeSubnets(&ec2.DescribeSubnetsInput{ + DescribeImages(gomock.Eq(&ec2.DescribeImagesInput{ Filters: []*ec2.Filter{ - filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), - {Name: aws.String("tag:some-tag"), Values: aws.StringSlice([]string{"some-value"})}, + { + Name: aws.String("owner-id"), + Values: []*string{aws.String("cluster-level-image-lookup-org")}, + }, + { + Name: aws.String("name"), + Values: []*string{aws.String(amiName)}, + }, + { + Name: aws.String("architecture"), + Values: []*string{aws.String("x86_64")}, + }, + { + Name: aws.String("state"), + Values: []*string{aws.String("available")}, + }, + { + Name: aws.String("virtualization-type"), + Values: []*string{aws.String("hvm")}, + }, + }, + })). + Return(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("ami-1"), + CreationDate: aws.String("2006-01-02T15:04:05.000Z"), + }, }, - }). - Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{{ - SubnetId: aws.String("filtered-subnet-1"), - AvailabilityZone: aws.String("us-east-1b"), - }}, }, nil) - m. + m. // TODO: Restore these parameters, but with the tags as well RunInstances(gomock.Any()). Return(&ec2.Reservation{ Instances: []*ec2.Instance{ @@ -1142,7 +1355,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with subnet ID that belongs to Cluster", + name: "AWSMachine ImageLookupOrg overrides AWSCluster ImageLookupOrg", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1151,28 +1364,31 @@ func TestCreateInstance(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: pointer.String("bootstrap-data"), }, + Version: pointer.String("v1.16.1"), }, }, machineConfig: &infrav1.AWSMachineSpec{ - AMI: infrav1.AMIReference{ - ID: aws.String("abc"), - }, - InstanceType: "m5.large", - Subnet: &infrav1.AWSResourceReference{ - ID: aws.String("matching-subnet"), - }, + InstanceType: "m5.large", + ImageLookupOrg: "machine-level-image-lookup-org", }, 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-id", + ID: "vpc-test", }, - Subnets: infrav1.Subnets{{ - ID: "matching-subnet", - }}, }, + ImageLookupOrg: "cluster-level-image-lookup-org", }, Status: infrav1.AWSClusterStatus{ Network: infrav1.NetworkStatus{ @@ -1194,20 +1410,10 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { - m. - DescribeSubnets(&ec2.DescribeSubnetsInput{ - Filters: []*ec2.Filter{ - filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), - {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"matching-subnet"})}, - }, - }). - Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{{ - SubnetId: aws.String("matching-subnet"), - AvailabilityZone: aws.String("us-east-1b"), - }}, - }, nil) + amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1") + if err != nil { + t.Fatalf("Failed to process ami format: %v", err) + } m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ @@ -1225,30 +1431,64 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) + // verify that the ImageLookupOrg is used when finding AMIs m. - RunInstances(gomock.Any()). - Return(&ec2.Reservation{ - Instances: []*ec2.Instance{ + DescribeImages(gomock.Eq(&ec2.DescribeImagesInput{ + Filters: []*ec2.Filter{ { - 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("matching-subnet"), - 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"), - }, - }, - }, + Name: aws.String("owner-id"), + Values: []*string{aws.String("machine-level-image-lookup-org")}, + }, + { + Name: aws.String("name"), + Values: []*string{aws.String(amiName)}, + }, + { + Name: aws.String("architecture"), + Values: []*string{aws.String("x86_64")}, + }, + { + Name: aws.String("state"), + Values: []*string{aws.String("available")}, + }, + { + Name: aws.String("virtualization-type"), + Values: []*string{aws.String("hvm")}, + }, + }, + })). + Return(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("ami-1"), + CreationDate: aws.String("2006-01-02T15:04:05.000Z"), + }, + }, + }, nil) + m. // TODO: Restore these parameters, but with the tags as well + RunInstances(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, }, @@ -1269,7 +1509,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with subnet ID that does not exist", + name: "subnet filter and failureDomain defined", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1278,6 +1518,7 @@ func TestCreateInstance(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: pointer.String("bootstrap-data"), }, + FailureDomain: aws.String("us-east-1b"), }, }, machineConfig: &infrav1.AWSMachineSpec{ @@ -1286,7 +1527,10 @@ func TestCreateInstance(t *testing.T) { }, InstanceType: "m5.large", Subnet: &infrav1.AWSResourceReference{ - ID: aws.String("non-matching-subnet"), + Filters: []infrav1.Filter{{ + Name: "tag:some-tag", + Values: []string{"some-value"}, + }}, }, }, awsCluster: &infrav1.AWSCluster{ @@ -1296,9 +1540,6 @@ func TestCreateInstance(t *testing.T) { VPC: infrav1.VPCSpec{ ID: "vpc-id", }, - Subnets: infrav1.Subnets{{ - ID: "subnet-1", - }}, }, }, Status: infrav1.AWSClusterStatus{ @@ -1342,27 +1583,60 @@ func TestCreateInstance(t *testing.T) { DescribeSubnets(&ec2.DescribeSubnetsInput{ Filters: []*ec2.Filter{ filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), - {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"non-matching-subnet"})}, + {Name: aws.String("tag:some-tag"), Values: aws.StringSlice([]string{"some-value"})}, }, }). Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{}, + Subnets: []*ec2.Subnet{{ + SubnetId: aws.String("filtered-subnet-1"), + AvailabilityZone: aws.String("us-east-1b"), + }}, + }, nil) + m. + RunInstances(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, + }, + }, + }, + }, nil) + m. + DescribeNetworkInterfaces(gomock.Any()). + Return(&ec2.DescribeNetworkInterfacesOutput{ + NetworkInterfaces: []*ec2.NetworkInterface{}, + NextToken: nil, }, nil) }, check: func(instance *infrav1.Instance, err error) { - expectedErrMsg := "failed to run machine \"aws-test1\", no subnets available matching criteria" - 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()) + if err != nil { + t.Fatalf("did not expect error: %v", err) } }, }, { - name: "with subnet ID that does not belong to Cluster", + name: "with subnet ID that belongs to Cluster", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1390,7 +1664,7 @@ func TestCreateInstance(t *testing.T) { ID: "vpc-id", }, Subnets: infrav1.Subnets{{ - ID: "subnet-1", + ID: "matching-subnet", }}, }, }, @@ -1414,6 +1688,36 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { + m. + DescribeSubnets(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"matching-subnet"})}, + }, + }). + Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{{ + SubnetId: aws.String("matching-subnet"), + AvailabilityZone: aws.String("us-east-1b"), + }}, + }, 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. RunInstances(gomock.Any()). Return(&ec2.Reservation{ @@ -1444,36 +1748,6 @@ func TestCreateInstance(t *testing.T) { }, }, }, 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. - DescribeSubnets(&ec2.DescribeSubnetsInput{ - Filters: []*ec2.Filter{ - filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), - {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"matching-subnet"})}, - }, - }). - Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{{ - SubnetId: aws.String("matching-subnet"), - }}, - }, nil) m. DescribeNetworkInterfaces(gomock.Any()). Return(&ec2.DescribeNetworkInterfacesOutput{ @@ -1488,7 +1762,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "subnet id and failureDomain don't match", + name: "with subnet ID that does not exist", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1497,7 +1771,6 @@ func TestCreateInstance(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: pointer.String("bootstrap-data"), }, - FailureDomain: aws.String("us-east-1b"), }, }, machineConfig: &infrav1.AWSMachineSpec{ @@ -1506,7 +1779,7 @@ func TestCreateInstance(t *testing.T) { }, InstanceType: "m5.large", Subnet: &infrav1.AWSResourceReference{ - ID: aws.String("subnet-1"), + ID: aws.String("non-matching-subnet"), }, }, awsCluster: &infrav1.AWSCluster{ @@ -1517,8 +1790,7 @@ func TestCreateInstance(t *testing.T) { ID: "vpc-id", }, Subnets: infrav1.Subnets{{ - ID: "subnet-1", - AvailabilityZone: "us-west-1b", + ID: "subnet-1", }}, }, }, @@ -1542,20 +1814,6 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { - m. - DescribeSubnets(&ec2.DescribeSubnetsInput{ - Filters: []*ec2.Filter{ - filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), - {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"subnet-1"})}, - }, - }). - Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{{ - SubnetId: aws.String("subnet-1"), - AvailabilityZone: aws.String("us-west-1b"), - }}, - }, nil) m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ @@ -1573,20 +1831,30 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) + m. + DescribeSubnets(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"non-matching-subnet"})}, + }, + }). + Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{}, + }, nil) }, check: func(instance *infrav1.Instance, err error) { - expectedErrMsg := "failed to run machine \"aws-test1\", found 1 subnets matching criteria but post-filtering failed. subnet \"subnet-1\" availability zone \"us-west-1b\" does not match failure domain \"us-east-1b\"" + expectedErrMsg := "failed to run machine \"aws-test1\", no subnets available matching criteria" 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()) + t.Fatalf("Expected error: %s\nInstead got: %s", expectedErrMsg, err.Error()) } }, }, { - name: "public IP true and failureDomain doesn't have public subnet", + name: "with subnet ID that does not belong to Cluster", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -1595,7 +1863,6 @@ func TestCreateInstance(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: pointer.String("bootstrap-data"), }, - FailureDomain: aws.String("us-east-1b"), }, }, machineConfig: &infrav1.AWSMachineSpec{ @@ -1603,7 +1870,9 @@ func TestCreateInstance(t *testing.T) { ID: aws.String("abc"), }, InstanceType: "m5.large", - PublicIP: aws.Bool(true), + Subnet: &infrav1.AWSResourceReference{ + ID: aws.String("matching-subnet"), + }, }, awsCluster: &infrav1.AWSCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, @@ -1613,9 +1882,7 @@ func TestCreateInstance(t *testing.T) { ID: "vpc-id", }, Subnets: infrav1.Subnets{{ - ID: "private-subnet-1", - AvailabilityZone: "us-east-1b", - IsPublic: false, + ID: "subnet-1", }}, }, }, @@ -1639,6 +1906,36 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { + m. + RunInstances(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("matching-subnet"), + 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, + }, + }, + }, + }, nil) m. DescribeInstanceTypes(gomock.Eq(&ec2.DescribeInstanceTypesInput{ InstanceTypes: []*string{ @@ -1656,7 +1953,200 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) - }, + m. + DescribeSubnets(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"matching-subnet"})}, + }, + }). + Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{{ + SubnetId: aws.String("matching-subnet"), + }}, + }, 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: "subnet id and failureDomain don't match", + machine: clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"set": "node"}, + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: pointer.String("bootstrap-data"), + }, + FailureDomain: aws.String("us-east-1b"), + }, + }, + machineConfig: &infrav1.AWSMachineSpec{ + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.large", + Subnet: &infrav1.AWSResourceReference{ + ID: aws.String("subnet-1"), + }, + }, + awsCluster: &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: infrav1.AWSClusterSpec{ + NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: "vpc-id", + }, + Subnets: infrav1.Subnets{{ + ID: "subnet-1", + AvailabilityZone: "us-west-1b", + }}, + }, + }, + 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. + DescribeSubnets(&ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{ + filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), + {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"subnet-1"})}, + }, + }). + Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{{ + SubnetId: aws.String("subnet-1"), + AvailabilityZone: aws.String("us-west-1b"), + }}, + }, 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) + }, + check: func(instance *infrav1.Instance, err error) { + expectedErrMsg := "failed to run machine \"aws-test1\", found 1 subnets matching criteria but post-filtering failed. subnet \"subnet-1\" availability zone \"us-west-1b\" does not match failure domain \"us-east-1b\"" + 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: "public IP true and failureDomain doesn't have public subnet", + machine: clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"set": "node"}, + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: pointer.String("bootstrap-data"), + }, + FailureDomain: aws.String("us-east-1b"), + }, + }, + machineConfig: &infrav1.AWSMachineSpec{ + AMI: infrav1.AMIReference{ + ID: aws.String("abc"), + }, + InstanceType: "m5.large", + PublicIP: aws.Bool(true), + }, + awsCluster: &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: infrav1.AWSClusterSpec{ + NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: "vpc-id", + }, + Subnets: infrav1.Subnets{{ + ID: "private-subnet-1", + AvailabilityZone: "us-east-1b", + 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. + 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) + }, check: func(instance *infrav1.Instance, err error) { expectedErrMsg := "failed to run machine \"aws-test1\" with public IP, no public subnets available in availability zone \"us-east-1b\"" if err == nil { @@ -1727,7 +2217,6 @@ func TestCreateInstance(t *testing.T) { DescribeSubnets(&ec2.DescribeSubnetsInput{ Filters: []*ec2.Filter{ filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"public-subnet-1"})}, }, }). @@ -1857,7 +2346,6 @@ func TestCreateInstance(t *testing.T) { DescribeSubnets(&ec2.DescribeSubnetsInput{ Filters: []*ec2.Filter{ filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{"private-subnet-1"})}, }, }). @@ -1982,7 +2470,6 @@ func TestCreateInstance(t *testing.T) { DescribeSubnets(&ec2.DescribeSubnetsInput{ Filters: []*ec2.Filter{ filter.EC2.SubnetStates(ec2.SubnetStatePending, ec2.SubnetStateAvailable), - filter.EC2.VPC("vpc-id"), {Name: aws.String("tag:some-tag"), Values: aws.StringSlice([]string{"some-value"})}, }, }). @@ -2343,9 +2830,427 @@ func TestCreateInstance(t *testing.T) { }, }, }, - Placement: &ec2.Placement{ - AvailabilityZone: &az, - }, + Placement: &ec2.Placement{ + AvailabilityZone: &az, + }, + }, + }, + }, 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 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", + Tenancy: "dedicated", + 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, + }, + }, + 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. // 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{ + Tenancy: &tenancy, + }, + 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"), + }, + }, + }, + { + ResourceType: aws.String("volume"), + 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"), + }, + }, + }, + { + ResourceType: aws.String("network-interface"), + 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, + Tenancy: &tenancy, + }, + }, + }, + }, 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 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, + }, + }, + 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. // 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"), + }, + }, + }, + { + ResourceType: aws.String("volume"), + 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"), + }, + }, + }, + { + ResourceType: aws.String("network-interface"), + 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) @@ -2363,7 +3268,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with dedicated tenancy cloud-config", + name: "with dedicated tenancy and placement group ignition", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -2382,9 +3287,12 @@ func TestCreateInstance(t *testing.T) { }, InstanceType: "m5.large", Tenancy: "dedicated", - UncompressedUserData: &isUncompressedFalse, + PlacementGroupName: "placement-group1", + UncompressedUserData: &isUncompressedTrue, + Ignition: &infrav1.Ignition{}, }, awsCluster: &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: infrav1.AWSClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ Subnets: infrav1.Subnets{ @@ -2421,6 +3329,23 @@ func TestCreateInstance(t *testing.T) { }, }, expect: func(m *mocks.MockEC2APIMockRecorder) { + 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. // TODO: Restore these parameters, but with the tags as well RunInstances(gomock.Eq(&ec2.RunInstancesInput{ ImageId: aws.String("abc"), @@ -2429,7 +3354,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"), @@ -2459,8 +3385,58 @@ func TestCreateInstance(t *testing.T) { }, }, }, + { + ResourceType: aws.String("volume"), + 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"), + }, + }, + }, + { + ResourceType: aws.String("network-interface"), + 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)), + UserData: aws.String(base64.StdEncoding.EncodeToString(data)), })). Return(&ec2.Reservation{ Instances: []*ec2.Instance{ @@ -2491,23 +3467,6 @@ func TestCreateInstance(t *testing.T) { }, }, }, 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{ @@ -2522,7 +3481,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with custom placement group cloud-config", + name: "with custom placement group and partition number", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -2539,9 +3498,10 @@ func TestCreateInstance(t *testing.T) { AMI: infrav1.AMIReference{ ID: aws.String("abc"), }, - InstanceType: "m5.large", - PlacementGroupName: "placement-group1", - UncompressedUserData: &isUncompressedFalse, + InstanceType: "m5.large", + PlacementGroupName: "placement-group1", + PlacementGroupPartition: 2, + UncompressedUserData: &isUncompressedFalse, }, awsCluster: &infrav1.AWSCluster{ Spec: infrav1.AWSClusterSpec{ @@ -2555,9 +3515,6 @@ func TestCreateInstance(t *testing.T) { IsPublic: false, }, }, - VPC: infrav1.VPCSpec{ - ID: "vpc-test", - }, }, }, Status: infrav1.AWSClusterStatus{ @@ -2588,7 +3545,8 @@ func TestCreateInstance(t *testing.T) { MaxCount: aws.Int64(1), MinCount: aws.Int64(1), Placement: &ec2.Placement{ - GroupName: aws.String("placement-group1"), + GroupName: aws.String("placement-group1"), + PartitionNumber: aws.Int64(2), }, SecurityGroupIds: []*string{aws.String("2"), aws.String("3")}, SubnetId: aws.String("subnet-1"), @@ -2618,6 +3576,56 @@ func TestCreateInstance(t *testing.T) { }, }, }, + { + ResourceType: aws.String("volume"), + 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"), + }, + }, + }, + { + ResourceType: aws.String("network-interface"), + 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)), })). @@ -2646,6 +3654,7 @@ func TestCreateInstance(t *testing.T) { Placement: &ec2.Placement{ AvailabilityZone: &az, GroupName: aws.String("placement-group1"), + PartitionNumber: aws.Int64(2), }, }, }, @@ -2681,7 +3690,7 @@ func TestCreateInstance(t *testing.T) { }, }, { - name: "with dedicated tenancy and placement group ignition", + name: "expect error when placementGroupPartition is set, but placementGroupName is empty", machine: clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"set": "node"}, @@ -2698,14 +3707,11 @@ func TestCreateInstance(t *testing.T) { AMI: infrav1.AMIReference{ ID: aws.String("abc"), }, - InstanceType: "m5.large", - Tenancy: "dedicated", - PlacementGroupName: "placement-group1", - UncompressedUserData: &isUncompressedTrue, - Ignition: &infrav1.Ignition{}, + InstanceType: "m5.large", + PlacementGroupPartition: 2, + UncompressedUserData: &isUncompressedFalse, }, awsCluster: &infrav1.AWSCluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: infrav1.AWSClusterSpec{ NetworkSpec: infrav1.NetworkSpec{ Subnets: infrav1.Subnets{ @@ -2717,9 +3723,6 @@ func TestCreateInstance(t *testing.T) { IsPublic: false, }, }, - VPC: infrav1.VPCSpec{ - ID: "vpc-test", - }, }, }, Status: infrav1.AWSClusterStatus{ @@ -2759,87 +3762,14 @@ func TestCreateInstance(t *testing.T) { }, }, }, nil) - 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{ - Tenancy: &tenancy, - 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(data)), - })). - 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, - Tenancy: &tenancy, - }, - }, - }, - }, 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) + expectedErrMsg := "placementGroupPartition is set but placementGroupName is empty" + 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()) } }, },