feat: wait with cluster deletion until foreign finalizers are removed (#132)

Author: Diaphteiros

api/core/v1alpha1/constants.go

@@ -43,6 +43,7 @@ const (
 
 	ClusterConditionShootManagement       = "ShootManagement"
 	ClusterConditionClusterConfigurations = "ClusterConfigurations"
+	ClusterConditionForeignFinalizers     = "ForeignFinalizers"
 
 	ProviderConfigConditionCloudProfile             = "CloudProfile"
 	ProviderConfigConditionClusterProfileManagement = "ClusterProfileManagement"

internal/controllers/cluster/controller.go

@@ -12,6 +12,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -270,6 +271,20 @@ func (r *ClusterReconciler) reconcile(ctx context.Context, req reconcile.Request
 		// DELETE
 		log.Info("Deleting resource")
 
+		// check if there are any foreign finalizers on the Cluster resource
+		foreignFinalizers := make([]string, 0, len(c.Finalizers))
+		for _, fin := range c.Finalizers {
+			if fin != providerv1alpha1.ClusterFinalizer {
+				foreignFinalizers = append(foreignFinalizers, fin)
+			}
+		}
+		if len(foreignFinalizers) > 0 {
+			log.Info("Postponing shoot deletion until foreign finalizers are removed", "foreignFinalizers", foreignFinalizers)
+			createCon(providerv1alpha1.ClusterConditionForeignFinalizers, metav1.ConditionFalse, cconst.ReasonWaitingForDeletion, "Waiting for foreign finalizers to be removed: "+fmt.Sprintf("%v", foreignFinalizers))
+			return rr
+		}
+		createCon(providerv1alpha1.ClusterConditionForeignFinalizers, metav1.ConditionTrue, "", "")
+
 		if exists {
 			// shoot is still there
 			if shoot.DeletionTimestamp == nil {
@@ -346,6 +361,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			predicate.Or(
 				predicate.GenerationChangedPredicate{},
 				ctrlutils.DeletionTimestampChangedPredicate{},
+				finalizersLostWhileInDeletionPredicate{},
 				ctrlutils.GotAnnotationPredicate(openmcpconst.OperationAnnotation, openmcpconst.OperationAnnotationValueReconcile),
 				ctrlutils.LostAnnotationPredicate(openmcpconst.OperationAnnotation, openmcpconst.OperationAnnotationValueIgnore),
 			),
@@ -549,3 +565,31 @@ func (r *ClusterReconciler) getClusterConfigs(ctx context.Context, c *clustersv1
 
 	return clusterConfigs, nil
 }
+
+// TODO: move to controller-utils library
+type finalizersLostWhileInDeletionPredicate struct{}
+
+var _ predicate.Predicate = finalizersLostWhileInDeletionPredicate{}
+
+// Create implements predicate.TypedPredicate.
+func (f finalizersLostWhileInDeletionPredicate) Create(event.TypedCreateEvent[client.Object]) bool {
+	return false
+}
+
+// Delete implements predicate.TypedPredicate.
+func (f finalizersLostWhileInDeletionPredicate) Delete(event.TypedDeleteEvent[client.Object]) bool {
+	return false
+}
+
+// Generic implements predicate.TypedPredicate.
+func (f finalizersLostWhileInDeletionPredicate) Generic(event.TypedGenericEvent[client.Object]) bool {
+	return false
+}
+
+// Update implements predicate.TypedPredicate.
+func (f finalizersLostWhileInDeletionPredicate) Update(e event.TypedUpdateEvent[client.Object]) bool {
+	if e.ObjectOld == nil || e.ObjectNew == nil {
+		return false
+	}
+	return !e.ObjectNew.GetDeletionTimestamp().IsZero() && len(e.ObjectNew.GetFinalizers()) < len(e.ObjectOld.GetFinalizers())
+}

internal/controllers/cluster/controller_test.go

@@ -6,8 +6,10 @@ import (
 	. "github.com/onsi/gomega/gstruct"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	clustersv1alpha1 "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1"
@@ -240,6 +242,55 @@ var _ = Describe("Cluster Controller", func() {
 		Expect(env.Client(platformCluster).Get(env.Ctx, client.ObjectKeyFromObject(c), c)).To(MatchError(apierrors.IsNotFound, "cluster should be deleted"))
 	})
 
+	It("should not start with shoot deletion if there are foreign finalizers on the Cluster", func() {
+		env := defaultTestSetup("..", "cluster", "testdata", "test-05")
+
+		c := &clustersv1alpha1.Cluster{}
+		c.SetName("advanced")
+		c.SetNamespace("clusters")
+		env.ShouldReconcile(cRec, testutils.RequestFromObject(c))
+		Expect(env.Client(platformCluster).Get(env.Ctx, client.ObjectKeyFromObject(c), c)).To(Succeed())
+		c.Finalizers = append(c.Finalizers, "example.com/finalizer")
+		Expect(env.Client(platformCluster).Update(env.Ctx, c)).To(Succeed())
+
+		// verify shoot existence
+		Expect(c.Status.ProviderStatus).ToNot(BeNil())
+		cs := &providerv1alpha1.ClusterStatus{}
+		Expect(c.Status.GetProviderStatus(cs)).To(Succeed())
+		Expect(cs.Shoot).ToNot(BeNil())
+		shoot := &gardenv1beta1.Shoot{}
+		shoot.SetName(cs.Shoot.Name)
+		shoot.SetNamespace(cs.Shoot.Namespace)
+		Expect(env.Client(gardenCluster).Get(env.Ctx, client.ObjectKeyFromObject(shoot), shoot)).To(Succeed())
+
+		// delete the cluster
+		Expect(env.Client(platformCluster).Delete(env.Ctx, c)).To(Succeed())
+		env.ShouldReconcile(cRec, testutils.RequestFromObject(c))
+		Expect(env.Client(platformCluster).Get(env.Ctx, client.ObjectKeyFromObject(c), c)).To(Succeed())
+
+		// shoot should not be deleted because of the unknown finalizer
+		Expect(env.Client(gardenCluster).Get(env.Ctx, client.ObjectKeyFromObject(shoot), shoot)).To(Succeed())
+		Expect(shoot.DeletionTimestamp.IsZero()).To(BeTrue())
+
+		// there should be a condition indicating that there are foreign finalizers
+		Expect(c.Status.Conditions).To(ContainElement(MatchFields(IgnoreExtras, Fields{
+			"Type":    Equal(providerv1alpha1.ClusterConditionForeignFinalizers),
+			"Status":  Equal(metav1.ConditionFalse),
+			"Message": ContainSubstring("example.com/finalizer"),
+		})))
+
+		// remove the foreign finalizer
+		Expect(controllerutil.RemoveFinalizer(c, "example.com/finalizer")).To(BeTrue())
+		Expect(env.Client(platformCluster).Update(env.Ctx, c)).To(Succeed())
+		env.ShouldReconcile(cRec, testutils.RequestFromObject(c))
+
+		// verify shoot deletion
+		Expect(env.Client(gardenCluster).Get(env.Ctx, client.ObjectKeyFromObject(shoot), shoot)).To(MatchError(apierrors.IsNotFound, "shoot should be deleted"))
+		// shoot is gone, reconcile again to remove cluster finalizer
+		env.ShouldReconcile(cRec, testutils.RequestFromObject(c))
+		Expect(env.Client(platformCluster).Get(env.Ctx, client.ObjectKeyFromObject(c), c)).To(MatchError(apierrors.IsNotFound, "cluster should be deleted"))
+	})
+
 	It("should handle cluster configs correctly", func() {
 		env := defaultTestSetup("..", "cluster", "testdata", "test-04")