-
Notifications
You must be signed in to change notification settings - Fork 483
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
fix(probes) allows overriding the default liveness/readiness probe with custom commands #1070
base: main
Are you sure you want to change the base?
fix(probes) allows overriding the default liveness/readiness probe with custom commands #1070
Conversation
…th custom commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Do you mind adding a CI values.yaml
that could test this? You can add this in here: https://github.com/Kong/charts/tree/main/charts/kong/ci
Updated tests, and also updated PR description with the example rendered manifest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at linter errors in charts/kong/ci/probe-test-values.yaml
@@ -823,6 +823,7 @@ resources: {} | |||
# memory: 2G | |||
|
|||
# readinessProbe for Kong pods | |||
# replace httpGet with exec if you want to use a custom command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please specify in the comments here an exemplary use case for exec? So that potential users know how exactly this can be used? I'm thinking something that could simply be uncommented to work ( assuming that httpGet
section gets commented ). WDYT?
initialDelaySeconds: {{ .Values.livenessProbe.initialDelaySeconds }} | ||
timeoutSeconds: {{ .Values.livenessProbe.timeoutSeconds }} | ||
periodSeconds: {{ .Values.livenessProbe.periodSeconds }} | ||
successThreshold: {{ .Values.livenessProbe.successThreshold }} | ||
failureThreshold: {{ .Values.livenessProbe.failureThreshold }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about making this into a small helm template which could be reused for both liveness
and readiness
probe?
What this PR does / why we need it:
This allows folks to override the liveness/readiness probe with a custom exec/command instead of defaulting to the httpGet. Keeps the existing behavior as default so no breaking changes.
Which issue this PR fixes
Special notes for your reviewer:
example values with execGet
helm template . --values values.yaml
values.yaml
rendered out manifest
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
main
branch.