Implement Readiness Probe #6
Conversation
phrynchanka
changed the title
phrynchanka
added
ready for review
and removed
question
There was a problem hiding this comment.
Nice work @phrynchanka ! Left a few comments, mostly around consistency but overall things look good!
operator/templates/stateful-set.yaml
Outdated
| initialDelaySeconds: {{ .Params.READINESS_CHECK_INITIAL_DELAY }} | ||
| periodSeconds: {{ .Params.READINESS_CHECK_PERIOD }} | ||
| timeoutSeconds: {{ .Params.READINESS_CHECK_TIMEOUT }} | ||
| successThreshold: {{ .Params.READINESS_SUCCESS_THRESHOLD }} |
There was a problem hiding this comment.
I wonder if this param is even needed to be configurable since we only need 1 successful check. But in the spirit of configuring :all-the-things:...
There was a problem hiding this comment.
I've exposed all properties which k8s offer for readiness/liveness probe. Maybe it will be used :)
Co-Authored-By: Sam Tran <[email protected]>
Co-Authored-By: Sam Tran <[email protected]>
Co-Authored-By: Sam Tran <[email protected]>
There was a problem hiding this comment.
Thanks for the PR @phrynchanka! The changes look good to me. I pushed a PR against this PR with a few ideas for improvements. Please take a look: #8
I'll try to get the PR with the testing structure merged asap so that we can update this PR's branch with it and then we can merge it with the green checks.
Also, I noticed that a slightly different pattern is used elsewhere for grepping the nodetool output. Did it also work with the pattern we're using here?
Yep, our pattern works great. |
* Move "readiness probe"-related params to the operator settings block. Also, prepend "NODE" since they're for the "node" container. It's likely that we'll add readiness probes for other containers. * s/cassandra-readiness-probe/node-readiness-probe/ To be consistent with per-container naming. * Simplify phrasing. * Be consistent with adding unit suffixes to param names.
|
|
I think for now the existing test should be sufficient, since a successful install is dependent on the readiness probes. We might want to add more specific tests for it in the near future, but for now it should be ok. |
|
🚢 |
What changes are proposed in this PR?
Resolves DCOS-58972
This PR Implement Readiness Probe for Kudo Cassandra operator.
LivenessProbe wasn't implemented because we need clear understand when pods definitely must be restarted.
Implementation steps :
params.ymlstateful-set.yamlDC/OSbut without security support (which should be implemented and script modified)How were these changes tested?
Manually tested
kubectl get event --namespace $kudo_cassandra_instance_namespace --field-selector involvedObject.name=cassandra-node-0and
kubectl describe pod cassandra-node-0