Skip to content

Commit 56cb4fa

Browse files
skeeeyskeeey
authored
fix flaky upgrade test (#436)
Signed-off-by: Wei Liu <liuweixa@redhat.com>
1 parent 512df4a commit 56cb4fa

File tree

5 files changed

+96
-73
lines changed

5 files changed

+96
-73
lines changed

‎test/e2e/istio/test.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ if kubectl --kubeconfig=$server_kubeconfig -n clusters-service wait --for=condit
8181
succeeded=$(kubectl --kubeconfig=$server_kubeconfig -n clusters-service get job maestro-e2e-tests -o jsonpath='{.status.succeeded}')
8282
if [ "$succeeded" = "1" ]; then
8383
echo "Job completed successfully"
84-
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs -l app=maestro-e2e
84+
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs jobs/maestro-e2e-tests
8585
exit 0
8686
fi
8787
fi
8888

8989
echo "Job clusters-service/maestro-e2e-tests failed"
90-
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs -l app=maestro-e2e
90+
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs jobs/maestro-e2e-tests
9191
exit 1

‎test/e2e/pkg/spec_resync_test.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,18 @@ var _ = Describe("Spec Resync After Restart", Ordered, Label("e2e-tests-spec-res
264264

265265
By("check the resource deletion via source workclient")
266266
Eventually(func() error {
267-
mwGRPCClient := sourceWorkClient.ManifestWorks(agentTestOpts.consumerName)
268-
workList, err := mwGRPCClient.List(ctx, metav1.ListOptions{})
269-
if err != nil {
270-
return fmt.Errorf("failed to list manifestwork: %v", err)
267+
if err := AssertWorkNotFound(workNameA); err != nil {
268+
return err
269+
}
270+
271+
if err := AssertWorkNotFound(workNameB); err != nil {
272+
return err
271273
}
272-
if len(workList.Items) != 0 {
273-
return fmt.Errorf("expected no resources returned by maestro api")
274+
275+
if err := AssertWorkNotFound(workNameC); err != nil {
276+
return err
274277
}
278+
275279
return nil
276280
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
277281
})

‎test/e2e/pkg/status_resync_test.go‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -275,16 +275,7 @@ var _ = Describe("Status Resync After Restart", Ordered, Label("e2e-tests-status
275275

276276
By("check the resource deletion via source workclient")
277277
Eventually(func() error {
278-
mwGRPCClient := sourceWorkClient.ManifestWorks(agentTestOpts.consumerName)
279-
workList, err := mwGRPCClient.List(ctx, metav1.ListOptions{})
280-
if err != nil {
281-
return fmt.Errorf("failed to list manifestwork: %v", err)
282-
}
283-
284-
if len(workList.Items) != 0 {
285-
return fmt.Errorf("expected no resources returned by maestro api")
286-
}
287-
return nil
278+
return AssertWorkNotFound(workName)
288279
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
289280

290281
watcherCancel()

‎test/upgrade/pkg/upgrade_test.go‎

Lines changed: 81 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ const (
2828
nestedWorkName = "maestro-e2e-upgrade-test-work"
2929
)
3030

31+
const (
32+
timeout = 2 * time.Minute
33+
polling = 10 * time.Second
34+
)
35+
3136
var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
3237

3338
var deployWorkName string
@@ -38,62 +43,85 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
3843
deployClient := kubeClientSet.AppsV1().Deployments(namespace)
3944

4045
By("create a deployment for readonly work", func() {
41-
_, err := deployClient.Get(ctx, deployReadonlyName, metav1.GetOptions{})
42-
if errors.IsNotFound(err) {
43-
deploy := utils.NewDeployment(namespace, deployReadonlyName, 0)
44-
_, newErr := deployClient.Create(ctx, deploy, metav1.CreateOptions{})
45-
Expect(newErr).ShouldNot(HaveOccurred())
46-
} else {
47-
Expect(err).ShouldNot(HaveOccurred())
48-
}
46+
Eventually(func() error {
47+
_, err := deployClient.Get(ctx, deployReadonlyName, metav1.GetOptions{})
48+
if errors.IsNotFound(err) {
49+
deploy := utils.NewDeployment(namespace, deployReadonlyName, 0)
50+
_, newErr := deployClient.Create(ctx, deploy, metav1.CreateOptions{})
51+
return newErr
52+
}
53+
if err != nil {
54+
return err
55+
}
56+
return nil
57+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
4958
})
5059

5160
By("create a work to apply a deployment", func() {
5261
deploy := utils.NewDeployment(namespace, deployName, 0)
5362
deployWorkName = utils.WorkName(utils.DeploymentGVK, deploy)
54-
_, err := workServerClient.Get(ctx, deployWorkName)
55-
if errors.IsNotFound(err) {
56-
work, newErr := utils.NewManifestWork(utils.DeploymentGVK, utils.DeploymentGVR, deploy)
57-
Expect(newErr).ShouldNot(HaveOccurred())
58-
59-
work.Labels["maestro.e2e.test.name"] = "upgrade"
60-
_, newErr = workServerClient.Create(ctx, work)
61-
Expect(newErr).ShouldNot(HaveOccurred())
62-
} else {
63-
Expect(err).ShouldNot(HaveOccurred())
64-
}
63+
64+
Eventually(func() error {
65+
_, err := workServerClient.Get(ctx, deployWorkName)
66+
if errors.IsNotFound(err) {
67+
work, newErr := utils.NewManifestWork(utils.DeploymentGVK, utils.DeploymentGVR, deploy)
68+
if newErr != nil {
69+
return newErr
70+
}
71+
72+
work.Labels["maestro.e2e.test.name"] = "upgrade"
73+
_, newErr = workServerClient.Create(ctx, work)
74+
return newErr
75+
}
76+
if err != nil {
77+
return err
78+
}
79+
return nil
80+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
6581
})
6682

6783
By("create a readonly work to retrieve a deployment status", func() {
6884
deployReadonly := utils.NewDeploymentReadonly(namespace, deployReadonlyName)
6985
deployReadonlyWorkName = utils.WorkName(utils.DeploymentGVK, deployReadonly)
70-
_, err := workServerClient.Get(ctx, deployReadonlyWorkName)
71-
if errors.IsNotFound(err) {
72-
work, newErr := utils.NewManifestWork(utils.DeploymentGVK, utils.DeploymentGVR, deployReadonly)
73-
Expect(newErr).ShouldNot(HaveOccurred())
74-
75-
work.Labels["maestro.e2e.test.name"] = "upgrade"
76-
_, newErr = workServerClient.Create(ctx, work)
77-
Expect(newErr).ShouldNot(HaveOccurred())
78-
} else {
79-
Expect(err).ShouldNot(HaveOccurred())
80-
}
86+
Eventually(func() error {
87+
_, err := workServerClient.Get(ctx, deployReadonlyWorkName)
88+
if errors.IsNotFound(err) {
89+
work, newErr := utils.NewManifestWork(utils.DeploymentGVK, utils.DeploymentGVR, deployReadonly)
90+
if newErr != nil {
91+
return newErr
92+
}
93+
94+
work.Labels["maestro.e2e.test.name"] = "upgrade"
95+
_, newErr = workServerClient.Create(ctx, work)
96+
return newErr
97+
}
98+
if err != nil {
99+
return err
100+
}
101+
return nil
102+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
81103
})
82104

83105
By("create a work to apply a nested manifestwork", func() {
84106
nestedWork := utils.NewDeploymentManifestWork(namespace, nestedWorkName)
85107
nestedWorkWorkName = utils.WorkName(utils.ManifestWorkGVK, nestedWork)
86-
_, err := workServerClient.Get(ctx, nestedWorkWorkName)
87-
if errors.IsNotFound(err) {
88-
work, newErr := utils.NewManifestWork(utils.ManifestWorkGVK, utils.ManifestWorkGVR, nestedWork)
89-
Expect(newErr).ShouldNot(HaveOccurred())
90-
91-
work.Labels["maestro.e2e.test.name"] = "upgrade"
92-
_, newErr = workServerClient.Create(ctx, work)
93-
Expect(newErr).ShouldNot(HaveOccurred())
94-
} else {
95-
Expect(err).ShouldNot(HaveOccurred())
96-
}
108+
Eventually(func() error {
109+
_, err := workServerClient.Get(ctx, nestedWorkWorkName)
110+
if errors.IsNotFound(err) {
111+
work, newErr := utils.NewManifestWork(utils.ManifestWorkGVK, utils.ManifestWorkGVR, nestedWork)
112+
if newErr != nil {
113+
return newErr
114+
}
115+
116+
work.Labels["maestro.e2e.test.name"] = "upgrade"
117+
_, newErr = workServerClient.Create(ctx, work)
118+
return newErr
119+
}
120+
if err != nil {
121+
return err
122+
}
123+
return nil
124+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
97125
})
98126
})
99127

@@ -114,7 +142,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
114142
lastGeneration = deploy.Generation
115143
lastReplicas = *deploy.Spec.Replicas
116144
return nil
117-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
145+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
118146
})
119147

120148
expectedReplicas = utils.UpdateReplicas(lastReplicas)
@@ -144,7 +172,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
144172
}
145173

146174
return nil
147-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
175+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
148176
})
149177

150178
By("ensure the deployment is updated", func() {
@@ -154,16 +182,16 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
154182
return err
155183
}
156184

157-
if deploy.Generation != lastGeneration+1 {
158-
return fmt.Errorf("expected generation %d, but got %d", lastGeneration+1, deploy.Generation)
185+
if deploy.Generation == lastGeneration {
186+
return fmt.Errorf("expected deployment %s updated, but failed", deployName)
159187
}
160188

161189
if *deploy.Spec.Replicas != expectedReplicas {
162190
return fmt.Errorf("expected replicas %d, but got %d", expectedReplicas, *deploy.Spec.Replicas)
163191
}
164192

165193
return nil
166-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
194+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
167195
})
168196

169197
By("ensure the deployment new status is watched", func() {
@@ -173,7 +201,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
173201
return err
174202
}
175203
return AssertReplicas(watchedWork, expectedReplicas)
176-
}, 5*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
204+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
177205
})
178206
})
179207

@@ -187,7 +215,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
187215
return err
188216
}
189217
return AssertStatusFeedbackSynced(watchedWork)
190-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
218+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
191219
})
192220

193221
By("update the deployment", func() {
@@ -208,7 +236,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
208236
}
209237

210238
return nil
211-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
239+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
212240

213241
})
214242

@@ -219,7 +247,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
219247
return err
220248
}
221249
return AssertReplicas(watchedWork, expectedReplicas)
222-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
250+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
223251
})
224252
})
225253

