openshift-evangelists / workshopper

Workshop content rendering tool
63 stars 42 forks source link

Add /healthz endpoint (or similar) #56

Closed jorgemoralespou closed 6 years ago

jorgemoralespou commented 6 years ago

I had today a deployment with liveness/readiness probes configured to /, but that url redirected to the content, which had an error and returned 500, so the deployment never finished (was never ready).

If there's an error on the content, the deployment will never finish.

marekjelen commented 6 years ago

Hmm, with a non-OK content it should probably fail. What does @siamaksade @thoraxe think?

I am not against having health endpoints, but how should they behave? The integral part of the deployment is the content, if the content is not OK, why would health endpoint report healthy state?

siamaksade commented 6 years ago

I don't think it's needed. Each deployment need to define probes that are specific to its content to be able to detect issues like that for example to /workshop/cloudnative/lab/getting-started

thoraxe commented 6 years ago

Jorge's content wasn't rendering and therefore the liveness/readiness probes got 500s which count as failures. The container would've been restarted (failed liveness) and eventually the deployment would be considered failed.

It sounds like the behavior was correct and is exactly what we want and what's expected. If we had a /healthz endpoint, you wouldn't have known you had a problem unless you visited the app. The app would've been "broken" but happily passed liveness/readiness, which doesn't sound right.

This doesn't sound like a bug or a reason to implement /healthz.

Am I misinterpreting?

marekjelen commented 6 years ago

OK, the agreement seems to be that it should not be there.

Let's reopen in case we find a use-case for such endpoints.