🐛 Revert "Supporting externally managed Control Plane"

api/v1beta2/awscluster_types.go

@@ -169,11 +169,10 @@ type Bastion struct {
 type LoadBalancerType string
 
 var (
-	LoadBalancerTypeClassic  = LoadBalancerType("classic")
-	LoadBalancerTypeELB      = LoadBalancerType("elb")
-	LoadBalancerTypeALB      = LoadBalancerType("alb")
-	LoadBalancerTypeNLB      = LoadBalancerType("nlb")
-	LoadBalancerTypeDisabled = LoadBalancerType("disabled")
+	LoadBalancerTypeClassic = LoadBalancerType("classic")
+	LoadBalancerTypeELB     = LoadBalancerType("elb")
+	LoadBalancerTypeALB     = LoadBalancerType("alb")
+	LoadBalancerTypeNLB     = LoadBalancerType("nlb")
 )
 
 // AWSLoadBalancerSpec defines the desired state of an AWS load balancer.
@@ -232,7 +231,7 @@ type AWSLoadBalancerSpec struct {
 
 	// LoadBalancerType sets the type for a load balancer. The default type is classic.
 	// +kubebuilder:default=classic
-	// +kubebuilder:validation:Enum:=classic;elb;alb;nlb;disabled
+	// +kubebuilder:validation:Enum:=classic;elb;alb;nlb
 	LoadBalancerType LoadBalancerType `json:"loadBalancerType,omitempty"`
 
 	// DisableHostsRewrite disabled the hair pinning issue solution that adds the NLB's address as 127.0.0.1 to the hosts

api/v1beta2/awscluster_webhook.go

@@ -298,49 +298,5 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
 		}
 	}
 
-	if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeDisabled {
-		if r.Spec.ControlPlaneLoadBalancer.Name != nil {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "name"), r.Spec.ControlPlaneLoadBalancer.Name, "cannot configure a name if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if r.Spec.ControlPlaneLoadBalancer.CrossZoneLoadBalancing {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "crossZoneLoadBalancing"), r.Spec.ControlPlaneLoadBalancer.CrossZoneLoadBalancing, "cross-zone load balancing cannot be set if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if len(r.Spec.ControlPlaneLoadBalancer.Subnets) > 0 {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "subnets"), r.Spec.ControlPlaneLoadBalancer.Subnets, "subnets cannot be set if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol != nil {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthCheckProtocol"), r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol, "healthcheck protocol cannot be set if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if len(r.Spec.ControlPlaneLoadBalancer.AdditionalSecurityGroups) > 0 {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "additionalSecurityGroups"), r.Spec.ControlPlaneLoadBalancer.AdditionalSecurityGroups, "additional Security Groups cannot be set if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if len(r.Spec.ControlPlaneLoadBalancer.AdditionalListeners) > 0 {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "additionalListeners"), r.Spec.ControlPlaneLoadBalancer.AdditionalListeners, "cannot set additional listeners if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if len(r.Spec.ControlPlaneLoadBalancer.IngressRules) > 0 {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "ingress rules cannot be set if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if r.Spec.ControlPlaneLoadBalancer.PreserveClientIP {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "preserveClientIP"), r.Spec.ControlPlaneLoadBalancer.PreserveClientIP, "cannot preserve client IP if the LoadBalancer reconciliation is disabled"))
-		}
-
-		if r.Spec.ControlPlaneLoadBalancer.DisableHostsRewrite {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "disableHostsRewrite"), r.Spec.ControlPlaneLoadBalancer.DisableHostsRewrite, "cannot disable hosts rewrite if the LoadBalancer reconciliation is disabled"))
-		}
-	}
-
-	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"))
-		}
-	}
-
 	return allErrs
 }

api/v1beta2/awscluster_webhook_test.go