@@ -230,7 +258,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
230258
Eventually(func() error {
231259
_, err := workClient.Get(ctx, nestedWorkName, metav1.GetOptions{})
232260
return err
233-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
261+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
234262
})
235263

236264
By("update the nested work via work", func() {
@@ -259,7 +287,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
259287
}
260288

261289
return nil
262-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
290+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
263291
})
264292

265293
By("ensure the manifestwork new status is watched", func() {
@@ -273,7 +301,7 @@ var _ = Describe("Upgrade Test", Ordered, Label("e2e-tests-upgrade"), func() {
273301
return err
274302
}
275303
return AssertNestedWorkAvailable(watchedWork)
276-
}, 1*time.Minute, 1*time.Second).ShouldNot(HaveOccurred())
304+
}).WithTimeout(timeout).WithPolling(polling).ShouldNot(HaveOccurred())
277305
})
278306
})
279307
})

‎test/upgrade/script/run.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ if kubectl --kubeconfig=$server_kubeconfig -n clusters-service wait --for=condit
7373
succeeded=$(kubectl --kubeconfig=$server_kubeconfig -n clusters-service get job maestro-upgrade-tests -o jsonpath='{.status.succeeded}')
7474
if [ "$succeeded" = "1" ]; then
7575
echo "Job completed successfully"
76-
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs -l app=maestro-upgrade
76+
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs jobs/maestro-upgrade-tests
7777
exit 0
7878
fi
7979
fi
8080

8181
echo "Job clusters-service/maestro-upgrade-tests failed"
82-
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs -l app=maestro-upgrade
82+
kubectl --kubeconfig=$server_kubeconfig -n clusters-service logs jobs/maestro-upgrade-tests
8383
exit 1

0 commit comments

Comments
 (0)