Add liveness/readiness probes #79
Conversation
|
/lgtm |
openshift-ci-robot
added
the
approved
|
/test e2e-metal-ipi |
|
/retest |
| @@ -106,6 +109,14 @@ func main() { | |||
| os.Exit(1) | |||
| } | |||
|
|
|||
| if err := mgr.AddReadyzCheck("ping", healthz.Ping); err != nil { | |||
There was a problem hiding this comment.
It looks like these health and readiness checks will always return "ready" or "healthy". They don't actually check anything, other than verifying that the process can respond to an HTTP request. Is that the intent? It would be a good idea to document the criteria either way, both in the commit message, and also in the README or similar documentation.
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold Would like to get thoughts on my doc request. |
openshift-ci-robot
added
the
do-not-merge/hold
Danil-Grigorev
force-pushed
the
add-health-check
branch
from
d928ad3
to
7c61e8e
Compare
openshift-ci-robot
added
needs-rebase
|
@mhrivnak I added a small doc file to the PR. This is the intent for probes right now. |
Danil-Grigorev
force-pushed
the
add-health-check
branch
2 times, most recently
from
1b8d07f
to
999eddd
Compare
Danil-Grigorev
force-pushed
the
add-health-check
branch
from
999eddd
to
1c4e8c0
Compare
openshift-ci-robot
removed
the
needs-rebase
There was a problem hiding this comment.
To alleviate any ambiguity or assumptions people might make about what controller-runtime probes do, I suggest a very direct statement such as: "The process will be considered ready and healthy by kubernetes based solely on whether it can respond to an HTTP request."
docs/probes.md
Outdated
| @@ -0,0 +1,30 @@ | |||
| # Baremetal Machine Controller readiness | |||
|
|
|||
| For the perpouse of integration with MAO, baremethal provider is leveraging controller-runtime readiness and liveness probes, checking ability to serve HTTP requests in the manager process, to encrease robustness for cluster operator status reporting. | |||
- Introducing simple readiness probes for controller-runtime, checking the manager container could respond to HTTP requests.
Danil-Grigorev
force-pushed
the
add-health-check
branch
from
1c4e8c0
to
24f66fd
Compare
|
/unhold |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Danil-Grigorev, dhellmann, enxebre, mhrivnak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Depends on openshift/machine-api-operator#602
Close #78