From 609067081f66b9ac5603bed074259d5c83936759 Mon Sep 17 00:00:00 2001 From: yunbo Date: Wed, 5 Jun 2024 09:53:24 +0800 Subject: [PATCH] improve existing e2e logic to avoid unexpected behaviour Signed-off-by: yunbo --- pkg/controller/rollout/rollout_canary.go | 1 + test/e2e/deployment_test.go | 21 +++++++++++++++++++++ test/e2e/rollout_test.go | 19 +++++++++++++------ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index c69ccb96..0b93d76d 100644 --- 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 index b857f929..c8379bc4 100644 --- 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 index c8a7cae2..c2ae86c8 100644 --- 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) {