TREE-679: Added support for health probes#12
Conversation
sujaya-sys
force-pushed
the
add-healthprobes
branch
from
febfd25 to
bdc353c
Compare
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
Co-authored-by: pelzerim <immanuel.pelzer@googlemail.com>
|
Thanks for the quick review @pelzerim! Added your changes. |
|
After we release this lets update the chart here https://github.com/checkly/infra/blob/a0aa06c510ad8725ebbd09b5802bf82bd6735df6/backend/agent.tf#L69 to see if it actually works |
| ports: | ||
| - name: metrics | ||
| containerPort: {{ .Values.metrics.port }} | ||
| - name: health |
There was a problem hiding this comment.
Ok, so i just realized that these are two ports. HOWEVER its the same server in the agent. So you can technically currently pass 2 ports but only one is used
Lets get rid of healthPort and just use metrics.port only. in the broader sense health checks are kinda metrics
There was a problem hiding this comment.
Gotcha, makes sense, I missed this. Added in my latest commit: cb4417d
sujaya-sys
changed the title
|
@pelzerim is this PR g2g from your side? If so I'd kick off testing as you suggested here: #12 (comment) Thank you! |
| {{- end }} | ||
| - name: "AGENT_SERVER_PORT" | ||
| value: "{{ .Values.metrics.port }}" | ||
| - name: "AGENT_HEALTH_PORT" |
There was a problem hiding this comment.
I am sorry for the confusion here, but I had another look and saw this comment in the code
// HealthPort is the port for health probe endpoints (/-/readiness, /-/liveness, /health).
// Default 8081 avoids conflicts with Node agent (which uses 8080).
// Configure via AGENT_HEALTH_PORT if needed.
HealthPort int `conf:"default:8081,env:AGENT_HEALTH_PORT"`
This makes things a tad more complicated.
I think we now need 2 readiness/liveness probes for each of the processes in the agent.
Additionally, this comes with a breaking change of the chart. It will only work with a certain version of the agent and up
There was a problem hiding this comment.
Hmm, what if we had a single endpoint but had one agent call the other to confirm readiness?
There was a problem hiding this comment.
Lets just do a breaking change release of this chart? I'd rather not leak this problem into the runners
ejanusevicius
left a comment
ejanusevicius
left a comment
There was a problem hiding this comment.
Can we remove the .DS_Store?
3 participants
Context
As part of the Checkly agent v.6.3.1 support for health probe endpoints has been added:
More info in our Dev docs: health probe endpoints
Request
Add liveness and readiness probes to the helm chart