Merge pull request #11001 from k8s-infra-cherrypick-robot/cherry-pick…

…-10975-to-release-1.7 [release-1.7] 🌱 test: stop using Consistently for rollout checks
kubernetes-sigs · Aug 2, 2024 · 16fa6e3 · 16fa6e3
2 parents 60f232c + c726331
commit 16fa6e3
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 19 deletions.
diff --git a/test/e2e/clusterctl_upgrade.go b/test/e2e/clusterctl_upgrade.go
@@ -627,43 +627,42 @@ func ClusterctlUpgradeSpec(ctx context.Context, inputGetter func() ClusterctlUpg
 				input.PostUpgrade(managementClusterProxy, workloadClusterNamespace, managementClusterName)
 			}
 
-			// After the upgrade check that MachineList is available. This ensures the APIServer is serving without
-			// error before checking that it `Consistently` returns the MachineList later on.
-			Byf("[%d] Waiting for MachineList to be available", i)
+			// After the upgrade: wait for MachineList to be available after the upgrade.
 			Eventually(func() error {
 				postUpgradeMachineList := &unstructured.UnstructuredList{}
 				postUpgradeMachineList.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   clusterv1.GroupVersion.Group,
 					Version: coreCAPIStorageVersion,
 					Kind:    "MachineList",
 				})
-				err = managementClusterProxy.GetClient().List(
+				return managementClusterProxy.GetClient().List(
 					ctx,
 					postUpgradeMachineList,
 					client.InNamespace(workloadCluster.GetNamespace()),
 					client.MatchingLabels{clusterv1.ClusterNameLabel: workloadCluster.GetName()},
 				)
-				return err
 			}, "3m", "30s").ShouldNot(HaveOccurred(), "MachineList should be available after the upgrade")
 
-			// After the upgrade check that there were no unexpected rollouts.
-			Byf("[%d] Verify there are no unexpected rollouts", i)
-			Consistently(func() bool {
-				postUpgradeMachineList := &unstructured.UnstructuredList{}
+			Byf("[%d] Waiting for three minutes before checking if an unexpected rollout happened", i)
+			time.Sleep(time.Minute * 3)
+
+			// After the upgrade: check that there were no unexpected rollouts.
+			postUpgradeMachineList := &unstructured.UnstructuredList{}
+			Byf("[%d] Verifing there are no unexpected rollouts", i)
+			Eventually(func() error {
 				postUpgradeMachineList.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   clusterv1.GroupVersion.Group,
 					Version: coreCAPIStorageVersion,
 					Kind:    "MachineList",
 				})
-				err = managementClusterProxy.GetClient().List(
+				return managementClusterProxy.GetClient().List(
 					ctx,
 					postUpgradeMachineList,
 					client.InNamespace(workloadCluster.GetNamespace()),
 					client.MatchingLabels{clusterv1.ClusterNameLabel: workloadCluster.GetName()},
 				)
-				Expect(err).ToNot(HaveOccurred())
-				return validateMachineRollout(preUpgradeMachineList, postUpgradeMachineList)
-			}, "3m", "30s").Should(BeTrue(), "Machines should remain the same after the upgrade")
+			}, "3m", "30s").ShouldNot(HaveOccurred(), "MachineList should be available after the upgrade")
+			Expect(validateMachineRollout(preUpgradeMachineList, postUpgradeMachineList)).To(BeTrue(), "Machines should remain the same after the upgrade")
 
 			// Scale up to 2 and back down to 1 so we can repeat this multiple times.
 			Byf("[%d] Scale MachineDeployment to ensure the providers work", i)

diff --git a/test/e2e/self_hosted.go b/test/e2e/self_hosted.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -296,20 +297,35 @@ func SelfHostedSpec(ctx context.Context, inputGetter func() SelfHostedSpecInput)
 		})
 		Expect(controlPlane).ToNot(BeNil())
 
-		// After the move check that there were no unexpected rollouts.
+		// After the move: wait for MachineList to be available after the upgrade.
 		log.Logf("Verify there are no unexpected rollouts")
-		Consistently(func() bool {
+		Eventually(func() error {
 			postMoveMachineList := &unstructured.UnstructuredList{}
 			postMoveMachineList.SetGroupVersionKind(clusterv1.GroupVersion.WithKind("MachineList"))
-			err = selfHostedClusterProxy.GetClient().List(
+			return selfHostedClusterProxy.GetClient().List(
 				ctx,
 				postMoveMachineList,
 				client.InNamespace(namespace.Name),
 				client.MatchingLabels{clusterv1.ClusterNameLabel: workloadClusterName},
 			)
-			Expect(err).NotTo(HaveOccurred(), "Failed to list machines after move")
-			return validateMachineRollout(preMoveMachineList, postMoveMachineList)
-		}, "3m", "30s").Should(BeTrue(), "Machines should not roll out after move to self-hosted cluster")
+		}, "3m", "30s").ShouldNot(HaveOccurred(), "MachineList should be available after move to self-hosted cluster")
+
+		log.Logf("Waiting for three minutes before checking if an unexpected rollout happened")
+		time.Sleep(time.Minute * 3)
+
+		// After the move: check that there were no unexpected rollouts.
+		postMoveMachineList := &unstructured.UnstructuredList{}
+		log.Logf("Verify there are no unexpected rollouts")
+		Eventually(func() error {
+			postMoveMachineList.SetGroupVersionKind(clusterv1.GroupVersion.WithKind("MachineList"))
+			return selfHostedClusterProxy.GetClient().List(
+				ctx,
+				postMoveMachineList,
+				client.InNamespace(namespace.Name),
+				client.MatchingLabels{clusterv1.ClusterNameLabel: workloadClusterName},
+			)
+		}, "3m", "30s").ShouldNot(HaveOccurred(), "MachineList should be available after move to self-hosted cluster")
+		Expect(validateMachineRollout(preMoveMachineList, postMoveMachineList)).To(BeTrue(), "Machines should not roll out after move to self-hosted cluster")
 
 		if input.SkipUpgrade {
 			// Only do upgrade step if defined by test input.