Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add support for a secondary control plane load balancer #4733

Merged
merged 1 commit into from
Feb 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions api/v1beta1/awscluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions api/v1beta2/awscluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
136 changes: 84 additions & 52 deletions api/v1beta2/awscluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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{}) &&
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
nrb marked this conversation as resolved.
Show resolved Hide resolved
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"))
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions api/v1beta2/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions api/v1beta2/network_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would be good to mark it optional


// NatGatewaysIPs contains the public IPs of the NAT Gateways
NatGatewaysIPs []string `json:"natGatewaysIPs,omitempty"`
}
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading