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

kubelet should set readiness to true as soon as possible for containers without readiness probe defined #11234

Closed
yujuhong opened this issue Jul 14, 2015 · 10 comments
Assignees
Labels
area/kubelet priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@yujuhong
Copy link
Contributor

Forked from #11007

kubelet set readiness for containers when checking container health in SyncPod. For containers that are created for the first time, the readiness of the containers would only be set in the next sync (~10s or less). This applies to containers with NO readiness probe defined, so that they'd stay as not ready until the next sync. Instead, kubelet could perform an extra probe right after the container creation to make sure the readiness is set correctly as soon as possible.

@yujuhong yujuhong added priority/backlog Higher priority than priority/awaiting-more-evidence. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jul 14, 2015
@yujuhong yujuhong added this to the v1.0-post milestone Jul 14, 2015
@bgrant0607 bgrant0607 removed this from the v1.0-post milestone Jul 24, 2015
@bgrant0607 bgrant0607 added this to the v1.1 milestone Aug 11, 2015
@yujuhong
Copy link
Contributor Author

@vishh, I am assigning this to you since you (sort of) expressed interests in today's meeting. If you are swamped with other issues, feel free to unassign!

@bgrant0607
Copy link
Member

cc @nikhiljindal

@nikhiljindal
Copy link
Contributor

@vishh I can take this up if you havent started working on it.
I need it for Deployment #12236.
There are a few other changes I need to make to the Ready PodCondition. I can make all of them together.

@nikhiljindal nikhiljindal assigned nikhiljindal and unassigned vishh Aug 18, 2015
@yujuhong
Copy link
Contributor Author

@nikhiljindal, @timstclair is working on separating probing from the kubelet sync period (#12866). You may want to coordinate with him.

@nikhiljindal
Copy link
Contributor

Removing 1.1 milestone

@nikhiljindal nikhiljindal removed this from the v1.1 milestone Oct 5, 2015
@timstclair
Copy link

I'm not sure how this would work with a temporary readiness probe. The problem with using a readiness probe is that the probe needs to get the container ID before it can probe, and it relies on the sync loop for that (currently). Also, I think you want to declare the container Ready as soon as it's ID exists, so setting up a probe doesn't seem like the right approach.

@yujuhong
Copy link
Contributor Author

yujuhong commented Oct 5, 2015

@timstclair. we don't need to setup an actual probe, but we'd like the readiness be true if there is no probe defined. By the way, your PR which got merged today already solved this problem.

@timstclair
Copy link

Yes, if there is no probe defined it defaults to readiness (although I believe that was already true). If the container is created by the time syncPod exits, it should be set to ready. Otherwise, it will still need to wait for the next syncPod execution.

@yujuhong
Copy link
Contributor Author

yujuhong commented Oct 5, 2015

Yes, if there is no probe defined it defaults to readiness (although I believe that was already true)

This wasn't technically true until your PR landed. Before, we only probe at the beginning of SyncPod (before the container creation), and the readiness would be false for the first sync.

Otherwise, it will still need to wait for the next syncPod execution.

I think the scope of this issue is more towards setting readiness for pods that do not have probe defined, so I will go ahead and close this bug.

@yujuhong yujuhong closed this as completed Oct 5, 2015
@timstclair
Copy link

Ah, got it, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubelet priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests

5 participants