Merge pull request kubernetes-sigs#10975 from chrischdi/pr-test-improve-rollout-check

🌱 test: stop using Consistently for rollout checks

Author: k8s-ci-robot

test/e2e/clusterctl_upgrade.go

@@ -612,43 +612,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)

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.