chore: review

Signed-off-by: Armando Ruocco <[email protected]>

tests/e2e/asserts_test.go

@@ -504,25 +504,27 @@ func insertRecordIntoTable(tableName string, value int, conn *sql.DB) {
 	Expect(err).ToNot(HaveOccurred())
 }
 
-func AssertQueryEventuallyMatchExpectation(
+func QueryMatchExpectationPredicate(
 	pod *corev1.Pod,
 	dbname testsUtils.DatabaseName,
 	query string,
 	expectedOutput string,
-) {
-	Eventually(func(g Gomega) {
+) func(g Gomega) {
+	return func(g Gomega) {
+		// executor
 		stdout, stderr, err := env.ExecQueryInInstancePod(
-			testsUtils.PodLocator{
-				Namespace: pod.Namespace,
-				PodName:   pod.Name,
-			}, dbname, query)
+			testsUtils.PodLocator{Namespace: pod.Namespace, PodName: pod.Name},
+			dbname,
+			query,
+		)
+
 		if err != nil {
 			GinkgoWriter.Printf("stdout: %v\nstderr: %v", stdout, stderr)
 		}
 		g.Expect(err).ToNot(HaveOccurred())
 		g.Expect(strings.Trim(stdout, "\n")).To(BeEquivalentTo(expectedOutput),
 			fmt.Sprintf("expected query %q to return %q", query, expectedOutput))
-	}, 30).Should(Succeed())
+	}
 }
 
 func roleExistsQuery(roleName string) string {
@@ -973,10 +975,10 @@ func AssertReplicaModeCluster(
 		// verify the replica database created followed the source database, rather than
 		// default to the "app" db and user
 		By("checking that in replica cluster there is no database app and user app", func() {
-			AssertQueryEventuallyMatchExpectation(primaryReplicaCluster, testsUtils.PostgresDBName,
-				databaseExistsQuery("app"), "f")
-			AssertQueryEventuallyMatchExpectation(primaryReplicaCluster, testsUtils.PostgresDBName,
-				roleExistsQuery("app"), "f")
+			Eventually(QueryMatchExpectationPredicate(primaryReplicaCluster, testsUtils.PostgresDBName,
+				databaseExistsQuery("app"), "f"), 30).Should(Succeed())
+			Eventually(QueryMatchExpectationPredicate(primaryReplicaCluster, testsUtils.PostgresDBName,
+				roleExistsQuery("app"), "f"), 30).Should(Succeed())
 		})
 	}
 }
@@ -1056,10 +1058,10 @@ func AssertDetachReplicaModeCluster(
 	By("verifying the replica database doesn't exist in the replica cluster", func() {
 		// Application database configuration is skipped for replica clusters,
 		// so we expect these to not be present
-		AssertQueryEventuallyMatchExpectation(primaryReplicaCluster, testsUtils.PostgresDBName,
-			databaseExistsQuery(replicaDatabaseName), "f")
-		AssertQueryEventuallyMatchExpectation(primaryReplicaCluster, testsUtils.PostgresDBName,
-			roleExistsQuery(replicaUserName), "f")
+		Eventually(QueryMatchExpectationPredicate(primaryReplicaCluster, testsUtils.PostgresDBName,
+			databaseExistsQuery(replicaDatabaseName), "f"), 30).Should(Succeed())
+		Eventually(QueryMatchExpectationPredicate(primaryReplicaCluster, testsUtils.PostgresDBName,
+			roleExistsQuery(replicaUserName), "f"), 30).Should(Succeed())
 	})
 
 	By("writing some new data to the source cluster", func() {

tests/e2e/cluster_microservice_test.go

@@ -261,8 +261,8 @@ func assertTableAndDataOnImportedCluster(
 		})
 
 		By("verifying the user named 'micro' on source is not in imported database", func() {
-			AssertQueryEventuallyMatchExpectation(pod, testsUtils.PostgresDBName,
-				roleExistsQuery("micro"), "f")
+			Eventually(QueryMatchExpectationPredicate(pod, testsUtils.PostgresDBName,
+				roleExistsQuery("micro"), "f"), 30).Should(Succeed())
 		})
 	})
 }
