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

Unexpected startupProbe behavior. #89995

Closed
heckad opened this issue Apr 9, 2020 · 20 comments · Fixed by #92196
Closed

Unexpected startupProbe behavior. #89995

heckad opened this issue Apr 9, 2020 · 20 comments · Fixed by #92196
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@heckad
Copy link

heckad commented Apr 9, 2020

If I have startupProbe probe and readinessProbe then all good. Previous version of the pod is terminated after startup probe success, but if readinessProbe wasn't specified the pod becomes ready immediately. It would be nice to add new state (for example starting) which would have pods until a startupProbe is successful. Also, it would mean that container not ready for connection.

Thank you for attention!

@heckad heckad added the kind/support Categorizes issue or PR as a support question. label Apr 9, 2020
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Apr 9, 2020
@heckad
Copy link
Author

heckad commented Apr 10, 2020

/sig cluster-lifecycle

@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 10, 2020
@heckad
Copy link
Author

heckad commented Apr 23, 2020

@neolit123?

@neolit123
Copy link
Member

/remove-sig cluster-lifecycle
/sig apps

try asking in #kubernetes-users on k8s slack.

@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Apr 23, 2020
@heckad
Copy link
Author

heckad commented Apr 23, 2020

@neolit123, they are silent in the chat. Who is involved in the development of startup probe?

@neolit123
Copy link
Member

neolit123 commented Apr 23, 2020

i think this topic better fits SIG Node.

try asking in their channel on slack #sig-node, but be patient ;)
/sig node
/remove-sig apps

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed sig/apps Categorizes an issue or PR as relevant to SIG Apps. labels Apr 23, 2020
@k8s-ci-robot
Copy link
Contributor

@neolit123: Those labels are not set on the issue: sig/apps

In response to this:

i think this topic better fits SIG Node.

try asking in their channel on slack #sig-node, but be patient ;)
/sig node
/remove-sig apps

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.

@thockin thockin added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/support Categorizes issue or PR as a support question. labels Apr 23, 2020
@thockin
Copy link
Member

thockin commented Apr 23, 2020

I think this is a feature request, not support, so I changed labels. On its face it SEEMS reasonable, but I am not close enough to the details to say. This is definitely sig-node.

@riking
Copy link

riking commented Apr 23, 2020

Adding a new pod lifecycle state has been determined to be intractable in the past.

Is the proposed change keeping the pod in !Ready until the first success from startupProbe?

@heckad
Copy link
Author

heckad commented Apr 23, 2020

Adding a new pod lifecycle state has been determined to be intractable in the past.

Where do you usually discuss this? Just wondering.

Is the proposed change keeping the pod in !Ready until the first success from startupProbe?

Yes, I want that pod considered not ready until startupProbe will not be successful. If readiness probe doesn't present then a pod can become ready. How can a pod be ready if it is not running?

@heckad
Copy link
Author

heckad commented May 1, 2020

@riking?

@heckad
Copy link
Author

heckad commented Jun 16, 2020

@riking how long wait?

@thockin
Copy link
Member

thockin commented Jun 16, 2020

Looking at this issue, I can confirm it, at least superficially - a pod with a perma-failing startupProbe is "ready" if there is no readinessGate defined and "not ready" if it is defined.

The question I have is whether we think that is "correct" or not? Initially I thought it was correct, but the more I think about it the less convinced I am.

If I have defined a startupProbe and it is not passing, my app should not be ready.

Does anyone disagree?

@heckad
Copy link
Author

heckad commented Jun 16, 2020

Also, the app should not be ready in terminating status. It's funny, but without a readiness probe, pods in the final state have the status ready and accept requests. This is unexpected.

@thockin
Copy link
Member

thockin commented Jun 16, 2020

Whether a pod is ready while terminating is an entirely different question, IMO. Apps that server network traffic probably need to "drain" and to do that they need to stay "ready" for a while as upstream LBs de-program.

Let's not conflate issues, please.

@thockin
Copy link
Member

thockin commented Jun 16, 2020

#92196

The fix seems simple - lets see what CI says.

@matthyx
Copy link
Contributor

matthyx commented Jun 16, 2020

@neolit123, they are silent in the chat. Who is involved in the development of startup probe?

Sorry, I don't spend much time on slack... you could have checked the release notes in 1.18 ;-)
Tim's fix seems good, thanks!

@matthyx
Copy link
Contributor

matthyx commented Jun 18, 2020

/reopen
Until we merge all related PRs...

@k8s-ci-robot k8s-ci-robot reopened this Jun 18, 2020
@k8s-ci-robot
Copy link
Contributor

@matthyx: Reopened this issue.

In response to this:

/reopen
Until we merge all related PRs...

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.

@matthyx
Copy link
Contributor

matthyx commented Jul 10, 2020

All merged.
/close

@k8s-ci-robot
Copy link
Contributor

@matthyx: Closing this issue.

In response to this:

All merged.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants