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.

Sign up for GitHub

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

Jump to bottom

kubeadm: begin polling for bootstrap cluster info immediately #48737

Merged
merged 1 commit into from
Jul 12, 2017
Merged

kubeadm: begin polling for bootstrap cluster info immediately #48737

merged 1 commit into from
Jul 12, 2017

Conversation

mattmoyer
Copy link
Contributor

@mattmoyer mattmoyer commented Jul 10, 2017 •
edited by luxas
Loading

What this PR does / why we need it:
This PR changes the behavior of the kubeadm loop that polls the API server waiting for discovery information (cluster-info). The previous (inadvertent?) behavior was to sleep for constants.DiscoveryRetryInterval (5 seconds) before the first request (PollInfinite vs. PollImmediateInfinite).

After this change, kubeadm begins does the first check immediately before it moves into the slower polling mode. This takes around 5 seconds off the average time for a new node to join.

Before (~5.5s)

root@worker:~# time kubeadm join --token abc.123 192.168.42.10:6443
[...]
real	0m5.523s
user	0m0.112s
sys	0m0.124s

After (~0.5s)

root@worker:~# time kubeadm join --token abc.123 192.168.42.10:6443
[...]
real	0m0.587s
user	0m0.092s
sys	0m0.132s

Which issue this PR fixes:

Special notes for your reviewer:
This is my first Kubernetes PR, so please let me know if it's formatted correctly.

Release note:

kubeadm: Fix a bug where `kubeadm join` would wait 5 seconds without doing anything. Now `kubeadm join` executes the tasks immediately.

@mattmoyer
Begin polling for bootstrap cluster info immediately.
1d9bc00 
The previous behavior was to sleep for `constants.DiscoveryRetryInterval` (5 seconds) before the first request.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 10, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @mattmoyer. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 10, 2017
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 10, 2017
@jbeda jbeda assigned jbeda and unassigned justinsb and mikedanese Jul 10, 2017
@jbeda
Copy link
Contributor

jbeda commented Jul 10, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 10, 2017
@justinsb
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 10, 2017
@jbeda
Copy link
Contributor

jbeda commented Jul 10, 2017 via email

@jbeda
Copy link
Contributor

jbeda commented Jul 11, 2017

/test pull-kubernetes-e2e-kops-aws
/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbeda, justinsb, mattmoyer

Associated issue requirement bypassed by: jbeda

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 /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2017
@mattmoyer
Copy link
Contributor Author

/test pull-kubernetes-e2e-kops-aws

@luxas
Copy link
Member

luxas commented Jul 12, 2017

Thanks @mattmoyer! No, this wasn't expected.
Actually I wondered one day why the token discovery was slower than expected, and thought I'd look into it one day. I'm very glad you found it and could contributed it 🎉

Happy first contribution 👍 !

@luxas
Copy link
Member

luxas commented Jul 12, 2017

@mattmoyer Feel free to cherrypick this afterwards to the release-1.7 branch with the hack/cherry_pick_pull.sh script. It's pretty straightforward to use as long as you have hub installed and git remote -v outputs something like this:

origin	[email protected]:luxas/kubernetes.git (fetch)
origin	[email protected]:luxas/kubernetes.git (push)
upstream	https://github.com/kubernetes/kubernetes (fetch)
upstream	DISABLE (push)

@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 7d97208 into kubernetes:master Jul 12, 2017
@luxas luxas added this to the v1.7 milestone Jul 12, 2017
@luxas luxas added the kind/bug Categorizes issue or PR as related to a bug. label Jul 12, 2017
@luxas
Copy link
Member

luxas commented Jul 12, 2017

@mattmoyer I added a release note to this, since that's required for a cherrypick

@luxas luxas added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 12, 2017
@mattmoyer mattmoyer deleted the faster-kubeadm-join branch July 12, 2017 20:17
@mattmoyer mattmoyer mentioned this pull request Jul 12, 2017
Merged
@wojtek-t
Copy link
Member

@luxas - thanks for adding release note. Cherrypick approved.

@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 13, 2017
k8s-github-robot pushed a commit that referenced this pull request Jul 18, 2017
Merge pull request #48834 from mattmoyer/automated-cherry-pick-of-#48…
edd592f 
…737-upstream-release-1.7

Automatic merge from submit-queue

Automated cherry pick of #48737

Cherry pick of #48737 on release-1.7.

#48737: Begin polling for bootstrap cluster info immediately.
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jbeda jbeda

@justinsb justinsb

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
9 participants
@mattmoyer @k8s-ci-robot @jbeda @justinsb @k8s-github-robot @luxas @k8s-cherrypick-bot @wojtek-t @mikedanese