[improvement] standardize predicates on controllers (#454)

* clean up delete predicates * ignore changes to metadata for controllers * don't enqueue updates for changes the controller makes to the resource ID for VPC and PG
linode · Aug 16, 2024 · 232a4fc · 232a4fc
1 parent 24d71b0
commit 232a4fc
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 28 deletions.
diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go
@@ -306,6 +306,7 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager, options crc
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1alpha2.LinodeCluster{}).
 		WithOptions(options).
+		// we care about reconciling on metadata updates for LinodeClusters because the OwnerRef for the Cluster is needed
 		WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)).
 		Watches(
 			&clusterv1.Cluster{},

diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go
@@ -809,6 +809,7 @@ func (r *LinodeMachineReconciler) SetupWithManager(mgr ctrl.Manager, options crc
 			handler.EnqueueRequestsFromMapFunc(linodeMachineMapper),
 			builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
 		).
+		// we care about reconciling on metadata updates for LinodeMachines because the OwnerRef for the Machine is needed
 		WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)).
 		Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
 	if err != nil {

diff --git a/controller/linodeplacementgroup_controller.go b/controller/linodeplacementgroup_controller.go
@@ -305,20 +305,24 @@ func (r *LinodePlacementGroupReconciler) SetupWithManager(mgr ctrl.Manager, opti
 	err = ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1alpha2.LinodePlacementGroup{}).
 		WithOptions(options).
-		WithEventFilter(
-			predicate.And(
-				// Filter for objects with a specific WatchLabel.
-				predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue),
-				// Do not reconcile the Delete events generated by the
-				// controller itself.
-				predicate.Funcs{
-					DeleteFunc: func(e event.DeleteEvent) bool { return false },
-				},
-			)).Watches(
-		&clusterv1.Cluster{},
-		handler.EnqueueRequestsFromMapFunc(linodePlacementGroupMapper),
-		builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
-	).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
+		WithEventFilter(predicate.And(
+			predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue),
+			predicate.GenerationChangedPredicate{},
+			predicate.Funcs{UpdateFunc: func(e event.UpdateEvent) bool {
+				oldObject, okOld := e.ObjectOld.(*infrav1alpha2.LinodePlacementGroup)
+				newObject, okNew := e.ObjectNew.(*infrav1alpha2.LinodePlacementGroup)
+				if okOld && okNew && oldObject.Spec.PGID == nil && newObject.Spec.PGID != nil {
+					// We just created the PG, don't enqueue and update
+					return false
+				}
+				return true
+			}},
+		)).
+		Watches(
+			&clusterv1.Cluster{},
+			handler.EnqueueRequestsFromMapFunc(linodePlacementGroupMapper),
+			builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
+		).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
 	if err != nil {
 		return fmt.Errorf("failed to build controller: %w", err)
 	}

diff --git a/controller/linodevpc_controller.go b/controller/linodevpc_controller.go
@@ -335,20 +335,24 @@ func (r *LinodeVPCReconciler) SetupWithManager(mgr ctrl.Manager, options crcontr
 	err = ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1alpha2.LinodeVPC{}).
 		WithOptions(options).
-		WithEventFilter(
-			predicate.And(
-				// Filter for objects with a specific WatchLabel.
-				predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue),
-				// Do not reconcile the Delete events generated by the
-				// controller itself.
-				predicate.Funcs{
-					DeleteFunc: func(e event.DeleteEvent) bool { return false },
-				},
-			)).Watches(
-		&clusterv1.Cluster{},
-		handler.EnqueueRequestsFromMapFunc(linodeVPCMapper),
-		builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
-	).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
+		WithEventFilter(predicate.And(
+			predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue),
+			predicate.GenerationChangedPredicate{},
+			predicate.Funcs{UpdateFunc: func(e event.UpdateEvent) bool {
+				oldObject, okOld := e.ObjectOld.(*infrav1alpha2.LinodeVPC)
+				newObject, okNew := e.ObjectNew.(*infrav1alpha2.LinodeVPC)
+				if okOld && okNew && oldObject.Spec.VPCID == nil && newObject.Spec.VPCID != nil {
+					// We just created the VPC, don't enqueue and update
+					return false
+				}
+				return true
+			}},
+		)).
+		Watches(
+			&clusterv1.Cluster{},
+			handler.EnqueueRequestsFromMapFunc(linodeVPCMapper),
+			builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
+		).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
 	if err != nil {
 		return fmt.Errorf("failed to build controller: %w", err)
 	}