Skip to content

fix(helm): add liveliness and readiness probes to helm chart #143

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

Merged
alukach merged 4 commits into from
Mar 28, 2026
+88 −1
Merged

fix(helm): add liveliness and readiness probes to helm chart #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc43a52
fix(helm): add liveliness and readiness probes to helm chart
alukach Mar 9, 2026
d66b1d4
chore: release 1.0.3
alukach Mar 9, 2026
4a6d0a5
Apply feedback
alukach Mar 9, 2026
2b70f3b
small fixes.
pantierra Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions helm/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,12 @@ Render env var value based on type
{{- . | toJson | quote -}}
{{- end -}}
{{- end -}}

{{/*
Validate terminationGracePeriodSeconds > preStopSleepSeconds
*/}}
{{- define "stac-auth-proxy.validateTerminationGracePeriod" -}}
{{- if not (gt .Values.terminationGracePeriodSeconds .Values.preStopSleepSeconds) -}}
{{- fail "terminationGracePeriodSeconds must be greater than preStopSleepSeconds" -}}
{{- end -}}
{{- end -}}
20 changes: 20 additions & 0 deletions helm/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{{- include "stac-auth-proxy.validateTerminationGracePeriod" . -}}
apiVersion: apps/v1
kind: Deployment
metadata:
Expand All @@ -14,6 +15,7 @@ spec:
labels:
{{- include "stac-auth-proxy.selectorLabels" . | nindent 8 }}
spec:
terminationGracePeriodSeconds: {{ .Values.terminationGracePeriodSeconds }}
serviceAccountName: {{ include "stac-auth-proxy.serviceAccountName" . }}
securityContext:
{{- toYaml .Values.securityContext | nindent 8 }}
Expand All @@ -31,6 +33,24 @@ spec:
- name: http
containerPort: 8000
protocol: TCP
{{- with .Values.startupProbe }}
startupProbe:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.livenessProbe }}
livenessProbe:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.readinessProbe }}
readinessProbe:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.preStopSleepSeconds }}
lifecycle:
preStop:
exec:
command: ["sleep", "{{ .Values.preStopSleepSeconds }}"]
{{- end }}
Comment thread
alukach marked this conversation as resolved.
resources:
{{- toYaml .Values.resources | nindent 12 }}
env:
Expand Down
29 changes: 28 additions & 1 deletion helm/values.schema.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,33 @@ properties:
description: "List of capabilities to drop"
description: "Container-level security context"

terminationGracePeriodSeconds:
type: integer
minimum: 1
description: "Duration in seconds the pod needs to terminate gracefully. Must be greater than preStopSleepSeconds."
Copy link
Copy Markdown
Contributor

@pantierra pantierra Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be greater than preStopSleepSeconds.

Can we have a check for this?

Copy link
Copy Markdown
Contributor

@pantierra pantierra Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check for this in the helm/templates/_helpers.tpl

default: 30

preStopSleepSeconds:
type: integer
minimum: 0
description: "Seconds to sleep in preStop hook before SIGTERM, allowing Kubernetes endpoint propagation. Set to 0 to disable."
default: 15
Copy link
Copy Markdown
Contributor

@pantierra pantierra Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default value below is 5. Please make sure they are the same, either 5 or 15.

Copy link
Copy Markdown
Member Author

@alukach alukach Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we make these types of check in a helm chart?

Copy link
Copy Markdown
Contributor

@pantierra pantierra Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the schema is ignored in the checks because it is a yaml not json. This should fix. I am not entirely sure if checks would include the standard values, though.

In this PR i just changed the preStopSleepSeconds in values.yaml to 15.


startupProbe:
type: object
additionalProperties: true
description: "Startup probe configuration. Disables liveness/readiness probes until startup succeeds."

livenessProbe:
type: object
additionalProperties: true
description: "Liveness probe configuration. Determines if the container should be restarted."

readinessProbe:
type: object
additionalProperties: true
description: "Readiness probe configuration. Determines if the container should receive traffic."

nodeSelector:
type: object
additionalProperties:
Expand Down Expand Up @@ -299,4 +326,4 @@ properties:
description: "Name of the image pull secret"

required:
- service
- service
31 changes: 31 additions & 0 deletions helm/values.yaml
View file
Open in desktop
Copy link
Copy Markdown

@thenav56 thenav56 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to enable the health endpoint?

Currently, when running this inside a stac-auth-proxy pod container, it returns a 404:

>>> import httpx
>>> httpx.get("http://localhost:8000/healthz")
<Response [404 Not Found]>

Copy link
Copy Markdown
Member Author

@alukach alukach Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The healthz path DOES include the root_path, are you using that? ie does http://localhost:8000/stac/healthz work?

Comment thread
alukach marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,37 @@ containerSecurityContext:
drop:
- ALL

# Graceful shutdown: delays SIGTERM to allow Kubernetes endpoint propagation.
# The preStop hook runs BEFORE SIGTERM is sent, giving kube-proxy time to
# remove the pod from service endpoints so no new traffic arrives during shutdown.
# terminationGracePeriodSeconds must be > preStopSleepSeconds + app shutdown time.
terminationGracePeriodSeconds: 30
preStopSleepSeconds: 15

# Probes
# startupProbe disables liveness/readiness checks until startup succeeds,
# giving the app time to wait for upstream services (e.g. during node scaling).
startupProbe:
httpGet:
path: /healthz
port: http
periodSeconds: 2
failureThreshold: 30 # 60s total for startup

livenessProbe:
httpGet:
path: /healthz
port: http
periodSeconds: 60
failureThreshold: 3

readinessProbe:
httpGet:
path: /healthz
port: http
periodSeconds: 5
failureThreshold: 3

nodeSelector: {}
tolerations: []
affinity: {}
Expand Down
Loading