diff --git a/api/internal/controller/api_controller.go b/api/internal/controller/api_controller.go index 7c47bbb65..048606bf8 100644 --- a/api/internal/controller/api_controller.go +++ b/api/internal/controller/api_controller.go @@ -48,10 +48,10 @@ func (r *ApiReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&api.ApiHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&apiapi.Api{}). + For(&apiapi.Api{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Watches(&apiapi.Api{}, handler.EnqueueRequestsFromMapFunc(r.MapApiToApi), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). WithOptions(controller.Options{ MaxConcurrentReconciles: cconfig.MaxConcurrentReconciles, diff --git a/api/internal/controller/apiexposure_controller.go b/api/internal/controller/apiexposure_controller.go index 34d69314e..36fa7f686 100644 --- a/api/internal/controller/apiexposure_controller.go +++ b/api/internal/controller/apiexposure_controller.go @@ -58,14 +58,14 @@ func (r *ApiExposureReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&apiexposure.ApiExposureHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&apiv1.ApiExposure{}). + For(&apiv1.ApiExposure{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Watches(&apiv1.Api{}, handler.EnqueueRequestsFromMapFunc(r.MapApiToApiExposure), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). Watches(&apiv1.ApiExposure{}, handler.EnqueueRequestsFromMapFunc(r.MapApiExposureToApiExposure), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). // Watch ApiSubscription with ResourceVersionChangedPredicate (not GenerationChangedPredicate) // because we need to react to approval status changes, which update Status (not Spec). @@ -78,6 +78,8 @@ func (r *ApiExposureReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.MapRouteToApiExposure), builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). + // Watch Zone with ResourceVersionChangedPredicate because zone readiness is conveyed + // via status conditions (not spec), and exposures must react when a zone becomes ready. Watches(&adminv1.Zone{}, handler.EnqueueRequestsFromMapFunc(r.MapZoneToApiExposure), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), diff --git a/api/internal/controller/apisubscription_controller.go b/api/internal/controller/apisubscription_controller.go index 7c6495b2b..6c05bb0d9 100644 --- a/api/internal/controller/apisubscription_controller.go +++ b/api/internal/controller/apisubscription_controller.go @@ -65,14 +65,17 @@ func (r *ApiSubscriptionReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&apisubscription.ApiSubscriptionHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&apiapi.ApiSubscription{}, builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + For(&apiapi.ApiSubscription{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + // ApprovalRequest and Approval are watched without a predicate so that status + // changes (approval decisions) re-enqueue the parent ApiSubscription. + // GenerationChangedPredicate would miss these since decisions update Status, not Spec. Owns(&approvalapi.ApprovalRequest{}). Owns(&approvalapi.Approval{}). Owns(&gatewayapi.ConsumeRoute{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&apiapi.RemoteApiSubscription{}). Watches(&apiapi.Api{}, handler.EnqueueRequestsFromMapFunc(r.MapApiToApiSubscription), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + builder.WithPredicates(predicate.GenerationChangedPredicate{}), ). Watches(&apiapi.ApiExposure{}, handler.EnqueueRequestsFromMapFunc(r.MapApiExposureToApiSubscription), diff --git a/common/pkg/controller/controller.go b/common/pkg/controller/controller.go index 204fe41e0..b483183be 100644 --- a/common/pkg/controller/controller.go +++ b/common/pkg/controller/controller.go @@ -6,6 +6,7 @@ package controller import ( "context" + "reflect" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -66,7 +67,10 @@ func (c *ControllerImpl[T]) Reconcile(ctx context.Context, req reconcile.Request if changed, setupErr := FirstSetup(ctx, c.Client, object); setupErr != nil { return HandleError(ctx, setupErr, object, c.Recorder), nil } else if changed { - return reconcile.Result{}, nil + // Requeue explicitly so the full reconcile runs after the finalizer write, + // regardless of whether the watch event fires (finalizer updates do not bump + // Generation, so GenerationChangedPredicate would otherwise filter it out). + return reconcile.Result{Requeue: true}, nil } logger.V(1).Info("Fetched object") @@ -118,9 +122,19 @@ func (c *ControllerImpl[T]) Reconcile(ctx context.Context, req reconcile.Request c.Event(ctx, object, "Warning", "UnknownReady", "Resource has an unknown ready status") } + // Snapshot conditions before stamping so we can detect real changes. + // StampObservedGeneration mutates ObservedGeneration on each condition in-place, + // so any generation advance will show up as a diff. + condsBefore := append([]metav1.Condition(nil), object.GetConditions()...) StampObservedGeneration(object) - if err = c.Client.Status().Update(ctx, object); err != nil { - return HandleError(ctx, err, object, c.Recorder), nil + + // Skip the status write when nothing actually changed. This prevents a + // ResourceVersion bump that would otherwise re-enqueue this controller + // (via For()) and cross-trigger downstream watchers on every periodic reconcile. + if !reflect.DeepEqual(condsBefore, object.GetConditions()) { + if err = c.Client.Status().Update(ctx, object); err != nil { + return HandleError(ctx, err, object, c.Recorder), nil + } } requeueAfter := config.RequeueWithJitter() diff --git a/common/pkg/controller/controller_test.go b/common/pkg/controller/controller_test.go index 95e84da24..8ae8eeae3 100644 --- a/common/pkg/controller/controller_test.go +++ b/common/pkg/controller/controller_test.go @@ -138,7 +138,7 @@ var _ = Describe("Controller", func() { res, err := controller.Reconcile(ctx, req, &test.TestResource{}) Expect(err).ToNot(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) + Expect(res).To(Equal(reconcile.Result{Requeue: true})) Expect(k8sClient.Get(ctx, req.NamespacedName, obj)).To(Succeed()) Expect(controllerutil.ContainsFinalizer(obj, config.FinalizerName)).To(BeTrue()) diff --git a/gateway/internal/controller/consumer_controller.go b/gateway/internal/controller/consumer_controller.go index 3a4e37188..f3ce4026b 100644 --- a/gateway/internal/controller/consumer_controller.go +++ b/gateway/internal/controller/consumer_controller.go @@ -12,8 +12,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1 "github.com/telekom/controlplane/gateway/api/v1" consumer_handler "github.com/telekom/controlplane/gateway/internal/handler/consumer" @@ -43,7 +45,7 @@ func (r *ConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&consumer_handler.ConsumerHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&gatewayv1.Consumer{}). + For(&gatewayv1.Consumer{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). WithOptions(controller.Options{ MaxConcurrentReconciles: cconfig.MaxConcurrentReconciles, RateLimiter: cc.NewRateLimiter(), diff --git a/gateway/internal/controller/consumeroute_controller.go b/gateway/internal/controller/consumeroute_controller.go index dc99dc2af..cf6b0d494 100644 --- a/gateway/internal/controller/consumeroute_controller.go +++ b/gateway/internal/controller/consumeroute_controller.go @@ -47,7 +47,7 @@ func (r *ConsumeRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&consumeroute_handler.ConsumeRouteHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&gatewayv1.ConsumeRoute{}). + For(&gatewayv1.ConsumeRoute{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). WithOptions(controller.Options{ MaxConcurrentReconciles: cconfig.MaxConcurrentReconciles, RateLimiter: cc.NewRateLimiter(), diff --git a/gateway/internal/controller/gateway_controller.go b/gateway/internal/controller/gateway_controller.go index 6bf89ac7f..eba4c3ef4 100644 --- a/gateway/internal/controller/gateway_controller.go +++ b/gateway/internal/controller/gateway_controller.go @@ -12,8 +12,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/predicate" v1 "github.com/telekom/controlplane/gateway/api/v1" handler "github.com/telekom/controlplane/gateway/internal/handler/gateway" @@ -43,7 +45,7 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&handler.GatewayHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&v1.Gateway{}). + For(&v1.Gateway{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). WithOptions(controller.Options{ MaxConcurrentReconciles: cconfig.MaxConcurrentReconciles, RateLimiter: cc.NewRateLimiter(), diff --git a/gateway/internal/controller/route_controller.go b/gateway/internal/controller/route_controller.go index 72b8c0dbe..100c8e4c7 100644 --- a/gateway/internal/controller/route_controller.go +++ b/gateway/internal/controller/route_controller.go @@ -47,7 +47,7 @@ func (r *RouteReconciler) SetupWithManager(mgr ctrl.Manager) error { r.Controller = cc.NewController(&routehandler.RouteHandler{}, r.Client, r.Recorder) return ctrl.NewControllerManagedBy(mgr). - For(&gatewayv1.Route{}). + For(&gatewayv1.Route{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Watches(&gatewayv1.ConsumeRoute{}, handler.EnqueueRequestsFromMapFunc(r.mapConsumeRouteToRoute), builder.WithPredicates(predicate.GenerationChangedPredicate{})).