K8SPSMDB-1089: ignore finalizers on errors when `.status.state="Error…

…"` (#1589)

https://perconadev.atlassian.net/browse/K8SPSMDB-1089

Co-authored-by: Viacheslav Sarzhan <slava.sarzhan@percona.com>

pkg/controller/perconaservermongodb/finalizers.go

@@ -5,7 +5,6 @@ import (
 	"sort"
 
 	"github.com/pkg/errors"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
@@ -46,35 +45,29 @@ func (r *ReconcilePerconaServerMongoDB) checkFinalizers(ctx context.Context, cr
 			case errWaitingTermination:
 			default:
 				log.Error(err, "failed to run finalizer", "finalizer", f)
+				if cr.Status.State == api.AppStateError {
+					continue
+				}
 			}
 			finalizers = append(finalizers, orderedFinalizers[i:]...)
 			break
 		}
 	}
 
 	cr.SetFinalizers(finalizers)
-	err = r.client.Update(ctx, cr)
+	if err := r.client.Update(ctx, cr); err != nil {
+		return false, errors.Wrap(err, "update")
+	}
 
-	return shouldReconcile, err
+	return shouldReconcile, nil
 }
 
-func (r *ReconcilePerconaServerMongoDB) deletePSMDBPods(ctx context.Context, cr *api.PerconaServerMongoDB) (err error) {
+func (r *ReconcilePerconaServerMongoDB) deletePSMDBPods(ctx context.Context, cr *api.PerconaServerMongoDB) error {
 	log := logf.FromContext(ctx)
 
 	if cr.Spec.Sharding.Enabled {
 		cr.Spec.Sharding.Mongos.Size = 0
 
-		sts := new(appsv1.StatefulSet)
-		err := r.client.Get(ctx, cr.MongosNamespacedName(), sts)
-		if client.IgnoreNotFound(err) != nil {
-			return errors.Wrap(err, "failed to get mongos statefulset")
-		}
-		if sts.Spec.Replicas != nil && *sts.Spec.Replicas > 0 {
-			err = r.disableBalancer(ctx, cr)
-			if err != nil {
-				return errors.Wrap(err, "failed to disable balancer")
-			}
-		}
 		list, err := r.getMongosPods(ctx, cr)
 		if err != nil {
 			return errors.Wrap(err, "get mongos pods")
@@ -84,20 +77,21 @@ func (r *ReconcilePerconaServerMongoDB) deletePSMDBPods(ctx context.Context, cr
 		}
 	}
 
-	replsetsDeleted := true
+	rsDeleted := true
 	for _, rs := range cr.Spec.Replsets {
 		if err := r.deleteRSPods(ctx, cr, rs); err != nil {
+			rsDeleted = false
 			switch err {
 			case errWaitingTermination, errWaitingFirstPrimary:
 				log.Info(err.Error(), "rs", rs.Name)
+				continue
 			default:
 				log.Error(err, "failed to delete rs pods", "rs", rs.Name)
+				return err
 			}
-			replsetsDeleted = false
-			continue
 		}
 	}
-	if !replsetsDeleted {
+	if !rsDeleted {
 		return errWaitingTermination
 	}
 

pkg/controller/perconaservermongodb/finalizers_test.go

@@ -0,0 +1,110 @@
+package perconaservermongodb
+
+import (
+	"context"
+	"slices"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/types"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+
+	api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
+	"github.com/percona/percona-server-mongodb-operator/version"
+)
+
+func TestCheckFinalizers(t *testing.T) {
+	ctx := context.Background()
+	crName := "check-finalizers"
+	ns := crName + "-ns"
+
+	defaultCR := readDefaultCR(t, crName, ns)
+	obj := append(
+		fakePodsForRS(defaultCR, defaultCR.Spec.Replsets[0]),
+		fakeStatefulset(defaultCR, defaultCR.Spec.Replsets[0].Name, defaultCR.Spec.Replsets[0].Size, ""),
+	)
+
+	tests := []struct {
+		name string
+		cr   *api.PerconaServerMongoDB
+
+		expectedShouldReconcile bool
+		expectedFinalizers      []string
+	}{
+		{
+			name: "no-finalizers",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = nil
+			}),
+			expectedShouldReconcile: false,
+		},
+		{
+			name: "delete-pvc pass",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = []string{api.FinalizerDeletePVC}
+			}),
+			expectedShouldReconcile: false,
+			expectedFinalizers:      nil,
+		},
+		{
+			name: "delete pods fails",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = []string{api.FinalizerDeletePSMDBPodsInOrder}
+			}),
+			expectedFinalizers: []string{api.FinalizerDeletePSMDBPodsInOrder},
+		},
+		{
+			name: "cr with error state, delete pods fails with delete-pvc",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = []string{api.FinalizerDeletePSMDBPodsInOrder}
+				cr.Status.State = api.AppStateError
+			}),
+			expectedFinalizers: []string{},
+		},
+		{
+			name: "delete pods fails with delete-pvc",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = []string{api.FinalizerDeletePVC, api.FinalizerDeletePSMDBPodsInOrder}
+			}),
+			expectedFinalizers: []string{api.FinalizerDeletePSMDBPodsInOrder, api.FinalizerDeletePVC},
+		},
+		{
+			name: "cr with error state, delete pods fails with delete-pvc",
+			cr: updateObj(t, defaultCR.DeepCopy(), func(cr *api.PerconaServerMongoDB) {
+				cr.Finalizers = []string{api.FinalizerDeletePVC, api.FinalizerDeletePSMDBPodsInOrder}
+				cr.Status.State = api.AppStateError
+			}),
+			expectedFinalizers: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := buildFakeClient(append(obj, tt.cr)...)
+
+			cr := &api.PerconaServerMongoDB{}
+			if err := r.client.Get(ctx, types.NamespacedName{Name: crName, Namespace: ns}, cr); err != nil {
+				t.Fatal(err)
+			}
+
+			if err := cr.CheckNSetDefaults(version.PlatformKubernetes, logf.FromContext(ctx)); err != nil {
+				t.Fatal(err)
+			}
+
+			shouldReconcile, err := r.checkFinalizers(ctx, cr)
+			if err != nil {
+				t.Fatal("unexpected error:", err.Error())
+			}
+			if shouldReconcile != tt.expectedShouldReconcile {
+				t.Fatal("unexpected shouldReconcile:", shouldReconcile)
+			}
+
+			if err := r.client.Get(ctx, types.NamespacedName{Name: crName, Namespace: ns}, cr); err != nil {
+				t.Fatal(err)
+			}
+
+			if !slices.Equal(cr.Finalizers, tt.expectedFinalizers) {
+				t.Fatal("unexpected finalizers:", cr.Finalizers, "; expected:", tt.expectedFinalizers)
+			}
+		})
+	}
+}

pkg/controller/perconaservermongodb/helpers_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"k8s.io/apimachinery/pkg/util/yaml"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
 )
@@ -29,3 +30,11 @@ func readDefaultCR(t *testing.T, name, namespace string) *api.PerconaServerMongo
 
 	return cr
 }
+
+func updateObj[T client.Object](t *testing.T, obj T, f func(obj T)) T {
+	t.Helper()
+
+	f(obj)
+
+	return obj
+}