Use finalizers to block CR deletion until all resources are deleted

pkg/reconciler/knativeeventing/knativeeventing.go

@@ -40,6 +40,10 @@ import (
 	"knative.dev/pkg/controller"
 )
 
+const (
+	finalizerName = "delete-knative-eventing-manifest"
+)
+
 var (
 	platform    common.Platforms
 	role        mf.Predicate = mf.Any(mf.ByKind("ClusterRole"), mf.ByKind("Role"))
@@ -71,25 +75,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
 	// Get the KnativeEventing resource with this namespace/name.
 	original, err := r.knativeEventingLister.KnativeEventings(namespace).Get(name)
 	if apierrs.IsNotFound(err) {
-		// The resource was deleted
-		r.eventings.Delete(key)
-		var RBAC = mf.Any(role, rolebinding)
-
-		if r.eventings.Len() == 0 {
-			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
-
 	} else if err != nil {
 		r.Logger.Error(err, "Error getting KnativeEventings")
 		return err
 	}
+
+	if original.GetDeletionTimestamp() != nil {
+		if _, ok := r.eventings[key]; ok {
+			delete(r.eventings, key)
+		}
+		return r.delete(original)
+	}
+
 	// Keep track of the number of Eventings in the cluster
 	r.eventings.Insert(key)
 
@@ -122,6 +120,7 @@ func (r *Reconciler) reconcile(ctx context.Context, ke *eventingv1alpha1.Knative
 	reqLogger.Infow("Reconciling KnativeEventing", "status", ke.Status)
 
 	stages := []func(*mf.Manifest, *eventingv1alpha1.KnativeEventing) error{
+		r.ensureFinalizer,
 		r.initStatus,
 		r.install,
 		r.checkDeployments,
@@ -130,6 +129,7 @@ func (r *Reconciler) reconcile(ctx context.Context, ke *eventingv1alpha1.Knative
 
 	manifest, err := r.transform(ke)
 	if err != nil {
+		ke.Status.MarkEventingFailed("Manifest Installation", err.Error())
 		return err
 	}
 
@@ -229,6 +229,53 @@ func (r *Reconciler) updateStatus(desired *eventingv1alpha1.KnativeEventing) (*e
 	return r.KnativeEventingClientSet.OperatorV1alpha1().KnativeEventings(desired.Namespace).UpdateStatus(existing)
 }
 
+// ensureFinalizer attaches a "delete manifest" finalizer to the instance
+func (r *Reconciler) ensureFinalizer(manifest *mf.Manifest, instance *eventingv1alpha1.KnativeEventing) error {
+	for _, finalizer := range instance.GetFinalizers() {
+		if finalizer == finalizerName {
+			return nil
+		}
+	}
+	instance.SetFinalizers(append(instance.GetFinalizers(), finalizerName))
+	instance, err := r.KnativeEventingClientSet.OperatorV1alpha1().KnativeEventings(instance.Namespace).Update(instance)
+	return err
+}
+
+// delete all the resources in the release manifest
+func (r *Reconciler) delete(instance *eventingv1alpha1.KnativeEventing) error {
+	if len(instance.GetFinalizers()) == 0 || instance.GetFinalizers()[0] != finalizerName {
+		return nil
+	}
+	r.Logger.Info("Deleting resources")
+	var RBAC = mf.Any(role, rolebinding)
+	if len(r.eventings) == 0 {
+		// delete the deployments first
+		if err := r.config.Filter(mf.ByKind("Deployment")).Delete(); err != nil {
+			r.Logger.Warn(err, "Error deleting deployments")
+			return err
+		}
+		if err := r.config.Filter(mf.NoCRDs, mf.None(RBAC)).Delete(); err != nil {
+			r.Logger.Warn(err, "Error deleting resources")
+			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 {
+			r.Logger.Warn(err, "Error deleting RBAC resources")
+			return err
+		}
+
+		r.Logger.Info("Resources are deleted")
+	}
+	// The deletionTimestamp might've changed. Fetch the resource again.
+	refetched, err := r.knativeEventingLister.KnativeEventings(instance.Namespace).Get(instance.Name)
+	if err != nil {
+		return err
+	}
+	refetched.SetFinalizers(refetched.GetFinalizers()[1:])
+	_, err = r.KnativeEventingClientSet.OperatorV1alpha1().KnativeEventings(refetched.Namespace).Update(refetched)
+	return err
+}
+
 // Delete obsolete resources from previous versions
 func (r *Reconciler) deleteObsoleteResources(manifest *mf.Manifest, instance *eventingv1alpha1.KnativeEventing) error {
 	resource := &unstructured.Unstructured{}

test/resources/verify.go

@@ -99,6 +99,16 @@ func KEOperatorCRDelete(t *testing.T, clients *test.Clients, names test.Resource
 	if err := clients.KnativeEventing().Delete(names.KnativeEventing, &metav1.DeleteOptions{}); err != nil {
 		t.Fatalf("KnativeEventing %q failed to delete: %v", names.KnativeEventing, err)
 	}
+	err := wait.PollImmediate(Interval, Timeout, func() (bool, error) {
+		_, err := clients.KnativeEventing().Get(names.KnativeEventing, metav1.GetOptions{})
+		if apierrs.IsNotFound(err) {
+			return true, nil
+		}
+		return false, err
+	})
+	if err != nil {
+		t.Fatal("Timed out waiting on KnativeServing to delete", err)
+	}
 	_, b, _, _ := runtime.Caller(0)
 	m, err := mfc.NewManifest(filepath.Join((filepath.Dir(b)+"/.."), "config/"), clients.Config)
 	if err != nil {
@@ -109,14 +119,8 @@ func KEOperatorCRDelete(t *testing.T, clients *test.Clients, names test.Resource
 	}
 	// verify all but the CRD's and the Namespace are gone
 	for _, u := range m.Filter(mf.NoCRDs, mf.None(mf.ByKind("Namespace"))).Resources() {
-		waitErr := wait.PollImmediate(Interval, Timeout, func() (bool, error) {
-			if _, err := m.Client.Get(&u); apierrs.IsNotFound(err) {
-				return true, nil
-			}
-			return false, err
-		})
-		if waitErr != nil {
-			t.Fatalf("The %s %s failed to be deleted: %v", u.GetKind(), u.GetName(), waitErr)
+		if _, err := m.Client.Get(&u); !apierrs.IsNotFound(err) {
+			t.Fatalf("The %s %s failed to be deleted: %v", u.GetKind(), u.GetName(), err)
 		}
 	}
 	// verify all the CRD's remain