diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go index 91026d6727..5eaa3c42d4 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/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/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go index ef40fe4c21..f28289ba07 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go @@ -35,6 +35,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 client 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 @@ -339,6 +357,78 @@ 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1.CustomResourceDefinition, 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1.CustomResourceDefinition, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ApiextensionsV1().CustomResourceDefinitions() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1.CustomResourceDefinition, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +518,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Custom } // 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.ApiextensionsV1().CustomResourceDefinitions() + + _, 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/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go index a5cbe322dc..d981794069 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/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/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go index c1651658a4..7b4921b9b6 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go @@ -35,6 +35,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 client 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 @@ -339,6 +357,78 @@ 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1beta1.CustomResourceDefinition, 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1beta1.CustomResourceDefinition, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ApiextensionsV1beta1().CustomResourceDefinitions() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.CustomResourceDefinition, desiredFinalizers sets.Set[string]) (*v1beta1.CustomResourceDefinition, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +518,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.C } // 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.ApiextensionsV1beta1().CustomResourceDefinitions() + + _, 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/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/controller.go b/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/controller.go index a006f0b9b0..2d6c368859 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/controller.go +++ b/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/reconciler.go b/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/reconciler.go index 215dc90be8..1ad3409a72 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/reconciler.go +++ b/client/injection/kube/reconciler/admissionregistration/v1/mutatingwebhookconfiguration/reconciler.go @@ -32,6 +32,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" admissionregistrationv1 "k8s.io/client-go/listers/admissionregistration/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,78 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.MutatingWebhookConfiguration, 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.MutatingWebhookConfiguration, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AdmissionregistrationV1().MutatingWebhookConfigurations() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.MutatingWebhookConfiguration, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +451,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Mutati } // 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.AdmissionregistrationV1().MutatingWebhookConfigurations() + + _, 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/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/controller.go b/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/controller.go index a71c59bc72..679e07ccba 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/controller.go +++ b/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/reconciler.go b/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/reconciler.go index 6ebc844923..cebbc7349c 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/reconciler.go +++ b/client/injection/kube/reconciler/admissionregistration/v1/validatingwebhookconfiguration/reconciler.go @@ -32,6 +32,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" admissionregistrationv1 "k8s.io/client-go/listers/admissionregistration/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,78 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.ValidatingWebhookConfiguration, 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.ValidatingWebhookConfiguration, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AdmissionregistrationV1().ValidatingWebhookConfigurations() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1.ValidatingWebhookConfiguration, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +451,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Valida } // 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.AdmissionregistrationV1().ValidatingWebhookConfigurations() + + _, 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/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/controller.go b/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/controller.go index 60a336dc9d..28b703e00e 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/controller.go +++ b/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/reconciler.go b/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/reconciler.go index c8ebd2123d..8fc8f5c424 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/reconciler.go +++ b/client/injection/kube/reconciler/admissionregistration/v1beta1/mutatingwebhookconfiguration/reconciler.go @@ -32,6 +32,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" admissionregistrationv1beta1 "k8s.io/client-go/listers/admissionregistration/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,78 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.MutatingWebhookConfiguration, 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.MutatingWebhookConfiguration, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.MutatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.MutatingWebhookConfiguration, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +451,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.M } // 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.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() + + _, 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/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/controller.go b/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/controller.go index 1d09ccbb2e..6c67cdeb73 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/controller.go +++ b/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/reconciler.go b/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/reconciler.go index 12d1879b67..c128d8fadb 100644 --- a/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/reconciler.go +++ b/client/injection/kube/reconciler/admissionregistration/v1beta1/validatingwebhookconfiguration/reconciler.go @@ -32,6 +32,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" admissionregistrationv1beta1 "k8s.io/client-go/listers/admissionregistration/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,78 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.ValidatingWebhookConfiguration, 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.ValidatingWebhookConfiguration, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.ValidatingWebhookConfiguration, desiredFinalizers sets.Set[string]) (*v1beta1.ValidatingWebhookConfiguration, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +451,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.V } // 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.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations() + + _, 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/client/injection/kube/reconciler/apps/v1/deployment/controller.go b/client/injection/kube/reconciler/apps/v1/deployment/controller.go index c6e60be43e..098f6da20f 100644 --- a/client/injection/kube/reconciler/apps/v1/deployment/controller.go +++ b/client/injection/kube/reconciler/apps/v1/deployment/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/client/injection/kube/reconciler/apps/v1/deployment/reconciler.go b/client/injection/kube/reconciler/apps/v1/deployment/reconciler.go index 89fb3306f0..76c3999244 100644 --- a/client/injection/kube/reconciler/apps/v1/deployment/reconciler.go +++ b/client/injection/kube/reconciler/apps/v1/deployment/reconciler.go @@ -34,6 +34,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" appsv1 "k8s.io/client-go/listers/apps/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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.Deployment, desiredFinalizers sets.Set[string]) (*v1.Deployment, 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.Deployment, desiredFinalizers sets.Set[string]) (*v1.Deployment, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AppsV1().Deployments(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Deployment, desiredFinalizers sets.Set[string]) (*v1.Deployment, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Deploy } // 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.AppsV1().Deployments(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/client/injection/kube/reconciler/apps/v1beta1/deployment/controller.go b/client/injection/kube/reconciler/apps/v1beta1/deployment/controller.go index ed4c55916f..3b7964a391 100644 --- a/client/injection/kube/reconciler/apps/v1beta1/deployment/controller.go +++ b/client/injection/kube/reconciler/apps/v1beta1/deployment/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/client/injection/kube/reconciler/apps/v1beta1/deployment/reconciler.go b/client/injection/kube/reconciler/apps/v1beta1/deployment/reconciler.go index b63221063b..5d432aca48 100644 --- a/client/injection/kube/reconciler/apps/v1beta1/deployment/reconciler.go +++ b/client/injection/kube/reconciler/apps/v1beta1/deployment/reconciler.go @@ -34,6 +34,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" appsv1beta1 "k8s.io/client-go/listers/apps/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AppsV1beta1().Deployments(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,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.AppsV1beta1().Deployments(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/client/injection/kube/reconciler/apps/v1beta2/deployment/controller.go b/client/injection/kube/reconciler/apps/v1beta2/deployment/controller.go index 3895861b4d..f11af483c0 100644 --- a/client/injection/kube/reconciler/apps/v1beta2/deployment/controller.go +++ b/client/injection/kube/reconciler/apps/v1beta2/deployment/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/client/injection/kube/reconciler/apps/v1beta2/deployment/reconciler.go b/client/injection/kube/reconciler/apps/v1beta2/deployment/reconciler.go index 7470501c85..f0f98a965f 100644 --- a/client/injection/kube/reconciler/apps/v1beta2/deployment/reconciler.go +++ b/client/injection/kube/reconciler/apps/v1beta2/deployment/reconciler.go @@ -34,6 +34,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" appsv1beta2 "k8s.io/client-go/listers/apps/v1beta2" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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 *v1beta2.Deployment, desiredFinalizers sets.Set[string]) (*v1beta2.Deployment, 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 *v1beta2.Deployment, desiredFinalizers sets.Set[string]) (*v1beta2.Deployment, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.AppsV1beta2().Deployments(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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 *v1beta2.Deployment, desiredFinalizers sets.Set[string]) (*v1beta2.Deployment, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta2.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.AppsV1beta2().Deployments(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/client/injection/kube/reconciler/batch/v1/cronjob/controller.go b/client/injection/kube/reconciler/batch/v1/cronjob/controller.go index fc7f54ab3b..245f7ce490 100644 --- a/client/injection/kube/reconciler/batch/v1/cronjob/controller.go +++ b/client/injection/kube/reconciler/batch/v1/cronjob/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/client/injection/kube/reconciler/batch/v1/cronjob/reconciler.go b/client/injection/kube/reconciler/batch/v1/cronjob/reconciler.go index c5c1344d9a..eb4ef0e84e 100644 --- a/client/injection/kube/reconciler/batch/v1/cronjob/reconciler.go +++ b/client/injection/kube/reconciler/batch/v1/cronjob/reconciler.go @@ -34,6 +34,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" batchv1 "k8s.io/client-go/listers/batch/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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.CronJob, desiredFinalizers sets.Set[string]) (*v1.CronJob, 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.CronJob, desiredFinalizers sets.Set[string]) (*v1.CronJob, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.BatchV1().CronJobs(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.CronJob, desiredFinalizers sets.Set[string]) (*v1.CronJob, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.CronJo } // 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.BatchV1().CronJobs(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/client/injection/kube/reconciler/batch/v1beta1/cronjob/controller.go b/client/injection/kube/reconciler/batch/v1beta1/cronjob/controller.go index 5e194b710f..c23c5399d0 100644 --- a/client/injection/kube/reconciler/batch/v1beta1/cronjob/controller.go +++ b/client/injection/kube/reconciler/batch/v1beta1/cronjob/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/client/injection/kube/reconciler/batch/v1beta1/cronjob/reconciler.go b/client/injection/kube/reconciler/batch/v1beta1/cronjob/reconciler.go index 36fac38e1c..2466b437e0 100644 --- a/client/injection/kube/reconciler/batch/v1beta1/cronjob/reconciler.go +++ b/client/injection/kube/reconciler/batch/v1beta1/cronjob/reconciler.go @@ -34,6 +34,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" batchv1beta1 "k8s.io/client-go/listers/batch/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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.CronJob, desiredFinalizers sets.Set[string]) (*v1beta1.CronJob, 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.CronJob, desiredFinalizers sets.Set[string]) (*v1beta1.CronJob, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.BatchV1beta1().CronJobs(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.CronJob, desiredFinalizers sets.Set[string]) (*v1beta1.CronJob, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.C } // 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.BatchV1beta1().CronJobs(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/client/injection/kube/reconciler/core/v1/configmap/controller.go b/client/injection/kube/reconciler/core/v1/configmap/controller.go index 65df48dbb7..7f4887acb0 100644 --- a/client/injection/kube/reconciler/core/v1/configmap/controller.go +++ b/client/injection/kube/reconciler/core/v1/configmap/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/core/v1/configmap/reconciler.go b/client/injection/kube/reconciler/core/v1/configmap/reconciler.go index 6be19856a5..cd7677263c 100644 --- a/client/injection/kube/reconciler/core/v1/configmap/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/configmap/reconciler.go @@ -31,6 +31,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" @@ -96,6 +97,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 } // Check that our Reconciler implements controller.Reconciler. @@ -150,6 +160,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 @@ -271,6 +289,80 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.ConfigMap, desiredFinalizers sets.Set[string]) (*v1.ConfigMap, 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.ConfigMap, desiredFinalizers sets.Set[string]) (*v1.ConfigMap, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().ConfigMaps(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.ConfigMap, desiredFinalizers sets.Set[string]) (*v1.ConfigMap, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -360,5 +452,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.CoreV1().ConfigMaps(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/client/injection/kube/reconciler/core/v1/namespace/controller.go b/client/injection/kube/reconciler/core/v1/namespace/controller.go index 34c0fcd3be..57b6f71b3a 100644 --- a/client/injection/kube/reconciler/core/v1/namespace/controller.go +++ b/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/client/injection/kube/reconciler/core/v1/namespace/reconciler.go b/client/injection/kube/reconciler/core/v1/namespace/reconciler.go index 04a3fe0cbd..1b698c5202 100644 --- a/client/injection/kube/reconciler/core/v1/namespace/reconciler.go +++ b/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,78 @@ 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...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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 +517,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/client/injection/kube/reconciler/core/v1/node/controller.go b/client/injection/kube/reconciler/core/v1/node/controller.go index e35d7c7316..dd85e9fa01 100644 --- a/client/injection/kube/reconciler/core/v1/node/controller.go +++ b/client/injection/kube/reconciler/core/v1/node/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/client/injection/kube/reconciler/core/v1/node/reconciler.go b/client/injection/kube/reconciler/core/v1/node/reconciler.go index b6ede315ad..581f98555c 100644 --- a/client/injection/kube/reconciler/core/v1/node/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/node/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,78 @@ 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.Node, desiredFinalizers sets.Set[string]) (*v1.Node, 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.Node, desiredFinalizers sets.Set[string]) (*v1.Node, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().Nodes() + + 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Node, desiredFinalizers sets.Set[string]) (*v1.Node, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -427,5 +517,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Node, } // 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().Nodes() + + _, 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/client/injection/kube/reconciler/core/v1/pod/controller.go b/client/injection/kube/reconciler/core/v1/pod/controller.go index ca96c4878c..e97c5f07a8 100644 --- a/client/injection/kube/reconciler/core/v1/pod/controller.go +++ b/client/injection/kube/reconciler/core/v1/pod/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/client/injection/kube/reconciler/core/v1/pod/reconciler.go b/client/injection/kube/reconciler/core/v1/pod/reconciler.go index e8fcc6524c..06cf6de976 100644 --- a/client/injection/kube/reconciler/core/v1/pod/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/pod/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,80 @@ 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.Pod, desiredFinalizers sets.Set[string]) (*v1.Pod, 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.Pod, desiredFinalizers sets.Set[string]) (*v1.Pod, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().Pods(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Pod, desiredFinalizers sets.Set[string]) (*v1.Pod, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -427,5 +519,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Pod, r } // 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().Pods(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/client/injection/kube/reconciler/core/v1/secret/controller.go b/client/injection/kube/reconciler/core/v1/secret/controller.go index 16a3aa2105..660359d628 100644 --- a/client/injection/kube/reconciler/core/v1/secret/controller.go +++ b/client/injection/kube/reconciler/core/v1/secret/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/core/v1/secret/reconciler.go b/client/injection/kube/reconciler/core/v1/secret/reconciler.go index d25d4b3a68..b01bbaf825 100644 --- a/client/injection/kube/reconciler/core/v1/secret/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/secret/reconciler.go @@ -31,6 +31,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" @@ -96,6 +97,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 } // Check that our Reconciler implements controller.Reconciler. @@ -150,6 +160,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 @@ -271,6 +289,80 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.Secret, desiredFinalizers sets.Set[string]) (*v1.Secret, 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.Secret, desiredFinalizers sets.Set[string]) (*v1.Secret, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().Secrets(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Secret, desiredFinalizers sets.Set[string]) (*v1.Secret, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -360,5 +452,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Secret } // 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().Secrets(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/client/injection/kube/reconciler/core/v1/service/controller.go b/client/injection/kube/reconciler/core/v1/service/controller.go index d73019e125..52445d8b9a 100644 --- a/client/injection/kube/reconciler/core/v1/service/controller.go +++ b/client/injection/kube/reconciler/core/v1/service/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/client/injection/kube/reconciler/core/v1/service/reconciler.go b/client/injection/kube/reconciler/core/v1/service/reconciler.go index e23302aedf..ba7faf25fe 100644 --- a/client/injection/kube/reconciler/core/v1/service/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/service/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,80 @@ 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...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().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, v1.EventTypeWarning, "FinalizerUpdateFailed", + "Failed to update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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() @@ -427,5 +519,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.CoreV1().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/client/injection/kube/reconciler/core/v1/serviceaccount/controller.go b/client/injection/kube/reconciler/core/v1/serviceaccount/controller.go index cae21cc885..441f102bb6 100644 --- a/client/injection/kube/reconciler/core/v1/serviceaccount/controller.go +++ b/client/injection/kube/reconciler/core/v1/serviceaccount/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/core/v1/serviceaccount/reconciler.go b/client/injection/kube/reconciler/core/v1/serviceaccount/reconciler.go index bb6d8aa33a..af426003e0 100644 --- a/client/injection/kube/reconciler/core/v1/serviceaccount/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/serviceaccount/reconciler.go @@ -31,6 +31,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" @@ -96,6 +97,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 } // Check that our Reconciler implements controller.Reconciler. @@ -150,6 +160,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 @@ -271,6 +289,80 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.ServiceAccount, desiredFinalizers sets.Set[string]) (*v1.ServiceAccount, 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.ServiceAccount, desiredFinalizers sets.Set[string]) (*v1.ServiceAccount, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.CoreV1().ServiceAccounts(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.ServiceAccount, desiredFinalizers sets.Set[string]) (*v1.ServiceAccount, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -360,5 +452,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.CoreV1().ServiceAccounts(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/client/injection/kube/reconciler/extensions/v1beta1/deployment/controller.go b/client/injection/kube/reconciler/extensions/v1beta1/deployment/controller.go index 6e7a912ebc..8eb1f55c9d 100644 --- a/client/injection/kube/reconciler/extensions/v1beta1/deployment/controller.go +++ b/client/injection/kube/reconciler/extensions/v1beta1/deployment/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/client/injection/kube/reconciler/extensions/v1beta1/deployment/reconciler.go b/client/injection/kube/reconciler/extensions/v1beta1/deployment/reconciler.go index b83e54416e..1ad31848e1 100644 --- a/client/injection/kube/reconciler/extensions/v1beta1/deployment/reconciler.go +++ b/client/injection/kube/reconciler/extensions/v1beta1/deployment/reconciler.go @@ -34,6 +34,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" extensionsv1beta1 "k8s.io/client-go/listers/extensions/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -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 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 @@ -339,6 +357,80 @@ 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ExtensionsV1beta1().Deployments(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.Deployment, desiredFinalizers sets.Set[string]) (*v1beta1.Deployment, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -428,5 +520,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.ExtensionsV1beta1().Deployments(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/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/controller.go b/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/controller.go index 72051d41a8..626e9a2ce1 100644 --- a/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/controller.go +++ b/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/reconciler.go b/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/reconciler.go index 548bbb5ee8..ef3cf58d12 100644 --- a/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/reconciler.go +++ b/client/injection/kube/reconciler/extensions/v1beta1/networkpolicy/reconciler.go @@ -32,6 +32,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" extensionsv1beta1 "k8s.io/client-go/listers/extensions/v1beta1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,80 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1beta1.NetworkPolicy, 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1beta1.NetworkPolicy, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.ExtensionsV1beta1().NetworkPolicies(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, v1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1beta1.NetworkPolicy, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +453,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1beta1.N } // 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.ExtensionsV1beta1().NetworkPolicies(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/client/injection/kube/reconciler/networking/v1/networkpolicy/controller.go b/client/injection/kube/reconciler/networking/v1/networkpolicy/controller.go index 132dcc4ff0..8cc4fec043 100644 --- a/client/injection/kube/reconciler/networking/v1/networkpolicy/controller.go +++ b/client/injection/kube/reconciler/networking/v1/networkpolicy/controller.go @@ -128,6 +128,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/client/injection/kube/reconciler/networking/v1/networkpolicy/reconciler.go b/client/injection/kube/reconciler/networking/v1/networkpolicy/reconciler.go index 892f14e457..9446df5ffa 100644 --- a/client/injection/kube/reconciler/networking/v1/networkpolicy/reconciler.go +++ b/client/injection/kube/reconciler/networking/v1/networkpolicy/reconciler.go @@ -32,6 +32,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" networkingv1 "k8s.io/client-go/listers/networking/v1" record "k8s.io/client-go/tools/record" controller "knative.dev/pkg/controller" @@ -97,6 +98,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 } // Check that our Reconciler implements controller.Reconciler. @@ -151,6 +161,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 @@ -272,6 +290,80 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1.NetworkPolicy, 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1.NetworkPolicy, error) { + // Check if we need to do anything + existingFinalizers := sets.New[string](resource.Finalizers...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := scheme.Scheme.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, fmt.Errorf("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": finalizers, + }, + } + + applyConfig["metadata"].(map[string]interface{})["namespace"] = resource.Namespace + + patch, err := json.Marshal(applyConfig) + if err != nil { + return resource, err + } + + patcher := r.Client.NetworkingV1().NetworkPolicies(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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, corev1.EventTypeNormal, "FinalizerUpdate", + "Updated finalizers 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.NetworkPolicy, desiredFinalizers sets.Set[string]) (*v1.NetworkPolicy, error) { // Don't modify the informers copy. existing := resource.DeepCopy() @@ -361,5 +453,28 @@ func (r *reconcilerImpl) clearFinalizer(ctx context.Context, resource *v1.Networ } // 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.NetworkingV1().NetworkPolicies(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/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go b/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go index 771fdecd39..a641670d38 100644 --- a/codegen/cmd/injection-gen/generators/reconciler/reconciler_controller.go +++ b/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/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go b/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go index c416bcae8c..f0a3f59050 100644 --- a/codegen/cmd/injection-gen/generators/reconciler/reconciler_reconciler.go +++ b/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,85 @@ 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...) + + var finalizers []string + 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. + finalizers = []string{r.finalizerName} + } 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. + finalizers = []string{} // Empty array removes our managed finalizers + } + + // Determine GVK + gvks, _, err := {{.schemeScheme|raw}}.ObjectKinds(resource) + if err != nil || len(gvks) == 0 { + return resource, {{.fmtErrorf|raw}}("failed to determine GVK for resource: %w", err) + } + gvk := gvks[0] + + // Create apply configuration + applyConfig := map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "name": resource.Name, + "uid": resource.UID, + "finalizers": 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 update finalizers for %q via server-side apply: %v", resource.Name, err) + } else { + r.Recorder.Eventf(updated, {{.corev1EventTypeNormal|raw}}, "FinalizerUpdate", + "Updated finalizers 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 +801,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/controller/options.go b/controller/options.go index 574ef003f4..e20cb7cba5 100644 --- a/controller/options.go +++ b/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