Merge pull request #2262 from sbueringer/pr-consistent-setup

🌱 Avoid adding watches after building controllers

Author: k8s-ci-robot

controllers/vmware/vspherecluster_reconciler.go

@@ -63,7 +63,7 @@ type ClusterReconciler struct {
 // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;update;create;delete
 // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims/status,verbs=get;update;patch
 
-func (r ClusterReconciler) Reconcile(_ goctx.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+func (r *ClusterReconciler) Reconcile(_ goctx.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
 	logger := r.Logger.WithName(req.Namespace).WithName(req.Name)
 	logger.V(3).Info("Starting Reconcile vsphereCluster")
 

controllers/vspherecluster_controller.go

@@ -88,7 +88,7 @@ func AddClusterControllerToManager(ctx *context.ControllerManagerContext, mgr ma
 		if err != nil {
 			return errors.Wrap(err, "failed to create a network provider")
 		}
-		reconciler := vmware.ClusterReconciler{
+		reconciler := &vmware.ClusterReconciler{
 			ControllerContext:     controllerContext,
 			ResourcePolicyService: vmoperator.RPService{},
 			ControlPlaneService:   vmoperator.CPService{},

controllers/vspheremachine_controller.go

@@ -155,21 +155,17 @@ func AddMachineControllerToManager(ctx *context.ControllerManagerContext, mgr ma
 		)
 	}
 
-	c, err := builder.Build(r)
-	if err != nil {
-		return err
-	}
-
 	if !supervisorBased {
-		err = c.Watch(
-			source.Kind(mgr.GetCache(), &clusterv1.Cluster{}),
+		builder.Watches(
+			&clusterv1.Cluster{},
 			handler.EnqueueRequestsFromMapFunc(r.clusterToVSphereMachines),
-			predicates.ClusterUnpausedAndInfrastructureReady(r.Logger))
-		if err != nil {
-			return err
-		}
+			ctrlbldr.WithPredicates(
+				predicates.ClusterUnpausedAndInfrastructureReady(r.Logger),
+			),
+		)
 	}
-	return nil
+
+	return builder.Complete(r)
 }
 
 type machineReconciler struct {

controllers/vspherevm_controller.go

@@ -38,6 +38,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlbldr "sigs.k8s.io/controller-runtime/pkg/builder"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -92,7 +93,8 @@ func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager
 		VMService:                 &govmomi.VMService{},
 		remoteClusterCacheTracker: tracker,
 	}
-	controller, err := ctrl.NewControllerManagedBy(mgr).
+
+	return ctrl.NewControllerManagedBy(mgr).
 		// Watch the controlled, infrastructure resource.
 		For(controlledType).
 		WithOptions(options).
@@ -106,54 +108,43 @@ func AddVMControllerToManager(ctx *context.ControllerManagerContext, mgr manager
 			&handler.EnqueueRequestForObject{},
 		).
 		WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), ctx.WatchFilterValue)).
-		Build(r)
-	if err != nil {
-		return err
-	}
-
-	err = controller.Watch(
-		source.Kind(mgr.GetCache(), &clusterv1.Cluster{}),
-		handler.EnqueueRequestsFromMapFunc(r.clusterToVSphereVMs),
-		predicate.Funcs{
-			UpdateFunc: func(e event.UpdateEvent) bool {
-				newCluster := e.ObjectNew.(*clusterv1.Cluster)
-				// check whether cluster has either spec.paused or pasued annotation
-				return !annotations.IsPaused(newCluster, newCluster)
-			},
-			CreateFunc: func(e event.CreateEvent) bool {
-				cluster := e.Object.(*clusterv1.Cluster)
-				// check whether cluster has either spec.paused or pasued annotation
-				return annotations.IsPaused(cluster, cluster)
-			},
-		})
-	if err != nil {
-		return err
-	}
-
-	err = controller.Watch(
-		source.Kind(mgr.GetCache(), &infrav1.VSphereCluster{}),
-		handler.EnqueueRequestsFromMapFunc(r.vsphereClusterToVSphereVMs),
-		predicate.Funcs{
-			UpdateFunc: func(e event.UpdateEvent) bool {
-				oldCluster := e.ObjectOld.(*infrav1.VSphereCluster)
-				newCluster := e.ObjectNew.(*infrav1.VSphereCluster)
-				return !clustermodule.Compare(oldCluster.Spec.ClusterModules, newCluster.Spec.ClusterModules)
-			},
-			CreateFunc:  func(e event.CreateEvent) bool { return false },
-			DeleteFunc:  func(e event.DeleteEvent) bool { return false },
-			GenericFunc: func(e event.GenericEvent) bool { return false },
-		})
-	if err != nil {
-		return err
-	}
-
-	err = controller.Watch(
-		source.Kind(mgr.GetCache(), &ipamv1.IPAddressClaim{}),
-		handler.EnqueueRequestsFromMapFunc(r.ipAddressClaimToVSphereVM))
-	if err != nil {
-		return err
-	}
-	return nil
+		Watches(
+			&clusterv1.Cluster{},
+			handler.EnqueueRequestsFromMapFunc(r.clusterToVSphereVMs),
+			ctrlbldr.WithPredicates(
+				predicate.Funcs{
+					UpdateFunc: func(e event.UpdateEvent) bool {
+						newCluster := e.ObjectNew.(*clusterv1.Cluster)
+						// check whether cluster has either spec.paused or pasued annotation
+						return !annotations.IsPaused(newCluster, newCluster)
+					},
+					CreateFunc: func(e event.CreateEvent) bool {
+						cluster := e.Object.(*clusterv1.Cluster)
+						// check whether cluster has either spec.paused or pasued annotation
+						return annotations.IsPaused(cluster, cluster)
+					},
+				}),
+		).
+		Watches(
+			&infrav1.VSphereCluster{},
+			handler.EnqueueRequestsFromMapFunc(r.vsphereClusterToVSphereVMs),
+			ctrlbldr.WithPredicates(
+				predicate.Funcs{
+					UpdateFunc: func(e event.UpdateEvent) bool {
+						oldCluster := e.ObjectOld.(*infrav1.VSphereCluster)
+						newCluster := e.ObjectNew.(*infrav1.VSphereCluster)
+						return !clustermodule.Compare(oldCluster.Spec.ClusterModules, newCluster.Spec.ClusterModules)
+					},
+					CreateFunc:  func(e event.CreateEvent) bool { return false },
+					DeleteFunc:  func(e event.DeleteEvent) bool { return false },
+					GenericFunc: func(e event.GenericEvent) bool { return false },
+				}),
+		).
+		Watches(
+			&ipamv1.IPAddressClaim{},
+			handler.EnqueueRequestsFromMapFunc(r.ipAddressClaimToVSphereVM),
+		).
+		Complete(r)
 }
 
 type vmReconciler struct {