test(AKS): improves E2E test resiliency to transient network failures

This patch wraps several AKS API calls inside `Eventually` or `Retry`
statements to avoid faulty E2E test results due to transient network issues.

Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>

Author: wadlejitendra

tests/e2e/asserts_test.go

@@ -551,11 +551,13 @@ func AssertNewPrimary(namespace string, clusterName string, oldPrimary string) {
 }
 
 func AssertStorageCredentialsAreCreated(namespace string, name string, id string, key string) {
-	_, _, err := testsUtils.Run(fmt.Sprintf("kubectl create secret generic %v -n %v "+
-		"--from-literal='ID=%v' "+
-		"--from-literal='KEY=%v'",
-		name, namespace, id, key))
-	Expect(err).ToNot(HaveOccurred())
+	Eventually(func() error {
+		_, _, err := testsUtils.Run(fmt.Sprintf("kubectl create secret generic %v -n %v "+
+			"--from-literal='ID=%v' "+
+			"--from-literal='KEY=%v'",
+			name, namespace, id, key))
+		return err
+	}, 60, 5).Should(BeNil())
 }
 
 // AssertArchiveWalOnMinio to archive walls and verify that exists
@@ -735,7 +737,8 @@ func AssertWritesToReplicaFails(
 			Expect(value, err).To(Equal("t"))
 
 			// Expect to be in a read-only transaction
-			_, _, err = env.ExecCommand(env.Ctx, *connectingPod, specs.PostgresContainerName, &timeout,
+			_, _, err = utils.ExecCommand(env.Ctx, env.Interface, env.RestClientConfig, *connectingPod,
+				specs.PostgresContainerName, &timeout,
 				"psql", dsn, "-tAc", "CREATE TABLE table1(var1 text);")
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).Should(
@@ -2015,7 +2018,7 @@ func CreateResourceFromFile(namespace, sampleFilePath string) {
 			return err
 		}
 		return nil
-	}, 60, 5).Should(BeNil())
+	}, RetryTimeout, PollingTime).Should(BeNil())
 }
 
 func AssertBackupConditionInClusterStatus(namespace, clusterName string) {

tests/e2e/configuration_update_test.go

@@ -15,6 +15,7 @@ import (
 
 	apiv1 "github.com/EnterpriseDB/cloud-native-postgresql/api/v1"
 	"github.com/EnterpriseDB/cloud-native-postgresql/pkg/specs"
+	devUtils "github.com/EnterpriseDB/cloud-native-postgresql/pkg/utils"
 	"github.com/EnterpriseDB/cloud-native-postgresql/tests"
 	"github.com/EnterpriseDB/cloud-native-postgresql/tests/utils"
 
@@ -24,12 +25,42 @@ import (
 
 var _ = Describe("Configuration update", func() {
 	const (
-		clusterName = "postgresql-storage-class"
-		namespace   = "cluster-update-config-e2e"
-		sampleFile  = fixturesDir + "/base/cluster-storage-class.yaml"
-		level       = tests.High
+		clusterName          = "postgresql-storage-class"
+		namespace            = "cluster-update-config-e2e"
+		sampleFile           = fixturesDir + "/base/cluster-storage-class.yaml"
+		level                = tests.High
+		autoVacuumMaxWorkers = 4
 	)
 
+	checkErrorOutFixedAndBlockedConfigurationParameter := func(sample string) {
+		// Update the configuration
+		Eventually(func() error {
+			_, _, err := utils.RunUnchecked("kubectl apply -n " + namespace + " -f " + sample)
+			return err
+			// Expecting an error when a blockedConfigurationParameter is modified
+		}, RetryTimeout, PollingTime).ShouldNot(BeNil())
+
+		podList, err := env.GetClusterPodList(namespace, clusterName)
+		Expect(err).ToNot(HaveOccurred())
+
+		const timeout = 60
+		commandTimeout := time.Second * 2
+		// Expect other config parameters applied together with a blockedParameter to not have changed
+		for idx := range podList.Items {
+			pod := podList.Items[idx]
+			Eventually(func(g Gomega) int {
+				stdout, _, err := env.ExecCommand(env.Ctx, pod, specs.PostgresContainerName, &commandTimeout,
+					"psql", "-U", "postgres", "-tAc", "show autovacuum_max_workers")
+				g.Expect(err).ToNot(HaveOccurred())
+
+				value, atoiErr := strconv.Atoi(strings.Trim(stdout, "\n"))
+				g.Expect(atoiErr).ToNot(HaveOccurred())
+
+				return value
+			}, timeout).ShouldNot(BeEquivalentTo(autoVacuumMaxWorkers))
+		}
+	}
+
 	BeforeEach(func() {
 		if testLevelEnv.Depth < int(level) {
 			Skip("Test depth is lower than the amount requested for this test")
@@ -80,8 +111,15 @@ var _ = Describe("Configuration update", func() {
 			// Connection should fail now because we are not supplying a password
 			podList, err := env.GetClusterPodList(namespace, clusterName)
 			Expect(err).ToNot(HaveOccurred())
-			stdout, _, err := env.ExecCommand(env.Ctx, podList.Items[0], specs.PostgresContainerName, &commandtimeout,
-				"psql", "-U", "postgres", "-h", endpointName, "-tAc", "select 1")
+			stdout, _, err := devUtils.ExecCommand(
+				env.Ctx,
+				env.Interface,
+				env.RestClientConfig,
+				podList.Items[0],
+				specs.PostgresContainerName,
+				&commandtimeout,
+				"psql", "-U", "postgres", "-h", endpointName, "-tAc", "select 1",
+			)
 			Expect(err).To(HaveOccurred())
 			// Update the configuration
 			CreateResourceFromFile(namespace, sample)
@@ -180,45 +218,11 @@ var _ = Describe("Configuration update", func() {
 		})
 		By("Erroring out when a fixedConfigurationParameter is modified", func() {
 			sample := fixturesDir + "/config_update/05-fixed-params.yaml"
-			// Update the configuration
-			_, _, err := utils.RunUnchecked("kubectl apply -n " + namespace + " -f " + sample)
-			// Expecting an error when a fixedConfigurationParameter is modified
-			Expect(err).To(HaveOccurred())
-			podList, err := env.GetClusterPodList(namespace, clusterName)
-			Expect(err).ToNot(HaveOccurred())
-			timeout := 60
-			commandtimeout := time.Second * 2
-			// Expect other config parameters applied together with a fixedParameter to not have changed
-			for _, pod := range podList.Items {
-				pod := pod // pin the variable
-				Eventually(func() (int, error, error) {
-					stdout, _, err := env.ExecCommand(env.Ctx, pod, specs.PostgresContainerName, &commandtimeout,
-						"psql", "-U", "postgres", "-tAc", "show autovacuum_max_workers")
-					value, atoiErr := strconv.Atoi(strings.Trim(stdout, "\n"))
-					return value, err, atoiErr
-				}, timeout).ShouldNot(BeEquivalentTo(4))
-			}
+			checkErrorOutFixedAndBlockedConfigurationParameter(sample)
 		})
 		By("Erroring out when a blockedConfigurationParameter is modified", func() {
 			sample := fixturesDir + "/config_update/06-blocked-params.yaml"
-			// Update the configuration
-			_, _, err := utils.RunUnchecked("kubectl apply -n " + namespace + " -f " + sample)
-			// Expecting an error when a blockedConfigurationParameter is modified
-			Expect(err).To(HaveOccurred())
-			podList, err := env.GetClusterPodList(namespace, clusterName)
-			Expect(err).ToNot(HaveOccurred())
-			timeout := 60
-			commandtimeout := time.Second * 2
-			// Expect other config parameters applied together with a blockedParameter to not have changed
-			for _, pod := range podList.Items {
-				pod := pod
-				Eventually(func() (int, error, error) {
-					stdout, _, err := env.ExecCommand(env.Ctx, pod, specs.PostgresContainerName, &commandtimeout,
-						"psql", "-U", "postgres", "-tAc", "show autovacuum_max_workers")
-					value, atoiErr := strconv.Atoi(strings.Trim(stdout, "\n"))
-					return value, err, atoiErr
-				}, timeout).ShouldNot(BeEquivalentTo(4))
-			}
+			checkErrorOutFixedAndBlockedConfigurationParameter(sample)
 		})
 
 		// nolint:dupl
@@ -238,8 +242,7 @@ var _ = Describe("Configuration update", func() {
 				Expect(cluster.Status.CurrentPrimary, err).To(BeEquivalentTo(cluster.Status.TargetPrimary))
 				oldPrimary := cluster.Status.CurrentPrimary
 				// Update the configuration
-				_, _, err = utils.Run("kubectl apply -n " + namespace + " -f " + sample)
-				Expect(err).ToNot(HaveOccurred())
+				CreateResourceFromFile(namespace, sample)
 				timeout := 300
 				commandtimeout := time.Second * 2
 				// Check that the new parameter has been modified in every pod

tests/e2e/eviction_test.go

@@ -99,14 +99,6 @@ var _ = Describe("Pod eviction", Serial, Label(tests.LabelDisruptive), func() {
 			if testLevelEnv.Depth < int(level) {
 				Skip("Test depth is lower than the amount requested for this test")
 			}
-			// limit the case running on local kind env as we are using taint to simulate the eviction
-			// we do not know if other cloud vendor crd controller is running on the node been evicted
-			isIBM := env.IsIBM()
-			isAKS, _ := env.IsAKS()
-			isGKE, _ := env.IsGKE()
-			if isIBM || isAKS || isGKE {
-				Skip("Test runs only on local")
-			}
 		})
 		JustAfterEach(func() {
 			clusterName, err := env.GetResourceNameFromYAML(singleInstanceSampleFile)
@@ -117,6 +109,14 @@ var _ = Describe("Pod eviction", Serial, Label(tests.LabelDisruptive), func() {
 			}
 		})
 		BeforeAll(func() {
+			// limit the case running on local kind env as we are using taint to simulate the eviction
+			// we do not know if other cloud vendor crd controller is running on the node been evicted
+			isIBM := env.IsIBM()
+			isAKS, _ := env.IsAKS()
+			isGKE, _ := env.IsGKE()
+			if isIBM || isAKS || isGKE {
+				Skip("Test runs only on local")
+			}
 			namespace = "single-instance-pod-eviction"
 			err := env.CreateNamespace(namespace)
 			Expect(err).ToNot(HaveOccurred())

tests/e2e/logs_test.go

@@ -18,6 +18,7 @@ import (
 	apiv1 "github.com/EnterpriseDB/cloud-native-postgresql/api/v1"
 	"github.com/EnterpriseDB/cloud-native-postgresql/pkg/management/postgres/logpipe"
 	"github.com/EnterpriseDB/cloud-native-postgresql/pkg/specs"
+	"github.com/EnterpriseDB/cloud-native-postgresql/pkg/utils"
 	"github.com/EnterpriseDB/cloud-native-postgresql/tests"
 	testsUtils "github.com/EnterpriseDB/cloud-native-postgresql/tests/utils"
 
@@ -51,8 +52,8 @@ var _ = Describe("JSON log output", func() {
 		clusterName = "postgresql-json-logs"
 		const sampleFile = fixturesDir + "/json_logs/cluster-json-logs.yaml"
 		// Create a cluster in a namespace we'll delete after the test
-		err := env.CreateNamespace(namespace)
-		Expect(err).ToNot(HaveOccurred())
+		namespaceErr := env.CreateNamespace(namespace)
+		Expect(namespaceErr).ToNot(HaveOccurred())
 		AssertCreateCluster(namespace, clusterName, sampleFile, env)
 
 		By("verifying the presence of possible logger values", func() {
@@ -79,24 +80,29 @@ var _ = Describe("JSON log output", func() {
 			timeout := 300
 
 			for _, pod := range podList.Items {
+				var queryError error
 				// Run a wrong query and save its result
 				commandTimeout := time.Second * 5
-				_, _, err = env.ExecCommand(env.Ctx, pod, specs.PostgresContainerName,
-					&commandTimeout, "psql", "-U", "postgres", "app", "-tAc", errorTestQuery)
-				Expect(err).To(HaveOccurred())
-				expectedResult := err.Error()
+				Eventually(func(g Gomega) error {
+					_, _, queryError = utils.ExecCommand(env.Ctx, env.Interface, env.RestClientConfig, pod,
+						specs.PostgresContainerName, &commandTimeout, "psql", "-U", "postgres", "app", "-tAc",
+						errorTestQuery)
+					return queryError
+				}, RetryTimeout, PollingTime).ShouldNot(BeNil())
 
 				// Eventually the error log line will be logged
-				Eventually(func() (bool, error) {
+				Eventually(func(g Gomega) bool {
 					// Gather pod logs in the form of a Json Array
 					logEntries, err := testsUtils.ParseJSONLogs(namespace, pod.GetName(), env)
-					if err != nil {
-						return false, err
-					}
+					g.Expect(err).ToNot(HaveOccurred())
 
 					// Gather the record containing the wrong query result
-					return testsUtils.AssertQueryRecord(logEntries, errorTestQuery, expectedResult,
-						logpipe.LoggingCollectorRecordName), nil
+					return testsUtils.AssertQueryRecord(
+						logEntries,
+						errorTestQuery,
+						queryError.Error(),
+						logpipe.LoggingCollectorRecordName,
+					)
 				}, timeout).Should(BeTrue())
 			}
 		})
@@ -106,12 +112,15 @@ var _ = Describe("JSON log output", func() {
 			primaryPod, _ := env.GetClusterPrimary(namespace, clusterName)
 			timeout := 300
 
+			var queryError error
 			// Run a wrong query on just the primary and save its result
 			commandTimeout := time.Second * 5
-			_, _, err = env.ExecCommand(env.Ctx, *primaryPod, specs.PostgresContainerName,
-				&commandTimeout, "psql", "-U", "postgres", "app", "-tAc", errorTestQuery)
-			Expect(err).To(HaveOccurred())
-			expectedResult := err.Error()
+			Eventually(func() error {
+				_, _, queryError = utils.ExecCommand(env.Ctx, env.Interface, env.RestClientConfig,
+					*primaryPod, specs.PostgresContainerName,
+					&commandTimeout, "psql", "-U", "postgres", "app", "-tAc", errorTestQuery)
+				return queryError
+			}, RetryTimeout, PollingTime).ShouldNot(BeNil())
 
 			// Expect the query to be eventually logged on the primary
 			Eventually(func() (bool, error) {
@@ -123,17 +132,17 @@ var _ = Describe("JSON log output", func() {
 				}
 
 				// Gather the record containing the wrong query result
-				return testsUtils.AssertQueryRecord(logEntries, errorTestQuery, expectedResult,
+				return testsUtils.AssertQueryRecord(logEntries, errorTestQuery, queryError.Error(),
 					logpipe.LoggingCollectorRecordName), nil
 			}, timeout).Should(BeTrue())
 
 			// Retrieve cluster replicas
 			podList := &corev1.PodList{}
-			err = env.Client.List(
+			listError := env.Client.List(
 				env.Ctx, podList, client.InNamespace(namespace),
 				client.MatchingLabels{"postgresql": clusterName, "role": "replica"},
 			)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(listError).ToNot(HaveOccurred())
 
 			// Expect the query not to be logged on replicas
 			for _, pod := range podList.Items {
@@ -143,8 +152,14 @@ var _ = Describe("JSON log output", func() {
 				Expect(len(logEntries) > 0).To(BeTrue())
 
 				// No record should be returned in this case
-				Expect(testsUtils.AssertQueryRecord(logEntries, expectedResult, errorTestQuery,
-					logpipe.LoggingCollectorRecordName)).Should(BeFalse())
+				isQueryRecordContained := testsUtils.AssertQueryRecord(
+					logEntries,
+					queryError.Error(),
+					errorTestQuery,
+					logpipe.LoggingCollectorRecordName,
+				)
+
+				Expect(isQueryRecordContained).Should(BeFalse())
 			}
 		})
 
@@ -155,8 +170,9 @@ var _ = Describe("JSON log output", func() {
 			forceDelete := &client.DeleteOptions{
 				GracePeriodSeconds: &zero,
 			}
-			err = env.DeletePod(namespace, currentPrimary.GetName(), forceDelete)
-			Expect(err).ToNot(HaveOccurred())
+
+			deletePodError := env.DeletePod(namespace, currentPrimary.GetName(), forceDelete)
+			Expect(deletePodError).ToNot(HaveOccurred())
 
 			// Expect a new primary to be elected
 			timeout := 180

tests/e2e/rolling_update_test.go

@@ -12,7 +12,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/util/retry"
 
 	apiv1 "github.com/EnterpriseDB/cloud-native-postgresql/api/v1"
 	"github.com/EnterpriseDB/cloud-native-postgresql/internal/configuration"
@@ -76,13 +75,13 @@ var _ = Describe("Rolling updates", func() {
 			Namespace: namespace,
 			Name:      clusterName,
 		}
-		err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		Eventually(func(g Gomega) error {
 			err := env.Client.Get(env.Ctx, namespacedName, cluster)
-			Expect(err).ToNot(HaveOccurred())
+			g.Expect(err).ToNot(HaveOccurred())
+
 			cluster.Spec.ImageName = updatedImageName
 			return env.Client.Update(env.Ctx, cluster)
-		})
-		Expect(err).ToNot(HaveOccurred())
+		}, RetryTimeout, PollingTime).Should(BeNil())
 
 		// All the postgres containers should have the updated image
 		Eventually(func() (int32, error) {

tests/e2e/suite_test.go

@@ -25,6 +25,10 @@ import (
 const (
 	samplesDir  = "../../docs/src/samples"
 	fixturesDir = "./fixtures"
+	// RetryTimeout retry time when client api call or kubectl cli request get failed
+	RetryTimeout = 60
+	// PollingTime polling between retry
+	PollingTime = 5
 )
 
 var (

tests/e2e/syncreplicas_test.go

@@ -14,7 +14,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/util/retry"
 
 	clusterv1 "github.com/EnterpriseDB/cloud-native-postgresql/api/v1"
 	"github.com/EnterpriseDB/cloud-native-postgresql/pkg/specs"
@@ -89,16 +88,15 @@ var _ = Describe("Synchronous Replicas", func() {
 				Namespace: namespace,
 				Name:      clusterName,
 			}
-
 			// Set MaxSyncReplicas to 1
-			err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+			Eventually(func(g Gomega) error {
 				cluster := &clusterv1.Cluster{}
 				err := env.Client.Get(env.Ctx, namespacedName, cluster)
-				Expect(err).ToNot(HaveOccurred())
+				g.Expect(err).ToNot(HaveOccurred())
+
 				cluster.Spec.MaxSyncReplicas = 1
 				return env.Client.Update(env.Ctx, cluster)
-			})
-			Expect(err).ToNot(HaveOccurred())
+			}, 60, 5).Should(BeNil())
 
 			// Scale the cluster down to 2 pods
 			_, _, err := utils.Run(fmt.Sprintf("kubectl scale --replicas=2 -n %v cluster/%v", namespace, clusterName))