fix: let NNCResonciler process update events with different generations when IPAMv2 is enabled (#3279)

* fix: let NNCReconciler process every update when IPAMv2 is enabled

* 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

* revert: "refactor: cleanup PredicateFunc usage"

This reverts commit 67983d0.

* chore: reduce refactoring

* chore: just pass bool

Author: tyler-lloyd

cns/kubecontroller/nodenetworkconfig/reconciler.go

@@ -157,7 +157,9 @@ 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) error {
+// filterGenerationChange will check the old and new object's generation and only reconcile updates where the
+// generation is the same. This is typically used in IPAMv1 but should be set to false in IPAMv2.
+func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, node *v1.Node, filterGenerationChange bool) error {
 	r.nnccli = nodenetworkconfig.NewClient(mgr.GetClient())
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha.NodeNetworkConfig{}).
@@ -166,20 +168,20 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, node *v1.Node) error {
 			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 - status changes don't update generation.
 			UpdateFunc: func(ue event.UpdateEvent) bool {
 				if ue.ObjectOld == nil || ue.ObjectNew == nil {
 					return false
 				}
-				return ue.ObjectOld.GetGeneration() == ue.ObjectNew.GetGeneration()
+				if filterGenerationChange {
+					return ue.ObjectOld.GetGeneration() == ue.ObjectNew.GetGeneration()
+				}
+				return true
 			},
 		}).
+		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
+			// match on node controller ref for all other events.
+			return metav1.IsControlledBy(object, node)
+		})).
 		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
 			// only process events on objects that are not being deleted.
 			return object.GetDeletionTimestamp().IsZero()

cns/service/main.go

@@ -1032,7 +1032,7 @@ func main() {
 		// Start fs watcher here
 		z.Info("AsyncPodDelete is enabled")
 		logger.Printf("AsyncPodDelete is enabled")
-		cnsclient, err := cnsclient.New("", cnsReqTimeout) //nolint
+		cnsclient, err := cnsclient.New("", cnsReqTimeout) // nolint
 		if err != nil {
 			z.Error("failed to create cnsclient", zap.Error(err))
 		}
@@ -1548,7 +1548,10 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 	nodeIP := configuration.NodeIP()
 	nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP)
 	// pass Node to the Reconciler for Controller xref
-	if err := nncReconciler.SetupWithManager(manager, node); err != nil { //nolint:govet // intentional shadow
+	// IPAMv1 - reconcile only status changes (where generation doesn't change).
+	// IPAMv2 - reconcile all updates.
+	filterGenerationChange := !cnsconfig.EnableIPAMv2
+	if err := nncReconciler.SetupWithManager(manager, node, filterGenerationChange); err != nil { //nolint:govet // intentional shadow
 		return errors.Wrapf(err, "failed to setup nnc reconciler with manager")
 	}
 
@@ -1618,7 +1621,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 		// wait for the Reconciler to run once on a NNC that was made for this Node.
 		// the nncReadyCtx has a timeout of 15 minutes, after which we will consider
 		// this false and the NNC Reconciler stuck/failed, log and retry.
-		nncReadyCtx, cancel := context.WithTimeout(ctx, 15*time.Minute) //nolint // it will time out and not leak
+		nncReadyCtx, cancel := context.WithTimeout(ctx, 15*time.Minute) // nolint // it will time out and not leak
 		if started, err := nncReconciler.Started(nncReadyCtx); !started {
 			logger.Errorf("NNC reconciler has not started, does the NNC exist? err: %v", err)
 			nncReconcilerStartFailures.Inc()