@@ -331,10 +331,10 @@ func assertImportRenamesSelectedDatabase(
 		importedPrimaryPod, err := env.GetClusterPrimary(namespace, importedClusterName)
 		Expect(err).ToNot(HaveOccurred())
 
-		AssertQueryEventuallyMatchExpectation(importedPrimaryPod, testsUtils.PostgresDBName,
-			roleExistsQuery("db2"), "f")
-		AssertQueryEventuallyMatchExpectation(importedPrimaryPod, testsUtils.PostgresDBName,
-			roleExistsQuery("app"), "t")
+		Eventually(QueryMatchExpectationPredicate(importedPrimaryPod, testsUtils.PostgresDBName,
+			roleExistsQuery("db2"), "f"), 30).Should(Succeed())
+		Eventually(QueryMatchExpectationPredicate(importedPrimaryPod, testsUtils.PostgresDBName,
+			roleExistsQuery("app"), "t"), 30).Should(Succeed())
 	})
 
 	By("cleaning up the clusters", func() {

tests/e2e/declarative_database_management_test.go

@@ -119,8 +119,8 @@ var _ = Describe("Declarative database management", Label(tests.LabelSmoke, test
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, testsUtils.PostgresDBName,
-					databaseExistsQuery(dbname), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, testsUtils.PostgresDBName,
+					databaseExistsQuery(dbname), "t"), 30).Should(Succeed())
 
 				assertDatabaseHasExpectedFields(namespace, primaryPodInfo.Name, database)
 			})
@@ -133,8 +133,8 @@ var _ = Describe("Declarative database management", Label(tests.LabelSmoke, test
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, testsUtils.PostgresDBName,
-					databaseExistsQuery(dbname), boolPGOutput(retainOnDeletion))
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, testsUtils.PostgresDBName,
+					databaseExistsQuery(dbname), boolPGOutput(retainOnDeletion)), 30).Should(Succeed())
 			})
 		}
 

tests/e2e/managed_roles_test.go

@@ -139,14 +139,14 @@ var _ = Describe("Managed roles tests", Label(tests.LabelSmoke, tests.LabelBasic
 				primaryPod, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(username), "t")
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(userWithPerpetualPass), "t")
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(userWithHashedPassword), "t")
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(unrealizableUser), "f")
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(username), "t"), 30).Should(Succeed())
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(userWithPerpetualPass), "t"), 30).Should(Succeed())
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(userWithHashedPassword), "t"), 30).Should(Succeed())
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(unrealizableUser), "f"), 30).Should(Succeed())
 
 				query := fmt.Sprintf("SELECT true FROM pg_roles WHERE rolname='%s' and rolcanlogin=%v and rolsuper=%v "+
 					"and rolcreatedb=%v and rolcreaterole=%v and rolinherit=%v and rolreplication=%v "+
@@ -180,8 +180,8 @@ var _ = Describe("Managed roles tests", Label(tests.LabelSmoke, tests.LabelBasic
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, testsUtils.PostgresDBName,
-					roleExistsQuery(appUsername), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, testsUtils.PostgresDBName,
+					roleExistsQuery(appUsername), "t"), 30).Should(Succeed())
 
 				query := fmt.Sprintf("SELECT rolcreatedb and rolvaliduntil='infinity' "+
 					"FROM pg_roles WHERE rolname='%s'", appUsername)
@@ -377,8 +377,8 @@ var _ = Describe("Managed roles tests", Label(tests.LabelSmoke, tests.LabelBasic
 					Expect(err).ToNot(HaveOccurred())
 					return len(cluster.Status.ManagedRolesStatus.CannotReconcile)
 				}, 30).Should(Equal(0))
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(unrealizableUser), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(unrealizableUser), "t"), 30).Should(Succeed())
 			})
 
 			By("Add role in InRole for role new_role and verify in database", func() {
@@ -439,8 +439,8 @@ var _ = Describe("Managed roles tests", Label(tests.LabelSmoke, tests.LabelBasic
 				err = env.Client.Patch(env.Ctx, updated, client.MergeFrom(cluster))
 				Expect(err).ToNot(HaveOccurred())
 				// user not changed
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(unrealizableUser), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(unrealizableUser), "t"), 30).Should(Succeed())
 				Eventually(func() int {
 					cluster, err := env.GetCluster(namespace, clusterName)
 					Expect(err).ToNot(HaveOccurred())
@@ -553,8 +553,8 @@ var _ = Describe("Managed roles tests", Label(tests.LabelSmoke, tests.LabelBasic
 			By("Verify new_role not existed in db", func() {
 				primaryPod, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
-				AssertQueryEventuallyMatchExpectation(primaryPod, testsUtils.PostgresDBName,
-					roleExistsQuery(newUserName), "f")
+				Eventually(QueryMatchExpectationPredicate(primaryPod, testsUtils.PostgresDBName,
+					roleExistsQuery(newUserName), "f"), 30).Should(Succeed())
 			})
 		})
 	})

tests/e2e/publication_subscription_test.go

@@ -119,10 +119,10 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 
 			Expect(DeleteResourcesFromFile(namespace, destinationDatabaseManifest)).To(Succeed())
 			Expect(DeleteResourcesFromFile(namespace, sourceDatabaseManifest)).To(Succeed())
-			AssertQueryEventuallyMatchExpectation(sourcePrimaryPod, testsUtils.PostgresDBName,
-				databaseExistsQuery(dbname), "f")
-			AssertQueryEventuallyMatchExpectation(destPrimaryPod, testsUtils.PostgresDBName,
-				databaseExistsQuery(dbname), "f")
+			Eventually(QueryMatchExpectationPredicate(sourcePrimaryPod, testsUtils.PostgresDBName,
+				databaseExistsQuery(dbname), "f"), 30).Should(Succeed())
+			Eventually(QueryMatchExpectationPredicate(destPrimaryPod, testsUtils.PostgresDBName,
+				databaseExistsQuery(dbname), "f"), 30).Should(Succeed())
 		})
 
 		assertCreateDatabase := func(namespace, clusterName, databaseManifest string) {
@@ -151,8 +151,8 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, testsUtils.PostgresDBName,
-					databaseExistsQuery(databaseObject.Spec.Name), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, testsUtils.PostgresDBName,
+					databaseExistsQuery(databaseObject.Spec.Name), "t"), 30).Should(Succeed())
 			})
 		}
 
