From bda5223863931b058d1e3cf2bffdb666158c65c1 Mon Sep 17 00:00:00 2001 From: Hans Rakers Date: Fri, 16 Aug 2024 21:55:26 +0200 Subject: [PATCH] fix: Updated the mapper funcs so they actually work --- controllers/cloudstackmachine_controller.go | 10 +--- controllers/utils/utils.go | 61 ++++++++++++--------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go index fbbf1c25..68f9e973 100644 --- 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 index 0cd88110..a22bf315 100644 --- 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