K8SPS-367: delete-mysql-pvc finalizer (#726)

* Add delete-mysql-pvc finalizer.

* Add tests.

* Fix test.

* Fix

* Delete secrets properly

* Proper place to remove secrets.

* Refactor

* Run each finalizer to completion.

* Remove import.

* Cleanup

* Fix

deploy/cr.yaml

@@ -5,6 +5,7 @@ metadata:
   finalizers:
     - percona.com/delete-mysql-pods-in-order
     # - percona.com/delete-ssl
+    # - percona.com/delete-mysql-pvc
 spec:
 #  unsafeFlags:
 #    mysqlSize: false

pkg/controller/ps/controller.go

@@ -22,6 +22,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -35,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
@@ -141,34 +143,33 @@ func (r *PerconaServerMySQLReconciler) applyFinalizers(ctx context.Context, cr *
 
 	var err error
 
-	finalizers := []string{}
-	for _, f := range cr.GetFinalizers() {
-		switch f {
+	// Sorting finalizers to make sure that delete-mysql-pods-in-order runs before
+	// delete-mysql-pvc since the latter removes secrets needed for the former.
+	slices.Sort(cr.GetFinalizers())
+
+	for _, finalizer := range cr.GetFinalizers() {
+		switch finalizer {
 		case naming.FinalizerDeletePodsInOrder:
 			err = r.deleteMySQLPods(ctx, cr)
 		case naming.FinalizerDeleteSSL:
 			err = r.deleteCerts(ctx, cr)
+		case naming.FinalizerDeleteMySQLPvc:
+			err = r.deleteMySQLPvc(ctx, cr)
 		}
-
 		if err != nil {
-			switch err {
-			case psrestore.ErrWaitingTermination:
-				log.Info("waiting for pods to be deleted", "finalizer", f)
-			default:
-				log.Error(err, "failed to run finalizer", "finalizer", f)
-			}
-			finalizers = append(finalizers, f)
+			return err
 		}
 	}
 
-	cr.SetFinalizers(finalizers)
-
 	return k8sretry.RetryOnConflict(k8sretry.DefaultRetry, func() error {
-		err = r.Client.Update(ctx, cr)
+		c := new(apiv1alpha1.PerconaServerMySQL)
+		err := r.Client.Get(ctx, types.NamespacedName{Name: cr.Name, Namespace: cr.Namespace}, c)
 		if err != nil {
-			log.Error(err, "Client.Update failed")
+			return errors.Wrap(err, "get cr")
 		}
-		return err
+
+		c.SetFinalizers([]string{})
+		return r.Client.Update(ctx, c)
 	})
 }
 
@@ -356,6 +357,51 @@ func (r *PerconaServerMySQLReconciler) deleteCerts(ctx context.Context, cr *apiv
 	return nil
 }
 
+func (r *PerconaServerMySQLReconciler) deleteMySQLPvc(ctx context.Context, cr *apiv1alpha1.PerconaServerMySQL) error {
+	log := logf.FromContext(ctx)
+
+	log.Info(fmt.Sprintf("Applying %s finalizer", naming.FinalizerDeleteMySQLPvc))
+
+	exposer := mysql.Exposer(*cr)
+
+	list := corev1.PersistentVolumeClaimList{}
+
+	err := r.Client.List(ctx,
+		&list,
+		&client.ListOptions{
+			Namespace:     cr.Namespace,
+			LabelSelector: labels.SelectorFromSet(exposer.Labels()),
+		},
+	)
+	if err != nil {
+		return errors.Wrap(err, "get PVC list")
+	}
+
+	if list.Size() == 0 {
+		return nil
+	}
+
+	for _, pvc := range list.Items {
+		err := r.Client.Delete(ctx, &pvc, &client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &pvc.UID}})
+		if err != nil {
+			return errors.Wrapf(err, "delete PVC %s", pvc.Name)
+		}
+		log.Info("Removed MySQL PVC", "pvc", pvc.Name)
+	}
+
+	secretNames := []string{
+		cr.Spec.SecretsName,
+		"internal-" + cr.Name,
+	}
+	err = k8s.DeleteSecrets(ctx, r.Client, cr, secretNames)
+	if err != nil {
+		return errors.Wrap(err, "delete secrets")
+	}
+	log.Info("Removed secrets", "secrets", secretNames)
+
+	return nil
+}
+
 func (r *PerconaServerMySQLReconciler) doReconcile(
 	ctx context.Context,
 	cr *apiv1alpha1.PerconaServerMySQL,

pkg/controller/ps/controller_test.go

@@ -18,6 +18,9 @@ package ps
 
 import (
 	"context"
+	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -28,12 +31,14 @@ import (
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	psv1alpha1 "github.com/percona/percona-server-mysql-operator/api/v1alpha1"
 	"github.com/percona/percona-server-mysql-operator/pkg/mysql"
+	"github.com/percona/percona-server-mysql-operator/pkg/naming"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -518,3 +523,148 @@ var _ = Describe("Reconcile Binlog Server", Ordered, func() {
 		})
 	})
 })
