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

chart: add liveness/readiness probes to chartsvc deployment #645

Merged
merged 3 commits into from
Sep 18, 2018

Conversation

prydonius
Copy link
Contributor

chartsvc has endpoints for checking liveness and readiness but we were
not taking advantage of these in our Kubernetes configurations. This
updates the chart to configure the probes for this service.

chartsvc has endpoints for checking liveness and readiness but we were
not taking advantage of these in our Kubernetes configurations. This
updates the chart to configure the probes for this service.
@@ -124,6 +124,18 @@ chartsvc:
# requests:
# cpu: 100m
# memory: 128Mi
livenessProbe:
httpGet:
path: /live
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this route (/live) based on any common practice? I've seen the proliferation of healthz instead

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, you are just just using them.

Copy link
Contributor Author

@prydonius prydonius Sep 17, 2018

Choose a reason for hiding this comment

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

@migmartri when I implemented this in chartsvc, I used the https://github.com/heptiolabs/healthcheck library that suggests using /live and /ready https://github.com/heptiolabs/healthcheck#usage. Specifically, the difference from using a single /healthz is having two separate endpoints that might have different implementations of the healthcheck (e.g. /ready might check that MongoDB is connectable, though not currently implemented).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh nice, I was not aware of that library, thanks for the info.

Specifically, the difference from using a single /healthz is having two separate endpoints that might have different implementations of the healthcheck (e.g. /ready might check that MongoDB is connectable, though not currently implemented).

Right, in the past I've been handling this by passing a liveness parameter to the /healthz endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@migmartri makes sense, yeah, the name of the endpoints doesn't matter much here I guess, although I agree that healthz is more common. /live and /ready are also possibly too specific to Kubernetes, although the concept does transfer across. Either way, probably not important enough to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@migmartri makes sense, yeah, the name of the endpoints doesn't matter much here I guess, although I agree that healthz is more common. /live and /ready are also possibly too specific to Kubernetes, although the concept does transfer across. Either way, probably not important enough to change.

No it's not important, thanks for the info.

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at the implementation of those healthchecks and it seems that they do not do much apart of being mounted. So I think that this task is still valid.

@migmartri
Copy link
Contributor

Refs #502

httpGet:
path: /ready
port: 8080
initialDelaySeconds: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the default? Or you want to be explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oddly, the documentation doesn't specify what the default is: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.11/#probe-v1-core. I just copied these from the other services that have healtchecks (dashboard and frontend). I think it's good to be explicit (especially given it's unclear what the default is), and I think that it's fine for readiness to not have any delay. The initialDelaySeconds for liveness might actually be too long for a simple service like this, but this could give it more time to attempt to connect to MongoDB.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a funny note. In their advanced example, they mount the readiness check as healthz :P https://godoc.org/github.com/heptiolabs/healthcheck

@prydonius prydonius merged commit 0a67555 into vmware-tanzu:master Sep 18, 2018
@prydonius prydonius deleted the add-chartsvc-probes branch September 18, 2018 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants