[fix] - handling external network spec (#511)

* handling external network spec * fixing lint issues * adding ENUM external to the LoadBalancingType in linode-cluster * adding a mutating webhook to always override LoadBalancingType=external when hostname is provided during creation * handling controller logic * lint suggestions * handling cluster previously provisioned with empty Network Spec * handling cluster previously provisioned with empty Network Spec * deletion logic in linode cluster to handle three types of loadBalacing * better switch case * handling instanceID errors while deleting the cluster.yaml * removing mutating webhook
linode · Sep 18, 2024 · edc9a66 · edc9a66
1 parent 708b0ac
commit edc9a66
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 55 deletions.
diff --git a/api/v1alpha2/linodecluster_types.go b/api/v1alpha2/linodecluster_types.go
@@ -104,7 +104,8 @@ func (lm *LinodeCluster) SetConditions(conditions clusterv1.Conditions) {
 // NetworkSpec encapsulates Linode networking resources.
 type NetworkSpec struct {
 	// LoadBalancerType is the type of load balancer to use, defaults to NodeBalancer if not otherwise set
-	// +kubebuilder:validation:Enum=NodeBalancer;dns
+	// +kubebuilder:validation:Enum=NodeBalancer;dns;external
+	// +kubebuilder:default=NodeBalancer
 	// +optional
 	LoadBalancerType string `json:"loadBalancerType,omitempty"`
 	// DNSProvider is provider who manages the domain

diff --git a/api/v1alpha2/linodecluster_webhook.go b/api/v1alpha2/linodecluster_webhook.go
@@ -43,7 +43,7 @@ func (r *LinodeCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 }
 
 // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable update and deletion validation.
-//+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodecluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=create,versions=v1alpha2,name=validation.linodecluster.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
+// +kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1alpha2-linodecluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=create,versions=v1alpha2,name=validation.linodecluster.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1
 
 var _ webhook.Validator = &LinodeCluster{}
 

diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go
@@ -183,11 +183,16 @@ func DeleteNodesFromNB(ctx context.Context, logger logr.Logger, clusterScope *sc
 	}
 
 	for _, eachMachine := range clusterScope.LinodeMachines.Items {
+		instanceID, errorInstanceID := util.GetInstanceID(eachMachine.Spec.ProviderID)
+		if errorInstanceID != nil {
+			return errorInstanceID
+		}
+
 		err := clusterScope.LinodeClient.DeleteNodeBalancerNode(
 			ctx,
 			*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
 			*clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID,
-			*eachMachine.Spec.InstanceID,
+			instanceID,
 		)
 		if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
 			logger.Error(err, "Failed to update Node Balancer")
@@ -200,7 +205,7 @@ func DeleteNodesFromNB(ctx context.Context, logger logr.Logger, clusterScope *sc
 				ctx,
 				*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
 				*portConfig.NodeBalancerConfigID,
-				*eachMachine.Spec.InstanceID,
+				instanceID,
 			)
 			if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
 				logger.Error(err, "Failed to update Node Balancer")

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml
@@ -392,11 +392,13 @@ spec:
                       If not set, CAPL will create a unique identifier for you
                     type: string
                   loadBalancerType:
+                    default: NodeBalancer
                     description: LoadBalancerType is the type of load balancer to
                       use, defaults to NodeBalancer if not otherwise set
                     enum:
                     - NodeBalancer
                     - dns
+                    - external
                     type: string
                   nodeBalancerID:
                     description: NodeBalancerID is the id of NodeBalancer.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml
@@ -320,11 +320,13 @@ spec:
                               If not set, CAPL will create a unique identifier for you
                             type: string
                           loadBalancerType:
+                            default: NodeBalancer
                             description: LoadBalancerType is the type of load balancer
                               to use, defaults to NodeBalancer if not otherwise set
                             enum:
                             - NodeBalancer
                             - dns
+                            - external
                             type: string
                           nodeBalancerID:
                             description: NodeBalancerID is the id of NodeBalancer.

diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go
@@ -217,46 +217,44 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo
 
 func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error {
 	logger.Info("deleting cluster")
-	if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil && clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != lbTypeDNS {
-		logger.Info("NodeBalancer ID is missing, nothing to do")
-
-		if len(clusterScope.LinodeMachines.Items) > 0 {
-			return errors.New("Waiting for associated LinodeMachine objects to be deleted")
-		}
-
-		if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil {
-			logger.Error(err, "failed to remove credentials finalizer")
-			setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r)
-			return err
+	switch {
+	case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "external":
+		logger.Info("LoadBalacing managed externally, nothing to do.")
+		conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Deletion in progress")
+		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "LoadBalacing managed externally", "LoadBalacing managed externally, nothing to do.")
+
+	case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == lbTypeDNS:
+		if err := removeMachineFromDNS(ctx, logger, clusterScope); err != nil {
+			return fmt.Errorf("remove machine from loadbalancer: %w", err)
 		}
-		controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha2.ClusterFinalizer)
-		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "NodeBalancerIDMissing", "NodeBalancer ID is missing, nothing to do")
+		conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancing for Type DNS deleted")
+		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, clusterv1.DeletedReason, "Load balancing for Type DNS deleted")
 
-		return nil
-	}
+	case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil:
+		logger.Info("NodeBalancer ID is missing for Type NodeBalancer, nothing to do")
+		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "NodeBalancerIDMissing", "NodeBalancer already removed, nothing to do")
 
-	if err := removeMachineFromLB(ctx, logger, clusterScope); err != nil {
-		return fmt.Errorf("remove machine from loadbalancer: %w", err)
-	}
+	case clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != nil:
+		if err := removeMachineFromNB(ctx, logger, clusterScope); err != nil {
+			return fmt.Errorf("remove machine from loadbalancer: %w", err)
+		}
 
-	if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != lbTypeDNS && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != nil {
 		err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID)
 		if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
 			logger.Error(err, "failed to delete NodeBalancer")
 			setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r)
 			return err
 		}
