Merge pull request #651 from srteam2020/main

fix #648: delete the statefulset with precondition set to the correct…

Author: ChunyiLyu

controllers/reconcile_finalizer.go

@@ -41,6 +41,10 @@ func (r *RabbitmqClusterReconciler) removeFinalizer(ctx context.Context, rabbitm
 func (r *RabbitmqClusterReconciler) prepareForDeletion(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) error {
 	if controllerutil.ContainsFinalizer(rabbitmqCluster, deletionFinalizer) {
 		if err := clientretry.RetryOnConflict(clientretry.DefaultRetry, func() error {
+			uid, err := r.statefulSetUID(ctx, rabbitmqCluster)
+			if err != nil {
+				return err
+			}
 			sts := &appsv1.StatefulSet{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      rabbitmqCluster.ChildResourceName("server"),
@@ -54,7 +58,9 @@ func (r *RabbitmqClusterReconciler) prepareForDeletion(ctx context.Context, rabb
 			// Delete StatefulSet immediately after changing pod labels to minimize risk of them respawning.
 			// There is a window where the StatefulSet could respawn Pods without the deletion label in this order.
 			// But we can't delete it before because the DownwardAPI doesn't update once a Pod enters Terminating.
-			if err := r.Client.Delete(ctx, sts); client.IgnoreNotFound(err) != nil {
+			// Addressing #648: if both rabbitmqCluster and the statefulSet returned by r.Get() are stale (and match each other),
+			// setting the stale statefulSet's uid in the precondition can avoid mis-deleting any currently running statefulSet sharing the same name.
+			if err := r.Client.Delete(ctx, sts, &client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}); client.IgnoreNotFound(err) != nil {
 				return fmt.Errorf("cannot delete StatefulSet: %s", err.Error())
 			}
 

controllers/utils.go

@@ -2,11 +2,14 @@ package controllers
 
 import (
 	"context"
+	"fmt"
+
 	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -64,6 +67,23 @@ func (r *RabbitmqClusterReconciler) statefulSet(ctx context.Context, rmq *rabbit
 	return sts, nil
 }
 
+// statefulSetUID only returns the UID successfully when the controller reference uid matches the rmq uid
+func (r *RabbitmqClusterReconciler) statefulSetUID(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster) (types.UID, error) {
+	var err error
+	var sts *appsv1.StatefulSet
+	var ref *metav1.OwnerReference
+	if sts, err = r.statefulSet(ctx, rmq); err != nil {
+		return "", fmt.Errorf("failed to get statefulSet: %s", err.Error())
+	}
+	if ref = metav1.GetControllerOf(sts); ref == nil {
+		return "", fmt.Errorf("failed to get controller reference for statefulSet %s", sts.GetName())
+	}
+	if string(rmq.GetUID()) != string(ref.UID) {
+		return "", fmt.Errorf("statefulSet %s not owned by current RabbitmqCluster %s", sts.GetName(), rmq.GetName())
+	}
+	return sts.UID, nil
+}
+
 func (r *RabbitmqClusterReconciler) configMap(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster, name string) (*corev1.ConfigMap, error) {
 	configMap := &corev1.ConfigMap{}
 	if err := r.Get(ctx, types.NamespacedName{Namespace: rmq.Namespace, Name: name}, configMap); err != nil {