@@ -26,7 +26,6 @@ import (
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilfeature "k8s.io/component-base/featuregate/testing"
-	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/cluster-api-provider-aws/v2/feature"
@@ -51,126 +50,6 @@ func TestAWSClusterValidateCreate(t *testing.T) {
 		wantErr bool
 		expect  func(g *WithT, res *AWSLoadBalancerSpec)
 	}{
-		{
-			name: "No options are allowed when LoadBalancer is disabled (name)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						LoadBalancerType: LoadBalancerTypeDisabled,
-						Name:             ptr.To("name"),
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (crossZoneLoadBalancing)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						CrossZoneLoadBalancing: true,
-						LoadBalancerType:       LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (subnets)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						Subnets:          []string{"foo", "bar"},
-						LoadBalancerType: LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (healthCheckProtocol)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						HealthCheckProtocol: &ELBProtocolTCP,
-						LoadBalancerType:    LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (additionalSecurityGroups)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						AdditionalSecurityGroups: []string{"foo", "bar"},
-						LoadBalancerType:         LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (additionalListeners)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						AdditionalListeners: []AdditionalListenerSpec{
-							{
-								Port:     6443,
-								Protocol: ELBProtocolTCP,
-							},
-						},
-						LoadBalancerType: LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (ingressRules)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						IngressRules: []IngressRule{
-							{
-								Description: "ingress rule",
-								Protocol:    SecurityGroupProtocolTCP,
-								FromPort:    6443,
-								ToPort:      6443,
-							},
-						},
-						LoadBalancerType: LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (disableHostsRewrite)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						DisableHostsRewrite: true,
-						LoadBalancerType:    LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "No options are allowed when LoadBalancer is disabled (preserveClientIP)",
-			cluster: &AWSCluster{
-				Spec: AWSClusterSpec{
-					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
-						PreserveClientIP: true,
-						LoadBalancerType: LoadBalancerTypeDisabled,
-					},
-				},
-			},
-			wantErr: true,
-		},
 		// The SSHKeyName tests were moved to sshkeyname_test.go
 		{
 			name: "Supported schemes are 'internet-facing, Internet-facing, internal, or nil', rest will be rejected",

api/v1beta2/conditions_consts.go

@@ -125,9 +125,6 @@ const (
 	LoadBalancerReadyCondition clusterv1.ConditionType = "LoadBalancerReady"
 	// WaitForDNSNameReason used while waiting for a DNS name for the API server to be populated.
 	WaitForDNSNameReason = "WaitForDNSName"
-	// WaitForExternalControlPlaneEndpointReason is available when the AWS Cluster is waiting for an externally managed
-	// Load Balancer, such as an external Control Plane provider.
-	WaitForExternalControlPlaneEndpointReason = "WaitForExternalControlPlaneEndpoint"
 	// WaitForDNSNameResolveReason used while waiting for DNS name to resolve.
 	WaitForDNSNameResolveReason = "WaitForDNSNameResolve"
 	// LoadBalancerFailedReason used when an error occurs during load balancer reconciliation.

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml

@@ -1110,7 +1110,6 @@ spec:
                     - elb
                     - alb
                     - nlb
-                    - disabled
                     type: string
                   name:
                     description: Name sets the name of the classic ELB load balancer.
@@ -1692,7 +1691,6 @@ spec:
                     - elb
                     - alb
                     - nlb
-                    - disabled
                     type: string
                   name:
                     description: Name sets the name of the classic ELB load balancer.

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml

@@ -709,7 +709,6 @@ spec:
                             - elb
                             - alb
                             - nlb
-                            - disabled
                             type: string
                           name:
                             description: Name sets the name of the classic ELB load
@@ -1321,7 +1320,6 @@ spec:
                             - elb
                             - alb
                             - nlb
-                            - disabled
                             type: string
                           name:
                             description: Name sets the name of the classic ELB load

config/rbac/role.yaml

@@ -119,14 +119,6 @@ rules:
   - get
   - list
   - watch
-- apiGroups:
-  - controlplane.cluster.x-k8s.io
-  resources:
-  - '*'
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - controlplane.cluster.x-k8s.io
   resources:

controllers/awscluster_controller.go

@@ -266,45 +266,6 @@ func (r *AWSClusterReconciler) reconcileDelete(ctx context.Context, clusterScope
 	return nil
 }
 
-func (r *AWSClusterReconciler) reconcileLoadBalancer(clusterScope *scope.ClusterScope, awsCluster *infrav1.AWSCluster) (*time.Duration, error) {
-	retryAfterDuration := 15 * time.Second
-	if clusterScope.AWSCluster.Spec.ControlPlaneLoadBalancer.LoadBalancerType == infrav1.LoadBalancerTypeDisabled {
-		clusterScope.Debug("load balancer reconciliation shifted to external provider, checking external endpoint")
-
-		return r.checkForExternalControlPlaneLoadBalancer(clusterScope, awsCluster), nil
-	}
-
-	elbService := r.getELBService(clusterScope)
-
-	if err := elbService.ReconcileLoadbalancers(); err != nil {
-		clusterScope.Error(err, "failed to reconcile load balancer")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
-		return nil, err
-	}
-
-	if awsCluster.Status.Network.APIServerELB.DNSName == "" {
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameReason, clusterv1.ConditionSeverityInfo, "")
-		clusterScope.Info("Waiting on API server ELB DNS name")
-		return &retryAfterDuration, nil
-	}
-
-	clusterScope.Debug("Looking up IP address for DNS", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
-	if _, err := net.LookupIP(awsCluster.Status.Network.APIServerELB.DNSName); err != nil {
-		clusterScope.Error(err, "failed to get IP address for dns name", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameResolveReason, clusterv1.ConditionSeverityInfo, "")
-		clusterScope.Info("Waiting on API server ELB DNS name to resolve")
-		return &retryAfterDuration, nil
-	}
-	conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
-
-	awsCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
-		Host: awsCluster.Status.Network.APIServerELB.DNSName,
-		Port: clusterScope.APIServerPort(),
-	}
-
-	return nil, nil
-}
-
 func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope) (reconcile.Result, error) {
 	clusterScope.Info("Reconciling AWSCluster")
 
@@ -319,6 +280,7 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 	}
 
 	ec2Service := r.getEC2Service(clusterScope)
+	elbService := r.getELBService(clusterScope)
 	networkSvc := r.getNetworkService(*clusterScope)
 	sgService := r.getSecurityGroupService(*clusterScope)
 	s3Service := s3.NewService(clusterScope)
@@ -348,17 +310,37 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 		}
 	}
 
