Support a secondary control plane load balancer

* A secondary control plane load balancer can be added. * It will be a network load balancer. * Its scheme must not match that of the ControlPlaneLoadBalancer. Previously, NLBs could not have security groups attached. This has now changed, and an NLB can have a security group attached at creation. If a security group is _not_ present at creation, then the NLB can never have security groups added.
kubernetes-sigs · Jan 24, 2024 · c72f25d · c72f25d
1 parent 0f0e9b0
commit c72f25d
Show file tree

Hide file tree

Showing 24 changed files with 2,052 additions and 187 deletions.
diff --git 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
diff --git 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
@@ -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
@@ -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
@@ -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