@@ -184,8 +184,8 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, dbname,
-					publicationExistsQuery(pubName), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, dbname,
+					publicationExistsQuery(pubName), "t"), 30).Should(Succeed())
 			})
 		}
 
@@ -217,8 +217,8 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, clusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, dbname,
-					subscriptionExistsQuery(subName), "t")
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, dbname,
+					subscriptionExistsQuery(subName), "t"), 30).Should(Succeed())
 			})
 		}
 
@@ -263,13 +263,23 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 				Expect(err).NotTo(HaveOccurred())
 
 				Eventually(func(g Gomega) {
-					publication, err = testsUtils.GetPublicationObject(namespace, pubObjectName, env)
+					var pub apiv1.Publication
+					err = testsUtils.GetObject(
+						env,
+						types.NamespacedName{Namespace: namespace, Name: pubObjectName},
+						&pub,
+					)
 					g.Expect(err).ToNot(HaveOccurred())
 					publication.Spec.ReclaimPolicy = publicationReclaimPolicy
 					err = env.Client.Update(env.Ctx, publication)
 					g.Expect(err).ToNot(HaveOccurred())
 
-					subscription, err = testsUtils.GetSubscriptionObject(namespace, subObjectName, env)
+					var sub apiv1.Subscription
+					err = testsUtils.GetObject(
+						env,
+						types.NamespacedName{Namespace: namespace, Name: subObjectName},
+						&sub,
+					)
 					g.Expect(err).ToNot(HaveOccurred())
 					subscription.Spec.ReclaimPolicy = subscriptionReclaimPolicy
 					err = env.Client.Update(env.Ctx, subscription)
@@ -296,16 +306,16 @@ var _ = Describe("Publication and Subscription", Label(tests.LabelDeclarativePub
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, sourceClusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, dbname,
-					publicationExistsQuery(pubName), boolPGOutput(retainOnDeletion))
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, dbname,
+					publicationExistsQuery(pubName), boolPGOutput(retainOnDeletion)), 30).Should(Succeed())
 			})
 
 			By("verifying the subscription reclaim policy outcome", func() {
 				primaryPodInfo, err := env.GetClusterPrimary(namespace, destinationClusterName)
 				Expect(err).ToNot(HaveOccurred())
 
-				AssertQueryEventuallyMatchExpectation(primaryPodInfo, dbname,
-					subscriptionExistsQuery(subName), boolPGOutput(retainOnDeletion))
+				Eventually(QueryMatchExpectationPredicate(primaryPodInfo, dbname,
+					subscriptionExistsQuery(subName), boolPGOutput(retainOnDeletion)), 30).Should(Succeed())
 			})
 		}
 

tests/utils/postgres.go

@@ -21,9 +21,6 @@ import (
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
-
-	apiv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 )
 
 const (
@@ -58,31 +55,3 @@ func CountReplicas(env *TestingEnvironment, pod *corev1.Pod) (int, error) {
 	}
 	return strconv.Atoi(strings.Trim(stdOut, "\n"))
 }
-
-// GetPublicationObject gets a Publication given name and namespace
-func GetPublicationObject(namespace string, name string, env *TestingEnvironment) (*apiv1.Publication, error) {
-	namespacedName := types.NamespacedName{
-		Namespace: namespace,
-		Name:      name,
-	}
-	publication := &apiv1.Publication{}
-	err := GetObject(env, namespacedName, publication)
-	if err != nil {
-		return nil, err
-	}
-	return publication, nil
-}
-
-// GetSubscriptionObject gets a Subscription given name and namespace
-func GetSubscriptionObject(namespace string, name string, env *TestingEnvironment) (*apiv1.Subscription, error) {
-	namespacedName := types.NamespacedName{
-		Namespace: namespace,
-		Name:      name,
-	}
-	subscription := &apiv1.Subscription{}
-	err := GetObject(env, namespacedName, subscription)
-	if err != nil {
-		return nil, err
-	}
-	return subscription, nil
-}