Skip to content
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

Merged
merged 1 commit into from
Mar 8, 2016

Conversation

gmarek
Copy link
Contributor

@gmarek gmarek commented Dec 8, 2015

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

@gmarek gmarek added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 8, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 8, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 8, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 8, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 8, 2015

GCE e2e build/test failed for commit f062629114ce35a04fbb0bf945f3c378e1bd9309.

@k8s-bot
Copy link

k8s-bot commented Dec 8, 2015

GCE e2e build/test failed for commit d62b910.

@dchen1107
Copy link
Member

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.

@gmarek
Copy link
Contributor Author

gmarek commented Dec 8, 2015

@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.

@k8s-bot
Copy link

k8s-bot commented Dec 9, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 9, 2015

GCE e2e build/test failed for commit d62b910.

@roberthbailey
Copy link
Contributor

@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.

@gmarek
Copy link
Contributor Author

gmarek commented Dec 10, 2015

@roberthbailey: #18509

@gmarek
Copy link
Contributor Author

gmarek commented Dec 12, 2015

Quoting @bprashanth from another PR:

can you salt in the flannel change needed first?

Set this to false:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/server.go#L347

But still install flannel on the master node (so it's true everywhere else in salt, like: https://github.com/kubernetes/kubernetes/blob/master/cluster/saltbase/salt/top.sls#L46). This will tell the Kublete not to wait on flannel, but will still bringup the flannel daemon on the master node. The kubelet doesn't need to wait on flannel because master components run in host netowrking, but the master needs a flannel daemon because it needs flannel.1 so things like apiserver proxy to pod endpoint works.

@k8s-bot
Copy link

k8s-bot commented Dec 17, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 17, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 17, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 18, 2015

GCE e2e build/test failed for commit d62b910.

@k8s-bot
Copy link

k8s-bot commented Dec 22, 2015

GCE e2e build/test failed for commit d62b910.

@dchen1107
Copy link
Member

ok to test

@k8s-bot
Copy link

k8s-bot commented Feb 12, 2016

GCE e2e build/test failed for commit d62b910.

@gmarek gmarek removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 26, 2016
@gmarek gmarek changed the title Enable Masters Kubelet registration in gce-test by default DO NOT MERGE: Enable Masters Kubelet registration in gce-test by default Feb 29, 2016
@gmarek
Copy link
Contributor Author

gmarek commented Feb 29, 2016

Per PR tests should be fixed as soon as #22172 is merged. I'll run other tests to find other problematic ones.

@gmarek
Copy link
Contributor Author

gmarek commented Feb 29, 2016

I run e2e-gce suite and all tests passed.

@gmarek
Copy link
Contributor Author

gmarek commented Mar 1, 2016

Slow tests passed as well.

@gmarek
Copy link
Contributor Author

gmarek commented Mar 3, 2016

Serial tests passed. It seems that we may enable it once #22275 #22172 are merged.

@gmarek
Copy link
Contributor Author

gmarek commented Mar 7, 2016

@k8s-bot test this issue: #IGNORE

All failures should be fixed now.

@k8s-bot
Copy link

k8s-bot commented Mar 7, 2016

GCE e2e build/test passed for commit d62b910.

@gmarek
Copy link
Contributor Author

gmarek commented Mar 7, 2016

Aaand we're green. We need to decide what do do now: @mikedanese @davidopp @dchen1107 @bgrant0607

@bgrant0607
Copy link
Member

We should merge this so it doesn't bitrot.

LGTM

@bgrant0607 bgrant0607 added this to the v1.2 milestone Mar 8, 2016
@bgrant0607 bgrant0607 added lgtm "Looks good to me", indicates that a PR is ready to be merged. cherrypick-candidate labels Mar 8, 2016
@bgrant0607 bgrant0607 changed the title DO NOT MERGE: Enable Masters Kubelet registration in gce-test by default Enable Masters Kubelet registration in gce-test by default Mar 8, 2016
@bgrant0607 bgrant0607 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 8, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Mar 8, 2016

GCE e2e build/test passed for commit d62b910.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Mar 8, 2016

GCE e2e build/test passed for commit d62b910.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Mar 8, 2016

GCE e2e build/test passed for commit d62b910.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Mar 8, 2016
@k8s-github-robot k8s-github-robot merged commit 43aa3d3 into kubernetes:master Mar 8, 2016
eparis pushed a commit to eparis/kubernetes that referenced this pull request Mar 8, 2016
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.