refactor: cleanup PredicateFunc usage

Group the predicate funcs into either
specific event filters or the "universal"
filters which get applied to every event
type. Then And() them which is what
controller-runtime event_handler is
functionally doing anyway.

link: https://github.com/kubernetes-sigs/controller-runtime/blob/aea2e32a936584b06ae6f7992f856fe7292b0297/pkg/internal/source/event_handler.go#L116

Author: tyler-lloyd

cns/kubecontroller/nodenetworkconfig/reconciler.go

@@ -160,35 +160,47 @@ func (r *Reconciler) Started(ctx context.Context) (bool, error) {
 // SetupWithManager Sets up the reconciler with a new manager, filtering using NodeNetworkConfigFilter on nodeName.
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, node *v1.Node, cnsconfig *configuration.CNSConfig) error {
 	r.nnccli = nodenetworkconfig.NewClient(mgr.GetClient())
+
+	// specific event funcs
+	eventFuncs := predicate.Funcs{
+		// ignore delete events.
+		DeleteFunc: func(event.DeleteEvent) bool {
+			return false
+		},
+		UpdateFunc: func(ue event.UpdateEvent) bool {
+			if ue.ObjectOld == nil || ue.ObjectNew == nil {
+				return false
+			}
+			// check that the generation is the same - status changes don't update generation. But in IPAMv2
+			// it is safe to reconcile objects with a changed generation (typically means it was patched by
+			// CNS itself). This saves us from filtering out updates that were made while this controller's
+			// watch was down.
+			if cnsconfig != nil && cnsconfig.EnableIPAMv2 {
+				return true
+			}
+			return ue.ObjectOld.GetGeneration() == ue.ObjectNew.GetGeneration()
+		},
+	}
+
+	// these are applied to every event type
+	universalFuncs := predicate.NewPredicateFuncs(func(object client.Object) bool {
+		// match on node controller ref for all other events.
+		if !metav1.IsControlledBy(object, node) {
+			return false
+		}
+
+		// only process events on objects that are not being deleted.
+		if !object.GetDeletionTimestamp().IsZero() {
+			return false
+		}
+		return true
+	})
+
+	filters := predicate.And(eventFuncs, universalFuncs)
+
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha.NodeNetworkConfig{}).
-		WithEventFilter(predicate.Funcs{
-			// ignore delete events.
-			DeleteFunc: func(event.DeleteEvent) bool {
-				return false
-			},
-		}).
-		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
-			// match on node controller ref for all other events.
-			return metav1.IsControlledBy(object, node)
-		})).
-		WithEventFilter(predicate.Funcs{
-			// check that the generation is the same iff IPAMv1 - status changes don't update generation.
-			UpdateFunc: func(ue event.UpdateEvent) bool {
-				if ue.ObjectOld == nil || ue.ObjectNew == nil {
-					return false
-				}
-				// IPAMv2 is idempotent and can process every update event.
-				if cnsconfig != nil && cnsconfig.EnableIPAMv2 {
-					return true
-				}
-				return ue.ObjectOld.GetGeneration() == ue.ObjectNew.GetGeneration()
-			},
-		}).
-		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
-			// only process events on objects that are not being deleted.
-			return object.GetDeletionTimestamp().IsZero()
-		})).
+		WithEventFilter(filters).
 		Complete(r)
 	if err != nil {
 		return errors.Wrap(err, "failed to set up reconciler with manager")