-	if requeueAfter, err := r.reconcileLoadBalancer(clusterScope, awsCluster); err != nil {
+	if err := elbService.ReconcileLoadbalancers(); err != nil {
+		clusterScope.Error(err, "failed to reconcile load balancer")
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
 		return reconcile.Result{}, err
-	} else if requeueAfter != nil {
-		return reconcile.Result{RequeueAfter: *requeueAfter}, err
 	}
 
 	if err := s3Service.ReconcileBucket(); err != nil {
 		conditions.MarkFalse(awsCluster, infrav1.S3BucketReadyCondition, infrav1.S3BucketFailedReason, clusterv1.ConditionSeverityError, err.Error())
 		return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile S3 Bucket for AWSCluster %s/%s", awsCluster.Namespace, awsCluster.Name)
 	}
 
+	if awsCluster.Status.Network.APIServerELB.DNSName == "" {
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameReason, clusterv1.ConditionSeverityInfo, "")
+		clusterScope.Info("Waiting on API server ELB DNS name")
+		return reconcile.Result{RequeueAfter: 15 * time.Second}, nil
+	}
+
+	clusterScope.Debug("looking up IP address for DNS", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
+	if _, err := net.LookupIP(awsCluster.Status.Network.APIServerELB.DNSName); err != nil {
+		clusterScope.Error(err, "failed to get IP address for dns name", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameResolveReason, clusterv1.ConditionSeverityInfo, "")
+		clusterScope.Info("Waiting on API server ELB DNS name to resolve")
+		return reconcile.Result{RequeueAfter: 15 * time.Second}, nil
+	}
+	conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
+
+	awsCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
+		Host: awsCluster.Status.Network.APIServerELB.DNSName,
+		Port: clusterScope.APIServerPort(),
+	}
+
 	for _, subnet := range clusterScope.Subnets().FilterPrivate() {
 		found := false
 		for _, az := range awsCluster.Status.Network.APIServerELB.AvailabilityZones {
@@ -465,29 +447,3 @@ func (r *AWSClusterReconciler) requeueAWSClusterForUnpausedCluster(_ context.Con
 		}
 	}
 }
-
-func (r *AWSClusterReconciler) checkForExternalControlPlaneLoadBalancer(clusterScope *scope.ClusterScope, awsCluster *infrav1.AWSCluster) *time.Duration {
-	requeueAfterPeriod := 15 * time.Second
-
-	switch {
-	case len(awsCluster.Spec.ControlPlaneEndpoint.Host) == 0 && awsCluster.Spec.ControlPlaneEndpoint.Port == 0:
-		clusterScope.Info("AWSCluster control plane endpoint is still non-populated")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
-
-		return &requeueAfterPeriod
-	case len(awsCluster.Spec.ControlPlaneEndpoint.Host) == 0:
-		clusterScope.Info("AWSCluster control plane endpoint host is still non-populated")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
-
-		return &requeueAfterPeriod
-	case awsCluster.Spec.ControlPlaneEndpoint.Port == 0:
-		clusterScope.Info("AWSCluster control plane endpoint port is still non-populated")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
-
-		return &requeueAfterPeriod
-	default:
-		conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
-
-		return nil
-	}
-}