improve existing e2e logic to avoid unexpected behaviour

Signed-off-by: yunbo <[email protected]>
openkruise · Jun 5, 2024 · 6090670 · 6090670
1 parent 9836ca6
commit 6090670
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 6 deletions.
diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go
@@ -210,6 +210,7 @@ func (m *canaryReleaseManager) doCanaryMetricsAnalysis(c *RolloutContext) (bool,
 
 func (m *canaryReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error) {
 	if m.doCanaryJump(c) {
+		klog.Infof("rollout(%s/%s) canary step jumped", c.Rollout.Namespace, c.Rollout.Name)
 		return false, nil
 	}
 	canaryStatus := c.NewStatus.CanaryStatus

diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go
@@ -26,6 +26,16 @@ import (
 
 var _ = SIGDescribe("Advanced Deployment", func() {
 	var namespace string
+
+	DumpAllResources := func() {
+		deploy := &apps.DeploymentList{}
+		k8sClient.List(context.TODO(), deploy, client.InNamespace(namespace))
+		fmt.Println(util.DumpJSON(deploy))
+		rs := &apps.ReplicaSetList{}
+		k8sClient.List(context.TODO(), rs, client.InNamespace(namespace))
+		fmt.Println(util.DumpJSON(rs))
+	}
+
 	defaultRetry := wait.Backoff{
 		Steps:    10,
 		Duration: 10 * time.Millisecond,
@@ -132,7 +142,12 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 
 	CheckReplicas := func(deployment *apps.Deployment, replicas, available, updated int32) {
 		var clone *apps.Deployment
+		start := time.Now()
 		Eventually(func() bool {
+			if start.Add(time.Minute * 2).Before(time.Now()) {
+				DumpAllResources()
+				Expect(true).Should(BeFalse())
+			}
 			clone = &apps.Deployment{}
 			err := GetObject(deployment.Namespace, deployment.Name, clone)
 			Expect(err).NotTo(HaveOccurred())
@@ -239,6 +254,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 			deployment.Namespace = namespace
 			Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
 			CreateObject(deployment)
+			CheckReplicas(deployment, 5, 5, 5)
 			UpdateDeployment(deployment, "version2")
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(1))
@@ -255,6 +271,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 			Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
 			deployment.Spec.Replicas = pointer.Int32(10)
 			CreateObject(deployment)
+			CheckReplicas(deployment, 10, 10, 10)
 			UpdateDeployment(deployment, "version2")
 			UpdatePartitionWithCheck(deployment, intstr.FromString("0%"))
 			UpdatePartitionWithCheck(deployment, intstr.FromString("40%"))
@@ -287,6 +304,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 				`{"rollingStyle":"Partition","rollingUpdate":{"maxUnavailable":1,"maxSurge":0}}`
 			deployment.Spec.MinReadySeconds = 10
 			CreateObject(deployment)
+			CheckReplicas(deployment, 5, 5, 5)
 			UpdateDeployment(deployment, "version2")
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
 			UpdatePartitionWithoutCheck(deployment, intstr.FromInt(3))
@@ -303,6 +321,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 			deployment.Namespace = namespace
 			Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
 			CreateObject(deployment)
+			CheckReplicas(deployment, 5, 5, 5)
 			UpdateDeployment(deployment, "version2")
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(2))
@@ -317,6 +336,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 			deployment.Namespace = namespace
 			Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
 			CreateObject(deployment)
+			CheckReplicas(deployment, 5, 5, 5)
 			UpdateDeployment(deployment, "version2")
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
 			UpdatePartitionWithCheck(deployment, intstr.FromInt(2))
@@ -335,6 +355,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
 			Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
 			deployment.Annotations["rollouts.kruise.io/deployment-strategy"] = `{"rollingUpdate":{"maxUnavailable":0,"maxSurge":1}}`
 			CreateObject(deployment)
+			CheckReplicas(deployment, 5, 5, 5)
 			UpdateDeployment(deployment, "version2", "busybox:not-exists")
 			UpdatePartitionWithoutCheck(deployment, intstr.FromInt(1))
 			CheckReplicas(deployment, 6, 5, 1)

diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go
@@ -204,18 +204,25 @@ var _ = SIGDescribe("Rollout", func() {
 	}
 
 	ResumeRolloutCanary := func(name string) {
+		clone := &v1alpha1.Rollout{}
+		Expect(GetObject(name, clone)).NotTo(HaveOccurred())
+		currentIndex := clone.Status.CanaryStatus.CurrentStepIndex
 		Eventually(func() bool {
 			clone := &v1alpha1.Rollout{}
 			Expect(GetObject(name, clone)).NotTo(HaveOccurred())
-			if clone.Status.CanaryStatus.CurrentStepState != v1alpha1.CanaryStepStatePaused {
+			if clone.Status.CanaryStatus.CurrentStepIndex == currentIndex && clone.Status.CanaryStatus.CurrentStepState == v1alpha1.CanaryStepStatePaused {
+				klog.Info("patch to stepReady")
+				body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady)
+				Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred())
+				return false
+			} else {
 				fmt.Println("resume rollout success, and CurrentStepState", util.DumpJSON(clone.Status))
 				return true
 			}
-
-			body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady)
-			Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred())
-			return false
-		}, 10*time.Second, time.Second).Should(BeTrue())
+			// interval was critical before:
+			// too small: StepReady could be overidden by StepPaused
+			// too big: StepReady could progress to StepPaused of next Step
+		}, 10*time.Second, 2*time.Second).Should(BeTrue())
 	}
 
 	WaitDeploymentAllPodsReady := func(deployment *apps.Deployment) {