-
Notifications
You must be signed in to change notification settings - Fork 291
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 ownerReference resilience test #2209
🌱 Add ownerReference resilience test #2209
Conversation
d288e6d
to
17659f7
Compare
f5d4623
to
1facf1f
Compare
e40cdd8
to
363cbd3
Compare
/retest |
This error isn't happening for me locally - but it looks like a bug on the CAPI side from a ClusterResourceSetBinding with multiple ownerReferences - good we caught that here. Will bring it up over at the CAPI repo. /hold |
da986d5
to
87b17cc
Compare
/retest |
/hold cancel The ClusterResourceSet error was down to a race condition. I've update the code to ensure the CRS is reconciled before running the ownerReference check. |
87b17cc
to
6799904
Compare
/lgtm |
LGTM label has been added. Git tree hash: 6714c6928afd71e4e1ecd7a96027146901b61061
|
@killianmuldoon You probably want to squash |
1b15efd
to
e9d50ed
Compare
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.
small nit
/lgtm
e9d50ed
to
9bd2b40
Compare
@killianmuldoon: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
9bd2b40
to
d63e2bc
Compare
Thank you very much!! Nice work /lgtm Now onto manual cherry-picking I guess :D (or maybe git can deal with this delta) |
LGTM label has been added. Git tree hash: ed69c492a5e4bb6ce403c50a1d8565a9f0e03124
|
[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 |
Note: there is still a hold on this PR. |
/hold cancel |
@sbueringer: #2209 failed to apply on top of branch "release-1.8":
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: #2209 failed to apply on top of branch "release-1.7":
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. |
Add ownerReference validation to CAPV for the following types in the Cluster and ClusterClass tests:
The validations - similar to those in CAPI - assert that ownerReferences are re-reconciled if removed and also check that ownerReference apiVersions are always updated to the latest version.
Part of #2075