Adding uniqueness to cluster names when testing service type changes to work around race condition that is causing these tests to flake.

[sc-16571]

Author: dsessler7

internal/controller/postgrescluster/patroni_test.go

@@ -283,10 +283,21 @@ func TestReconcilePatroniLeaderLease(t *testing.T) {
 	// CRD validation looks only at the new/incoming value of fields. Confirm
 	// that each ServiceType can change to any other ServiceType. Forbidding
 	// certain transitions requires a validating webhook.
+	serviceTypeChangeClusterCounter := 0
 	for _, beforeType := range serviceTypes {
 		for _, changeType := range serviceTypes {
 			t.Run(beforeType+"To"+changeType, func(t *testing.T) {
-				cluster := cluster.DeepCopy()
+				// Creating fresh clusters for these tests
+				cluster := testCluster()
+				cluster.Namespace = ns.Name
+
+				// Note (dsessler): Adding a number to each cluster name to make cluster/service
+				// names unique to work around an intermittent race condition where a service
+				// from a prior test has not been deleted yet when the next test runs, causing
+				// the test to fail due to non-matching IP addresses.
+				cluster.Name += "-" + strconv.Itoa(serviceTypeChangeClusterCounter)
+				assert.NilError(t, cc.Create(ctx, cluster))
+
 				cluster.Spec.Service = &v1beta1.ServiceSpec{Type: beforeType}
 
 				before, err := reconciler.reconcilePatroniLeaderLease(ctx, cluster)
@@ -309,6 +320,7 @@ func TestReconcilePatroniLeaderLease(t *testing.T) {
 				assert.NilError(t, err, "\n%#v", errors.Unwrap(err))
 				assert.Equal(t, after.Spec.ClusterIP, before.Spec.ClusterIP,
 					"expected to keep the same ClusterIP")
+				serviceTypeChangeClusterCounter++
 			})
 		}
 	}

internal/controller/postgrescluster/pgadmin_test.go

@@ -21,6 +21,7 @@ package postgrescluster
 import (
 	"context"
 	"io"
+	"strconv"
 	"testing"
 
 	"github.com/pkg/errors"
@@ -417,10 +418,25 @@ func TestReconcilePGAdminService(t *testing.T) {
 	// CRD validation looks only at the new/incoming value of fields. Confirm
 	// that each ServiceType can change to any other ServiceType. Forbidding
 	// certain transitions requires a validating webhook.
+	serviceTypeChangeClusterCounter := 0
 	for _, beforeType := range serviceTypes {
 		for _, changeType := range serviceTypes {
 			t.Run(beforeType+"To"+changeType, func(t *testing.T) {
-				cluster := cluster.DeepCopy()
+				// Creating fresh clusters for these tests
+				clusterNamespace := cluster.Namespace
+				cluster := testCluster()
+				cluster.Namespace = clusterNamespace
+
+				// Note (dsessler): Adding a number to each cluster name to make cluster/service
+				// names unique to work around an intermittent race condition where a service
+				// from a prior test has not been deleted yet when the next test runs, causing
+				// the test to fail due to non-matching IP addresses.
+				cluster.Name += "-" + strconv.Itoa(serviceTypeChangeClusterCounter)
+				assert.NilError(t, cc.Create(ctx, cluster))
+
+				cluster.Spec.UserInterface = &v1beta1.UserInterfaceSpec{
+					PGAdmin: &v1beta1.PGAdminPodSpec{},
+				}
 				cluster.Spec.UserInterface.PGAdmin.Service = &v1beta1.ServiceSpec{Type: beforeType}
 
 				before, err := reconciler.reconcilePGAdminService(ctx, cluster)
@@ -443,6 +459,7 @@ func TestReconcilePGAdminService(t *testing.T) {
 				assert.NilError(t, err, "\n%#v", errors.Unwrap(err))
 				assert.Equal(t, after.Spec.ClusterIP, before.Spec.ClusterIP,
 					"expected to keep the same ClusterIP")
+				serviceTypeChangeClusterCounter++
 			})
 		}
 	}

internal/controller/postgrescluster/pgbouncer_test.go

@@ -20,6 +20,7 @@ package postgrescluster
 
 import (
 	"context"
+	"strconv"
 	"testing"
 
 	"github.com/pkg/errors"
@@ -326,10 +327,27 @@ func TestReconcilePGBouncerService(t *testing.T) {
 	// CRD validation looks only at the new/incoming value of fields. Confirm
 	// that each ServiceType can change to any other ServiceType. Forbidding
 	// certain transitions requires a validating webhook.
+	serviceTypeChangeClusterCounter := 0
 	for _, beforeType := range serviceTypes {
 		for _, changeType := range serviceTypes {
 			t.Run(beforeType+"To"+changeType, func(t *testing.T) {
-				cluster := cluster.DeepCopy()
+				// Creating fresh clusters for these tests
+				clusterNamespace := cluster.Namespace
+				cluster := testCluster()
+				cluster.Namespace = clusterNamespace
+
+				// Note (dsessler): Adding a number to each cluster name to make cluster/service
+				// names unique to work around an intermittent race condition where a service
+				// from a prior test has not been deleted yet when the next test runs, causing
+				// the test to fail due to non-matching IP addresses.
+				cluster.Name += "-" + strconv.Itoa(serviceTypeChangeClusterCounter)
+				assert.NilError(t, cc.Create(ctx, cluster))
+
+				cluster.Spec.Proxy = &v1beta1.PostgresProxySpec{
+					PGBouncer: &v1beta1.PGBouncerPodSpec{
+						Port: initialize.Int32(19041),
+					},
+				}
 				cluster.Spec.Proxy.PGBouncer.Service = &v1beta1.ServiceSpec{Type: beforeType}
 
 				before, err := reconciler.reconcilePGBouncerService(ctx, cluster)
@@ -352,6 +370,7 @@ func TestReconcilePGBouncerService(t *testing.T) {
 				assert.NilError(t, err, "\n%#v", errors.Unwrap(err))
 				assert.Equal(t, after.Spec.ClusterIP, before.Spec.ClusterIP,
 					"expected to keep the same ClusterIP")
+				serviceTypeChangeClusterCounter++
 			})
 		}
 	}