-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Enable Masters Kubelet registration in gce-test by default #18338
Conversation
Labelling this PR as size/XS |
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
f062629
to
d62b910
Compare
GCE e2e build/test failed for commit f062629114ce35a04fbb0bf945f3c378e1bd9309. |
GCE e2e build/test failed for commit d62b910. |
Yes, it should be enabled for e2e by default too. There are a bunch of tests failed, and I guess it is due to the master node registration messed up some assumption of the test harness. |
@dchen1107 - I think some (if not all) failures might be related with problems with e2e test infrastructure we currently have. I'll rerun them tomorrow. |
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
@gmarek The reason that this wasn't initially enabled by default was that e2e tests were failing. If we could get them to pass it'd be awesome to have this flag on by default. Thanks for looking into it. |
Quoting @bprashanth from another PR:
|
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
GCE e2e build/test failed for commit d62b910. |
ok to test |
GCE e2e build/test failed for commit d62b910. |
Per PR tests should be fixed as soon as #22172 is merged. I'll run other tests to find other problematic ones. |
I run e2e-gce suite and all tests passed. |
Slow tests passed as well. |
@k8s-bot test this issue: #IGNORE All failures should be fixed now. |
GCE e2e build/test passed for commit d62b910. |
Aaand we're green. We need to decide what do do now: @mikedanese @davidopp @dchen1107 @bgrant0607 |
We should merge this so it doesn't bitrot. LGTM |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit d62b910. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit d62b910. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit d62b910. |
Automatic merge from submit-queue |
Auto commit by PR queue bot
Auto commit by PR queue bot
Auto commit by PR queue bot
Auto commit by PR queue bot
Fixes #15803
I'm not sure why it wasn't enabled, but we probably should run the same configuration in tests as in production.
cc @wojtek-t @piosz @mikedanese @dchen1107