Fix Flaky results on Library and Gateway controller test#2039
Conversation
We are getting flaky results on this test and adding Eventually wrapper can help us to avoid this situations Signed-off-by: Francisco Herrera <fjglira@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2039 +/- ##
==========================================
- Coverage 76.57% 76.32% -0.26%
==========================================
Files 58 58
Lines 3181 3181
==========================================
- Hits 2436 2428 -8
- Misses 608 613 +5
- Partials 137 140 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| g.Expect(ready).To(BeTrue(), "curl-client pod is not ready") | ||
| }).Should(Succeed()) | ||
| Success("All pods in gateway namespace are ready") | ||
| Success("Curl client pod is ready") |
There was a problem hiding this comment.
Need to check if this work, because now we are only validating that curl pod is running, but the previous step validates that all the gateway deploy are Ready
There was a problem hiding this comment.
Pull request overview
This PR addresses flaky E2E failures in the Sail Operator test suite by making readiness/drift assertions more resilient to eventual consistency and background reconciliation, specifically in the Library reconciliation and Gateway controller E2E tests.
Changes:
- Wrap the ValidatingWebhookConfiguration drift mutation in an
Eventuallyblock to tolerate concurrent reconciler updates. - Narrow the Gateway controller “pod ready” check to the specific
curl-clientpod rather than requiring all pods in the gateway namespace to be ready.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/e2e/library/library_reconcile_test.go | Retries webhook drift mutation to reduce flakes from concurrent reconciliation. |
| tests/e2e/gatewaycontroller/gateway_controller_test.go | Makes the curl-client readiness check more targeted to avoid unrelated pod readiness causing flakes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The background reconciler may update the webhook concurrently; retry on conflict. | ||
| Eventually(func(g Gomega) { | ||
| webhook := &admissionv1.ValidatingWebhookConfiguration{} | ||
| g.Expect(cl.Get(ctx, webhookKey, webhook)).To(Succeed()) | ||
| webhook.Webhooks[0].Name = "xyz.xyz.xyz" | ||
| webhook.Webhooks[0].FailurePolicy = ptr.Of(admissionv1.Fail) | ||
| g.Expect(cl.Update(ctx, webhook)).To(Succeed()) |
|
Looks ok to me. |
Signed-off-by: Francisco Herrera <fjglira@gmail.com>
|
I had to do the change manually |
|
@fjglira: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
We are getting flaky results on this test and adding Eventually wrapper can help us to avoid this situations
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #2038
Related Issue/PR #
Additional information: