From d8c5838b8e4fd2235a4f8bfa4c84d8838481e612 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Mon, 3 Nov 2025 12:52:05 -0500 Subject: [PATCH 1/2] run codegen --- go.mod | 2 + go.sum | 4 +- .../autoscaling/v1alpha1/metric/controller.go | 8 + .../autoscaling/v1alpha1/metric/reconciler.go | 150 +++++++++++++++- .../v1alpha1/podautoscaler/controller.go | 8 + .../v1alpha1/podautoscaler/reconciler.go | 150 +++++++++++++++- .../serving/v1/configuration/controller.go | 8 + .../serving/v1/configuration/reconciler.go | 150 +++++++++++++++- .../serving/v1/revision/controller.go | 8 + .../serving/v1/revision/reconciler.go | 150 +++++++++++++++- .../reconciler/serving/v1/route/controller.go | 8 + .../reconciler/serving/v1/route/reconciler.go | 150 +++++++++++++++- .../serving/v1/service/controller.go | 8 + .../serving/v1/service/reconciler.go | 150 +++++++++++++++- .../v1beta1/domainmapping/controller.go | 8 + .../v1beta1/domainmapping/reconciler.go | 150 +++++++++++++++- .../core/v1/namespace/controller.go | 8 + .../core/v1/namespace/reconciler.go | 146 ++++++++++++++- .../reconciler/reconciler_controller.go | 8 + .../reconciler/reconciler_reconciler.go | 168 +++++++++++++++++- vendor/knative.dev/pkg/controller/options.go | 16 ++ .../pkg/controller/queue_metrics.go | 4 - vendor/modules.txt | 3 +- 23 files changed, 1449 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index e57c441f10ca..29064c365920 100644 --- a/go.mod +++ b/go.mod @@ -166,3 +166,5 @@ require ( sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect ) + +replace knative.dev/pkg => github.com/enarha/knative-pkg v0.0.0-20251021085811-363a01daebe3 diff --git a/go.sum b/go.sum index e58514636e48..da9d337d04e8 100644 --- a/go.sum +++ b/go.sum @@ -113,6 +113,8 @@ github.com/docker/docker-credential-helpers v0.8.2 h1:bX3YxiGzFP5sOXWc3bTPEXdEaZ github.com/docker/docker-credential-helpers v0.8.2/go.mod h1:P3ci7E3lwkZg6XiHdRKft1KckHiO9a2rNtyFbZ/ry9M= github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= +github.com/enarha/knative-pkg v0.0.0-20251021085811-363a01daebe3 h1:thmo9O5nk01o0ftphnB+ElVscRpf6g8O9/RsGDUO/Uk= +github.com/enarha/knative-pkg v0.0.0-20251021085811-363a01daebe3/go.mod h1:TNEqAnhRXuC9a+mUz10UEn7VqOsA2YMlqnZMfyBuoP8= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= @@ -592,8 +594,6 @@ knative.dev/hack v0.0.0-20251021013703-4fae78067103 h1:j96YY5CLCTytWZsGVzixVvNas knative.dev/hack v0.0.0-20251021013703-4fae78067103/go.mod h1:L5RzHgbvam0u8QFHfzCX6MKxu/a/gIGEdaRBqNiVbl0= knative.dev/networking v0.0.0-20251021092443-0bde19154dce h1:INRJxej8DPwRUfXzuBQ6Ldg2/QaHF1tZrBUA8LVTaso= knative.dev/networking v0.0.0-20251021092443-0bde19154dce/go.mod h1:CMOeNLRWFxcMQALbhhwr6XmsyMX7EuSHNi+Gzfq0HE0= -knative.dev/pkg v0.0.0-20251022152246-7bf6febca0b3 h1:472SARbX5rEir4g3QC/UsKnHNXqdRNbWOe2a4DcJRwk= -knative.dev/pkg v0.0.0-20251022152246-7bf6febca0b3/go.mod h1:8L1vgh3WoZ4OH9gspPSt3QFcMJsrOUBwOs0FuM5Jne8= pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= sigs.k8s.io/gateway-api v1.1.0 h1:DsLDXCi6jR+Xz8/xd0Z1PYl2Pn0TyaFMOPPZIj4inDM= diff --git a/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/controller.go b/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/controller.go index efe5c3dd2ca0..410fc3cb91f1 100644 --- a/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/controller.go +++ b/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/reconciler.go b/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/reconciler.go index c4ec7f538baa..a5ea63bd46c6 100644 --- a/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/reconciler.go +++ b/pkg/client/injection/reconciler/autoscaling/v1alpha1/metric/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1alpha1.Metric, desiredFinalizers sets.Set[string]) (*v1alpha1.Metric, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1alpha1.Metric, desiredFinalizers sets.Set[string]) (*v1alpha1.Metric, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AutoscalingV1alpha1().Metrics(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AutoscalingV1alpha1().Metrics(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1alpha1.Metric, desiredFinalizers sets.Set[string]) (*v1alpha1.Metric, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1alpha1. } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.AutoscalingV1alpha1().Metrics(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/controller.go b/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/controller.go index e8605bed2ac2..4bd35451e8e7 100644 --- a/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/controller.go +++ b/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/controller.go @@ -137,6 +137,14 @@ func NewImpl(ctx context.Context, r Interface, classValue string, optionsFns ... if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/reconciler.go b/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/reconciler.go index 473f8c4f6d81..089d482d8ea3 100644 --- a/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/reconciler.go +++ b/pkg/client/injection/reconciler/autoscaling/v1alpha1/podautoscaler/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -165,6 +175,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -358,6 +376,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1alpha1.PodAutoscaler, desiredFinalizers sets.Set[string]) (*v1alpha1.PodAutoscaler, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1alpha1.PodAutoscaler, desiredFinalizers sets.Set[string]) (*v1alpha1.PodAutoscaler, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AutoscalingV1alpha1().PodAutoscalers(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AutoscalingV1alpha1().PodAutoscalers(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1alpha1.PodAutoscaler, desiredFinalizers sets.Set[string]) (*v1alpha1.PodAutoscaler, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -447,5 +572,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1alpha1. } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.AutoscalingV1alpha1().PodAutoscalers(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/serving/v1/configuration/controller.go b/pkg/client/injection/reconciler/serving/v1/configuration/controller.go index dd2aa3986ac3..d9088d19f984 100644 --- a/pkg/client/injection/reconciler/serving/v1/configuration/controller.go +++ b/pkg/client/injection/reconciler/serving/v1/configuration/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/serving/v1/configuration/reconciler.go b/pkg/client/injection/reconciler/serving/v1/configuration/reconciler.go index 97fe189c299e..7527504e4829 100644 --- a/pkg/client/injection/reconciler/serving/v1/configuration/reconciler.go +++ b/pkg/client/injection/reconciler/serving/v1/configuration/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1.Configuration, desiredFinalizers sets.Set[string]) (*v1.Configuration, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1.Configuration, desiredFinalizers sets.Set[string]) (*v1.Configuration, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Configurations(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Configurations(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1.Configuration, desiredFinalizers sets.Set[string]) (*v1.Configuration, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Config } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.ServingV1().Configurations(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/serving/v1/revision/controller.go b/pkg/client/injection/reconciler/serving/v1/revision/controller.go index 3525174a6d89..bced01ad5a28 100644 --- a/pkg/client/injection/reconciler/serving/v1/revision/controller.go +++ b/pkg/client/injection/reconciler/serving/v1/revision/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/serving/v1/revision/reconciler.go b/pkg/client/injection/reconciler/serving/v1/revision/reconciler.go index a122b1c70db4..ef37d1b2f07a 100644 --- a/pkg/client/injection/reconciler/serving/v1/revision/reconciler.go +++ b/pkg/client/injection/reconciler/serving/v1/revision/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1.Revision, desiredFinalizers sets.Set[string]) (*v1.Revision, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1.Revision, desiredFinalizers sets.Set[string]) (*v1.Revision, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Revisions(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Revisions(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1.Revision, desiredFinalizers sets.Set[string]) (*v1.Revision, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Revisi } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.ServingV1().Revisions(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/serving/v1/route/controller.go b/pkg/client/injection/reconciler/serving/v1/route/controller.go index 7781ae752445..ca17c9fee113 100644 --- a/pkg/client/injection/reconciler/serving/v1/route/controller.go +++ b/pkg/client/injection/reconciler/serving/v1/route/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/serving/v1/route/reconciler.go b/pkg/client/injection/reconciler/serving/v1/route/reconciler.go index b3dfa2da83ba..ecc2513ceb43 100644 --- a/pkg/client/injection/reconciler/serving/v1/route/reconciler.go +++ b/pkg/client/injection/reconciler/serving/v1/route/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1.Route, desiredFinalizers sets.Set[string]) (*v1.Route, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1.Route, desiredFinalizers sets.Set[string]) (*v1.Route, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Routes(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Routes(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1.Route, desiredFinalizers sets.Set[string]) (*v1.Route, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Route, } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.ServingV1().Routes(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/serving/v1/service/controller.go b/pkg/client/injection/reconciler/serving/v1/service/controller.go index 7d4a99eeccd0..a5dad09b19bf 100644 --- a/pkg/client/injection/reconciler/serving/v1/service/controller.go +++ b/pkg/client/injection/reconciler/serving/v1/service/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/serving/v1/service/reconciler.go b/pkg/client/injection/reconciler/serving/v1/service/reconciler.go index 54cc0892f479..2b5f05416c39 100644 --- a/pkg/client/injection/reconciler/serving/v1/service/reconciler.go +++ b/pkg/client/injection/reconciler/serving/v1/service/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1.Service, desiredFinalizers sets.Set[string]) (*v1.Service, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1.Service, desiredFinalizers sets.Set[string]) (*v1.Service, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Services(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1().Services(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, corev1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1.Service, desiredFinalizers sets.Set[string]) (*v1.Service, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Servic } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.ServingV1().Services(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/controller.go b/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/controller.go index f39a1b6155e5..0ff55e6ae80b 100644 --- a/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/controller.go +++ b/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/controller.go @@ -133,6 +133,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/reconciler.go b/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/reconciler.go index e0ee2fcf7330..a1d80a76759f 100644 --- a/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/reconciler.go +++ b/pkg/client/injection/reconciler/serving/v1beta1/domainmapping/reconciler.go @@ -32,6 +32,7 @@ import ( labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" + scheme "k8s.io/client-go/kubernetes/scheme" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" kmp "knative.dev/pkg/kmp" @@ -101,6 +102,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -161,6 +171,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client versio if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -347,6 +365,113 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1beta1.DomainMapping, desiredFinalizers sets.Set[string]) (*v1beta1.DomainMapping, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1beta1.DomainMapping, desiredFinalizers sets.Set[string]) (*v1beta1.DomainMapping, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1beta1().DomainMappings(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ServingV1beta1().DomainMappings(resource.Namespace) + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1beta1.DomainMapping, desiredFinalizers sets.Set[string]) (*v1beta1.DomainMapping, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -436,5 +561,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.D } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.ServingV1beta1().DomainMappings(resource.Namespace) + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/controller.go b/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/controller.go index 34c0fcd3be4a..57b6f71b3a89 100644 --- a/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/controller.go +++ b/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/controller.go @@ -131,6 +131,14 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/reconciler.go b/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/reconciler.go index 04a3fe0cbde5..5a8e21796aac 100644 --- a/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/reconciler.go +++ b/vendor/knative.dev/pkg/client/injection/kube/reconciler/core/v1/namespace/reconciler.go @@ -33,6 +33,7 @@ import ( types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" kubernetes "k8s.io/client-go/kubernetes" + scheme "k8s.io/client-go/kubernetes/scheme" corev1 "k8s.io/client-go/listers/core/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -100,6 +101,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. skipStatusUpdates bool @@ -160,6 +170,14 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client kubern if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -338,6 +356,109 @@ func (r *reconcilerImpl) updateStatus(ctx context.Context, logger *zap.SugaredLo // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx context.Context, resource *v1.Namespace, desiredFinalizers sets.Set[string]) (*v1.Namespace, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx context.Context, resource *v1.Namespace, desiredFinalizers sets.Set[string]) (*v1.Namespace, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().Namespaces() + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().Namespaces() + + patchOpts := metav1.PatchOptions{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, types.ApplyPatchType, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx context.Context, resource *v1.Namespace, desiredFinalizers sets.Set[string]) (*v1.Namespace, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -427,5 +548,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Namesp } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := logging.FromContext(ctx) + + getter := r.Client.CoreV1().Namespaces() + + _, getErr := getter.Get(ctx, resource.Name, metav1.GetOptions{}) + if errors.IsNotFound(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go index 771fdecd393e..a641670d38d6 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go @@ -303,6 +303,14 @@ func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classValu if opts.PromoteFunc != nil { promoteFunc = opts.PromoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } rec.Recorder = createRecorder(ctx, agentName) diff --git a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go index c416bcae8c5f..c87d093a7883 100644 --- a/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go +++ b/vendor/knative.dev/pkg/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go @@ -178,6 +178,11 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty "equalitySemantic": c.Universe.Package("k8s.io/apimachinery/pkg/api/equality").Variable("Semantic"), "jsonMarshal": c.Universe.Package("encoding/json").Function("Marshal"), "typesMergePatchType": c.Universe.Package("k8s.io/apimachinery/pkg/types").Constant("MergePatchType"), + "typesApplyPatchType": c.Universe.Package("k8s.io/apimachinery/pkg/types").Constant("ApplyPatchType"), + "schemeScheme": c.Universe.Function(types.Name{ + Package: "k8s.io/client-go/kubernetes/scheme", + Name: "Scheme", + }), "syncRWMutex": c.Universe.Type(types.Name{ Package: "sync", Name: "RWMutex", @@ -309,6 +314,15 @@ type reconcilerImpl struct { // finalizerName is the name of the finalizer to reconcile. finalizerName string + // useServerSideApplyForFinalizers configures whether to use server-side apply for finalizer management + useServerSideApplyForFinalizers bool + + // finalizerFieldManager is the field manager name for server-side apply of finalizers + finalizerFieldManager string + + // forceApplyFinalizers configures whether to force server-side apply for finalizers + forceApplyFinalizers bool + {{if .hasStatus}} // skipStatusUpdates configures whether or not this reconciler automatically updates // the status of the reconciled resource. @@ -388,6 +402,14 @@ func NewReconciler(ctx {{.contextContext|raw}}, logger *{{.zapSugaredLogger|raw} if opts.DemoteFunc != nil { rec.DemoteFunc = opts.DemoteFunc } + if opts.UseServerSideApplyForFinalizers { + if opts.FinalizerFieldManager == "" { + logger.Fatal("FinalizerFieldManager must be provided when UseServerSideApplyForFinalizers is enabled") + } + rec.useServerSideApplyForFinalizers = true + rec.finalizerFieldManager = opts.FinalizerFieldManager + rec.forceApplyFinalizers = opts.ForceApplyFinalizers + } } return rec @@ -608,6 +630,123 @@ var reconcilerFinalizerFactory = ` // TODO: this method could be generic and sync all finalizers. For now it only // updates defaultFinalizerName or its override. func (r *reconcilerImpl) updateFinalizersFiltered(ctx {{.contextContext|raw}}, resource *{{.type|raw}}, desiredFinalizers {{.setsString|raw}}) (*{{.type|raw}}, error) { + if r.useServerSideApplyForFinalizers { + return r.updateFinalizersFilteredServerSideApply(ctx, resource, desiredFinalizers) + } + return r.updateFinalizersFilteredMergePatch(ctx, resource, desiredFinalizers) +} + +// updateFinalizersFilteredServerSideApply uses server-side apply to manage only this controller's finalizer. +func (r *reconcilerImpl) updateFinalizersFilteredServerSideApply(ctx {{.contextContext|raw}}, resource *{{.type|raw}}, desiredFinalizers {{.setsString|raw}}) (*{{.type|raw}}, error) { + // Check if we need to do anything + existingFinalizers := {{.setsNewString|raw}}(resource.Finalizers...) + + if desiredFinalizers.Has(r.finalizerName) { + if existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // Apply configuration with only our finalizer to add it. + gvks, _, err := {{.schemeScheme|raw}}.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, {{.fmtErrorf|raw}}("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{r.finalizerName}, + }, + } + {{if not .nonNamespaced}} + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + {{end}} + + patch, err := {{.jsonMarshal|raw}}(applyConfig) + if err != nil { + return resource, err + } + + {{if .nonNamespaced}} + patcher := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}() + {{else}} + patcher := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}(resource.Namespace) + {{end}} + + patchOpts := {{.metav1PatchOptions|raw}}{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, {{.typesApplyPatchType|raw}}, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, {{.corev1EventTypeWarning|raw}}, "FinalizerUpdateFailed", + "Failed to add finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, {{.corev1EventTypeNormal|raw}}, "FinalizerUpdate", + "Added finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } else { + if !existingFinalizers.Has(r.finalizerName) { + // Nothing to do. + return resource, nil + } + // For removal, we apply an empty configuration for our finalizer field manager. + // This effectively removes our finalizer while preserving others. + gvks, _, err := {{.schemeScheme|raw}}.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, {{.fmtErrorf|raw}}("failed to determine GVK for resource: %v", err) + } + gvk := gvks[0] + + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": []string{}, // Empty array removes our managed finalizers + }, + } + {{if not .nonNamespaced}} + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + {{end}} + + patch, err := {{.jsonMarshal|raw}}(applyConfig) + if err != nil { + return resource, err + } + + {{if .nonNamespaced}} + patcher := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}() + {{else}} + patcher := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}(resource.Namespace) + {{end}} + + patchOpts := {{.metav1PatchOptions|raw}}{ + FieldManager: r.finalizerFieldManager, + Force: &r.forceApplyFinalizers, + } + + updated, err := patcher.Patch(ctx, resource.Name, {{.typesApplyPatchType|raw}}, patch, patchOpts) + if err != nil { + r.Recorder.Eventf(resource, {{.corev1EventTypeWarning|raw}}, "FinalizerUpdateFailed", + "Failed to remove finalizer for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, {{.corev1EventTypeNormal|raw}}, "FinalizerUpdate", + "Removed finalizer for %q via server-side apply", resource.GetName()) + } + return updated, err + } +} + +// updateFinalizersFilteredMergePatch uses merge patch to manage finalizers (legacy behavior). +func (r *reconcilerImpl) updateFinalizersFilteredMergePatch(ctx {{.contextContext|raw}}, resource *{{.type|raw}}, desiredFinalizers {{.setsString|raw}}) (*{{.type|raw}}, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -700,7 +839,34 @@ func (r *reconcilerImpl) clearFinalizer(ctx {{.contextContext|raw}}, resource *{ } // Synchronize the finalizers filtered by r.finalizerName. - return r.updateFinalizersFiltered(ctx, resource, finalizers) + updated, err := r.updateFinalizersFiltered(ctx, resource, finalizers) + if err != nil { + // Check if the resource still exists by querying the API server to avoid logging errors + // when reconciling stale object from cache while the object is actually deleted. + logger := {{.loggingFromContext|raw}}(ctx) + + {{if .nonNamespaced}} + getter := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}() + {{else}} + getter := r.Client.{{.group}}{{.version}}().{{.type|apiGroup}}(resource.Namespace) + {{end}} + + _, getErr := getter.Get(ctx, resource.Name, {{.metav1GetOptions|raw}}{}) + if {{.apierrsIsNotFound|raw}}(getErr) { + // Resource no longer exists, which could happen during deletion + logger.Debugw("Resource no longer exists while clearing finalizers", + "resource", resource.GetName(), + "namespace", resource.GetNamespace(), + "originalError", err) + // Return the original resource since the finalizer clearing is effectively complete + return resource, nil + } + + // For other errors, return the original error + return updated, err + } + + return updated, nil } ` diff --git a/vendor/knative.dev/pkg/controller/options.go b/vendor/knative.dev/pkg/controller/options.go index 574ef003f4f3..e20cb7cba530 100644 --- a/vendor/knative.dev/pkg/controller/options.go +++ b/vendor/knative.dev/pkg/controller/options.go @@ -28,6 +28,22 @@ type Options struct { // overrides a default finalizer name assigned by the generator if needed. FinalizerName string + // UseServerSideApplyForFinalizers enables server-side apply for finalizer management. + // When enabled, this reconciler will use server-side apply instead of merge patch + // to manage its finalizer, reducing conflicts when multiple controllers manage + // different finalizers on the same resource. + UseServerSideApplyForFinalizers bool + + // FinalizerFieldManager specifies the field manager name for server-side apply + // finalizer operations. This is required when UseServerSideApplyForFinalizers + // is true and should be unique to avoid conflicts with other controllers. + FinalizerFieldManager string + + // ForceApplyFinalizers configures whether to use the Force option when applying + // finalizers via server-side apply. This can resolve conflicts but should be + // used carefully as it can override other field managers. + ForceApplyFinalizers bool + // AgentName is the name of the agent this reconciler uses. This overrides // the default controller's agent name. AgentName string diff --git a/vendor/knative.dev/pkg/controller/queue_metrics.go b/vendor/knative.dev/pkg/controller/queue_metrics.go index 462a0b91ca28..2e61e330ea51 100644 --- a/vendor/knative.dev/pkg/controller/queue_metrics.go +++ b/vendor/knative.dev/pkg/controller/queue_metrics.go @@ -121,10 +121,6 @@ func (m *queueMetrics) updateUnfinishedWork() { // doesn't seem to have non-hacky ways to reset the summary metrics. var total float64 var oldest float64 - - m.mu.Lock() - defer m.mu.Unlock() - for _, t := range m.processingStartTimes { age := m.sinceInSeconds(t) total += age diff --git a/vendor/modules.txt b/vendor/modules.txt index a9c18790a800..12aac615e45a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1514,7 +1514,7 @@ knative.dev/networking/pkg/http/stats knative.dev/networking/pkg/ingress knative.dev/networking/pkg/k8s knative.dev/networking/pkg/prober -# knative.dev/pkg v0.0.0-20251022152246-7bf6febca0b3 +# knative.dev/pkg v0.0.0-20251022152246-7bf6febca0b3 => github.com/enarha/knative-pkg v0.0.0-20251021085811-363a01daebe3 ## explicit; go 1.24.0 knative.dev/pkg/apiextensions/storageversion knative.dev/pkg/apiextensions/storageversion/cmd/migrate @@ -1662,3 +1662,4 @@ sigs.k8s.io/structured-merge-diff/v4/value ## explicit; go 1.22 sigs.k8s.io/yaml sigs.k8s.io/yaml/goyaml.v2 +# knative.dev/pkg => github.com/enarha/knative-pkg v0.0.0-20251021085811-363a01daebe3 From 8114073d64d180fcfdae3512b4b3806d83742b01 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Mon, 3 Nov 2025 12:55:23 -0500 Subject: [PATCH 2/2] enable SSA for controllers with finalizers --- pkg/reconciler/domainmapping/controller.go | 6 +++++- pkg/reconciler/labeler/controller.go | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/domainmapping/controller.go b/pkg/reconciler/domainmapping/controller.go index e22eb3e99021..a53091ea1e84 100644 --- a/pkg/reconciler/domainmapping/controller.go +++ b/pkg/reconciler/domainmapping/controller.go @@ -59,7 +59,11 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl }) configStore := config.NewStore(logging.WithLogger(ctx, logger.Named("config-store")), resync) configStore.WatchConfigs(cmw) - return controller.Options{ConfigStore: configStore} + return controller.Options{ + ConfigStore: configStore, + UseServerSideApplyForFinalizers: true, + FinalizerFieldManager: "domain-mapping", + } }) domainmappingInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) diff --git a/pkg/reconciler/labeler/controller.go b/pkg/reconciler/labeler/controller.go index 3585aab86c02..7563f9902730 100644 --- a/pkg/reconciler/labeler/controller.go +++ b/pkg/reconciler/labeler/controller.go @@ -54,7 +54,9 @@ func NewController( return controller.Options{ ConfigStore: configStore, // The labeler shouldn't mutate the route's status. - SkipStatusUpdates: true, + SkipStatusUpdates: true, + UseServerSideApplyForFinalizers: true, + FinalizerFieldManager: "labeler", } })