-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛 Add soft ownership from clusters to ClusterResourceSetBinding #8318
🐛 Add soft ownership from clusters to ClusterResourceSetBinding #8318
Conversation
13f1305
to
4847063
Compare
/test pull-cluster-api-e2e-full-main |
/test pull-cluster-api-e2e-informing-main |
first run passed, trying to see if it works consistently |
/assign @ykakarap |
also second run passed, trying one more |
3 over 3 success, last one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: e1804cf848676075c58163bf5157d921ef3f4e9d
|
/lgtm Thx for the quick fix! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-1.4 |
@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.4 in a new PR and assign it to you. In response to this:
Instructions 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/test-infra repository. |
@sbueringer: new pull request created: #8323 In response to this:
Instructions 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/test-infra repository. |
/area clusterctl |
What this PR does / why we need it:
#7680 removed the owner reference from cluster to ClusterResourceSetBinding, replacing it with a spec.ClusterName field
However, we did not fix the fake objects used for validating clusterctl move accordingly, and thus even if unit tests for move were passing, we are seeking flakiness in CI because there is no more a deterministic order between Cluster and ClusterResourceSetBinding (see details in the attached issue).
This PR fixes this issue by introducing soft ownership from clusters to ClusterResourceSetBinding; It also fixes the softOwnership test that was not really up to date with what the function is currently doing.
Which issue(s) this PR fixes:
Fixes #8316