+
+var _ = Describe("Finalizer delete-mysql-pvc", Ordered, func() {
+	ctx := context.Background()
+
+	const crName = "del-mysql-pvc-fnlz"
+	const ns = "del-mysql-pvc-fnlz"
+	crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns}
+
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      ns,
+			Namespace: ns,
+		},
+	}
+
+	BeforeAll(func() {
+		By("Creating the Namespace to perform the tests")
+		err := k8sClient.Create(ctx, namespace)
+		Expect(err).To(Not(HaveOccurred()))
+
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterAll(func() {
+		By("Deleting the Namespace to perform the tests")
+		_ = k8sClient.Delete(ctx, namespace)
+	})
+
+	Context("delete-mysql-pvc finalizer specified", Ordered, func() {
+
+		cr, err := readDefaultCR(crName, ns)
+
+		It("should read default cr.yaml", func() {
+			Expect(err).NotTo(HaveOccurred())
+		})
+		cr.Finalizers = append(cr.Finalizers, naming.FinalizerDeleteMySQLPvc)
+		cr.Spec.SecretsName = "cluster1-secrets"
+
+		sfsWithOwner := appsv1.StatefulSet{}
+		// stsApp := statefulset.NewNode(cr)
+		exposer := mysql.Exposer(*cr)
+
+		It("Should create PerconaXtraDBCluster", func() {
+			Expect(k8sClient.Create(ctx, cr)).Should(Succeed())
+		})
+
+		It("should reconcile once to create user secret", func() {
+			_, err := reconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("Should create mysql sts", func() {
+
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Name:      cr.Name + "-mysql",
+				Namespace: cr.Namespace,
+			}, &sfsWithOwner)).Should(Succeed())
+		})
+
+		It("Should create secrets", func() {
+			secret := &corev1.Secret{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{
+				Namespace: cr.Namespace,
+				Name:      cr.Spec.SecretsName,
+			}, secret)).Should(Succeed())
+		})
+
+		It("should create mysql PVC", func() {
+			for _, claim := range sfsWithOwner.Spec.VolumeClaimTemplates {
+				for i := 0; i < int(*sfsWithOwner.Spec.Replicas); i++ {
+					pvc := claim.DeepCopy()
+					pvc.Labels = exposer.Labels()
+					pvc.Name = strings.Join([]string{pvc.Name, sfsWithOwner.Name, strconv.Itoa(i)}, "-")
+					pvc.Namespace = ns
+					Expect(k8sClient.Create(ctx, pvc)).Should(Succeed())
+				}
+			}
+		})
+
+		It("controller should have mysql pvc", func() {
+			pvcList := corev1.PersistentVolumeClaimList{}
+			Eventually(func() bool {
+				err := k8sClient.List(ctx,
+					&pvcList,
+					&client.ListOptions{
+						Namespace: cr.Namespace,
+						LabelSelector: labels.SelectorFromSet(map[string]string{
+							"app.kubernetes.io/component": "mysql",
+						}),
+					})
+				return err == nil
+			}, time.Second*25, time.Millisecond*250).Should(BeTrue())
+			Expect(len(pvcList.Items)).Should(Equal(3))
+		})
+
+		When("mysql cluster is deleted with delete-mysql-pvc finalizer sts, pvc, and secrets should be removed", func() {
+			It("should delete mysql cluster and reconcile changes", func() {
+				Expect(k8sClient.Delete(ctx, cr)).Should(Succeed())
+
+				_, err := reconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName})
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("controller should remove pvc for mysql", func() {
+				pvcList := corev1.PersistentVolumeClaimList{}
+				Eventually(func() bool {
+					err := k8sClient.List(ctx, &pvcList, &client.ListOptions{
+						Namespace: cr.Namespace,
+						LabelSelector: labels.SelectorFromSet(map[string]string{
+							"app.kubernetes.io/component": "mysql",
+						}),
+					})
+					return err == nil
+				}, time.Second*15, time.Millisecond*250).Should(BeTrue())
+
+				for _, pvc := range pvcList.Items {
+					By(fmt.Sprintf("checking pvc/%s", pvc.Name))
+					Expect(pvc.DeletionTimestamp).ShouldNot(BeNil())
+				}
+			})
+
+			It("controller should delete secrets", func() {
+				secret := &corev1.Secret{}
+				Eventually(func() bool {
+					err := k8sClient.Get(ctx, types.NamespacedName{
+						Namespace: cr.Namespace,
+						Name:      cr.Spec.SecretsName,
+					}, secret)
+
+					return k8serrors.IsNotFound(err)
+				}, time.Second*15, time.Millisecond*250).Should(BeTrue())
+
+				Eventually(func() bool {
+					err := k8sClient.Get(ctx, types.NamespacedName{
+						Namespace: cr.Namespace,
+						Name:      "internal-" + cr.Name,
+					}, secret)
+
+					return k8serrors.IsNotFound(err)
+				}, time.Second*15, time.Millisecond*250).Should(BeTrue())
+
+			})
+		})
+	})
+})

pkg/k8s/utils.go

@@ -428,3 +428,24 @@ func GetCRWithDefaults(
 
 	return cr, nil
 }
+
+func DeleteSecrets(ctx context.Context, cl client.Client, cr *apiv1alpha1.PerconaServerMySQL, secretNames []string) error {
+	for _, secretName := range secretNames {
+		secret := &corev1.Secret{}
+		err := cl.Get(ctx, types.NamespacedName{
+			Namespace: cr.Namespace,
+			Name:      secretName,
+		}, secret)
+		if err != nil {
+			continue
+		}
+
+		err = cl.Delete(ctx, secret,
+			&client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &secret.UID}})
+		if err != nil {
+			return errors.Wrapf(err, "delete secret %s", secretName)
+		}
+	}
+
+	return nil
+}

pkg/naming/naming.go

@@ -30,8 +30,8 @@ const (
 const (
 	FinalizerDeleteSSL         = perconaPrefix + "delete-ssl"
 	FinalizerDeletePodsInOrder = perconaPrefix + "delete-mysql-pods-in-order"
-
-	FinalizerDeleteBackup = perconaPrefix + "delete-backup"
+	FinalizerDeleteBackup      = perconaPrefix + "delete-backup"
+	FinalizerDeleteMySQLPvc    = perconaPrefix + "delete-mysql-pvc"
 )
 
 type AnnotationKey string