Replace TestReconcilerHandleDelete With Kuttl Test (#3169)

TestReconcilerHandleDelete was prone to flakes when run with
`envtest-existing`, and so is here replaced by a KUTTL test
with matching functionality.

Issue [sc-14264]

Author: benjaminjb

internal/controller/postgrescluster/delete_test.go

@@ -20,7 +20,6 @@ package postgrescluster
 
 import (
 	"context"
-	"io"
 	"os"
 	"strings"
 	"testing"
@@ -30,294 +29,16 @@ import (
 	"gotest.tools/v3/assert"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/yaml"
 
-	"github.com/crunchydata/postgres-operator/internal/patroni"
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
-func TestReconcilerHandleDelete(t *testing.T) {
-	if !strings.EqualFold(os.Getenv("USE_EXISTING_CLUSTER"), "true") {
-		t.Skip("requires a running garbage collection controller")
-	}
-
-	ctx := context.Background()
-	env, cc := setupKubernetes(t)
-	require.ParallelCapacity(t, 2)
-
-	ns := setupNamespace(t, cc)
-	reconciler := Reconciler{
-		Client:   cc,
-		Owner:    client.FieldOwner(t.Name()),
-		Recorder: new(record.FakeRecorder),
-		Tracer:   otel.Tracer(t.Name()),
-	}
-
-	var err error
-	reconciler.PodExec, err = newPodExecutor(env.Config)
-	assert.NilError(t, err)
-
-	mustReconcile := func(t *testing.T, cluster *v1beta1.PostgresCluster) reconcile.Result {
-		t.Helper()
-		key := client.ObjectKeyFromObject(cluster)
-		request := reconcile.Request{NamespacedName: key}
-		result, err := reconciler.Reconcile(ctx, request)
-		assert.NilError(t, err, "%+v", err)
-		return result
-	}
-
-	for _, test := range []struct {
-		name         string
-		beforeCreate func(*testing.T, *v1beta1.PostgresCluster)
-		beforeDelete func(*testing.T, *v1beta1.PostgresCluster)
-		propagation  metav1.DeletionPropagation
-
-		waitForRunningInstances int32
-	}{
-		// Normal delete of a healthly cluster.
-		{
-			name: "Background", propagation: metav1.DeletePropagationBackground,
-			waitForRunningInstances: 2,
-		},
-		// TODO(cbandy): metav1.DeletePropagationForeground
-
-		// Normal delete of a healthy cluster after a failover.
-		{
-			name: "AfterFailover", propagation: metav1.DeletePropagationBackground,
-			waitForRunningInstances: 2,
-
-			beforeDelete: func(t *testing.T, cluster *v1beta1.PostgresCluster) {
-				list := corev1.PodList{}
-				selector, err := labels.Parse(strings.Join([]string{
-					"postgres-operator.crunchydata.com/cluster=" + cluster.Name,
-					"postgres-operator.crunchydata.com/instance",
-				}, ","))
-				assert.NilError(t, err)
-				assert.NilError(t, cc.List(ctx, &list,
-					client.InNamespace(cluster.Namespace),
-					client.MatchingLabelsSelector{Selector: selector}))
-
-				var primary *corev1.Pod
-				var replica *corev1.Pod
-				for i := range list.Items {
-					if list.Items[i].Labels["postgres-operator.crunchydata.com/role"] == "replica" {
-						replica = &list.Items[i]
-					} else {
-						primary = &list.Items[i]
-					}
-				}
-
-				if true &&
-					assert.Check(t, primary != nil, "expected to find a primary in %+v", list.Items) &&
-					assert.Check(t, replica != nil, "expected to find a replica in %+v", list.Items) {
-					success, err := patroni.Executor(
-						func(_ context.Context, stdin io.Reader, stdout, stderr io.Writer, command ...string) error {
-							return reconciler.PodExec(replica.Namespace, replica.Name, "database", stdin, stdout, stderr, command...)
-						},
-					).ChangePrimaryAndWait(ctx, primary.Name, replica.Name)
-
-					assert.NilError(t, err)
-					assert.Assert(t, success)
-				}
-			},
-		},
-
-		// Normal delete of cluster that could never run PostgreSQL.
-		{
-			name: "NeverRunning", propagation: metav1.DeletePropagationBackground,
-			waitForRunningInstances: 0,
-
-			beforeCreate: func(_ *testing.T, cluster *v1beta1.PostgresCluster) {
-				cluster.Spec.Image = "example.com/does-not-exist"
-			},
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			cluster := &v1beta1.PostgresCluster{}
-			assert.NilError(t, yaml.Unmarshal([]byte(`{
-				spec: {
-					postgresVersion: 13,
-					instances: [
-						{
-							replicas: 2,
-							dataVolumeClaimSpec: {
-								accessModes: [ReadWriteOnce],
-								resources: { requests: { storage: 1Gi } },
-							},
-						},
-					],
-					backups: { 
-						pgbackrest: {
-							repos: [{
-								name: repo1,
-								volume: {
-									volumeClaimSpec: {
-										accessModes: [ReadWriteOnce],
-										resources: { requests: { storage: 1Gi } },
-									},
-								},
-							}],
-						},
-					},
-				},
-			}`), cluster))
-
-			cluster.Namespace = ns.Name
-			cluster.Name = strings.ToLower(test.name)
-			cluster.Spec.Image = CrunchyPostgresHAImage
-			cluster.Spec.Backups.PGBackRest.Image = CrunchyPGBackRestImage
-
-			if test.beforeCreate != nil {
-				test.beforeCreate(t, cluster)
-			}
-
-			assert.NilError(t, cc.Create(ctx, cluster))
-
-			t.Cleanup(func() {
-				// Remove finalizers, if any, so the namespace can terminate.
-				assert.Check(t, client.IgnoreNotFound(
-					cc.Patch(ctx, cluster, client.RawPatch(
-						client.Merge.Type(), []byte(`{"metadata":{"finalizers":[]}}`)))))
-			})
-
-			// Start cluster.
-			mustReconcile(t, cluster)
-
-			assert.NilError(t,
-				cc.Get(ctx, client.ObjectKeyFromObject(cluster), cluster))
-			assert.Assert(t,
-				sets.NewString(cluster.Finalizers...).
-					Has("postgres-operator.crunchydata.com/finalizer"),
-				"cluster should immediately have a finalizer")
-
-			// Continue until instances are healthy.
-			if ready := int32(0); !assert.Check(t,
-				wait.Poll(time.Second, Scale(time.Minute), func() (bool, error) {
-					mustReconcile(t, cluster)
-					assert.NilError(t, cc.Get(ctx, client.ObjectKeyFromObject(cluster), cluster))
-
-					ready = 0
-					for _, set := range cluster.Status.InstanceSets {
-						ready += set.ReadyReplicas
-					}
-					return ready >= test.waitForRunningInstances, nil
-				}), "expected %v instances to be ready, got: %v", test.waitForRunningInstances, ready,
-			) {
-				t.FailNow()
-			}
-
-			if test.beforeDelete != nil {
-				test.beforeDelete(t, cluster)
-			}
-
-			switch test.propagation {
-			case metav1.DeletePropagationBackground:
-				// Background deletion is the default for custom resources.
-				// - https://issue.k8s.io/81628
-				assert.NilError(t, cc.Delete(ctx, cluster))
-			default:
-				assert.NilError(t, cc.Delete(ctx, cluster,
-					client.PropagationPolicy(test.propagation)))
-			}
-
-			// Stop cluster.
-			result := mustReconcile(t, cluster)
-
-			// If things started running, then they should stop in a certain order.
-			if test.waitForRunningInstances > 0 {
-
-				// Replicas should stop first, leaving just the one primary.
-				var instances []corev1.Pod
-				assert.NilError(t, wait.Poll(time.Second, Scale(time.Minute), func() (bool, error) {
-					if result.Requeue {
-						result = mustReconcile(t, cluster)
-					}
-
-					list := corev1.PodList{}
-					selector, err := labels.Parse(strings.Join([]string{
-						"postgres-operator.crunchydata.com/cluster=" + cluster.Name,
-						"postgres-operator.crunchydata.com/instance",
-					}, ","))
-					assert.NilError(t, err)
-					assert.NilError(t, cc.List(ctx, &list,
-						client.InNamespace(cluster.Namespace),
-						client.MatchingLabelsSelector{Selector: selector}))
-
-					instances = list.Items
-
-					// Patroni doesn't use "primary" to identify the primary.
-					return len(instances) == 1 &&
-						instances[0].Labels["postgres-operator.crunchydata.com/role"] == "master", nil
-				}), "expected one instance, got:\n%+v", instances)
-
-				// Patroni DCS objects should not be deleted yet.
-				{
-					list := corev1.EndpointsList{}
-					selector, err := labels.Parse(strings.Join([]string{
-						"postgres-operator.crunchydata.com/cluster=" + cluster.Name,
-						"postgres-operator.crunchydata.com/patroni",
-					}, ","))
-					assert.NilError(t, err)
-					assert.NilError(t, cc.List(ctx, &list,
-						client.InNamespace(cluster.Namespace),
-						client.MatchingLabelsSelector{Selector: selector}))
-
-					assert.Assert(t, len(list.Items) >= 2, // config + leader
-						"expected Patroni DCS objects to remain, there are %v",
-						len(list.Items))
-
-					// Endpoints are deleted differently than other resources, and
-					// Patroni might have recreated them to stay alive. Check that
-					// they are all from before the cluster delete operation.
-					// - https://issue.k8s.io/99407
-					assert.NilError(t,
-						cc.Get(ctx, client.ObjectKeyFromObject(cluster), cluster))
-
-					for _, endpoints := range list.Items {
-						assert.Assert(t,
-							endpoints.CreationTimestamp.Time.Before(cluster.DeletionTimestamp.Time),
-							`expected %q to be after %+v`, cluster.DeletionTimestamp, endpoints)
-					}
-				}
-			}
-
-			// Continue until cluster is gone.
-			assert.NilError(t, wait.Poll(time.Second, Scale(time.Minute), func() (bool, error) {
-				mustReconcile(t, cluster)
-
-				err := cc.Get(ctx, client.ObjectKeyFromObject(cluster), cluster)
-				return apierrors.IsNotFound(err), client.IgnoreNotFound(err)
-			}), "expected cluster to be deleted, got:\n%+v", *cluster)
-
-			var endpoints []corev1.Endpoints
-			assert.NilError(t, wait.Poll(time.Second, Scale(time.Minute/3), func() (bool, error) {
-				list := corev1.EndpointsList{}
-				selector, err := labels.Parse(strings.Join([]string{
-					"postgres-operator.crunchydata.com/cluster=" + cluster.Name,
-					"postgres-operator.crunchydata.com/patroni",
-				}, ","))
-				assert.NilError(t, err)
-				assert.NilError(t, cc.List(ctx, &list,
-					client.InNamespace(cluster.Namespace),
-					client.MatchingLabelsSelector{Selector: selector}))
-
-				endpoints = list.Items
-
-				return len(endpoints) == 0, nil
-			}), "Patroni DCS objects should be gone, got:\n%+v", endpoints)
-		})
-	}
-}
-
 func TestReconcilerHandleDeleteNamespace(t *testing.T) {
 	if !strings.EqualFold(os.Getenv("USE_EXISTING_CLUSTER"), "true") {
 		t.Skip("requires a running garbage collection controller")

testing/kuttl/e2e/delete/00--cluster.yaml

@@ -0,0 +1,27 @@
+---
+apiVersion: postgres-operator.crunchydata.com/v1beta1
+kind: PostgresCluster
+metadata:
+  name: delete
+spec:
+  postgresVersion: ${KUTTL_PG_VERSION}
+  instances:
+    - name: instance1
+      replicas: 1
+      dataVolumeClaimSpec:
+        accessModes:
+        - "ReadWriteOnce"
+        resources:
+          requests:
+            storage: 1Gi
+  backups:
+    pgbackrest:
+      repos:
+      - name: repo1
+        volume:
+          volumeClaimSpec:
+            accessModes:
+            - "ReadWriteOnce"
+            resources:
+              requests:
+                storage: 1Gi

testing/kuttl/e2e/delete/00-assert.yaml

@@ -0,0 +1,20 @@
+---
+apiVersion: postgres-operator.crunchydata.com/v1beta1
+kind: PostgresCluster
+metadata:
+  name: delete
+status:
+  instances:
+    - name: instance1
+      readyReplicas: 1
+      replicas: 1
+      updatedReplicas: 1
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  labels:
+    postgres-operator.crunchydata.com/cluster: delete
+    postgres-operator.crunchydata.com/pgbackrest-backup: replica-create
+status:
+  succeeded: 1

testing/kuttl/e2e/delete/01--delete-cluster.yaml

@@ -0,0 +1,8 @@
+---
+# Remove the cluster.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+delete:
+  - apiVersion: postgres-operator.crunchydata.com/v1beta1
+    kind: PostgresCluster
+    name: delete

testing/kuttl/e2e/delete/02-errors.yaml

@@ -0,0 +1,25 @@
+---
+apiVersion: postgres-operator.crunchydata.com/v1beta1
+kind: PostgresCluster
+labels:
+    postgres-operator.crunchydata.com/cluster: delete
+---
+apiVersion: apps/v1
+kind: StatefulSet
+labels:
+    postgres-operator.crunchydata.com/cluster: delete
+---
+apiVersion: v1
+kind: Pod
+labels:
+    postgres-operator.crunchydata.com/cluster: delete
+---
+apiVersion: v1
+kind: Service
+labels:
+    postgres-operator.crunchydata.com/cluster: delete
+---
+apiVersion: v1
+kind: Secret
+labels:
+    postgres-operator.crunchydata.com/cluster: delete