From e8200a1c11cb82b33e49f2e4e2fd75b15ee6ba24 Mon Sep 17 00:00:00 2001 From: Inel Pandzic Date: Tue, 20 Aug 2024 20:24:22 +0200 Subject: [PATCH] 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 | 1 + pkg/controller/ps/controller.go | 78 +++++++++++--- pkg/controller/ps/controller_test.go | 150 +++++++++++++++++++++++++++ pkg/k8s/utils.go | 21 ++++ pkg/naming/naming.go | 4 +- 5 files changed, 236 insertions(+), 18 deletions(-) diff --git a/deploy/cr.yaml b/deploy/cr.yaml index d29ef947d..8b5f464f5 100644 --- a/deploy/cr.yaml +++ b/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 diff --git a/pkg/controller/ps/controller.go b/pkg/controller/ps/controller.go index 2fc65673e..ebee799f5 100644 --- a/pkg/controller/ps/controller.go +++ b/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, diff --git a/pkg/controller/ps/controller_test.go b/pkg/controller/ps/controller_test.go index a23abc1af..ecca4b866 100644 --- a/pkg/controller/ps/controller_test.go +++ b/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()) + + }) + }) + }) +}) diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go index e5368fc3c..12948a6d0 100644 --- a/pkg/k8s/utils.go +++ b/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 +} diff --git a/pkg/naming/naming.go b/pkg/naming/naming.go index 2b694ca8c..23bdcfa20 100644 --- a/pkg/naming/naming.go +++ b/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