extended conformance - `GatewayClassObservedGenerationBump`

[mlavacca]

Is there an existing issue for this?

• [X] I have searched the existing issues

Problem Statement

Support the GatewayClassObservedGenerationBump feature in conformance tests (extended)

Acceptance Criteria

• [ ] the conformance test GatewayClassObservedGenerationBump is successfully ran

[pmalek]

I looked at this issue and it seems pretty straightforward to implement but 1 path I see for this is to implement a separate reconciler for GatewayClass solely for the needs of just updating the generation. The reason I'm seeing it this way is that Reconcile doesn't have any information passed down from the caller w.r.t to the old object when an update event happens:

Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error)

That can only be done with e.g. predicate.Funcs where both the old and the new objects are available for comparison. But then: since predicates passed to c.Watch(...) are logically ANDed we'd filter out all other events.

Any other ways of doing this other than a separate reconciler 🤔 ?

Below could be an implementation started but the predicate would have to be moved as mentioned to a separate reconciler:

diff --git a/internal/controllers/gateway/gateway_utils.go b/internal/controllers/gateway/gateway_utils.go
index 201b5b42..3826bece 100644
--- a/internal/controllers/gateway/gateway_utils.go
+++ b/internal/controllers/gateway/gateway_utils.go
@@ -9,6 +9,7 @@ import (
    "strings"

    "github.com/go-logr/logr"
+   "github.com/google/go-cmp/cmp"
    "github.com/samber/lo"
    corev1 "k8s.io/api/core/v1"
    apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -618,6 +619,35 @@ func isGatewayClassEventInClass(log logr.Logger, watchEvent interface{}) bool {
    return false
 }

+func isGatewayClassSpecChanged(log logr.Logger, watchEvent interface{}) bool {
+   gwFromClientObject := func(log logr.Logger, obj client.Object) (*GatewayClass, bool) {
+       gw, ok := obj.(*GatewayClass)
+       if !ok {
+           log.Error(fmt.Errorf("invalid type"),
+               "received invalid object type in event handlers",
+               "expected", "GatewayClass", "found", reflect.TypeOf(gw))
+           return nil, false
+       }
+       return gw, true
+   }
+
+   switch e := watchEvent.(type) {
+   case event.UpdateEvent:
+       gwcOld, ok := gwFromClientObject(log, e.ObjectOld)
+       if !ok {
+           return false
+       }
+       gwcNew, ok := gwFromClientObject(log, e.ObjectNew)
+       if !ok {
+           return false
+       }
+       return !cmp.Equal(gwcOld.Spec, gwcNew.Spec)
+   default:
+       log.Error(fmt.Errorf("invalid type"), "received invalid event type in event handlers", "found", reflect.TypeOf(watchEvent))
+       return false
+   }
+}
+
 // getListenerSupportedRouteKinds determines what RouteGroupKinds are supported by the Listener.
 // If no AllowedRoutes.Kinds are specified for the Listener, the supported RouteGroupKind is derived directly
 // from the Listener's Protocol.
diff --git a/internal/controllers/gateway/gatewayclass_controller.go b/internal/controllers/gateway/gatewayclass_controller.go
index 27fe9ad2..084a24c3 100644
--- a/internal/controllers/gateway/gatewayclass_controller.go
+++ b/internal/controllers/gateway/gatewayclass_controller.go
@@ -14,6 +14,7 @@ import (
    ctrl "sigs.k8s.io/controller-runtime"
    "sigs.k8s.io/controller-runtime/pkg/client"
    "sigs.k8s.io/controller-runtime/pkg/controller"
+   "sigs.k8s.io/controller-runtime/pkg/event"
    "sigs.k8s.io/controller-runtime/pkg/handler"
    "sigs.k8s.io/controller-runtime/pkg/predicate"
    "sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -77,6 +78,9 @@ func (r *GatewayClassReconciler) SetupWithManager(mgr ctrl.Manager) error {
        &source.Kind{Type: &gatewayv1beta1.GatewayClass{}},
        &handler.EnqueueRequestForObject{},
        predicate.NewPredicateFuncs(r.GatewayClassIsUnmanaged),
+       predicate.Funcs{
+           UpdateFunc: func(e event.UpdateEvent) bool { return isGatewayClassSpecChanged(r.Log, e) },
+       },
    )
 }

[mlavacca]

I think that adding a new GatewayClass controller with the sole objective of updating the GatewayClass generation seems a bit overkill, but the only clean way to do it. Hence 👍 from me.

[mlavacca]

We need to address this issue, as the GatewayClassObservedGenerationBump feature has been removed and this test is part of core tests starting from 0.8.0

[mlavacca]

Since GEP#2162 aims at adding the supported features into the GatewayClass, we'll need a GatewayClass reconciler without any doubt for that. This is just another reason to add it.