From b35048175f3f497a4d90bcf67e4cc9b9a8ead518 Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Mon, 13 Jul 2026 19:22:50 +0500 Subject: [PATCH 1/7] pre- and post- finalizers --- percona/controller/pgcluster/controller.go | 14 +++++--- percona/controller/pgcluster/finalizer.go | 41 ++++++++++++++-------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/percona/controller/pgcluster/controller.go b/percona/controller/pgcluster/controller.go index aacbb4efa1..4f8d0448da 100644 --- a/percona/controller/pgcluster/controller.go +++ b/percona/controller/pgcluster/controller.go @@ -256,18 +256,24 @@ func (r *PGClusterReconciler) Reconcile(ctx context.Context, request reconcile.R if cr.DeletionTimestamp != nil { log.Info("Deleting PerconaPGCluster", "deletionTimestamp", cr.DeletionTimestamp) - if err := r.runFinalizers(ctx, cr); err != nil { - return reconcile.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "run finalizers") - } - // We're deleting PostgresCluster explicitly to let Crunchy controller run its finalizers and not mess with us. if err := r.Client.Delete(ctx, postgresCluster); client.IgnoreNotFound(err) != nil { return ctrl.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "delete postgres cluster") } if err := r.Client.Get(ctx, client.ObjectKeyFromObject(postgresCluster), postgresCluster); err == nil { + if err := r.runPrePostgresClusterDeletionFinalizers(ctx, cr); err != nil { + return reconcile.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "run pre-postgrescluster finalizers") + } + log.Info("Waiting for PostgresCluster to be deleted") return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } else if client.IgnoreNotFound(err) != nil { + return ctrl.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "get postgres cluster") + } + + if err := r.runPostPostgresClusterDeletionFinalizers(ctx, cr); err != nil { + return reconcile.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "run post-postgrescluster finalizers") } return reconcile.Result{}, nil diff --git a/percona/controller/pgcluster/finalizer.go b/percona/controller/pgcluster/finalizer.go index 2e0256c909..25bc0e7a08 100644 --- a/percona/controller/pgcluster/finalizer.go +++ b/percona/controller/pgcluster/finalizer.go @@ -20,8 +20,6 @@ import ( v2 "github.com/percona/percona-postgresql-operator/v2/pkg/apis/pgv2.percona.com/v2" ) -type finalizerFunc func(context.Context, *v2.PerconaPGCluster) error - func (r *PGClusterReconciler) deletePVC(ctx context.Context, cr *v2.PerconaPGCluster) error { log := logging.FromContext(ctx) @@ -212,24 +210,39 @@ func (r *PGClusterReconciler) deleteBackups(ctx context.Context, cr *v2.PerconaP return nil } -func (r *PGClusterReconciler) runFinalizers(ctx context.Context, cr *v2.PerconaPGCluster) error { - type finalizerEntry struct { - name string - fn controller.FinalizerFunc[*v2.PerconaPGCluster] +func (r *PGClusterReconciler) runFinalizers(ctx context.Context, cr *v2.PerconaPGCluster, finalizers []finalizerEntry) error { + for _, entry := range finalizers { + if _, err := controller.RunFinalizer(ctx, r.Client, cr, entry.name, entry.fn); err != nil { + return errors.Wrapf(err, "run finalizer %s", entry.name) + } } - finalizers := []finalizerEntry{ - {pNaming.FinalizerDeletePVC, r.deletePVCAndSecrets}, - {pNaming.FinalizerDeleteSSL, r.deleteTLSSecrets}, + return nil +} + +type finalizerEntry struct { + name string + fn controller.FinalizerFunc[*v2.PerconaPGCluster] +} + +func (r *PGClusterReconciler) prePostgresClusterDeletionFinalizers() []finalizerEntry { + return []finalizerEntry{ {pNaming.FinalizerStopWatchers, r.stopExternalWatchers}, {pNaming.FinalizerDeleteBackups, r.deleteBackups}, } +} - for _, entry := range finalizers { - if _, err := controller.RunFinalizer(ctx, r.Client, cr, entry.name, entry.fn); err != nil { - return errors.Wrapf(err, "run finalizer %s", entry.name) - } +func (r *PGClusterReconciler) postPostgresClusterDeletionFinalizers() []finalizerEntry { + return []finalizerEntry{ + {pNaming.FinalizerDeletePVC, r.deletePVCAndSecrets}, + {pNaming.FinalizerDeleteSSL, r.deleteTLSSecrets}, } +} - return nil +func (r *PGClusterReconciler) runPrePostgresClusterDeletionFinalizers(ctx context.Context, cr *v2.PerconaPGCluster) error { + return r.runFinalizers(ctx, cr, r.prePostgresClusterDeletionFinalizers()) +} + +func (r *PGClusterReconciler) runPostPostgresClusterDeletionFinalizers(ctx context.Context, cr *v2.PerconaPGCluster) error { + return r.runFinalizers(ctx, cr, r.postPostgresClusterDeletionFinalizers()) } From d912b377af8374753d73bba4c688b165be2e6352 Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Mon, 13 Jul 2026 19:29:39 +0500 Subject: [PATCH 2/7] upd tests --- e2e-tests/tests/finalizers/01-assert.yaml | 1 + .../tests/finalizers/01-create-cluster.yaml | 2 +- .../tests/finalizers/02-delete-cluster.yaml | 24 +++++++++++++++++++ e2e-tests/tests/finalizers/03-assert.yaml | 1 + .../tests/finalizers/03-create-cluster.yaml | 2 +- 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/e2e-tests/tests/finalizers/01-assert.yaml b/e2e-tests/tests/finalizers/01-assert.yaml index e75519389a..ef0604f121 100644 --- a/e2e-tests/tests/finalizers/01-assert.yaml +++ b/e2e-tests/tests/finalizers/01-assert.yaml @@ -124,6 +124,7 @@ metadata: finalizers: - percona.com/delete-pvc - percona.com/delete-ssl + - percona.com/delete-backups - internal.percona.com/stop-watchers status: pgbouncer: diff --git a/e2e-tests/tests/finalizers/01-create-cluster.yaml b/e2e-tests/tests/finalizers/01-create-cluster.yaml index bdb0b42829..082fd66742 100644 --- a/e2e-tests/tests/finalizers/01-create-cluster.yaml +++ b/e2e-tests/tests/finalizers/01-create-cluster.yaml @@ -9,5 +9,5 @@ commands: source ../../functions get_cr \ - | yq eval '.metadata.finalizers += ["percona.com/delete-pvc", "percona.com/delete-ssl"]' \ + | yq eval '.metadata.finalizers += ["percona.com/delete-pvc", "percona.com/delete-ssl", "percona.com/delete-backups"]' \ | kubectl -n "${NAMESPACE}" apply -f - diff --git a/e2e-tests/tests/finalizers/02-delete-cluster.yaml b/e2e-tests/tests/finalizers/02-delete-cluster.yaml index bd18a13a90..a84610b74a 100644 --- a/e2e-tests/tests/finalizers/02-delete-cluster.yaml +++ b/e2e-tests/tests/finalizers/02-delete-cluster.yaml @@ -4,3 +4,27 @@ delete: - apiVersion: pgv2.percona.com/v2 kind: PerconaPGCluster name: finalizers +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + wait_for_delete "postgrescluster.upstream.pgv2.percona.com/finalizers" + wait_for_delete "pg/finalizers" + + assert_no_cluster_secrets() { + local count + count=$(kubectl -n "${NAMESPACE}" get secret -l postgres-operator.crunchydata.com/cluster=finalizers --no-headers 2>/dev/null | wc -l | tr -d ' ') + if [[ "${count}" != "0" ]]; then + echo "Found recreated secrets for deleted cluster finalizers" + kubectl -n "${NAMESPACE}" get secret -l postgres-operator.crunchydata.com/cluster=finalizers + exit 1 + fi + } + + # Check twice with a short delay to catch late secret recreation. + assert_no_cluster_secrets + sleep 15 + assert_no_cluster_secrets diff --git a/e2e-tests/tests/finalizers/03-assert.yaml b/e2e-tests/tests/finalizers/03-assert.yaml index 235feede3a..0092f97e57 100644 --- a/e2e-tests/tests/finalizers/03-assert.yaml +++ b/e2e-tests/tests/finalizers/03-assert.yaml @@ -124,6 +124,7 @@ metadata: finalizers: - percona.com/delete-pvc - percona.com/delete-ssl + - percona.com/delete-backups - internal.percona.com/stop-watchers status: pgbouncer: diff --git a/e2e-tests/tests/finalizers/03-create-cluster.yaml b/e2e-tests/tests/finalizers/03-create-cluster.yaml index cf3c84716f..1a2dae6439 100644 --- a/e2e-tests/tests/finalizers/03-create-cluster.yaml +++ b/e2e-tests/tests/finalizers/03-create-cluster.yaml @@ -9,5 +9,5 @@ commands: source ../../functions get_cr "finalizers-2" \ - | yq eval '.metadata.finalizers += ["percona.com/delete-pvc", "percona.com/delete-ssl"]' \ + | yq eval '.metadata.finalizers += ["percona.com/delete-pvc", "percona.com/delete-ssl", "percona.com/delete-backups"]' \ | kubectl -n "${NAMESPACE}" apply -f - From 2db3ec2be8b66457730cd46304809627ad2bb1ad Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Tue, 14 Jul 2026 16:49:28 +0500 Subject: [PATCH 3/7] add timeout --- e2e-tests/tests/finalizers/02-delete-cluster.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e-tests/tests/finalizers/02-delete-cluster.yaml b/e2e-tests/tests/finalizers/02-delete-cluster.yaml index a84610b74a..d574f39ed5 100644 --- a/e2e-tests/tests/finalizers/02-delete-cluster.yaml +++ b/e2e-tests/tests/finalizers/02-delete-cluster.yaml @@ -5,7 +5,8 @@ delete: kind: PerconaPGCluster name: finalizers commands: - - script: |- + - timeout: 180 + script: |- set -o errexit set -o xtrace From c056e553a98af5ee5aa3f59af01a1bdd3f53f5cf Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Tue, 14 Jul 2026 16:50:00 +0500 Subject: [PATCH 4/7] call pre-deletion finalizers before deleting upstream --- percona/controller/pgcluster/controller.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/percona/controller/pgcluster/controller.go b/percona/controller/pgcluster/controller.go index 4f8d0448da..2b06babcc3 100644 --- a/percona/controller/pgcluster/controller.go +++ b/percona/controller/pgcluster/controller.go @@ -256,15 +256,18 @@ func (r *PGClusterReconciler) Reconcile(ctx context.Context, request reconcile.R if cr.DeletionTimestamp != nil { log.Info("Deleting PerconaPGCluster", "deletionTimestamp", cr.DeletionTimestamp) + // Run pre-deletion finalizers before deleting PostgresCluster, + // because they may need to exec into pods (e.g. deleteBackups). + if err := r.runPrePostgresClusterDeletionFinalizers(ctx, cr); err != nil { + return reconcile.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "run pre-postgrescluster finalizers") + } + // We're deleting PostgresCluster explicitly to let Crunchy controller run its finalizers and not mess with us. if err := r.Client.Delete(ctx, postgresCluster); client.IgnoreNotFound(err) != nil { return ctrl.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "delete postgres cluster") } if err := r.Client.Get(ctx, client.ObjectKeyFromObject(postgresCluster), postgresCluster); err == nil { - if err := r.runPrePostgresClusterDeletionFinalizers(ctx, cr); err != nil { - return reconcile.Result{RequeueAfter: 5 * time.Second}, errors.Wrap(err, "run pre-postgrescluster finalizers") - } log.Info("Waiting for PostgresCluster to be deleted") return ctrl.Result{RequeueAfter: 5 * time.Second}, nil From c2dc7e7337feba6d385d178960f6de86f9c5d8e7 Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Wed, 15 Jul 2026 19:27:10 +0500 Subject: [PATCH 5/7] add unit test --- .../controller/pgcluster/finalizer_test.go | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/percona/controller/pgcluster/finalizer_test.go b/percona/controller/pgcluster/finalizer_test.go index e2c94c06c3..35a71f0c56 100644 --- a/percona/controller/pgcluster/finalizer_test.go +++ b/percona/controller/pgcluster/finalizer_test.go @@ -5,6 +5,7 @@ package pgcluster import ( "context" "fmt" + "io" "time" . "github.com/onsi/ginkgo/v2" @@ -353,6 +354,81 @@ var _ = Describe("Finalizers", Ordered, func() { }) }) + Context("pre-deletion finalizers removed before PostgresCluster is gone", Ordered, func() { + crName := ns + "-pre-finalizer-order" + crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns} + + When("CR has all four finalizers", func() { + cr, err := readDefaultCR(crName, ns) + It("should read default cr.yaml", func() { + Expect(err).NotTo(HaveOccurred()) + }) + + controllerutil.AddFinalizer(cr, pNaming.FinalizerStopWatchers) + controllerutil.AddFinalizer(cr, pNaming.FinalizerDeleteBackups) + controllerutil.AddFinalizer(cr, pNaming.FinalizerDeletePVC) + controllerutil.AddFinalizer(cr, pNaming.FinalizerDeleteSSL) + + It("should create PerconaPGCluster and a labeled pod for deleteBackups", func() { + status := cr.Status + Expect(k8sClient.Create(ctx, cr)).Should(Succeed()) + cr.Status = status + Expect(k8sClient.Status().Update(ctx, cr)).Should(Succeed()) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: crName + "-pod", + Namespace: ns, + Labels: map[string]string{ + "app.kubernetes.io/instance": crName, + "app.kubernetes.io/component": "pg", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "database", Image: "busybox"}, + }, + }, + } + Expect(k8sClient.Create(ctx, pod)).Should(Succeed()) + }) + + It("should reconcile to create PostgresCluster", func() { + rec := reconciler(cr) + rec.PodExec = func(_ context.Context, _, _, _ string, _ io.Reader, _ io.Writer, _ io.Writer, _ ...string) error { + return nil + } + _, err = rec.Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + _, err = crunchyReconciler().Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should delete PerconaPGCluster", func() { + Expect(k8sClient.Delete(ctx, cr)).Should(Succeed()) + }) + + It("should run one Reconcile pass (pre-finalizers run, PostgresCluster still exists)", func() { + rec := reconciler(cr) + rec.PodExec = func(_ context.Context, _, _, _ string, _ io.Reader, _ io.Writer, _ io.Writer, _ ...string) error { + return nil + } + _, err = rec.Reconcile(ctx, ctrl.Request{NamespacedName: crNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should have pre-deletion finalizers removed and post-deletion finalizers still present", func() { + fetched := &v2.PerconaPGCluster{} + Expect(k8sClient.Get(ctx, crNamespacedName, fetched)).Should(Succeed()) + + Expect(fetched.Finalizers).ShouldNot(ContainElement(pNaming.FinalizerStopWatchers)) + Expect(fetched.Finalizers).ShouldNot(ContainElement(pNaming.FinalizerDeleteBackups)) + Expect(fetched.Finalizers).Should(ContainElement(pNaming.FinalizerDeletePVC)) + Expect(fetched.Finalizers).Should(ContainElement(pNaming.FinalizerDeleteSSL)) + }) + }) + }) + Context(pNaming.FinalizerStopWatchers, Ordered, func() { When("without finalizer", func() { crName := ns + "-without-stop-watchers" From 4a9fbfce5319178ed6ca7b317c65b7ee8c1b4b7f Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Thu, 16 Jul 2026 13:15:02 +0500 Subject: [PATCH 6/7] add comment --- percona/controller/pgcluster/finalizer.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/percona/controller/pgcluster/finalizer.go b/percona/controller/pgcluster/finalizer.go index 25bc0e7a08..0198f425ae 100644 --- a/percona/controller/pgcluster/finalizer.go +++ b/percona/controller/pgcluster/finalizer.go @@ -225,6 +225,9 @@ type finalizerEntry struct { fn controller.FinalizerFunc[*v2.PerconaPGCluster] } +// prePostgresClusterDeletionFinalizers returns finalizers that must run while the +// PostgresCluster still exists (e.g. operations that require running pods or +// cluster resources such as stopping watchers and deleting backups from repos). func (r *PGClusterReconciler) prePostgresClusterDeletionFinalizers() []finalizerEntry { return []finalizerEntry{ {pNaming.FinalizerStopWatchers, r.stopExternalWatchers}, From 72c46dd24a0c8f153806b3eda1d6a0c1151810af Mon Sep 17 00:00:00 2001 From: Oksana Grishchenko Date: Thu, 16 Jul 2026 17:08:40 +0500 Subject: [PATCH 7/7] fix flaky test --- e2e-tests/tests/upgrade-minor/06-upgrade-cluster.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e-tests/tests/upgrade-minor/06-upgrade-cluster.yaml b/e2e-tests/tests/upgrade-minor/06-upgrade-cluster.yaml index ac7393f344..49cccd600e 100644 --- a/e2e-tests/tests/upgrade-minor/06-upgrade-cluster.yaml +++ b/e2e-tests/tests/upgrade-minor/06-upgrade-cluster.yaml @@ -31,7 +31,8 @@ commands: kubectl wait -n $NAMESPACE --timeout 180s --for=jsonpath='{.spec.template.spec.containers[0].image}'=$target_image_backrest sts/upgrade-minor-repo-host kubectl wait -n $NAMESPACE --timeout 180s --for=jsonpath='{.spec.template.spec.containers[0].image}'=$target_image_pgbouncer deployment/upgrade-minor-pgbouncer - for s in $(kubectl get sts --no-headers -l postgres-operator.crunchydata.com/instance-set=instance1 --output=custom-columns='NAME:.metadata.name'); do + for s in $(kubectl get sts -n $NAMESPACE --no-headers -l postgres-operator.crunchydata.com/instance-set=instance1 --output=custom-columns='NAME:.metadata.name'); do kubectl wait -n $NAMESPACE --timeout 180s --for=jsonpath='{.spec.template.spec.containers[0].image}'=$target_image_postgresql sts/${s} done timeout: 180 +