Remove obsolete finalizer from KnativeServing resources.

pkg/reconciler/knativeserving/knativeserving.go

@@ -18,6 +18,7 @@ package knativeserving
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	mf "github.com/manifestival/manifestival"
@@ -29,6 +30,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 	servingv1alpha1 "knative.dev/operator/pkg/apis/operator/v1alpha1"
@@ -43,10 +46,10 @@ import (
 )
 
 const (
-	finalizerName  = "delete-knative-serving-manifest"
-	creationChange = "creation"
-	editChange     = "edit"
-	deletionChange = "deletion"
+	oldFinalizerName = "delete-knative-serving-manifest"
+	creationChange   = "creation"
+	editChange       = "edit"
+	deletionChange   = "deletion"
 )
 
 var (
@@ -87,7 +90,22 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *servingv1alpha1
 	if _, ok := r.servings[key]; ok {
 		delete(r.servings, key)
 	}
-	return r.delete(ctx, original)
+
+	logger.Info("Deleting resources")
+	var RBAC = mf.Any(mf.ByKind("Role"), mf.ByKind("ClusterRole"), mf.ByKind("RoleBinding"), mf.ByKind("ClusterRoleBinding"))
+	if len(r.servings) == 0 {
+		if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil {
+			return err
+		}
+		if err := r.config.Filter(mf.NoCRDs, mf.None(RBAC)).Delete(); err != nil {
+			return err
+		}
+		// Delete Roles last, as they may be useful for human operators to clean up.
+		if err := r.config.Filter(RBAC).Delete(); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // ReconcileKind compares the actual state with the desired, and attempts to
@@ -135,7 +153,7 @@ func (r *Reconciler) reconcile(ctx context.Context, ks *servingv1alpha1.KnativeS
 	reqLogger.Infow("Reconciling KnativeServing", "status", ks.Status)
 
 	stages := []func(context.Context, *mf.Manifest, *servingv1alpha1.KnativeServing) error{
-		r.ensureFinalizer,
+		r.ensureFinalizerRemoval,
 		r.initStatus,
 		r.install,
 		r.checkDeployments,
@@ -251,47 +269,35 @@ func (r *Reconciler) checkDeployments(ctx context.Context, manifest *mf.Manifest
 	return nil
 }
 
-// ensureFinalizer attaches a "delete manifest" finalizer to the instance
-func (r *Reconciler) ensureFinalizer(ctx context.Context, manifest *mf.Manifest, instance *servingv1alpha1.KnativeServing) error {
-	for _, finalizer := range instance.GetFinalizers() {
-		if finalizer == finalizerName {
-			return nil
-		}
-	}
-	instance.SetFinalizers(append(instance.GetFinalizers(), finalizerName))
-	instance, err := r.knativeServingClientSet.OperatorV1alpha1().KnativeServings(instance.Namespace).Update(instance)
-	return err
-}
-
-// delete all the resources in the release manifest
-func (r *Reconciler) delete(ctx context.Context, instance *servingv1alpha1.KnativeServing) error {
-	logger := logging.FromContext(ctx)
+// ensureFinalizerRemoval ensures that the obsolete "delete-knative-serving-manifest" is removed from the resource.
+func (r *Reconciler) ensureFinalizerRemoval(_ context.Context, _ *mf.Manifest, instance *servingv1alpha1.KnativeServing) error {
+	finalizers := sets.NewString(instance.Finalizers...)
 
-	if len(instance.GetFinalizers()) == 0 || instance.GetFinalizers()[0] != finalizerName {
+	if !finalizers.Has(oldFinalizerName) {
+		// Nothing to do.
 		return nil
 	}
-	logger.Info("Deleting resources")
-	var RBAC = mf.Any(mf.ByKind("Role"), mf.ByKind("ClusterRole"), mf.ByKind("RoleBinding"), mf.ByKind("ClusterRoleBinding"))
-	if len(r.servings) == 0 {
-		if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil {
-			return err
-		}
-		if err := r.config.Filter(mf.NoCRDs, mf.None(RBAC)).Delete(); err != nil {
-			return err
-		}
-		// Delete Roles last, as they may be useful for human operators to clean up.
-		if err := r.config.Filter(RBAC).Delete(); err != nil {
-			return err
-		}
+
+	// Remove the finalizer
+	finalizers.Delete(oldFinalizerName)
+
+	mergePatch := map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"finalizers":      finalizers.List(),
+			"resourceVersion": instance.ResourceVersion,
+		},
 	}
-	// The deletionTimestamp might've changed. Fetch the resource again.
-	refetched, err := r.knativeServingLister.KnativeServings(instance.Namespace).Get(instance.Name)
+
+	patch, err := json.Marshal(mergePatch)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to construct finalizer patch: %w", err)
 	}
-	refetched.SetFinalizers(refetched.GetFinalizers()[1:])
-	_, err = r.knativeServingClientSet.OperatorV1alpha1().KnativeServings(refetched.Namespace).Update(refetched)
-	return err
+
+	patcher := r.knativeServingClientSet.OperatorV1alpha1().KnativeServings(instance.Namespace)
+	if _, err := patcher.Patch(instance.Name, types.MergePatchType, patch); err != nil {
+		return fmt.Errorf("failed to patch finalizer away: %w", err)
+	}
+	return nil
 }
 
 // Delete obsolete resources from previous versions