test(e2e): synchronize ClusterRole propagation before cleanup#7692
test(e2e): synchronize ClusterRole propagation before cleanup#7692A69SHUBHAM wants to merge 1 commit into
Conversation
Signed-off-by: A69SHUBHAM <spacekrai0@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the reliability of the ClusterResourceBinding e2e tests by ensuring that propagated ClusterRoles are fully synchronized across member clusters before the test proceeds to cleanup. This change aligns the test logic with established patterns in other propagation tests, effectively resolving an identified source of test flakiness. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request updates the end-to-end tests in clusterresourcebinding_test.go by adding a call to framework.WaitClusterRolePresentOnClustersFitWith to ensure that the cluster role is present on the clusters during the test execution. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR addresses an e2e flake by adding an explicit synchronization barrier in clusterresourcebinding_test.go so the test waits for asynchronous ClusterRole propagation to member clusters before cleanup begins, matching the synchronization pattern used by similar propagation tests.
Changes:
- Add
WaitClusterRolePresentOnClustersFitWith(...)to thecreates work with permanent ID labeltest to ensure the propagatedClusterRoleexists on all member clusters before theDeferCleanupdisappearance check runs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7692 +/- ##
==========================================
- Coverage 42.05% 42.04% -0.01%
==========================================
Files 879 879
Lines 54831 54831
==========================================
- Hits 23059 23056 -3
- Misses 30026 30028 +2
- Partials 1746 1747 +1
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:
|
|
/cc @zhzhuang-zju Can you have a look |
What type of PR is this?
/kind flake
/kind failing-test
What this PR does / why we need it:
While investigating a recent e2e failure, I noticed that the
creates work with permanent ID labeltest inclusterresourcebinding_test.godoes not follow the synchronization pattern used by similar propagation tests.The test currently waits for the corresponding
Workobjects to be created in the control plane before completing. Since propagation to member clusters is asynchronous, cleanup can begin before the propagatedClusterRolehas been created.This PR adds
WaitClusterRolePresentOnClustersFitWith(...)before the test completes, aligning it with the synchronization pattern already used by similar e2e tests and by the secondItblock in the same file.Which issue(s) this PR fixes:
Fixes #7691
Special notes for your reviewer:
While investigating a recent e2e failure, I noticed this difference between the first and second
Itblocks inclusterresourcebinding_test.go. This change reuses the existing synchronization helper without changing the test assertions or production code.Does this PR introduce a user-facing change?