Move generation check from change_processor to controller

Problem: When processing updates to cluster resources, for some
resources, we check their generation, so that we don't trigger state
change (graph rebuild) if the generation didn't change. This is a
performance optimization so that we don't rebuild the graph and as a
result do not regenerate NGINX config and reload it. This is not a
K8s-native solution, and may introduce complexity for the codebase.

Solution: Use `GenerationChangedPredicate` in controller-runtime to
filter out the resource events at the controller level.

Author: kevin85421

internal/mode/static/manager.go

@@ -281,22 +281,28 @@ func registerControllers(
 		{
 			objectType: &gatewayv1.GatewayClass{},
 			options: []controller.Option{
+				controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
 				controller.WithK8sPredicate(predicate.GatewayClassPredicate{ControllerName: cfg.GatewayCtlrName}),
 			},
 		},
 		{
 			objectType: &gatewayv1.Gateway{},
 			options: func() []controller.Option {
+				options := []controller.Option{
+					controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
+				}
 				if cfg.GatewayNsName != nil {
-					return []controller.Option{
-						controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(*cfg.GatewayNsName)),
-					}
+					options = append(options,
+						controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(*cfg.GatewayNsName)))
 				}
-				return nil
+				return options
 			}(),
 		},
 		{
 			objectType: &gatewayv1.HTTPRoute{},
+			options: []controller.Option{
+				controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
+			},
 		},
 		{
 			objectType: &apiv1.Service{},
@@ -334,6 +340,9 @@ func registerControllers(
 		},
 		{
 			objectType: &gatewayv1beta1.ReferenceGrant{},
+			options: []controller.Option{
+				controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
+			},
 		},
 		{
 			objectType: &crdWithGVK,

internal/mode/static/state/change_processor.go

@@ -126,22 +126,22 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
 			{
 				gvk:       extractGVK(&v1.GatewayClass{}),
 				store:     newObjectStoreMapAdapter(clusterStore.GatewayClasses),
-				predicate: generationChangedPredicate{},
+				predicate: nil,
 			},
 			{
 				gvk:       extractGVK(&v1.Gateway{}),
 				store:     newObjectStoreMapAdapter(clusterStore.Gateways),
-				predicate: generationChangedPredicate{},
+				predicate: nil,
 			},
 			{
 				gvk:       extractGVK(&v1.HTTPRoute{}),
 				store:     newObjectStoreMapAdapter(clusterStore.HTTPRoutes),
-				predicate: generationChangedPredicate{},
+				predicate: nil,
 			},
 			{
 				gvk:       extractGVK(&v1beta1.ReferenceGrant{}),
 				store:     newObjectStoreMapAdapter(clusterStore.ReferenceGrants),
-				predicate: generationChangedPredicate{},
+				predicate: nil,
 			},
 			{
 				gvk:       extractGVK(&apiv1.Namespace{}),

internal/mode/static/state/changed_predicate.go

@@ -24,25 +24,6 @@ func (f funcPredicate) delete(object client.Object) bool {
 	return f.stateChanged(object)
 }
 
-// generationChangedPredicate implements stateChangedPredicate based on the generation of the object.
-// This predicate will return true on upsert if the object's generation has changed.
-// It always returns true on delete.
-type generationChangedPredicate struct{}
-
-func (generationChangedPredicate) delete(_ client.Object) bool { return true }
-
-func (generationChangedPredicate) upsert(oldObject, newObject client.Object) bool {
-	if oldObject == nil {
-		return true
-	}
-
-	if newObject == nil {
-		panic("Cannot determine if generation has changed on upsert because new object is nil")
-	}
-
-	return newObject.GetGeneration() != oldObject.GetGeneration()
-}
-
 // annotationChangedPredicate implements stateChangedPredicate based on the value of the annotation provided.
 // This predicate will return true on upsert if the annotation's value has changed.
 // It always returns true on delete.

internal/mode/static/state/changed_predicate_test.go

@@ -20,80 +20,6 @@ func TestFuncPredicate(t *testing.T) {
 	g.Expect(p.upsert(nil, nil)).To(BeTrue())
 }
 
-func TestGenerationChangedPredicate_Delete(t *testing.T) {
-	p := generationChangedPredicate{}
-
-	g := NewWithT(t)
-	g.Expect(p.delete(nil)).To(BeTrue())
-}
-
-func TestGenerationChangedPredicate_Update(t *testing.T) {
-	tests := []struct {
-		oldObj       client.Object
-		newObj       client.Object
-		name         string
-		stateChanged bool
-		expPanic     bool
-	}{
-		{
-			name: "generation has changed",
-			oldObj: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Generation: 1,
-				},
-			},
-			newObj: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Generation: 2,
-				},
-			},
-			stateChanged: true,
-		},
-		{
-			name: "generation has not changed",
-			oldObj: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Generation: 1,
-				},
-			},
-			newObj: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Generation: 1,
-				},
-			},
-			stateChanged: false,
-		},
-		{
-			name:         "old object is nil",
-			oldObj:       nil,
-			newObj:       &v1.Pod{},
-			stateChanged: true,
-		},
-		{
-			name:     "new object is nil",
-			oldObj:   &v1.Pod{},
-			newObj:   nil,
-			expPanic: true,
-		},
-	}
-
-	p := generationChangedPredicate{}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			g := NewWithT(t)
-			if test.expPanic {
-				upsert := func() {
-					p.upsert(test.oldObj, test.newObj)
-				}
-				g.Expect(upsert).Should(Panic())
-			} else {
-				g.Expect(p.upsert(test.oldObj, test.newObj)).To(Equal(test.stateChanged))
-			}
-		})
-	}
-}
-
 func TestAnnotationChangedPredicate_Delete(t *testing.T) {
 	p := annotationChangedPredicate{}