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.

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

Readiness + Startup probes #804

Merged
merged 21 commits into from
Jan 18, 2025
42 changes: 31 additions & 11 deletions infra/helm/meshdb/templates/meshweb.yaml
Original file line number Diff line number Diff line change
@@ -47,18 +47,38 @@ spec:
mountPath: /opt/meshdb/static
{{ if eq .Values.meshweb.liveness_probe "true" }}
livenessProbe:
exec:
command:
- curl
- http://127.0.0.1:{{ .Values.meshweb.port }}/api/v1
periodSeconds: 3
initialDelaySeconds: 4
timeoutSeconds: 3
{{ end }}
livenessProbe:
{{- toYaml .Values.livenessProbe | nindent 12 }}
exec:
command:
- bash
- -c
- 'curl http://127.0.0.1:{{ .Values.meshweb.port }}/api/v1/ -H "Host: db.nycmesh.net" -s | grep meshin'
Copy link
Collaborator

Choose a reason for hiding this comment

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

The grep is fine but knowing that it's we're meshin' confused my brain for a sec lol

periodSeconds: 10
initialDelaySeconds: 30
timeoutSeconds: 3
{{ end }}
{{ if eq .Values.meshweb.readiness_probe "true" }}
readinessProbe:
{{- toYaml .Values.readinessProbe | nindent 12 }}
exec:
command:
- bash
- -c
- 'curl http://127.0.0.1:{{ .Values.meshweb.port }}/api/v1/ -H "Host: db.nycmesh.net" -s | grep meshin'
periodSeconds: 3
initialDelaySeconds: 10
timeoutSeconds: 3
{{ end }}
{{ if eq .Values.meshweb.startup_probe "true" }}
startupProbe:
exec:
command:
- bash
- -c
- 'curl http://127.0.0.1:{{ .Values.meshweb.port }}/api/v1/ -H "Host: db.nycmesh.net" -s | grep meshin'
periodSeconds: 3
initialDelaySeconds: 20
timeoutSeconds: 3
failureThreshold: 20
{{ end }}
Comment on lines +61 to +81
Copy link
Collaborator

@WillNilges WillNilges Jan 10, 2025

Choose a reason for hiding this comment

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

I don't think it's worth having redundant probes. We should consider what kind of probes to use in place of these. Maybe we should just check if we get anything back from nginx at all as the liveness probe, then make the readiness probe check we're meshin'?

Not sure about startupProbe

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, I think I might be wrong to suggest that.

https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/

A common pattern for liveness probes is to use the same low-cost HTTP endpoint as for readiness probes, but with a higher failureThreshold. This ensures that the pod is observed as not-ready for some period of time before it is hard killed.

resources:
{{- toYaml .Values.meshweb.resources | nindent 12 }}
volumes:
2 changes: 2 additions & 0 deletions infra/helm/meshdb/values.yaml
Original file line number Diff line number Diff line change
@@ -38,6 +38,8 @@ meshweb:
static_pvc_name: "meshdb-static-pvc"
static_pvc_size: "1Gi"
liveness_probe: "true"
readiness_probe: "true"
startup_probe: "true"
image:
repository: willnilges/meshdb
tag: main