-
Notifications
You must be signed in to change notification settings - Fork 39.8k
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
Automated cherry pick of #49495 upstream release 1.7 #49767
Automated cherry pick of #49495 upstream release 1.7 #49767
Conversation
/test pull-kubernetes-unit |
1 similar comment
/test pull-kubernetes-unit |
@deads2k - it seems those failures are not flakes. Can you please take a look? |
Looks like an incomplete federation stub of some kind. Maybe they can just do whatever they change in 1.8. |
@deads2k - this is not failing on 1.7 branch and on other cherrypicks. This is related to this particular PR. |
/retest |
3 similar comments
/retest |
/retest |
/retest |
/test pull-kubernetes-unit |
/lgtm |
@kubernetes/sig-federation-bugs @nikhiljindal PTAL. |
/retest Review the full test history for this PR. |
1 similar comment
/retest Review the full test history for this PR. |
@foxish - I'm pretty convinced that failing unit tests are related to this cl - I have seen this failure on any other PR to 1.7 branch and on this one I've seen them 5+ times |
Sorry, I see that now. I was thrown off by @deads2k's comment. Investigating further. |
Looks like these tests were deleted entirely in master in #47890. |
Spent some time on this, and raised #50771 to still let those federated namespace unit tests run ok with the changes in this cherry-pick PR.
@foxish you are right, we have done away with these tests completely and no longer exist on master (thanks to @maruns efforts). In the tests which still exist on 1.7, the tests actually do not at all use the discovery functions, but there is no faked method which can bypass the checks introduced in this PR during initialising the controller. This cherry-pick should work if its put on top of the PR that I submitted. |
/retest |
/lgtm |
@wojtek-t: you cannot LGTM your own PR. 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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: foxish, wojtek-t Associated issue: 49495 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/retest |
1 similar comment
/retest |
I'm not convinced that those flakes aren't real issues - those are to frequent comparing to other PRs. |
/test pull-kubernetes-e2e-gce-etcd3 They don't look related to the change though. |
Different tests failing each time, which make me suspect it is indeed a flake. Retesting. |
@wojtek-t, removed do-not-merge. 1.7.4 is cut already as I understand. Targeting this for 1.7.5. |
/test pull-kubernetes-e2e-gce-etcd3 |
/retest Review the full test history for this PR. |
2 similar comments
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Cherry pick of #49495 on release-1.7.
#49495: make it possible to allow discovery errors for controllers