fix: Make sure VM's get assigned to lb rules when a change in lb rule…

…s occurs.
leaseweb · Aug 16, 2024 · ac8dfc3 · ac8dfc3
1 parent 66afeb0
commit ac8dfc3
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 2 deletions.
diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go
@@ -360,7 +360,7 @@ func (r *CloudStackMachineReconciliationRunner) ReconcileDelete() (retRes ctrl.R
 // SetupWithManager registers the machine reconciler to the CAPI controller manager.
 func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts controller.Options) error {
 	reconciler.Recorder = mgr.GetEventRecorderFor("capc-machine-controller")
-	CloudStackClusterToCloudStackMachines, err := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx))
+	cloudStackClusterToCloudStackMachines, err := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx))
 	if err != nil {
 		return errors.Wrap(err, "failed to create CloudStackClusterToCloudStackMachines mapper")
 	}
@@ -369,6 +369,10 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont
 	if err != nil {
 		return errors.Wrap(err, "failed to create mapper for Cluster to CloudStackMachines")
 	}
+	cloudStackIsolatedNetworkToControlPlaneCloudStackMachines, err := utils.CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx))
+	if err != nil {
+		return errors.Wrap(err, "failed to create CloudStackIsolatedNetworkToControlPlaneCloudStackMachines mapper")
+	}
 
 	err = ctrl.NewControllerManagedBy(mgr).
 		WithOptions(opts).
@@ -389,7 +393,7 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont
 		).
 		Watches(
 			&infrav1.CloudStackCluster{},
-			handler.EnqueueRequestsFromMapFunc(CloudStackClusterToCloudStackMachines),
+			handler.EnqueueRequestsFromMapFunc(cloudStackClusterToCloudStackMachines),
 		).
 		WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), reconciler.WatchFilterValue)).
 		WithEventFilter(
@@ -436,6 +440,27 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont
 				predicates.ClusterUnpausedAndInfrastructureReady(ctrl.LoggerFrom(ctx)),
 			),
 		).
+		Watches(
+			// This watch is here to assign VM's to loadbalancer rules
+			&infrav1.CloudStackIsolatedNetwork{},
+			handler.EnqueueRequestsFromMapFunc(cloudStackIsolatedNetworkToControlPlaneCloudStackMachines),
+			builder.WithPredicates(
+				predicate.Funcs{
+					UpdateFunc: func(e event.UpdateEvent) bool {
+						oldCSIsoNet := e.ObjectOld.(*infrav1.CloudStackIsolatedNetwork)
+						newCSIsoNet := e.ObjectNew.(*infrav1.CloudStackIsolatedNetwork)
+
+						// We're only interested in status updates, not Spec updates
+						if oldCSIsoNet.Generation != newCSIsoNet.Generation {
+							return false
+						}
+
+						// Only trigger a CloudStackMachine reconcile if the loadbalancer rules changed.
+						return len(oldCSIsoNet.Status.LoadBalancerRuleIDs) != len(newCSIsoNet.Status.LoadBalancerRuleIDs)
+					},
+				},
+			),
+		).
 		Complete(reconciler)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go
@@ -213,6 +213,58 @@ func CloudStackClusterToCloudStackIsolatedNetworks(c client.Client, obj client.O
 	}, nil
 }
 
+// CloudStackIsolatedNetworkToControlPlaneCloudStackMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
+// of CloudStackMachines that are part of the control plane.
+func CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(c client.Client, obj client.ObjectList, scheme *runtime.Scheme, log logr.Logger) (handler.MapFunc, error) {
+	gvk, err := apiutil.GVKForObject(obj, scheme)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to find GVK for CloudStackMachine")
+	}
+
+	return func(ctx context.Context, o client.Object) []reconcile.Request {
+		csIsoNet, ok := o.(*infrav1.CloudStackIsolatedNetwork)
+		if !ok {
+			log.Error(fmt.Errorf("expected a CloudStackIsolatedNetwork but got a %T", o), "Error in CloudStackIsolatedNetworkToControlPlaneCloudStackMachines")
+			return nil
+		}
+
+		log = log.WithValues("objectMapper", "cloudStackIsolatedNetworkToControlPlaneCloudStackMachines", "isonet", klog.KRef(csIsoNet.Namespace, csIsoNet.Name))
+
+		// Don't handle deleted CloudStackIsolatedNetworks
+		if !csIsoNet.ObjectMeta.DeletionTimestamp.IsZero() {
+			log.V(4).Info("CloudStackIsolatedNetwork has a deletion timestamp, skipping mapping.")
+			return nil
+		}
+
+		clusterName, err := GetOwnerClusterName(csIsoNet.ObjectMeta)
+		if err != nil {
+			log.Error(err, "Failed to get owning cluster, skipping mapping.")
+			return nil
+		}
+
+		machineList := &clusterv1.MachineList{}
+		machineList.SetGroupVersionKind(gvk)
+		// list all the requested objects within the cluster namespace with the cluster name and control plane label.
+		err = c.List(ctx, machineList, client.InNamespace(csIsoNet.Namespace), client.MatchingLabels{
+			clusterv1.ClusterNameLabel:         clusterName,
+			clusterv1.MachineControlPlaneLabel: "",
+		})
+		if err != nil {
+			return nil
+		}
+
+		mapFunc := util.MachineToInfrastructureMapFunc(gvk)
+		var results []ctrl.Request
+		for _, machine := range machineList.Items {
+			m := machine
+			csMachines := mapFunc(ctx, &m)
+			results = append(results, csMachines...)
+		}
+
+		return results
+	}, nil
+}
+
 // DebugPredicate returns a predicate that logs the event that triggered the reconciliation
 func DebugPredicate(logger logr.Logger) predicate.Funcs {
 	return predicate.Funcs{

diff --git a/pkg/cloud/isolated_network_test.go b/pkg/cloud/isolated_network_test.go
@@ -349,6 +349,28 @@ var _ = Describe("Network", func() {
 			Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed())
 		})
 
+		It("With additionalPorts defined, associates VM to all related LB rules", func() {
+			dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts, 456)
+			dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{dummies.LBRuleID, "FakeLBRuleID2"}
+			lbip := &csapi.ListLoadBalancerRuleInstancesParams{}
+			albp := &csapi.AssignToLoadBalancerRuleParams{}
+			gomock.InOrder(
+				lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]).
+					Return(lbip),
+				lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil),
+				lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]).Return(albp),
+				lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(&csapi.AssignToLoadBalancerRuleResponse{}, nil),
+
+				lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[1]).
+					Return(lbip),
+				lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil),
+				lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[1]).Return(albp),
+				lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(&csapi.AssignToLoadBalancerRuleResponse{}, nil),
+			)
+
+			Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed())
+		})
+
 		It("Associating VM to LB rule fails", func() {
 			dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{"lbruleid"}
 			lbip := &csapi.ListLoadBalancerRuleInstancesParams{}