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

Add liveness and readiness probes #1855

Closed
ncdc opened this issue Dec 6, 2019 · 8 comments · Fixed by #2156
Closed

Add liveness and readiness probes #1855

ncdc opened this issue Dec 6, 2019 · 8 comments · Fixed by #2156
Assignees
@prankul88
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone
v0.3.0

Comments

@ncdc
Copy link
Contributor

ncdc commented Dec 6, 2019

User Story

As a developer/user/operator I would like to have liveness and readiness probes for the Cluster API manager so that I am better informed when it is operational and ready to receive requests.

Detailed Description

See user story.

Anything else you would like to add:

It would be ideal if the readiness probe could only return true once the webhook server is online and ready to receive requests. I'm not sure how feasible this is, though.

/kind feature
/milestone v0.3.0
/priority important-soon
/help
@k8s-ci-robot
Copy link
Contributor

@ncdc:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

User Story

As a developer/user/operator I would like to have liveness and readiness probes for the Cluster API manager so that I am better informed when it is operational and ready to receive requests.

Detailed Description

See user story.

Anything else you would like to add:

It would be ideal if the readiness probe could only return true once the webhook server is online and ready to receive requests. I'm not sure how feasible this is, though.

/kind feature
/milestone v0.3.0
/priority important-soon
/help

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.

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 6, 2019
@k8s-ci-robot k8s-ci-robot added this to the v0.3.0 milestone Dec 6, 2019
@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Dec 6, 2019
@ncdc ncdc mentioned this issue Dec 6, 2019
Closed
@prankul88
Copy link
Contributor

@ncdc I would like to work on this if possible

@vincepri
Copy link
Member

/assign @prankul88

@chuckha
Copy link
Contributor

chuckha commented Dec 12, 2019

/cc

@chuckha
Copy link
Contributor

chuckha commented Dec 12, 2019

This will be important for the e2e tests. I pretty much have to resort to retrying my creates until the webhook services are ready to respond. Would be happy to work with you @prankul88 if you'd like, feel free to reach out on slack or here. No obligation though, if you'd like to tackle it yourself that works for me too.

@ncdc
Copy link
Contributor Author

ncdc commented Dec 12, 2019

Note, whatever we don't won't be 100% accurate and may still flake because the goroutine our readiness checks run in is different from the goroutine for the webhook server. See kubernetes-sigs/controller-runtime#723 for more details.

@prankul88
Copy link
Contributor

prankul88 commented Dec 13, 2019 •
edited
Loading

@chuckha Thanks. Would love your help. Give me some time to have a little more knowledge about it. Will reach out to you:)

phoracek added a commit to phoracek/kubemacpool that referenced this issue Jan 10, 2020
@phoracek
add readiness probe
32c2116 
Currently, manager pods turn Ready as soon as their binary starts.
However, it takes ~40 more seconds for the webhook to actually start
serving requests. In order to avoid that, we introduce a readiness
probe that marks the pod as Ready only once the server starts.

Please note that there is no way to do it properly with the current
controller-runtime. We don't have a hook point that would get executed
once the webhook has started. Therefore we have a Sleep there. That
should be dropped once
kubernetes-sigs/cluster-api#1855
is implemented.

Signed-off-by: Petr Horacek <[email protected]>
@phoracek phoracek mentioned this issue Jan 10, 2020
Closed
@jayunit100 jayunit100 mentioned this issue Jan 15, 2020
Closed
@ncdc
Copy link
Contributor Author

ncdc commented Jan 22, 2020

There is work in progress for CAPA here: kubernetes-sigs/cluster-api-provider-aws#1487. We should be consistent.

@cpanato cpanato mentioned this issue Jan 27, 2020
Merged
k8s-ci-robot added a commit that referenced this issue Jan 28, 2020
@k8s-ci-robot
Merge pull request #2156 from cpanato/GH-1855
54420c4 
✨ Add liveness/readiness probes
@cpanato cpanato mentioned this issue Mar 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@prankul88 prankul88

Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Milestone
v0.3.0
Development

Successfully merging a pull request may close this issue.

✨ Add liveness/readiness probes cpanato/cluster-api
5 participants
@ncdc @chuckha @vincepri @k8s-ci-robot @prankul88