fix: Separate load balancer reconciliation from GetOrCreateIsolatedNe…

…twork
Leaseweb · Aug 27, 2024 · 8bd85c7 · 8bd85c7
1 parent c26adc4
commit 8bd85c7
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 43 deletions.
diff --git a/controllers/cloudstackisolatednetwork_controller.go b/controllers/cloudstackisolatednetwork_controller.go
@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"reflect"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -72,14 +73,14 @@ func (reconciler *CloudStackIsoNetReconciler) Reconcile(ctx context.Context, req
 	return r.RunBaseReconciliationStages()
 }
 
-func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result, retErr error) {
+func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (ctrl.Result, error) {
 	controllerutil.AddFinalizer(r.ReconciliationSubject, infrav1.IsolatedNetworkFinalizer)
 
 	// Setup isolated network, endpoint, egress, and load balancing.
 	// Set endpoint of CloudStackCluster if it is not currently set. (uses patcher to do so)
 	csClusterPatcher, err := patch.NewHelper(r.CSCluster, r.K8sClient)
 	if err != nil {
-		return r.ReturnWrappedError(retErr, "setting up CloudStackCluster patcher")
+		return ctrl.Result{}, errors.Wrap(err, "setting up CloudStackCluster patcher")
 	}
 	if r.FailureDomain.Spec.Zone.ID == "" {
 		return r.RequeueWithMessage("Zone ID not resolved yet.")
@@ -91,8 +92,14 @@ func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result,
 	if err := r.CSUser.AddClusterTag(cloud.ResourceTypeNetwork, r.ReconciliationSubject.Spec.ID, r.CSCluster); err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "tagging network with id %s", r.ReconciliationSubject.Spec.ID)
 	}
+	// Configure API server load balancer, if enabled and this cluster is not externally managed.
+	if !annotations.IsExternallyManaged(r.CSCluster) {
+		if err := r.CSUser.ReconcileLoadBalancer(r.FailureDomain, r.ReconciliationSubject, r.CSCluster); err != nil {
+			return ctrl.Result{}, errors.Wrap(err, "reconciling load balancer")
+		}
+	}
 	if err := csClusterPatcher.Patch(r.RequestCtx, r.CSCluster); err != nil {
-		return r.ReturnWrappedError(err, "patching endpoint update to CloudStackCluster")
+		return ctrl.Result{}, errors.Wrap(err, "patching endpoint update to CloudStackCluster")
 	}
 
 	r.ReconciliationSubject.Status.Ready = true

diff --git a/pkg/cloud/isolated_network.go b/pkg/cloud/isolated_network.go
@@ -35,6 +35,7 @@ import (
 
 type IsoNetworkIface interface {
 	GetOrCreateIsolatedNetwork(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error
+	ReconcileLoadBalancer(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error
 
 	AssociatePublicIPAddress(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error
 	CreateEgressFirewallRules(*infrav1.CloudStackIsolatedNetwork) error
@@ -737,54 +738,61 @@ func (c *client) GetOrCreateIsolatedNetwork(
 		isoNet.Status.PublicIPID = publicAddresses.PublicIpAddresses[0].Id
 	}
 
-	if !annotations.IsExternallyManaged(csCluster) {
-		// Check/set ControlPlaneEndpoint port.
-		// Prefer csCluster ControlPlaneEndpoint port. Use isonet port if CP missing. Set to default if both missing.
-		if csCluster.Spec.ControlPlaneEndpoint.Port != 0 {
-			isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port
-		} else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified.
-			csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port
-		} else {
-			csCluster.Spec.ControlPlaneEndpoint.Port = 6443
-			isoNet.Spec.ControlPlaneEndpoint.Port = 6443
-		}
-
-		// Associate public IP with the load balancer if enabled.
-		if csCluster.Spec.APIServerLoadBalancer.IsEnabled() {
-			// Associate Public IP with CloudStackIsolatedNetwork
-			if err := c.AssociatePublicIPAddress(fd, isoNet, csCluster); err != nil {
-				return errors.Wrapf(err, "associating public IP address to csCluster")
-			}
-		}
+	// Open the Isolated Network egress firewall.
+	return errors.Wrap(c.CreateEgressFirewallRules(isoNet), "opening the isolated network's egress firewall")
+}
 
-		// Set up load balancing rules to map VM ports to Public IP ports.
-		if err := c.ReconcileLoadBalancerRules(isoNet, csCluster); err != nil {
-			return errors.Wrap(err, "reconciling load balancing rules")
-		}
+// ReconcileLoadBalancer configures the API server load balancer.
+func (c *client) ReconcileLoadBalancer(
+	fd *infrav1.CloudStackFailureDomain,
+	isoNet *infrav1.CloudStackIsolatedNetwork,
+	csCluster *infrav1.CloudStackCluster,
+) error {
+	// Check/set ControlPlaneEndpoint port.
+	// Prefer csCluster ControlPlaneEndpoint port. Use isonet port if CP missing. Set to default if both missing.
+	if csCluster.Spec.ControlPlaneEndpoint.Port != 0 {
+		isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port
+	} else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified.
+		csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port
+	} else {
+		csCluster.Spec.ControlPlaneEndpoint.Port = 6443
+		isoNet.Spec.ControlPlaneEndpoint.Port = 6443
+	}
 
-		// Set up firewall rules to manage access to load balancer public IP ports.
-		if err := c.ReconcileFirewallRules(isoNet, csCluster); err != nil {
-			return errors.Wrap(err, "reconciling firewall rules")
+	// Associate public IP with the load balancer if enabled.
+	if csCluster.Spec.APIServerLoadBalancer.IsEnabled() {
+		// Associate Public IP with CloudStackIsolatedNetwork
+		if err := c.AssociatePublicIPAddress(fd, isoNet, csCluster); err != nil {
+			return errors.Wrapf(err, "associating public IP address to csCluster")
 		}
+	}
 
-		if !csCluster.Spec.APIServerLoadBalancer.IsEnabled() && isoNet.Status.APIServerLoadBalancer != nil {
-			// If the APIServerLoadBalancer has been disabled, release its IP unless it's the SNAT IP.
-			released, err := c.DisassociatePublicIPAddressIfNotInUse(isoNet.Status.APIServerLoadBalancer.IPAddressID)
-			if err != nil {
-				return errors.Wrap(err, "disassociating public IP address")
-			}
-			if released {
-				isoNet.Status.APIServerLoadBalancer.IPAddress = ""
-				isoNet.Status.APIServerLoadBalancer.IPAddressID = ""
-			}
+	// Set up load balancing rules to map VM ports to Public IP ports.
+	if err := c.ReconcileLoadBalancerRules(isoNet, csCluster); err != nil {
+		return errors.Wrap(err, "reconciling load balancing rules")
+	}
+
+	// Set up firewall rules to manage access to load balancer public IP ports.
+	if err := c.ReconcileFirewallRules(isoNet, csCluster); err != nil {
+		return errors.Wrap(err, "reconciling firewall rules")
+	}
 
-			// Clear the load balancer status as it is disabled.
-			isoNet.Status.APIServerLoadBalancer = nil
+	if !csCluster.Spec.APIServerLoadBalancer.IsEnabled() && isoNet.Status.APIServerLoadBalancer != nil {
+		// If the APIServerLoadBalancer has been disabled, release its IP unless it's the SNAT IP.
+		released, err := c.DisassociatePublicIPAddressIfNotInUse(isoNet.Status.APIServerLoadBalancer.IPAddressID)
+		if err != nil {
+			return errors.Wrap(err, "disassociating public IP address")
+		}
+		if released {
+			isoNet.Status.APIServerLoadBalancer.IPAddress = ""
+			isoNet.Status.APIServerLoadBalancer.IPAddressID = ""
 		}
+
+		// Clear the load balancer status as it is disabled.
+		isoNet.Status.APIServerLoadBalancer = nil
 	}
 
-	// Open the Isolated Network egress firewall.
-	return errors.Wrap(c.CreateEgressFirewallRules(isoNet), "opening the isolated network's egress firewall")
+	return nil
 }
 
 // AssignVMToLoadBalancerRules assigns a VM instance to load balancing rules (specifying lb membership).