diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index ca199770ec..5761b39e67 100644 --- a/api/v1beta1/awscluster_conversion.go +++ b/api/v1beta1/awscluster_conversion.go @@ -44,6 +44,14 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error { } restoreControlPlaneLoadBalancerStatus(&restored.Status.Network.APIServerELB, &dst.Status.Network.APIServerELB) + if restored.Spec.SecondaryControlPlaneLoadBalancer != nil { + if dst.Spec.SecondaryControlPlaneLoadBalancer == nil { + dst.Spec.SecondaryControlPlaneLoadBalancer = &infrav2.AWSLoadBalancerSpec{} + } + restoreControlPlaneLoadBalancer(restored.Spec.SecondaryControlPlaneLoadBalancer, dst.Spec.SecondaryControlPlaneLoadBalancer) + } + restoreControlPlaneLoadBalancerStatus(&restored.Status.Network.SecondaryAPIServerELB, &dst.Status.Network.SecondaryAPIServerELB) + dst.Spec.S3Bucket = restored.Spec.S3Bucket if restored.Status.Bastion != nil { dst.Status.Bastion.InstanceMetadataOptions = restored.Status.Bastion.InstanceMetadataOptions @@ -115,6 +123,16 @@ func restoreControlPlaneLoadBalancerStatus(restored, dst *infrav2.LoadBalancer) dst.LoadBalancerType = restored.LoadBalancerType dst.ELBAttributes = restored.ELBAttributes dst.ELBListeners = restored.ELBListeners + dst.Name = restored.Name + dst.DNSName = restored.DNSName + dst.Scheme = restored.Scheme + dst.SubnetIDs = restored.SubnetIDs + dst.SecurityGroupIDs = restored.SecurityGroupIDs + dst.HealthCheck = restored.HealthCheck + dst.ClassicElbAttributes = restored.ClassicElbAttributes + dst.Tags = restored.Tags + dst.ClassicELBListeners = restored.ClassicELBListeners + dst.AvailabilityZones = restored.AvailabilityZones } // restoreIPAMPool manually restores the ipam pool data. @@ -135,6 +153,10 @@ func restoreControlPlaneLoadBalancer(restored, dst *infrav2.AWSLoadBalancerSpec) dst.PreserveClientIP = restored.PreserveClientIP dst.IngressRules = restored.IngressRules dst.AdditionalListeners = restored.AdditionalListeners + dst.AdditionalSecurityGroups = restored.AdditionalSecurityGroups + dst.Scheme = restored.Scheme + dst.CrossZoneLoadBalancing = restored.CrossZoneLoadBalancing + dst.Subnets = restored.Subnets } // ConvertFrom converts the v1beta1 AWSCluster receiver to a v1beta1 AWSCluster. diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 347e67fd35..45567bd837 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -932,6 +932,7 @@ func autoConvert_v1beta2_AWSClusterSpec_To_v1beta1_AWSClusterSpec(in *v1beta2.AW } else { out.ControlPlaneLoadBalancer = nil } + // WARNING: in.SecondaryControlPlaneLoadBalancer requires manual conversion: does not exist in peer-type out.ImageLookupFormat = in.ImageLookupFormat out.ImageLookupOrg = in.ImageLookupOrg out.ImageLookupBaseOS = in.ImageLookupBaseOS @@ -2101,6 +2102,7 @@ func autoConvert_v1beta2_NetworkStatus_To_v1beta1_NetworkStatus(in *v1beta2.Netw if err := Convert_v1beta2_LoadBalancer_To_v1beta1_ClassicELB(&in.APIServerELB, &out.APIServerELB, s); err != nil { return err } + // WARNING: in.SecondaryAPIServerELB requires manual conversion: does not exist in peer-type // WARNING: in.NatGatewaysIPs requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta2/awscluster_types.go b/api/v1beta2/awscluster_types.go index 0a1bdafbfc..05b3887b2d 100644 --- a/api/v1beta2/awscluster_types.go +++ b/api/v1beta2/awscluster_types.go @@ -60,6 +60,14 @@ type AWSClusterSpec struct { // +optional ControlPlaneLoadBalancer *AWSLoadBalancerSpec `json:"controlPlaneLoadBalancer,omitempty"` + // SecondaryControlPlaneLoadBalancer is an additional load balancer that can be used for the control plane. + // + // An example use case is to have a separate internal load balancer for internal traffic, + // and a separate external load balancer for external traffic. + // + // +optional + SecondaryControlPlaneLoadBalancer *AWSLoadBalancerSpec `json:"secondaryControlPlaneLoadBalancer,omitempty"` + // ImageLookupFormat is the AMI naming format to look up machine images when // a machine does not specify an AMI. When set, this will be used for all // cluster machines unless a machine specifies a different ImageLookupOrg. diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go index 3ffdfaa707..bbc3eb6a8f 100644 --- a/api/v1beta2/awscluster_webhook.go +++ b/api/v1beta2/awscluster_webhook.go @@ -58,7 +58,7 @@ func (r *AWSCluster) ValidateCreate() (admission.Warnings, error) { allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...) allErrs = append(allErrs, r.Spec.S3Bucket.Validate()...) allErrs = append(allErrs, r.validateNetwork()...) - allErrs = append(allErrs, r.validateControlPlaneLB()...) + allErrs = append(allErrs, r.validateControlPlaneLBs()...) return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) } @@ -85,51 +85,18 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, err ) } - newLoadBalancer := &AWSLoadBalancerSpec{} - existingLoadBalancer := &AWSLoadBalancerSpec{} - - if r.Spec.ControlPlaneLoadBalancer != nil { - newLoadBalancer = r.Spec.ControlPlaneLoadBalancer.DeepCopy() + // Validate the control plane load balancers. + lbs := map[*AWSLoadBalancerSpec]*AWSLoadBalancerSpec{ + oldC.Spec.ControlPlaneLoadBalancer: r.Spec.ControlPlaneLoadBalancer, + oldC.Spec.SecondaryControlPlaneLoadBalancer: r.Spec.SecondaryControlPlaneLoadBalancer, } - if oldC.Spec.ControlPlaneLoadBalancer != nil { - existingLoadBalancer = oldC.Spec.ControlPlaneLoadBalancer.DeepCopy() - } - if oldC.Spec.ControlPlaneLoadBalancer == nil { - // If old scheme was nil, the only value accepted here is the default value: internet-facing - if newLoadBalancer.Scheme != nil && newLoadBalancer.Scheme.String() != ELBSchemeInternetFacing.String() { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"), - r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable, default value was set to internet-facing"), - ) - } - } else { - // If old scheme was not nil, the new scheme should be the same. - if !cmp.Equal(existingLoadBalancer.Scheme, newLoadBalancer.Scheme) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"), - r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable"), - ) - } - // The name must be defined when the AWSCluster is created. If it is not defined, - // then the controller generates a default name at runtime, but does not store it, - // so the name remains nil. In either case, the name cannot be changed. - if !cmp.Equal(existingLoadBalancer.Name, newLoadBalancer.Name) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "name"), - r.Spec.ControlPlaneLoadBalancer.Name, "field is immutable"), - ) + for oldLB, newLB := range lbs { + if oldLB == nil && newLB == nil { + continue } - } - // Block the update for Protocol : - // - if it was not set in old spec but added in new spec - // - if it was set in old spec but changed in new spec - if !cmp.Equal(newLoadBalancer.HealthCheckProtocol, existingLoadBalancer.HealthCheckProtocol) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthCheckProtocol"), - newLoadBalancer.HealthCheckProtocol, "field is immutable once set"), - ) + allErrs = append(allErrs, r.validateControlPlaneLoadBalancerUpdate(oldLB, newLB)...) } if !cmp.Equal(oldC.Spec.ControlPlaneEndpoint, clusterv1.APIEndpoint{}) && @@ -174,6 +141,49 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, err return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) } +func (r *AWSCluster) validateControlPlaneLoadBalancerUpdate(oldlb, newlb *AWSLoadBalancerSpec) field.ErrorList { + var allErrs field.ErrorList + + if oldlb == nil { + // If old scheme was nil, the only value accepted here is the default value: internet-facing + if newlb.Scheme != nil && newlb.Scheme.String() != ELBSchemeInternetFacing.String() { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"), + newlb.Scheme, "field is immutable, default value was set to internet-facing"), + ) + } + } else { + // If old scheme was not nil, the new scheme should be the same. + if !cmp.Equal(oldlb.Scheme, newlb.Scheme) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"), + newlb.Scheme, "field is immutable"), + ) + } + // The name must be defined when the AWSCluster is created. If it is not defined, + // then the controller generates a default name at runtime, but does not store it, + // so the name remains nil. In either case, the name cannot be changed. + if !cmp.Equal(oldlb.Name, newlb.Name) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "name"), + newlb.Name, "field is immutable"), + ) + } + } + + // Block the update for Protocol : + // - if it was not set in old spec but added in new spec + // - if it was set in old spec but changed in new spec + if !cmp.Equal(newlb.HealthCheckProtocol, oldlb.HealthCheckProtocol) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthCheckProtocol"), + newlb.HealthCheckProtocol, "field is immutable once set"), + ) + } + + return allErrs +} + // Default satisfies the defaulting webhook interface. func (r *AWSCluster) Default() { SetObjectDefaults_AWSCluster(r) @@ -243,26 +253,48 @@ func (r *AWSCluster) validateNetwork() field.ErrorList { allErrs = append(allErrs, field.Invalid(field.NewPath("additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together")) } } + return allErrs } -func (r *AWSCluster) validateControlPlaneLB() field.ErrorList { +func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList { var allErrs field.ErrorList - if r.Spec.ControlPlaneLoadBalancer == nil { - return allErrs + // If the secondary is defined, check that the name is not empty and different from the primary. + // Also, ensure that the secondary load balancer is an NLB + if r.Spec.SecondaryControlPlaneLoadBalancer != nil { + if r.Spec.SecondaryControlPlaneLoadBalancer.Name == nil || *r.Spec.SecondaryControlPlaneLoadBalancer.Name == "" { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "name"), r.Spec.SecondaryControlPlaneLoadBalancer.Name, "secondary controlPlaneLoadBalancer.name cannot be empty")) + } + + if r.Spec.SecondaryControlPlaneLoadBalancer.Name == r.Spec.ControlPlaneLoadBalancer.Name { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "name"), r.Spec.SecondaryControlPlaneLoadBalancer.Name, "field must be different from controlPlaneLoadBalancer.name")) + } + + if r.Spec.SecondaryControlPlaneLoadBalancer.Scheme.Equals(r.Spec.ControlPlaneLoadBalancer.Scheme) { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "scheme"), r.Spec.SecondaryControlPlaneLoadBalancer.Scheme, "control plane load balancers must have different schemes")) + } + + if r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType != LoadBalancerTypeNLB { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "loadBalancerType"), r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType, "secondary control plane load balancer must be a Network Load Balancer")) + } } // Additional listeners are only supported for NLBs. - if len(r.Spec.ControlPlaneLoadBalancer.AdditionalListeners) > 0 { - if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType != LoadBalancerTypeNLB { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "additionalListeners"), r.Spec.ControlPlaneLoadBalancer.AdditionalListeners, "additional listeners are only supported for NLB load balancers")) - } + // Validate the control plane load balancers. + loadBalancers := []*AWSLoadBalancerSpec{ + r.Spec.ControlPlaneLoadBalancer, + r.Spec.SecondaryControlPlaneLoadBalancer, } + for _, cp := range loadBalancers { + if cp == nil { + continue + } - for _, rule := range r.Spec.ControlPlaneLoadBalancer.IngressRules { - if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together")) + for _, rule := range cp.IngressRules { + if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together")) + } } } diff --git a/api/v1beta2/defaults.go b/api/v1beta2/defaults.go index 50a917df37..f10bb895c1 100644 --- a/api/v1beta2/defaults.go +++ b/api/v1beta2/defaults.go @@ -69,6 +69,14 @@ func SetDefaults_AWSClusterSpec(s *AWSClusterSpec) { //nolint:golint,stylecheck if s.ControlPlaneLoadBalancer.LoadBalancerType == "" { s.ControlPlaneLoadBalancer.LoadBalancerType = LoadBalancerTypeClassic } + if s.SecondaryControlPlaneLoadBalancer != nil { + if s.SecondaryControlPlaneLoadBalancer.LoadBalancerType == "" { + s.SecondaryControlPlaneLoadBalancer.LoadBalancerType = LoadBalancerTypeNLB + } + if s.SecondaryControlPlaneLoadBalancer.Scheme == nil { + s.SecondaryControlPlaneLoadBalancer.Scheme = &ELBSchemeInternal + } + } } // SetDefaults_Labels is used to default cluster scope resources for clusterctl move. diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go index 8b4ba3ac4e..df5f13fea9 100644 --- a/api/v1beta2/network_types.go +++ b/api/v1beta2/network_types.go @@ -37,6 +37,9 @@ type NetworkStatus struct { // APIServerELB is the Kubernetes api server load balancer. APIServerELB LoadBalancer `json:"apiServerElb,omitempty"` + // SecondaryAPIServerELB is the secondary Kubernetes api server load balancer. + SecondaryAPIServerELB LoadBalancer `json:"secondaryAPIServerELB,omitempty"` + // NatGatewaysIPs contains the public IPs of the NAT Gateways NatGatewaysIPs []string `json:"natGatewaysIPs,omitempty"` } diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index cd5ba0a0f9..a037772772 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -308,6 +308,11 @@ func (in *AWSClusterSpec) DeepCopyInto(out *AWSClusterSpec) { *out = new(AWSLoadBalancerSpec) (*in).DeepCopyInto(*out) } + if in.SecondaryControlPlaneLoadBalancer != nil { + in, out := &in.SecondaryControlPlaneLoadBalancer, &out.SecondaryControlPlaneLoadBalancer + *out = new(AWSLoadBalancerSpec) + (*in).DeepCopyInto(*out) + } in.Bastion.DeepCopyInto(&out.Bastion) if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef @@ -1632,6 +1637,7 @@ func (in *NetworkStatus) DeepCopyInto(out *NetworkStatus) { } } in.APIServerELB.DeepCopyInto(&out.APIServerELB) + in.SecondaryAPIServerELB.DeepCopyInto(&out.SecondaryAPIServerELB) if in.NatGatewaysIPs != nil { in, out := &in.NatGatewaysIPs, &out.NatGatewaysIPs *out = make([]string, len(*in)) 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 8c9cdd638a..3c30e1f660 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -1500,6 +1500,215 @@ spec: items: type: string type: array + secondaryAPIServerELB: + description: SecondaryAPIServerELB is the secondary Kubernetes + api server load balancer. + properties: + arn: + description: ARN of the load balancer. Unlike the ClassicLB, + ARN is used mostly to define and get it. + type: string + attributes: + description: ClassicElbAttributes defines extra attributes + associated with the load balancer. + properties: + crossZoneLoadBalancing: + description: CrossZoneLoadBalancing enables the classic + load balancer load balancing. + type: boolean + idleTimeout: + description: IdleTimeout is time that the connection is + allowed to be idle (no data has been sent over the connection) + before it is closed by the load balancer. + format: int64 + type: integer + type: object + availabilityZones: + description: AvailabilityZones is an array of availability + zones in the VPC attached to the load balancer. + items: + type: string + type: array + dnsName: + description: DNSName is the dns name of the load balancer. + type: string + elbAttributes: + additionalProperties: + type: string + description: ELBAttributes defines extra attributes associated + with v2 load balancers. + type: object + elbListeners: + description: ELBListeners is an array of listeners associated + with the load balancer. There must be at least one. + items: + description: Listener defines an AWS network load balancer + listener. + properties: + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + targetGroup: + description: TargetGroupSpec specifies target group + settings for a given listener. This is created first, + and the ARN is then passed to the listener. + properties: + name: + description: Name of the TargetGroup. Must be unique + over the same group of listeners. + type: string + port: + description: Port is the exposed port + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + enum: + - tcp + - tls + - udp + - TCP + - TLS + - UDP + type: string + targetGroupHealthCheck: + description: HealthCheck is the elb health check + associated with the load balancer. + properties: + intervalSeconds: + format: int64 + type: integer + path: + type: string + port: + type: string + protocol: + type: string + thresholdCount: + format: int64 + type: integer + timeoutSeconds: + format: int64 + type: integer + type: object + vpcId: + type: string + required: + - name + - port + - protocol + - vpcId + type: object + required: + - port + - protocol + - targetGroup + type: object + type: array + healthChecks: + description: HealthCheck is the classic elb health check associated + with the load balancer. + properties: + healthyThreshold: + format: int64 + type: integer + interval: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + target: + type: string + timeout: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + unhealthyThreshold: + format: int64 + type: integer + required: + - healthyThreshold + - interval + - target + - timeout + - unhealthyThreshold + type: object + listeners: + description: ClassicELBListeners is an array of classic elb + listeners associated with the load balancer. There must + be at least one. + items: + description: ClassicELBListener defines an AWS classic load + balancer listener. + properties: + instancePort: + format: int64 + type: integer + instanceProtocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + required: + - instancePort + - instanceProtocol + - port + - protocol + type: object + type: array + loadBalancerType: + description: LoadBalancerType sets the type for a load balancer. + The default type is classic. + enum: + - classic + - elb + - alb + - nlb + type: string + name: + description: The name of the load balancer. It must be unique + within the set of load balancers defined in the region. + It also serves as identifier. + type: string + scheme: + description: Scheme is the load balancer scheme, either internet-facing + or private. + type: string + securityGroupIds: + description: SecurityGroupIDs is an array of security groups + assigned to the load balancer. + items: + type: string + type: array + subnetIds: + description: SubnetIDs is an array of subnets in the VPC attached + to the load balancer. + items: + type: string + type: array + tags: + additionalProperties: + type: string + description: Tags is a map of tags associated with the load + balancer. + type: object + type: object securityGroups: additionalProperties: description: SecurityGroup defines an AWS security group. @@ -3110,6 +3319,215 @@ spec: items: type: string type: array + secondaryAPIServerELB: + description: SecondaryAPIServerELB is the secondary Kubernetes + api server load balancer. + properties: + arn: + description: ARN of the load balancer. Unlike the ClassicLB, + ARN is used mostly to define and get it. + type: string + attributes: + description: ClassicElbAttributes defines extra attributes + associated with the load balancer. + properties: + crossZoneLoadBalancing: + description: CrossZoneLoadBalancing enables the classic + load balancer load balancing. + type: boolean + idleTimeout: + description: IdleTimeout is time that the connection is + allowed to be idle (no data has been sent over the connection) + before it is closed by the load balancer. + format: int64 + type: integer + type: object + availabilityZones: + description: AvailabilityZones is an array of availability + zones in the VPC attached to the load balancer. + items: + type: string + type: array + dnsName: + description: DNSName is the dns name of the load balancer. + type: string + elbAttributes: + additionalProperties: + type: string + description: ELBAttributes defines extra attributes associated + with v2 load balancers. + type: object + elbListeners: + description: ELBListeners is an array of listeners associated + with the load balancer. There must be at least one. + items: + description: Listener defines an AWS network load balancer + listener. + properties: + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + targetGroup: + description: TargetGroupSpec specifies target group + settings for a given listener. This is created first, + and the ARN is then passed to the listener. + properties: + name: + description: Name of the TargetGroup. Must be unique + over the same group of listeners. + type: string + port: + description: Port is the exposed port + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + enum: + - tcp + - tls + - udp + - TCP + - TLS + - UDP + type: string + targetGroupHealthCheck: + description: HealthCheck is the elb health check + associated with the load balancer. + properties: + intervalSeconds: + format: int64 + type: integer + path: + type: string + port: + type: string + protocol: + type: string + thresholdCount: + format: int64 + type: integer + timeoutSeconds: + format: int64 + type: integer + type: object + vpcId: + type: string + required: + - name + - port + - protocol + - vpcId + type: object + required: + - port + - protocol + - targetGroup + type: object + type: array + healthChecks: + description: HealthCheck is the classic elb health check associated + with the load balancer. + properties: + healthyThreshold: + format: int64 + type: integer + interval: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + target: + type: string + timeout: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + unhealthyThreshold: + format: int64 + type: integer + required: + - healthyThreshold + - interval + - target + - timeout + - unhealthyThreshold + type: object + listeners: + description: ClassicELBListeners is an array of classic elb + listeners associated with the load balancer. There must + be at least one. + items: + description: ClassicELBListener defines an AWS classic load + balancer listener. + properties: + instancePort: + format: int64 + type: integer + instanceProtocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + required: + - instancePort + - instanceProtocol + - port + - protocol + type: object + type: array + loadBalancerType: + description: LoadBalancerType sets the type for a load balancer. + The default type is classic. + enum: + - classic + - elb + - alb + - nlb + type: string + name: + description: The name of the load balancer. It must be unique + within the set of load balancers defined in the region. + It also serves as identifier. + type: string + scheme: + description: Scheme is the load balancer scheme, either internet-facing + or private. + type: string + securityGroupIds: + description: SecurityGroupIDs is an array of security groups + assigned to the load balancer. + items: + type: string + type: array + subnetIds: + description: SubnetIDs is an array of subnets in the VPC attached + to the load balancer. + items: + type: string + type: array + tags: + additionalProperties: + type: string + description: Tags is a map of tags associated with the load + balancer. + type: object + type: object securityGroups: additionalProperties: description: SecurityGroup defines an AWS security group. 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 b30f43b245..b8ff27e9a7 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -1527,6 +1527,187 @@ spec: required: - name type: object + secondaryControlPlaneLoadBalancer: + description: "SecondaryControlPlaneLoadBalancer is an additional load + balancer that can be used for the control plane. \n An example use + case is to have a separate internal load balancer for internal traffic, + and a separate external load balancer for external traffic." + properties: + additionalListeners: + description: AdditionalListeners sets the additional listeners + for the control plane load balancer. This is only applicable + to Network Load Balancer (NLB) types for the time being. + items: + description: AdditionalListenerSpec defines the desired state + of an additional listener on an AWS load balancer. + properties: + port: + description: Port sets the port for the additional listener. + format: int64 + maximum: 65535 + minimum: 1 + type: integer + protocol: + default: TCP + description: Protocol sets the protocol for the additional + listener. Currently only TCP is supported. + enum: + - TCP + type: string + required: + - port + type: object + type: array + x-kubernetes-list-map-keys: + - port + x-kubernetes-list-type: map + additionalSecurityGroups: + description: AdditionalSecurityGroups sets the security groups + used by the load balancer. Expected to be security group IDs + This is optional - if not provided new security groups will + be created for the load balancer + items: + type: string + type: array + crossZoneLoadBalancing: + description: "CrossZoneLoadBalancing enables the classic ELB cross + availability zone balancing. \n With cross-zone load balancing, + each load balancer node for your Classic Load Balancer distributes + requests evenly across the registered instances in all enabled + Availability Zones. If cross-zone load balancing is disabled, + each load balancer node distributes requests evenly across the + registered instances in its Availability Zone only. \n Defaults + to false." + type: boolean + disableHostsRewrite: + description: DisableHostsRewrite disabled the hair pinning issue + solution that adds the NLB's address as 127.0.0.1 to the hosts + file of each instance. This is by default, false. + type: boolean + healthCheckProtocol: + description: HealthCheckProtocol sets the protocol type for ELB + health check target default value is ELBProtocolSSL + enum: + - TCP + - SSL + - HTTP + - HTTPS + - TLS + - UDP + type: string + ingressRules: + description: IngressRules sets the ingress rules for the control + plane load balancer. + items: + description: IngressRule defines an AWS ingress rule for security + groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access from. Cannot + be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information about + the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access from. + Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + protocol: + description: Protocol is the protocol for the ingress rule. + Accepted values are "-1" (all), "4" (IP in IP),"tcp", + "udp", "icmp", and "58" (ICMPv6), "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access from. + Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: The security group role to allow access from. + Cannot be specified with CidrBlocks. The field will be + combined with source security group IDs if specified. + items: + description: SecurityGroupRole defines the unique role + of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array + loadBalancerType: + default: classic + description: LoadBalancerType sets the type for a load balancer. + The default type is classic. + enum: + - classic + - elb + - alb + - nlb + type: string + name: + description: Name sets the name of the classic ELB load balancer. + As per AWS, the name must be unique within your set of load + balancers for the region, must have a maximum of 32 characters, + must contain only alphanumeric characters or hyphens, and cannot + begin or end with a hyphen. Once set, the value cannot be changed. + maxLength: 32 + pattern: ^[A-Za-z0-9]([A-Za-z0-9]{0,31}|[-A-Za-z0-9]{0,30}[A-Za-z0-9])$ + type: string + preserveClientIP: + description: PreserveClientIP lets the user control if preservation + of client ips must be retained or not. If this is enabled 6443 + will be opened to 0.0.0.0/0. + type: boolean + scheme: + default: internet-facing + description: Scheme sets the scheme of the load balancer (defaults + to internet-facing) + enum: + - internet-facing + - internal + type: string + subnets: + description: Subnets sets the subnets that should be applied to + the control plane load balancer (defaults to discovered subnets + for managed VPCs or an empty set for unmanaged VPCs) + items: + type: string + type: array + type: object sshKeyName: description: SSHKeyName is the name of the ssh key to attach to the bastion host. Valid values are empty string (do not use SSH keys), @@ -2063,6 +2244,215 @@ spec: items: type: string type: array + secondaryAPIServerELB: + description: SecondaryAPIServerELB is the secondary Kubernetes + api server load balancer. + properties: + arn: + description: ARN of the load balancer. Unlike the ClassicLB, + ARN is used mostly to define and get it. + type: string + attributes: + description: ClassicElbAttributes defines extra attributes + associated with the load balancer. + properties: + crossZoneLoadBalancing: + description: CrossZoneLoadBalancing enables the classic + load balancer load balancing. + type: boolean + idleTimeout: + description: IdleTimeout is time that the connection is + allowed to be idle (no data has been sent over the connection) + before it is closed by the load balancer. + format: int64 + type: integer + type: object + availabilityZones: + description: AvailabilityZones is an array of availability + zones in the VPC attached to the load balancer. + items: + type: string + type: array + dnsName: + description: DNSName is the dns name of the load balancer. + type: string + elbAttributes: + additionalProperties: + type: string + description: ELBAttributes defines extra attributes associated + with v2 load balancers. + type: object + elbListeners: + description: ELBListeners is an array of listeners associated + with the load balancer. There must be at least one. + items: + description: Listener defines an AWS network load balancer + listener. + properties: + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + targetGroup: + description: TargetGroupSpec specifies target group + settings for a given listener. This is created first, + and the ARN is then passed to the listener. + properties: + name: + description: Name of the TargetGroup. Must be unique + over the same group of listeners. + type: string + port: + description: Port is the exposed port + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + enum: + - tcp + - tls + - udp + - TCP + - TLS + - UDP + type: string + targetGroupHealthCheck: + description: HealthCheck is the elb health check + associated with the load balancer. + properties: + intervalSeconds: + format: int64 + type: integer + path: + type: string + port: + type: string + protocol: + type: string + thresholdCount: + format: int64 + type: integer + timeoutSeconds: + format: int64 + type: integer + type: object + vpcId: + type: string + required: + - name + - port + - protocol + - vpcId + type: object + required: + - port + - protocol + - targetGroup + type: object + type: array + healthChecks: + description: HealthCheck is the classic elb health check associated + with the load balancer. + properties: + healthyThreshold: + format: int64 + type: integer + interval: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + target: + type: string + timeout: + description: A Duration represents the elapsed time between + two instants as an int64 nanosecond count. The representation + limits the largest representable duration to approximately + 290 years. + format: int64 + type: integer + unhealthyThreshold: + format: int64 + type: integer + required: + - healthyThreshold + - interval + - target + - timeout + - unhealthyThreshold + type: object + listeners: + description: ClassicELBListeners is an array of classic elb + listeners associated with the load balancer. There must + be at least one. + items: + description: ClassicELBListener defines an AWS classic load + balancer listener. + properties: + instancePort: + format: int64 + type: integer + instanceProtocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + port: + format: int64 + type: integer + protocol: + description: ELBProtocol defines listener protocols + for a load balancer. + type: string + required: + - instancePort + - instanceProtocol + - port + - protocol + type: object + type: array + loadBalancerType: + description: LoadBalancerType sets the type for a load balancer. + The default type is classic. + enum: + - classic + - elb + - alb + - nlb + type: string + name: + description: The name of the load balancer. It must be unique + within the set of load balancers defined in the region. + It also serves as identifier. + type: string + scheme: + description: Scheme is the load balancer scheme, either internet-facing + or private. + type: string + securityGroupIds: + description: SecurityGroupIDs is an array of security groups + assigned to the load balancer. + items: + type: string + type: array + subnetIds: + description: SubnetIDs is an array of subnets in the VPC attached + to the load balancer. + items: + type: string + type: array + tags: + additionalProperties: + type: string + description: Tags is a map of tags associated with the load + balancer. + type: object + type: object securityGroups: additionalProperties: description: SecurityGroup defines an AWS security group. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml index a8dbe5cbb1..0da685658a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml @@ -1148,6 +1148,195 @@ spec: required: - name type: object + secondaryControlPlaneLoadBalancer: + description: "SecondaryControlPlaneLoadBalancer is an additional + load balancer that can be used for the control plane. \n + An example use case is to have a separate internal load + balancer for internal traffic, and a separate external load + balancer for external traffic." + properties: + additionalListeners: + description: AdditionalListeners sets the additional listeners + for the control plane load balancer. This is only applicable + to Network Load Balancer (NLB) types for the time being. + items: + description: AdditionalListenerSpec defines the desired + state of an additional listener on an AWS load balancer. + properties: + port: + description: Port sets the port for the additional + listener. + format: int64 + maximum: 65535 + minimum: 1 + type: integer + protocol: + default: TCP + description: Protocol sets the protocol for the + additional listener. Currently only TCP is supported. + enum: + - TCP + type: string + required: + - port + type: object + type: array + x-kubernetes-list-map-keys: + - port + x-kubernetes-list-type: map + additionalSecurityGroups: + description: AdditionalSecurityGroups sets the security + groups used by the load balancer. Expected to be security + group IDs This is optional - if not provided new security + groups will be created for the load balancer + items: + type: string + type: array + crossZoneLoadBalancing: + description: "CrossZoneLoadBalancing enables the classic + ELB cross availability zone balancing. \n With cross-zone + load balancing, each load balancer node for your Classic + Load Balancer distributes requests evenly across the + registered instances in all enabled Availability Zones. + If cross-zone load balancing is disabled, each load + balancer node distributes requests evenly across the + registered instances in its Availability Zone only. + \n Defaults to false." + type: boolean + disableHostsRewrite: + description: DisableHostsRewrite disabled the hair pinning + issue solution that adds the NLB's address as 127.0.0.1 + to the hosts file of each instance. This is by default, + false. + type: boolean + healthCheckProtocol: + description: HealthCheckProtocol sets the protocol type + for ELB health check target default value is ELBProtocolSSL + enum: + - TCP + - SSL + - HTTP + - HTTPS + - TLS + - UDP + type: string + ingressRules: + description: IngressRules sets the ingress rules for the + control plane load balancer. + items: + description: IngressRule defines an AWS ingress rule + for security groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access + from. Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information + about the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access + from. Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + protocol: + description: Protocol is the protocol for the ingress + rule. Accepted values are "-1" (all), "4" (IP + in IP),"tcp", "udp", "icmp", and "58" (ICMPv6), + "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access + from. Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: The security group role to allow access + from. Cannot be specified with CidrBlocks. The + field will be combined with source security group + IDs if specified. + items: + description: SecurityGroupRole defines the unique + role of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array + loadBalancerType: + default: classic + description: LoadBalancerType sets the type for a load + balancer. The default type is classic. + enum: + - classic + - elb + - alb + - nlb + type: string + name: + description: Name sets the name of the classic ELB load + balancer. As per AWS, the name must be unique within + your set of load balancers for the region, must have + a maximum of 32 characters, must contain only alphanumeric + characters or hyphens, and cannot begin or end with + a hyphen. Once set, the value cannot be changed. + maxLength: 32 + pattern: ^[A-Za-z0-9]([A-Za-z0-9]{0,31}|[-A-Za-z0-9]{0,30}[A-Za-z0-9])$ + type: string + preserveClientIP: + description: PreserveClientIP lets the user control if + preservation of client ips must be retained or not. + If this is enabled 6443 will be opened to 0.0.0.0/0. + type: boolean + scheme: + default: internet-facing + description: Scheme sets the scheme of the load balancer + (defaults to internet-facing) + enum: + - internet-facing + - internal + type: string + subnets: + description: Subnets sets the subnets that should be applied + to the control plane load balancer (defaults to discovered + subnets for managed VPCs or an empty set for unmanaged + VPCs) + items: + type: string + type: array + type: object sshKeyName: description: SSHKeyName is the name of the ssh key to attach to the bastion host. Valid values are empty string (do not diff --git a/controllers/awsmachine_controller.go b/controllers/awsmachine_controller.go index 20d3832c0d..32a0863cdb 100644 --- a/controllers/awsmachine_controller.go +++ b/controllers/awsmachine_controller.go @@ -32,6 +32,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "k8s.io/utils/ptr" @@ -331,7 +332,8 @@ func (r *AWSMachineReconciler) reconcileDelete(machineScope *scope.MachineScope, if err := r.reconcileLBAttachment(machineScope, elbScope, instance); err != nil { // We are tolerating AccessDenied error, so this won't block for users with older version of IAM; // all the other errors are blocking. - if !elb.IsAccessDenied(err) && !elb.IsNotFound(err) { + // Because we are reconciling all load balancers, attempt to treat the error as a list of errors. + if err = kerrors.FilterOut(err, elb.IsAccessDenied, elb.IsNotFound); err != nil { conditions.MarkFalse(machineScope.AWSMachine, infrav1.ELBAttachedCondition, "DeletingFailed", clusterv1.ConditionSeverityWarning, err.Error()) return ctrl.Result{}, errors.Errorf("failed to reconcile LB attachment: %+v", err) } @@ -853,6 +855,8 @@ func (r *AWSMachineReconciler) deleteIgnitionBootstrapDataFromS3(machineScope *s return nil } +// reconcileLBAttachment reconciles attachment to _all_ defined load balancers. +// Callers are expected to filter out known-good errors out of the aggregate error list. func (r *AWSMachineReconciler) reconcileLBAttachment(machineScope *scope.MachineScope, elbScope scope.ELBScope, i *infrav1.Instance) error { if !machineScope.IsControlPlane() { return nil @@ -860,18 +864,33 @@ func (r *AWSMachineReconciler) reconcileLBAttachment(machineScope *scope.Machine elbsvc := r.getELBService(elbScope) - // In order to prevent sending request to a "not-ready" control plane machines, it is required to remove the machine - // from the ELB as soon as the machine or infra machine gets deleted or when the machine is in a not running state. - if machineScope.AWSMachineIsDeleted() || machineScope.MachineIsDeleted() || !machineScope.InstanceIsRunning() { - if elbScope.ControlPlaneLoadBalancer().LoadBalancerType == infrav1.LoadBalancerTypeClassic { - machineScope.Debug("deregistering from classic load balancer") - return r.deregisterInstanceFromClassicLB(machineScope, elbsvc, i) + errs := []error{} + for _, lbSpec := range elbScope.ControlPlaneLoadBalancers() { + if lbSpec == nil { + continue + } + // In order to prevent sending request to a "not-ready" control plane machines, it is required to remove the machine + // from the ELB as soon as the machine or infra machine gets deleted or when the machine is in a not running state. + if machineScope.AWSMachineIsDeleted() || machineScope.MachineIsDeleted() || !machineScope.InstanceIsRunning() { + if lbSpec.LoadBalancerType == infrav1.LoadBalancerTypeClassic { + machineScope.Debug("deregistering from classic load balancer") + return r.deregisterInstanceFromClassicLB(machineScope, elbsvc, i) + } + machineScope.Debug("deregistering from v2 load balancer") + errs = append(errs, r.deregisterInstanceFromV2LB(machineScope, elbsvc, i, lbSpec)) + continue + } + + if err := r.registerInstanceToLBs(machineScope, elbsvc, i, lbSpec); err != nil { + errs = append(errs, errors.Wrapf(err, "could not register machine to load balancer")) } - machineScope.Debug("deregistering from v2 load balancer") - return r.deregisterInstanceFromV2LB(machineScope, elbsvc, i) } - switch elbScope.ControlPlaneLoadBalancer().LoadBalancerType { + return kerrors.NewAggregate(errs) +} + +func (r *AWSMachineReconciler) registerInstanceToLBs(machineScope *scope.MachineScope, elbsvc services.ELBInterface, i *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) error { + switch lb.LoadBalancerType { case infrav1.LoadBalancerTypeClassic: fallthrough case "": @@ -884,10 +903,10 @@ func (r *AWSMachineReconciler) reconcileLBAttachment(machineScope *scope.Machine fallthrough case infrav1.LoadBalancerTypeNLB: machineScope.Debug("registering to v2 load balancer") - return r.registerInstanceToV2LB(machineScope, elbsvc, i) + return r.registerInstanceToV2LB(machineScope, elbsvc, i, lb) } - return errors.Errorf("unknown load balancer type %q", elbScope.ControlPlaneLoadBalancer().LoadBalancerType) + return errors.Errorf("unknown load balancer type %q", lb.LoadBalancerType) } func (r *AWSMachineReconciler) registerInstanceToClassicLB(machineScope *scope.MachineScope, elbsvc services.ELBInterface, i *infrav1.Instance) error { @@ -914,8 +933,8 @@ func (r *AWSMachineReconciler) registerInstanceToClassicLB(machineScope *scope.M return nil } -func (r *AWSMachineReconciler) registerInstanceToV2LB(machineScope *scope.MachineScope, elbsvc services.ELBInterface, instance *infrav1.Instance) error { - _, registered, err := elbsvc.IsInstanceRegisteredWithAPIServerLB(instance) +func (r *AWSMachineReconciler) registerInstanceToV2LB(machineScope *scope.MachineScope, elbsvc services.ELBInterface, instance *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) error { + _, registered, err := elbsvc.IsInstanceRegisteredWithAPIServerLB(instance, lb) if err != nil { r.Recorder.Eventf(machineScope.AWSMachine, corev1.EventTypeWarning, "FailedAttachControlPlaneELB", "Failed to register control plane instance %q with load balancer: failed to determine registration status: %v", instance.ID, err) @@ -926,7 +945,7 @@ func (r *AWSMachineReconciler) registerInstanceToV2LB(machineScope *scope.Machin return nil } - if err := elbsvc.RegisterInstanceWithAPIServerLB(instance); err != nil { + if err := elbsvc.RegisterInstanceWithAPIServerLB(instance, lb); err != nil { r.Recorder.Eventf(machineScope.AWSMachine, corev1.EventTypeWarning, "FailedAttachControlPlaneELB", "Failed to register control plane instance %q with load balancer: %v", instance.ID, err) conditions.MarkFalse(machineScope.AWSMachine, infrav1.ELBAttachedCondition, infrav1.ELBAttachFailedReason, clusterv1.ConditionSeverityError, err.Error()) @@ -962,8 +981,8 @@ func (r *AWSMachineReconciler) deregisterInstanceFromClassicLB(machineScope *sco return nil } -func (r *AWSMachineReconciler) deregisterInstanceFromV2LB(machineScope *scope.MachineScope, elbsvc services.ELBInterface, i *infrav1.Instance) error { - targetGroupARNs, registered, err := elbsvc.IsInstanceRegisteredWithAPIServerLB(i) +func (r *AWSMachineReconciler) deregisterInstanceFromV2LB(machineScope *scope.MachineScope, elbsvc services.ELBInterface, i *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) error { + targetGroupARNs, registered, err := elbsvc.IsInstanceRegisteredWithAPIServerLB(i, lb) if err != nil { r.Recorder.Eventf(machineScope.AWSMachine, corev1.EventTypeWarning, "FailedDetachControlPlaneELB", "Failed to deregister control plane instance %q from load balancer: failed to determine registration status: %v", i.ID, err) diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go index e998ff06e5..f4511e9508 100644 --- a/controllers/helpers_test.go +++ b/controllers/helpers_test.go @@ -285,6 +285,10 @@ func mockedCreateLBV2Calls(t *testing.T, m *mocks.MockELBV2APIMockRecorder) { ResourceArns: []*string{lbArn}, TagKeys: []*string{aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster-apiserver")}, })).MaxTimes(1) + m.SetSecurityGroups(gomock.Eq(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: lbArn, + SecurityGroups: aws.StringSlice([]string{"sg-apiserver-lb"}), + })).MaxTimes(1) } func mockedDeleteLBCalls(expectV2Call bool, mv2 *mocks.MockELBV2APIMockRecorder, m *mocks.MockELBAPIMockRecorder) { diff --git a/docs/book/src/SUMMARY_PREFIX.md b/docs/book/src/SUMMARY_PREFIX.md index 765c63312b..55a3083769 100644 --- a/docs/book/src/SUMMARY_PREFIX.md +++ b/docs/book/src/SUMMARY_PREFIX.md @@ -38,3 +38,5 @@ - [Ignition support](./topics/ignition-support.md) - [External Resource Garbage Collection](./topics/external-resource-gc.md) - [Instance Metadata](./topics/instance-metadata.md) + - [Network Load Balancers](./topics/network-load-balancer-with-awscluster.md) + - [Secondary Control Plane Load Balancer](./topics/secondary-load-balancer.md) diff --git a/docs/book/src/crd/index.md b/docs/book/src/crd/index.md index 71dccd1323..9ee3ff5135 100644 --- a/docs/book/src/crd/index.md +++ b/docs/book/src/crd/index.md @@ -15653,6 +15653,22 @@ AWSLoadBalancerSpec
secondaryControlPlaneLoadBalancer
SecondaryControlPlaneLoadBalancer is an additional load balancer that can be used for the control plane.
+An example use case is to have a separate internal load balancer for internal traffic, +and a separate external load balancer for external traffic.
+imageLookupFormat
secondaryControlPlaneLoadBalancer
SecondaryControlPlaneLoadBalancer is an additional load balancer that can be used for the control plane.
+An example use case is to have a separate internal load balancer for internal traffic, +and a separate external load balancer for external traffic.
+imageLookupFormat
secondaryControlPlaneLoadBalancer
SecondaryControlPlaneLoadBalancer is an additional load balancer that can be used for the control plane.
+An example use case is to have a separate internal load balancer for internal traffic, +and a separate external load balancer for external traffic.
+imageLookupFormat
secondaryAPIServerELB
SecondaryAPIServerELB is the secondary Kubernetes api server load balancer.
+natGatewaysIPs
emptyRoutesDefaultVPCSecurityGroup
EmptyRoutesDefaultVPCSecurityGroup specifies whether the default VPC security group ingress +and egress rules should be removed.
+By default, when creating a VPC, AWS creates a security group called default
with ingress and egress
+rules that allow traffic from anywhere. The group could be used as a potential surface attack and
+it’s generally suggested that the group rules are removed or modified appropriately.
NOTE: This only applies when the VPC is managed by the Cluster API AWS controller.
++
ROSAMachinePool is the Schema for the rosamachinepools API.
+ +Field | +Description | +||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
+metadata + + +Kubernetes meta/v1.ObjectMeta + + + |
+
+Refer to the Kubernetes API documentation for the fields of the
+metadata field.
+ |
+||||||||||||||||
+spec + + +RosaMachinePoolSpec + + + |
+
+ + +
|
+||||||||||||||||
+status + + +RosaMachinePoolStatus + + + |
++ | +
@@ -24054,6 +24288,228 @@ during an instance refresh. The default is 90.
++(Appears on:RosaMachinePoolSpec) +
++
RosaMachinePoolAutoScaling specifies scaling options.
+ +Field | +Description | +
---|---|
+minReplicas + +int + + |
++ | +
+maxReplicas + +int + + |
++ | +
+(Appears on:ROSAMachinePool) +
++
RosaMachinePoolSpec defines the desired state of RosaMachinePool.
+ +Field | +Description | +
---|---|
+nodePoolName + +string + + |
+
+ NodePoolName specifies the name of the nodepool in Rosa +must be a valid DNS-1035 label, so it must consist of lower case alphanumeric and have a max length of 15 characters. + |
+
+availabilityZone + +string + + |
+
+(Optional)
+ AvailabilityZone is an optinal field specifying the availability zone where instances of this machine pool should run +For Multi-AZ clusters, you can create a machine pool in a Single-AZ of your choice. + |
+
+subnet + +string + + |
++(Optional) + | +
+labels + +map[string]string + + |
+
+(Optional)
+ Labels specifies labels for the Kubernetes node objects + |
+
+autoRepair + +bool + + |
+
+(Optional)
+ AutoRepair specifies whether health checks should be enabled for machines +in the NodePool. The default is false. + |
+
+instanceType + +string + + |
+
+ InstanceType specifies the AWS instance type + |
+
+autoscaling + + +RosaMachinePoolAutoScaling + + + |
+
+(Optional)
+ Autoscaling specifies auto scaling behaviour for this MachinePool. +required if Replicas is not configured + |
+
+providerIDList + +[]string + + |
+
+(Optional)
+ ProviderIDList contain a ProviderID for each machine instance that’s currently managed by this machine pool. + |
+
+(Appears on:ROSAMachinePool) +
++
RosaMachinePoolStatus defines the observed state of RosaMachinePool.
+ +Field | +Description | +
---|---|
+ready + +bool + + |
+
+ Ready denotes that the RosaMachinePool nodepool has joined +the cluster + |
+
+replicas + +int32 + + |
+
+(Optional)
+ Replicas is the most recently observed number of replicas. + |
+
+conditions + + +Cluster API api/v1beta1.Conditions + + + |
+
+(Optional)
+ Conditions defines current service state of the managed machine pool + |
+
+id + +string + + |
+
+ ID is the ID given by ROSA. + |
+
string
alias)diff --git a/docs/book/src/topics/network-load-balancer-with-awscluster.md b/docs/book/src/topics/network-load-balancer-with-awscluster.md index a309d822db..8b4de79983 100644 --- a/docs/book/src/topics/network-load-balancer-with-awscluster.md +++ b/docs/book/src/topics/network-load-balancer-with-awscluster.md @@ -50,17 +50,10 @@ spec: ## Security -NLBs cannot use Security Groups. Therefore, the following steps have been taken to increase security for nodes -communication. NLBs need access to the node in order to send traffic its way. A port has to be opened using an ip -address range instead of a security group as a _source_. There are two scenarios and CIDRs that can be enabled. +NLBs can use security groups, but only if one is associated at the time of creation. +CAPA will associate the default control plane security groups with a new NLB by default. -First, if client IP preservation is _disabled_ we only add the VPC's private CIDR range as allowed source for the API -server's port (usually 6443). This will work because then the NLB will use its dynamically allocated internal IP -address as source. - -Second, if client IP preservation is _enabled_ we MUST set `0.0.0.0/0` as communication source because then the -incoming IP address will be that of the client's that might not be in the current VPC. This shouldn't be too much of a -problem, but user's need to be aware of this restriction. +For more information, see AWS's [Network Load Balancer and Security Groups](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-security-groups.html) documentation. ## Extension of the code diff --git a/docs/book/src/topics/secondary-load-balancer.md b/docs/book/src/topics/secondary-load-balancer.md new file mode 100644 index 0000000000..2b2ea450a7 --- /dev/null +++ b/docs/book/src/topics/secondary-load-balancer.md @@ -0,0 +1,36 @@ +# Enabling a Secondary Control Plane Load Balancer + +## Overview + +It is possible to use a second control plane load balancer within a CAPA cluster. +This secondary control plane load balancer is primarily meant to be used for internal cluster traffic, for use cases where traffic between nodes and pods should be kept internal to the VPC network. +This adds a layer of privacy to traffic, as well as potentially saving on egress costs for traffic to the Kubernetes API server. + +A dual load balancer topology is not used as a default in order to maintain backward compatibility with existing CAPA clusters. + +## Requirements and defaults + +- A secondary control plane load balancer is _not_ created by default. +- The secondary control plane load balancer _must_ be a [Network Load Balancer](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/introduction.html), and will default to this type. +- The secondary control plane load balancer must also be provided a name. +- The secondary control plane's `Scheme` defaults to `internal`, and _must_ be different from the `spec.controlPlaneLoadBalancer`'s `Scheme`. + +The secondary load balancer will use the same Security Group information as the primary control plane load balancer. + +## Creating a secondary load balancer + +To create a secondary load balancer, add the `secondaryControlPlaneLoadBalancer` stanza to your `AWSCluster`. + +```yaml +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta2 +kind: AWSCluster +metadata: + name: test-aws-cluster +spec: + region: us-east-2 + sshKeyName: nrb-default + secondaryControlPlaneLoadBalancer: + name: internal-apiserver + scheme: internal # optional +``` diff --git a/docs/proposal/20220712-garbage-collection-delete.svg b/docs/proposal/20220712-garbage-collection-delete.svg index ae8cca8629..81e06de85e 100644 --- a/docs/proposal/20220712-garbage-collection-delete.svg +++ b/docs/proposal/20220712-garbage-collection-delete.svg @@ -1,4 +1,4 @@ - \ No newline at end of file diff --git a/docs/proposal/IPv6 Sequence Diagram.svg b/docs/proposal/IPv6 Sequence Diagram.svg new file mode 100644 index 0000000000..55e7c40fae --- /dev/null +++ b/docs/proposal/IPv6 Sequence Diagram.svg @@ -0,0 +1,53 @@ + \ No newline at end of file diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go index 592bd8c100..fd67eede86 100644 --- a/pkg/cloud/scope/cluster.go +++ b/pkg/cloud/scope/cluster.go @@ -183,6 +183,13 @@ func (s *ClusterScope) ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec { return s.AWSCluster.Spec.ControlPlaneLoadBalancer } +func (s *ClusterScope) ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec { + return []*infrav1.AWSLoadBalancerSpec{ + s.AWSCluster.Spec.ControlPlaneLoadBalancer, + s.AWSCluster.Spec.SecondaryControlPlaneLoadBalancer, + } +} + // ControlPlaneLoadBalancerScheme returns the Classic ELB scheme (public or internal facing). func (s *ClusterScope) ControlPlaneLoadBalancerScheme() infrav1.ELBScheme { if s.ControlPlaneLoadBalancer() != nil && s.ControlPlaneLoadBalancer().Scheme != nil { diff --git a/pkg/cloud/scope/elb.go b/pkg/cloud/scope/elb.go index 0a5c653775..53b3d6db99 100644 --- a/pkg/cloud/scope/elb.go +++ b/pkg/cloud/scope/elb.go @@ -39,6 +39,7 @@ type ELBScope interface { VPC() *infrav1.VPCSpec // ControlPlaneLoadBalancer returns the AWSLoadBalancerSpec + // Deprecated: Use ControlPlaneLoadBalancers() ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec // ControlPlaneLoadBalancerScheme returns the Classic ELB scheme (public or internal facing) @@ -49,4 +50,8 @@ type ELBScope interface { // ControlPlaneEndpoint returns AWSCluster control plane endpoint ControlPlaneEndpoint() clusterv1.APIEndpoint + + // ControlPlaneLoadBalancers returns both the ControlPlaneLoadBalancer and SecondaryControlPlaneLoadBalancer AWSLoadBalancerSpecs. + // The control plane load balancers should always be returned in the above order. + ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec } diff --git a/pkg/cloud/services/elb/loadbalancer.go b/pkg/cloud/services/elb/loadbalancer.go index a4a322324c..d962b7aefe 100644 --- a/pkg/cloud/services/elb/loadbalancer.go +++ b/pkg/cloud/services/elb/loadbalancer.go @@ -32,6 +32,7 @@ import ( rgapi "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" @@ -57,37 +58,45 @@ const maxELBsDescribeTagsRequest = 20 func (s *Service) ReconcileLoadbalancers() error { s.scope.Debug("Reconciling load balancers") - // do a switch and reconcile different load-balancer types - switch s.scope.ControlPlaneLoadBalancer().LoadBalancerType { - case infrav1.LoadBalancerTypeClassic: - return s.reconcileClassicLoadBalancer() - case infrav1.LoadBalancerTypeNLB, infrav1.LoadBalancerTypeALB, infrav1.LoadBalancerTypeELB: - return s.reconcileV2LB() - default: - return fmt.Errorf("unknown or unsupported load balancer type: %s", s.scope.ControlPlaneLoadBalancer().LoadBalancerType) + var errs []error + + for _, lbSpec := range s.scope.ControlPlaneLoadBalancers() { + if lbSpec == nil { + continue + } + switch lbSpec.LoadBalancerType { + case infrav1.LoadBalancerTypeClassic: + errs = append(errs, s.reconcileClassicLoadBalancer()) + case infrav1.LoadBalancerTypeNLB, infrav1.LoadBalancerTypeALB, infrav1.LoadBalancerTypeELB: + errs = append(errs, s.reconcileV2LB(lbSpec)) + default: + errs = append(errs, fmt.Errorf("unknown or unsupported load balancer type on primary load balancer: %s", lbSpec.LoadBalancerType)) + } } + + return kerrors.NewAggregate(errs) } // reconcileV2LB creates a load balancer. It also takes care of generating unique names across // namespaces by appending the namespace to the name. -func (s *Service) reconcileV2LB() error { - name, err := LBName(s.scope) +func (s *Service) reconcileV2LB(lbSpec *infrav1.AWSLoadBalancerSpec) error { + name, err := LBName(s.scope, lbSpec) if err != nil { return errors.Wrap(err, "failed to get control plane load balancer name") } // Get default api server spec. - spec, err := s.getAPIServerLBSpec(name) + spec, err := s.getAPIServerLBSpec(name, lbSpec) if err != nil { return err } - lb, err := s.describeLB(name) + lb, err := s.describeLB(name, lbSpec) switch { case IsNotFound(err) && s.scope.ControlPlaneEndpoint().IsValid(): // if elb is not found and owner cluster ControlPlaneEndpoint is already populated, then we should not recreate the elb. return errors.Wrapf(err, "no loadbalancer exists for the AWSCluster %s, the cluster has become unrecoverable and should be deleted manually", s.scope.InfraClusterName()) case IsNotFound(err): - lb, err = s.createLB(spec) + lb, err = s.createLB(spec, lbSpec) if err != nil { s.scope.Error(err, "failed to create LB") return err @@ -100,7 +109,7 @@ func (s *Service) reconcileV2LB() error { } // set up the type for later processing - lb.LoadBalancerType = s.scope.ControlPlaneLoadBalancer().LoadBalancerType + lb.LoadBalancerType = lbSpec.LoadBalancerType if lb.IsManaged(s.scope.Name()) { if !cmp.Equal(spec.ELBAttributes, lb.ELBAttributes) { if err := s.configureLBAttributes(lb.ARN, spec.ELBAttributes); err != nil { @@ -128,7 +137,7 @@ func (s *Service) reconcileV2LB() error { } // Reconcile the security groups from the spec and the ones currently attached to the load balancer - if s.scope.ControlPlaneLoadBalancer().LoadBalancerType != infrav1.LoadBalancerTypeNLB && !sets.NewString(lb.SecurityGroupIDs...).Equal(sets.NewString(spec.SecurityGroupIDs...)) { + if !sets.NewString(lb.SecurityGroupIDs...).Equal(sets.NewString(spec.SecurityGroupIDs...)) { _, err := s.ELBV2Client.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ LoadBalancerArn: &lb.ARN, SecurityGroups: aws.StringSlice(spec.SecurityGroupIDs), @@ -140,21 +149,32 @@ func (s *Service) reconcileV2LB() error { } else { s.scope.Trace("Unmanaged control plane load balancer, skipping load balancer configuration", "api-server-elb", lb) } - lb.DeepCopyInto(&s.scope.Network().APIServerELB) + + if s.scope.ControlPlaneLoadBalancers()[1] != nil && lb.Name == *s.scope.ControlPlaneLoadBalancers()[1].Name { + lb.DeepCopyInto(&s.scope.Network().SecondaryAPIServerELB) + } else { + lb.DeepCopyInto(&s.scope.Network().APIServerELB) + } + return nil } -func (s *Service) getAPIServerLBSpec(elbName string) (*infrav1.LoadBalancer, error) { +func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBalancerSpec) (*infrav1.LoadBalancer, error) { var securityGroupIDs []string - controlPlaneLoadBalancer := s.scope.ControlPlaneLoadBalancer() - if controlPlaneLoadBalancer != nil && controlPlaneLoadBalancer.LoadBalancerType != infrav1.LoadBalancerTypeNLB { - securityGroupIDs = append(securityGroupIDs, controlPlaneLoadBalancer.AdditionalSecurityGroups...) + if lbSpec != nil { + securityGroupIDs = append(securityGroupIDs, lbSpec.AdditionalSecurityGroups...) securityGroupIDs = append(securityGroupIDs, s.scope.SecurityGroups()[infrav1.SecurityGroupAPIServerLB].ID) } + // Since we're no longer relying on s.scope.ControlPlaneLoadBalancerScheme to do the defaulting for us, do it here. + scheme := infrav1.ELBSchemeInternetFacing + if lbSpec != nil && lbSpec.Scheme != nil { + scheme = *lbSpec.Scheme + } + res := &infrav1.LoadBalancer{ Name: elbName, - Scheme: s.scope.ControlPlaneLoadBalancerScheme(), + Scheme: scheme, ELBAttributes: make(map[string]*string), ELBListeners: []infrav1.Listener{ { @@ -175,8 +195,8 @@ func (s *Service) getAPIServerLBSpec(elbName string) (*infrav1.LoadBalancer, err SecurityGroupIDs: securityGroupIDs, } - if s.scope.ControlPlaneLoadBalancer() != nil { - for _, additionalListeners := range controlPlaneLoadBalancer.AdditionalListeners { + if lbSpec != nil { + for _, additionalListeners := range lbSpec.AdditionalListeners { res.ELBListeners = append(res.ELBListeners, infrav1.Listener{ Protocol: additionalListeners.Protocol, Port: additionalListeners.Port, @@ -194,12 +214,12 @@ func (s *Service) getAPIServerLBSpec(elbName string) (*infrav1.LoadBalancer, err } } - if s.scope.ControlPlaneLoadBalancer() != nil && s.scope.ControlPlaneLoadBalancer().LoadBalancerType != infrav1.LoadBalancerTypeNLB { + if lbSpec != nil && lbSpec.LoadBalancerType != infrav1.LoadBalancerTypeNLB { res.ELBAttributes[infrav1.LoadBalancerAttributeIdleTimeTimeoutSeconds] = aws.String(infrav1.LoadBalancerAttributeIdleTimeDefaultTimeoutSecondsInSeconds) } - if s.scope.ControlPlaneLoadBalancer() != nil { - isCrossZoneLB := s.scope.ControlPlaneLoadBalancer().CrossZoneLoadBalancing + if lbSpec != nil { + isCrossZoneLB := lbSpec.CrossZoneLoadBalancing res.ELBAttributes[infrav1.LoadBalancerAttributeEnableLoadBalancingCrossZone] = aws.String(strconv.FormatBool(isCrossZoneLB)) } @@ -212,11 +232,11 @@ func (s *Service) getAPIServerLBSpec(elbName string) (*infrav1.LoadBalancer, err }) // If subnet IDs have been specified for this load balancer - if s.scope.ControlPlaneLoadBalancer() != nil && len(s.scope.ControlPlaneLoadBalancer().Subnets) > 0 { + if lbSpec != nil && len(lbSpec.Subnets) > 0 { // This set of subnets may not match the subnets specified on the Cluster, so we may not have already discovered them // We need to call out to AWS to describe them just in case input := &ec2.DescribeSubnetsInput{ - SubnetIds: aws.StringSlice(s.scope.ControlPlaneLoadBalancer().Subnets), + SubnetIds: aws.StringSlice(lbSpec.Subnets), } out, err := s.EC2Client.DescribeSubnetsWithContext(context.TODO(), input) if err != nil { @@ -251,9 +271,9 @@ func (s *Service) getAPIServerLBSpec(elbName string) (*infrav1.LoadBalancer, err return res, nil } -func (s *Service) createLB(spec *infrav1.LoadBalancer) (*infrav1.LoadBalancer, error) { +func (s *Service) createLB(spec *infrav1.LoadBalancer, lbSpec *infrav1.AWSLoadBalancerSpec) (*infrav1.LoadBalancer, error) { var t *string - switch s.scope.ControlPlaneLoadBalancer().LoadBalancerType { + switch lbSpec.LoadBalancerType { case infrav1.LoadBalancerTypeNLB: t = aws.String(elbv2.LoadBalancerTypeEnumNetwork) case infrav1.LoadBalancerTypeALB: @@ -262,14 +282,12 @@ func (s *Service) createLB(spec *infrav1.LoadBalancer) (*infrav1.LoadBalancer, e t = aws.String(elbv2.LoadBalancerTypeEnumGateway) } input := &elbv2.CreateLoadBalancerInput{ - Name: aws.String(spec.Name), - Subnets: aws.StringSlice(spec.SubnetIDs), - Tags: converters.MapToV2Tags(spec.Tags), - Scheme: aws.String(string(spec.Scheme)), - Type: t, - } - if s.scope.ControlPlaneLoadBalancer().LoadBalancerType != infrav1.LoadBalancerTypeNLB { - input.SecurityGroups = aws.StringSlice(spec.SecurityGroupIDs) + Name: aws.String(spec.Name), + Subnets: aws.StringSlice(spec.SubnetIDs), + Tags: converters.MapToV2Tags(spec.Tags), + Scheme: aws.String(string(spec.Scheme)), + SecurityGroups: aws.StringSlice(spec.SecurityGroupIDs), + Type: t, } if s.scope.VPC().IsIPv6Enabled() { @@ -313,7 +331,7 @@ func (s *Service) createLB(spec *infrav1.LoadBalancer) (*infrav1.LoadBalancer, e return nil, errors.New("no target group was created; the returned list is empty") } - if !s.scope.ControlPlaneLoadBalancer().PreserveClientIP { + if !lbSpec.PreserveClientIP { targetGroupAttributeInput := &elbv2.ModifyTargetGroupAttributesInput{ TargetGroupArn: group.TargetGroups[0].TargetGroupArn, Attributes: []*elbv2.TargetGroupAttribute{ @@ -358,7 +376,7 @@ func (s *Service) createLB(spec *infrav1.LoadBalancer) (*infrav1.LoadBalancer, e return res, nil } -func (s *Service) describeLB(name string) (*infrav1.LoadBalancer, error) { +func (s *Service) describeLB(name string, lbSpec *infrav1.AWSLoadBalancerSpec) (*infrav1.LoadBalancer, error) { input := &elbv2.DescribeLoadBalancersInput{ Names: aws.StringSlice([]string{name}), } @@ -383,15 +401,17 @@ func (s *Service) describeLB(name string) (*infrav1.LoadBalancer, error) { return nil, NewNotFound(fmt.Sprintf("no load balancer found with name %q", name)) } + // Direct usage of indices here is alright because the query to AWS is providing exactly one name, + // and the name uniqueness constraints prevent us from getting more than one entry back. if s.scope.VPC().ID != "" && s.scope.VPC().ID != *out.LoadBalancers[0].VpcId { return nil, errors.Errorf( "Load balancer names must be unique within a region: %q load balancer already exists in this region in VPC %q", name, *out.LoadBalancers[0].VpcId) } - if s.scope.ControlPlaneLoadBalancer() != nil && - s.scope.ControlPlaneLoadBalancer().Scheme != nil && - string(*s.scope.ControlPlaneLoadBalancer().Scheme) != aws.StringValue(out.LoadBalancers[0].Scheme) { + if lbSpec != nil && + lbSpec.Scheme != nil && + string(*lbSpec.Scheme) != aws.StringValue(out.LoadBalancers[0].Scheme) { return nil, errors.Errorf( "Load balancer names must be unique within a region: %q Load balancer already exists in this region with a different scheme %q", name, *out.LoadBalancers[0].Scheme) @@ -594,7 +614,20 @@ func (s *Service) DeleteLoadbalancers() error { } func (s *Service) deleteExistingNLBs() error { - name, err := LBName(s.scope) + errs := make([]error, 0) + + for _, lbSpec := range s.scope.ControlPlaneLoadBalancers() { + if lbSpec == nil { + continue + } + errs = append(errs, s.deleteExistingNLB(lbSpec)) + } + + return kerrors.NewAggregate(errs) +} + +func (s *Service) deleteExistingNLB(lbSpec *infrav1.AWSLoadBalancerSpec) error { + name, err := LBName(s.scope, lbSpec) if err != nil { return errors.Wrap(err, "failed to get control plane load balancer name") } @@ -603,7 +636,7 @@ func (s *Service) deleteExistingNLBs() error { return err } - lb, err := s.describeLB(name) + lb, err := s.describeLB(name, lbSpec) if IsNotFound(err) { return nil } @@ -622,7 +655,7 @@ func (s *Service) deleteExistingNLBs() error { } if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (done bool, err error) { - _, err = s.describeLB(name) + _, err = s.describeLB(name, lbSpec) done = IsNotFound(err) return done, nil }); err != nil { @@ -664,8 +697,8 @@ func (s *Service) IsInstanceRegisteredWithAPIServerELB(i *infrav1.Instance) (boo } // IsInstanceRegisteredWithAPIServerLB returns true if the instance is already registered with the APIServer LB. -func (s *Service) IsInstanceRegisteredWithAPIServerLB(i *infrav1.Instance) ([]string, bool, error) { - name, err := LBName(s.scope) +func (s *Service) IsInstanceRegisteredWithAPIServerLB(i *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) ([]string, bool, error) { + name, err := LBName(s.scope, lb) if err != nil { return nil, false, errors.Wrap(err, "failed to get control plane load balancer name") } @@ -762,12 +795,12 @@ func (s *Service) RegisterInstanceWithAPIServerELB(i *infrav1.Instance) error { } // RegisterInstanceWithAPIServerLB registers an instance with a LB. -func (s *Service) RegisterInstanceWithAPIServerLB(instance *infrav1.Instance) error { - name, err := LBName(s.scope) +func (s *Service) RegisterInstanceWithAPIServerLB(instance *infrav1.Instance, lbSpec *infrav1.AWSLoadBalancerSpec) error { + name, err := LBName(s.scope, lbSpec) if err != nil { return errors.Wrap(err, "failed to get control plane load balancer name") } - out, err := s.describeLB(name) + out, err := s.describeLB(name, lbSpec) if err != nil { return err } @@ -883,6 +916,7 @@ func (s *Service) DeregisterInstanceFromAPIServerLB(targetGroupArn string, i *in // ELBName returns the user-defined API Server ELB name, or a generated default if the user has not defined the ELB // name. +// This is only for the primary load balancer. func ELBName(s scope.ELBScope) (string, error) { if userDefinedName := s.ControlPlaneLoadBalancerName(); userDefinedName != nil { return *userDefinedName, nil @@ -894,11 +928,12 @@ func ELBName(s scope.ELBScope) (string, error) { return name, nil } -// LBName returns the user-defined API Server ELB name, or a generated default if the user has not defined the ELB +// LBName returns the user-defined API Server LB name, or a generated default if the user has not defined the LB // name. -func LBName(s scope.ELBScope) (string, error) { - if userDefinedName := s.ControlPlaneLoadBalancerName(); userDefinedName != nil { - return *userDefinedName, nil +// This is used for both the primary and secondary load balancers. +func LBName(s scope.ELBScope, lbSpec *infrav1.AWSLoadBalancerSpec) (string, error) { + if lbSpec != nil && lbSpec.Name != nil { + return *lbSpec.Name, nil } name, err := GenerateELBName(fmt.Sprintf("%s-%s", s.Namespace(), s.Name())) if err != nil { diff --git a/pkg/cloud/services/elb/loadbalancer_test.go b/pkg/cloud/services/elb/loadbalancer_test.go index 47531b4adf..c1f91613c9 100644 --- a/pkg/cloud/services/elb/loadbalancer_test.go +++ b/pkg/cloud/services/elb/loadbalancer_test.go @@ -379,20 +379,6 @@ func TestGetAPIServerV2ELBSpecControlPlaneLoadBalancer(t *testing.T) { } }, }, - { - name: "load balancer config with additional security groups specified for NLB has no security groups", - lb: &infrav1.AWSLoadBalancerSpec{ - AdditionalSecurityGroups: []string{"sg-00001", "sg-00002"}, - LoadBalancerType: infrav1.LoadBalancerTypeNLB, - }, - mocks: func(m *mocks.MockEC2APIMockRecorder) {}, - expect: func(t *testing.T, g *WithT, res *infrav1.LoadBalancer) { - t.Helper() - if len(res.SecurityGroupIDs) != 0 { - t.Errorf("Expected load balancer to be configured for 0 security groups, got %v", len(res.SecurityGroupIDs)) - } - }, - }, { name: "A base listener is set up for NLB", lb: &infrav1.AWSLoadBalancerSpec{ @@ -463,7 +449,7 @@ func TestGetAPIServerV2ELBSpecControlPlaneLoadBalancer(t *testing.T) { EC2Client: ec2Mock, } - spec, err := s.getAPIServerLBSpec(clusterScope.Name()) + spec, err := s.getAPIServerLBSpec(clusterScope.Name(), clusterScope.ControlPlaneLoadBalancer()) if err != nil { t.Fatal(err) } @@ -1147,7 +1133,7 @@ func TestRegisterInstanceWithAPIServerNLB(t *testing.T) { ELBV2Client: elbV2APIMocks, } - err = s.RegisterInstanceWithAPIServerLB(instance) + err = s.RegisterInstanceWithAPIServerLB(instance, clusterScope.ControlPlaneLoadBalancer()) tc.check(t, err) }) } @@ -1181,10 +1167,11 @@ func TestCreateNLB(t *testing.T) { }, elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ - Name: aws.String(elbName), - Scheme: aws.String("internet-facing"), - Type: aws.String("network"), - Subnets: aws.StringSlice([]string{clusterSubnetID}), + Name: aws.String(elbName), + Scheme: aws.String("internet-facing"), + SecurityGroups: []*string{}, + Type: aws.String("network"), + Subnets: aws.StringSlice([]string{clusterSubnetID}), Tags: []*elbv2.Tag{ { Key: aws.String("test"), @@ -1281,11 +1268,12 @@ func TestCreateNLB(t *testing.T) { }, elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ - Name: aws.String(elbName), - IpAddressType: aws.String("dualstack"), - Scheme: aws.String("internet-facing"), - Type: aws.String("network"), - Subnets: aws.StringSlice([]string{clusterSubnetID}), + Name: aws.String(elbName), + IpAddressType: aws.String("dualstack"), + Scheme: aws.String("internet-facing"), + SecurityGroups: aws.StringSlice([]string{}), + Type: aws.String("network"), + Subnets: aws.StringSlice([]string{clusterSubnetID}), Tags: []*elbv2.Tag{ { Key: aws.String("test"), @@ -1379,10 +1367,11 @@ func TestCreateNLB(t *testing.T) { }, elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ - Name: aws.String(elbName), - Scheme: aws.String("internet-facing"), - Type: aws.String("network"), - Subnets: aws.StringSlice([]string{clusterSubnetID}), + Name: aws.String(elbName), + Scheme: aws.String("internet-facing"), + SecurityGroups: []*string{}, + Type: aws.String("network"), + Subnets: aws.StringSlice([]string{clusterSubnetID}), Tags: []*elbv2.Tag{ { Key: aws.String("test"), @@ -1423,10 +1412,11 @@ func TestCreateNLB(t *testing.T) { }, elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ - Name: aws.String(elbName), - Scheme: aws.String("internet-facing"), - Type: aws.String("network"), - Subnets: aws.StringSlice([]string{clusterSubnetID}), + Name: aws.String(elbName), + Scheme: aws.String("internet-facing"), + SecurityGroups: aws.StringSlice([]string{}), + Type: aws.String("network"), + Subnets: aws.StringSlice([]string{clusterSubnetID}), Tags: []*elbv2.Tag{ { Key: aws.String("test"), @@ -1517,10 +1507,11 @@ func TestCreateNLB(t *testing.T) { }, elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ - Name: aws.String(elbName), - Scheme: aws.String("internet-facing"), - Type: aws.String("network"), - Subnets: aws.StringSlice([]string{clusterSubnetID}), + Name: aws.String(elbName), + Scheme: aws.String("internet-facing"), + SecurityGroups: aws.StringSlice([]string{}), + Type: aws.String("network"), + Subnets: aws.StringSlice([]string{clusterSubnetID}), Tags: []*elbv2.Tag{ { Key: aws.String("test"), @@ -1770,7 +1761,7 @@ func TestCreateNLB(t *testing.T) { } spec := tc.spec(*loadBalancerSpec) - lb, err := s.createLB(&spec) + lb, err := s.createLB(&spec, clusterScope.ControlPlaneLoadBalancer()) tc.check(t, lb, err) }) } @@ -1904,13 +1895,199 @@ func TestReconcileV2LB(t *testing.T) { scope: clusterScope, ELBV2Client: elbV2APIMocks, } - err = s.reconcileV2LB() + err = s.reconcileV2LB(clusterScope.ControlPlaneLoadBalancer()) lb := s.scope.Network().APIServerELB tc.check(t, &lb, err) }) } } +func TestReconcileLoadbalancers(t *testing.T) { + const ( + namespace = "foo" + clusterName = "bar" + clusterSubnetID = "subnet-1" + elbName = "bar-apiserver" + elbArn = "arn::apiserver" + secondElbName = "bar-apiserver2" + secondElbArn = "arn::apiserver2" + vpcID = "vpc-id" + az = "us-west-1a" + ) + + tests := []struct { + name string + elbV2APIMocks func(m *mocks.MockELBV2APIMockRecorder) + check func(t *testing.T, firstLB, secondLB *infrav1.LoadBalancer, err error) + awsCluster func(acl infrav1.AWSCluster) infrav1.AWSCluster + spec func(spec infrav1.LoadBalancer) infrav1.LoadBalancer + }{ + { + name: "ensure two load balancers are reconciled", + awsCluster: func(acl infrav1.AWSCluster) infrav1.AWSCluster { + acl.Spec.ControlPlaneLoadBalancer.Name = aws.String(elbName) + acl.Spec.SecondaryControlPlaneLoadBalancer = &infrav1.AWSLoadBalancerSpec{ + Name: aws.String(secondElbName), + Scheme: &infrav1.ELBSchemeInternal, + LoadBalancerType: infrav1.LoadBalancerTypeNLB, + } + return acl + }, + elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { + m.DescribeLoadBalancers(gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + Names: aws.StringSlice([]string{elbName}), + })). + Return(&elbv2.DescribeLoadBalancersOutput{ + LoadBalancers: []*elbv2.LoadBalancer{ + { + LoadBalancerArn: aws.String(elbArn), + LoadBalancerName: aws.String(elbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternetFacing)), + AvailabilityZones: []*elbv2.AvailabilityZone{ + { + SubnetId: aws.String(clusterSubnetID), + ZoneName: aws.String(az), + }, + }, + VpcId: aws.String(vpcID), + }, + }, + }, nil) + m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(elbArn)}).Return( + &elbv2.DescribeLoadBalancerAttributesOutput{ + Attributes: []*elbv2.LoadBalancerAttribute{ + { + Key: aws.String("load_balancing.cross_zone.enabled"), + Value: aws.String("false"), + }, + }, + }, + nil, + ) + m.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: []*string{aws.String(elbArn)}}).Return( + &elbv2.DescribeTagsOutput{ + TagDescriptions: []*elbv2.TagDescription{ + { + ResourceArn: aws.String(elbArn), + Tags: []*elbv2.Tag{}, + }, + }, + }, + nil, + ) + + m.DescribeLoadBalancers(gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + Names: aws.StringSlice([]string{secondElbName}), + })). + Return(&elbv2.DescribeLoadBalancersOutput{ + LoadBalancers: []*elbv2.LoadBalancer{ + { + LoadBalancerArn: aws.String(secondElbArn), + LoadBalancerName: aws.String(secondElbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternal)), + AvailabilityZones: []*elbv2.AvailabilityZone{ + { + SubnetId: aws.String(clusterSubnetID), + ZoneName: aws.String(az), + }, + }, + VpcId: aws.String(vpcID), + }, + }, + }, nil) + m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(secondElbArn)}).Return( + &elbv2.DescribeLoadBalancerAttributesOutput{ + Attributes: []*elbv2.LoadBalancerAttribute{ + { + Key: aws.String("load_balancing.cross_zone.enabled"), + Value: aws.String("false"), + }, + }, + }, + nil, + ) + m.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: []*string{aws.String(secondElbArn)}}).Return( + &elbv2.DescribeTagsOutput{ + TagDescriptions: []*elbv2.TagDescription{ + { + ResourceArn: aws.String(secondElbArn), + Tags: []*elbv2.Tag{}, + }, + }, + }, + nil, + ) + }, + check: func(t *testing.T, firstLB *infrav1.LoadBalancer, secondLB *infrav1.LoadBalancer, err error) { + t.Helper() + if err != nil { + t.Fatalf("did not expect error: %v", err) + } + + if len(firstLB.AvailabilityZones) != 1 { + t.Errorf("Expected first LB to contain 1 availability zone, got %v", len(firstLB.AvailabilityZones)) + } + if secondLB == nil { + t.Errorf("Expected second LB to be populated, was nil") + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + elbV2APIMocks := mocks.NewMockELBV2API(mockCtrl) + + scheme, err := setupScheme() + if err != nil { + t.Fatal(err) + } + awsCluster := &infrav1.AWSCluster{ + ObjectMeta: metav1.ObjectMeta{Name: clusterName}, + Spec: infrav1.AWSClusterSpec{ + ControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{ + Name: aws.String(elbName), + LoadBalancerType: infrav1.LoadBalancerTypeNLB, + }, + NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: vpcID, + }, + }, + }, + } + client := fake.NewClientBuilder().WithScheme(scheme).Build() + cluster := tc.awsCluster(*awsCluster) + clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: clusterName, + }, + }, + AWSCluster: &cluster, + }) + if err != nil { + t.Fatal(err) + } + + tc.elbV2APIMocks(elbV2APIMocks.EXPECT()) + + s := &Service{ + scope: clusterScope, + ELBV2Client: elbV2APIMocks, + } + err = s.ReconcileLoadbalancers() + firstLB := s.scope.Network().APIServerELB + secondLB := s.scope.Network().SecondaryAPIServerELB + tc.check(t, &firstLB, &secondLB, err) + }) + } +} + func TestDeleteAPIServerELB(t *testing.T) { clusterName := "bar" //nolint:goconst // does not need to be a package-level const elbName := "bar-apiserver" @@ -2600,7 +2777,7 @@ func TestDescribeV2Loadbalancers(t *testing.T) { ELBV2Client: elbV2ApiMock, } - _, err = s.describeLB(tc.lbName) + _, err = s.describeLB(tc.lbName, clusterScope.ControlPlaneLoadBalancer()) if err == nil { t.Fatal(err) } diff --git a/pkg/cloud/services/interfaces.go b/pkg/cloud/services/interfaces.go index df6b4c179e..116fd02be6 100644 --- a/pkg/cloud/services/interfaces.go +++ b/pkg/cloud/services/interfaces.go @@ -95,11 +95,11 @@ type ELBInterface interface { DeleteLoadbalancers() error ReconcileLoadbalancers() error IsInstanceRegisteredWithAPIServerELB(i *infrav1.Instance) (bool, error) - IsInstanceRegisteredWithAPIServerLB(i *infrav1.Instance) ([]string, bool, error) + IsInstanceRegisteredWithAPIServerLB(i *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) ([]string, bool, error) DeregisterInstanceFromAPIServerELB(i *infrav1.Instance) error DeregisterInstanceFromAPIServerLB(targetGroupArn string, i *infrav1.Instance) error RegisterInstanceWithAPIServerELB(i *infrav1.Instance) error - RegisterInstanceWithAPIServerLB(i *infrav1.Instance) error + RegisterInstanceWithAPIServerLB(i *infrav1.Instance, lb *infrav1.AWSLoadBalancerSpec) error } // NetworkInterface encapsulates the methods exposed to the cluster diff --git a/pkg/cloud/services/mock_services/elb_interface_mock.go b/pkg/cloud/services/mock_services/elb_interface_mock.go index 121d3e1b73..0af85fb047 100644 --- a/pkg/cloud/services/mock_services/elb_interface_mock.go +++ b/pkg/cloud/services/mock_services/elb_interface_mock.go @@ -108,9 +108,9 @@ func (mr *MockELBInterfaceMockRecorder) IsInstanceRegisteredWithAPIServerELB(arg } // IsInstanceRegisteredWithAPIServerLB mocks base method. -func (m *MockELBInterface) IsInstanceRegisteredWithAPIServerLB(arg0 *v1beta2.Instance) ([]string, bool, error) { +func (m *MockELBInterface) IsInstanceRegisteredWithAPIServerLB(arg0 *v1beta2.Instance, arg1 *v1beta2.AWSLoadBalancerSpec) ([]string, bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsInstanceRegisteredWithAPIServerLB", arg0) + ret := m.ctrl.Call(m, "IsInstanceRegisteredWithAPIServerLB", arg0, arg1) ret0, _ := ret[0].([]string) ret1, _ := ret[1].(bool) ret2, _ := ret[2].(error) @@ -118,9 +118,9 @@ func (m *MockELBInterface) IsInstanceRegisteredWithAPIServerLB(arg0 *v1beta2.Ins } // IsInstanceRegisteredWithAPIServerLB indicates an expected call of IsInstanceRegisteredWithAPIServerLB. -func (mr *MockELBInterfaceMockRecorder) IsInstanceRegisteredWithAPIServerLB(arg0 interface{}) *gomock.Call { +func (mr *MockELBInterfaceMockRecorder) IsInstanceRegisteredWithAPIServerLB(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsInstanceRegisteredWithAPIServerLB", reflect.TypeOf((*MockELBInterface)(nil).IsInstanceRegisteredWithAPIServerLB), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsInstanceRegisteredWithAPIServerLB", reflect.TypeOf((*MockELBInterface)(nil).IsInstanceRegisteredWithAPIServerLB), arg0, arg1) } // ReconcileLoadbalancers mocks base method. @@ -152,15 +152,15 @@ func (mr *MockELBInterfaceMockRecorder) RegisterInstanceWithAPIServerELB(arg0 in } // RegisterInstanceWithAPIServerLB mocks base method. -func (m *MockELBInterface) RegisterInstanceWithAPIServerLB(arg0 *v1beta2.Instance) error { +func (m *MockELBInterface) RegisterInstanceWithAPIServerLB(arg0 *v1beta2.Instance, arg1 *v1beta2.AWSLoadBalancerSpec) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RegisterInstanceWithAPIServerLB", arg0) + ret := m.ctrl.Call(m, "RegisterInstanceWithAPIServerLB", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } // RegisterInstanceWithAPIServerLB indicates an expected call of RegisterInstanceWithAPIServerLB. -func (mr *MockELBInterfaceMockRecorder) RegisterInstanceWithAPIServerLB(arg0 interface{}) *gomock.Call { +func (mr *MockELBInterfaceMockRecorder) RegisterInstanceWithAPIServerLB(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterInstanceWithAPIServerLB", reflect.TypeOf((*MockELBInterface)(nil).RegisterInstanceWithAPIServerLB), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterInstanceWithAPIServerLB", reflect.TypeOf((*MockELBInterface)(nil).RegisterInstanceWithAPIServerLB), arg0, arg1) }