-	}
 
-	conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted")
-	r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, clusterv1.DeletedReason, "Load balancer deleted")
-
-	clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = nil
-	clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil
-	clusterScope.LinodeCluster.Spec.Network.AdditionalPorts = []infrav1alpha2.LinodeNBPortConfig{}
+		conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer for Type NodeBalancer deleted")
+		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, clusterv1.DeletedReason, "Load balancer for Type NodeBalancer deleted")
 
+		clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = nil
+		clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil
+		clusterScope.LinodeCluster.Spec.Network.AdditionalPorts = []infrav1alpha2.LinodeNBPortConfig{}
+	}
 	if len(clusterScope.LinodeMachines.Items) > 0 {
-		return errors.New("Waiting for associated LinodeMachine objects to be deleted")
+		return errors.New("waiting for associated LinodeMachine objects to be deleted")
 	}
 
 	if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil {

diff --git a/controller/linodecluster_controller_helpers.go b/controller/linodecluster_controller_helpers.go
@@ -2,7 +2,6 @@ package controller
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"slices"
 	"strings"
@@ -25,17 +24,21 @@ import (
 
 func addMachineToLB(ctx context.Context, clusterScope *scope.ClusterScope) error {
 	logger := logr.FromContextOrDiscard(ctx)
+	if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "external" {
+		logger.Info("LoadBalancing is handled externally")
+		return nil
+	}
 	if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" {
 		if err := services.EnsureDNSEntries(ctx, clusterScope, "create"); err != nil {
 			return err
 		}
 		return nil
 	}
-	if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil {
-		return errors.New("nil NodeBalancer Config ID")
-	}
-	if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID == nil {
-		return errors.New("nil NodeBalancer Config ID")
+	// Reconcile clusters with Spec.Network = {} and ControlPlaneEndpoint.Host externally managed
+	if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil || clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID == nil {
+		logger.Info("NodeBalancerID or ApiserverNodeBalancerConfigID not set for Type NodeBalancer, this cluster is managed externally")
+		clusterScope.LinodeCluster.Spec.Network.LoadBalancerType = "external"
+		return nil
 	}
 	nodeBalancerNodes, err := clusterScope.LinodeClient.ListNodeBalancerNodes(
 		ctx,
@@ -64,17 +67,18 @@ func addMachineToLB(ctx context.Context, clusterScope *scope.ClusterScope) error
 	return nil
 }
 
-func removeMachineFromLB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error {
-	if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" {
-		if err := services.DeleteNodesFromNB(ctx, logger, clusterScope); err != nil {
-			logger.Error(err, "Failed to remove node from Node Balancer backend")
-			return err
-		}
-	} else if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" {
-		if err := services.EnsureDNSEntries(ctx, clusterScope, "delete"); err != nil {
-			logger.Error(err, "Failed to remove IP from DNS")
-			return err
-		}
+func removeMachineFromDNS(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error {
+	if err := services.EnsureDNSEntries(ctx, clusterScope, "delete"); err != nil {
+		logger.Error(err, "Failed to remove IP from DNS")
+		return err
+	}
+	return nil
+}
+
+func removeMachineFromNB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error {
+	if err := services.DeleteNodesFromNB(ctx, logger, clusterScope); err != nil {
+		logger.Error(err, "Failed to remove node from Node Balancer backend")
+		return err
 	}
 	return nil
 }

diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go
@@ -412,7 +412,7 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 					reconciler.Client = mck.K8sClient
 					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
 					Expect(err).NotTo(HaveOccurred())
-					Expect(mck.Events()).To(ContainSubstring("Warning NodeBalancerIDMissing NodeBalancer ID is missing, nothing to do"))
+					Expect(mck.Events()).To(ContainSubstring("Warning NodeBalancerIDMissing NodeBalancer already removed, nothing to do"))
 				}),
 			),
 			Path(
@@ -443,7 +443,7 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 					reconciler.Client = mck.K8sClient
 					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
 					Expect(err).To(HaveOccurred())
-					Expect(err.Error()).To(ContainSubstring("Waiting for associated LinodeMachine objects to be deleted"))
+					Expect(err.Error()).To(ContainSubstring("waiting for associated LinodeMachine objects to be deleted"))
 				}),
 			),
 			Path(
@@ -459,7 +459,7 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 					reconciler.Client = mck.K8sClient
 					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
 					Expect(err).To(HaveOccurred())
-					Expect(err.Error()).To(ContainSubstring("Waiting for associated LinodeMachine objects to be deleted"))
+					Expect(err.Error()).To(ContainSubstring("waiting for associated LinodeMachine objects to be deleted"))
 				}),
 			),
 		),

diff --git a/templates/infra/linodeMachineTemplate.yaml b/templates/infra/linodeMachineTemplate.yaml
@@ -6,8 +6,6 @@ metadata:
 spec:
   template:
     spec:
-      credentialsRef:
-        name: ${CLUSTER_NAME}-credentials
       image: ${LINODE_OS:="linode/ubuntu22.04"}
       type: ${LINODE_CONTROL_PLANE_MACHINE_TYPE}
       region: ${LINODE_REGION}
@@ -29,8 +27,6 @@ metadata:
 spec:
   template:
     spec:
-      credentialsRef:
-        name: ${CLUSTER_NAME}-credentials
       image: ${LINODE_OS:="linode/ubuntu22.04"}
       type: ${LINODE_MACHINE_TYPE}
       region: ${LINODE_REGION}