fix: Updated the mapper funcs so they actually work

Leaseweb · Aug 16, 2024 · bda5223 · bda5223
1 parent 56fd2f5
commit bda5223
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 35 deletions.
diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go
@@ -364,20 +364,14 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont
 
 	reconciler.Recorder = mgr.GetEventRecorderFor("capc-machine-controller")
 
-	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")
-	}
+	cloudStackClusterToCloudStackMachines := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, ctrl.LoggerFrom(ctx))
 
 	csMachineMapper, err := util.ClusterToTypedObjectsMapper(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme)
 	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")
-	}
+	cloudStackIsolatedNetworkToControlPlaneCloudStackMachines := utils.CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(reconciler.K8sClient, ctrl.LoggerFrom(ctx))
 
 	err = ctrl.NewControllerManagedBy(mgr).
 		WithOptions(opts).

diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go
@@ -115,12 +115,7 @@ func GetOwnerClusterName(obj metav1.ObjectMeta) (string, error) {
 
 // CloudStackClusterToCloudStackMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
 // of CloudStackMachines.
-func CloudStackClusterToCloudStackMachines(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")
-	}
-
+func CloudStackClusterToCloudStackMachines(c client.Client, log logr.Logger) handler.MapFunc {
 	return func(ctx context.Context, o client.Object) []reconcile.Request {
 		csCluster, ok := o.(*infrav1.CloudStackCluster)
 		if !ok {
@@ -143,22 +138,31 @@ func CloudStackClusterToCloudStackMachines(c client.Client, obj client.ObjectLis
 		}
 
 		machineList := &clusterv1.MachineList{}
-		machineList.SetGroupVersionKind(gvk)
 		// list all the requested objects within the cluster namespace with the cluster name label
 		if err := c.List(ctx, machineList, client.InNamespace(csCluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName}); err != nil {
+			log.Error(err, "Failed to get owned Machines, skipping mapping.")
 			return nil
 		}
 
-		mapFunc := util.MachineToInfrastructureMapFunc(gvk)
-		var results []ctrl.Request
+		results := make([]ctrl.Request, 0, len(machineList.Items))
 		for _, machine := range machineList.Items {
 			m := machine
-			csMachines := mapFunc(ctx, &m)
-			results = append(results, csMachines...)
+			log.WithValues("machine", klog.KObj(&m))
+			if m.Spec.InfrastructureRef.GroupVersionKind().Kind != "CloudStackMachine" {
+				log.V(4).Info("Machine has an InfrastructureRef for a different type, will not add to reconciliation request.")
+				continue
+			}
+			if m.Spec.InfrastructureRef.Name == "" {
+				log.V(4).Info("Machine has an InfrastructureRef with an empty name, will not add to reconciliation request.")
+				continue
+			}
+			log.WithValues("cloudStackMachine", klog.KRef(m.Spec.InfrastructureRef.Namespace, m.Spec.InfrastructureRef.Name))
+			log.V(4).Info("Adding CloudStackMachine to reconciliation request.")
+			results = append(results, ctrl.Request{NamespacedName: client.ObjectKey{Namespace: m.Namespace, Name: m.Spec.InfrastructureRef.Name}})
 		}
 
 		return results
-	}, nil
+	}
 }
 
 // CloudStackClusterToCloudStackIsolatedNetworks is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
@@ -215,12 +219,7 @@ func CloudStackClusterToCloudStackIsolatedNetworks(c client.Client, obj client.O
 
 // 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")
-	}
-
+func CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(c client.Client, log logr.Logger) handler.MapFunc {
 	return func(ctx context.Context, o client.Object) []reconcile.Request {
 		csIsoNet, ok := o.(*infrav1.CloudStackIsolatedNetwork)
 		if !ok {
@@ -238,35 +237,43 @@ func CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(c client.Client,
 
 		clusterName, ok := csIsoNet.GetLabels()[clusterv1.ClusterNameLabel]
 		if !ok {
-			log.Error(err, "CloudStackIsolatedNetwork is missing cluster name label or cluster does not exist, skipping mapping.")
+			log.Error(errors.New("failed to find cluster name label"), "CloudStackIsolatedNetwork is missing cluster name label or cluster does not exist, skipping mapping.")
 		}
 
 		log.V(4).Info("Looking up isolated network members with control plane label", "cluster", klog.KRef(csIsoNet.Namespace, clusterName))
 		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{
+		err := c.List(ctx, machineList, client.InNamespace(csIsoNet.Namespace), client.MatchingLabels{
 			clusterv1.ClusterNameLabel:         clusterName,
 			clusterv1.MachineControlPlaneLabel: "",
 		})
 		if err != nil {
+			log.Error(err, "Failed to get owned control plane Machines, skipping mapping.")
 			return nil
 		}
 
 		log.V(4).Info("Looked up members with control plane label", "found", len(machineList.Items))
 
-		mapFunc := util.MachineToInfrastructureMapFunc(gvk)
-		var results []ctrl.Request
+		results := make([]ctrl.Request, 0, len(machineList.Items))
 		for _, machine := range machineList.Items {
 			m := machine
-			log.V(4).Info("Check Machine", "machine", machine.Name, "machinegroupkind", m.Spec.InfrastructureRef.GroupVersionKind().GroupKind(), "groupkind", gvk.GroupKind())
-			csMachines := mapFunc(ctx, &m)
-			results = append(results, csMachines...)
+			log.WithValues("machine", klog.KObj(&m))
+			if m.Spec.InfrastructureRef.GroupVersionKind().Kind != "CloudStackMachine" {
+				log.V(4).Info("Machine has an InfrastructureRef for a different type, will not add to reconciliation request.")
+				continue
+			}
+			if m.Spec.InfrastructureRef.Name == "" {
+				log.V(4).Info("Machine has an InfrastructureRef with an empty name, will not add to reconciliation request.")
+				continue
+			}
+			log.WithValues("cloudStackMachine", klog.KRef(m.Spec.InfrastructureRef.Namespace, m.Spec.InfrastructureRef.Name))
+			log.V(4).Info("Adding CloudStackMachine to reconciliation request.")
+			results = append(results, ctrl.Request{NamespacedName: client.ObjectKey{Namespace: m.Namespace, Name: m.Spec.InfrastructureRef.Name}})
 		}
 		log.V(4).Info("returning the following requests", "requests", results)
 
 		return results
-	}, nil
+	}
 }
 
 // DebugPredicate returns a predicate that logs the event that triggered the reconciliation