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

Use Envoy's /ready endpoint for readiness probes #1433

Merged
merged 6 commits into from
Sep 10, 2019

Conversation

rochacon
Copy link
Contributor

@rochacon rochacon commented Sep 2, 2019

This PR enables Envoy's 1.11+ /ready endpoint for readiness probes.

Closes #1277

Copy link
Member

@stevesloka stevesloka left a comment

Choose a reason for hiding this comment

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

@ravilr
Copy link

ravilr commented Sep 2, 2019

we use the envoy.health_check filter today to achieve graceful connection draining during envoy shutdown/rolling deploys, i.e. issue an /healthcheck/fail and then wait for connections to drain.
Does /ready admin endpoint have the same behavior ?

@rochacon
Copy link
Contributor Author

rochacon commented Sep 3, 2019

@stevesloka I've left the filter on purpose, in the intent to use the /ready only for the readinessProbe, but given it is in the same listener it doesn't make sense to keep it. I'll test the change and update the PR later today.

@ravilr yes, both the health check filter and /ready behave quite similar.

@stevesloka
Copy link
Member

Thanks @rochacon, I think if we switch to /ready, there's no need for the healthz filter to be applied. One less thing to configure. =)

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM, with the caveat that this will need a section in the upgrade guide for 1.0-beta1 plus all 1.0 releases. I've added a note to #1476

@davecheney
Copy link
Contributor

davecheney commented Sep 10, 2019 via email

@youngnick youngnick merged commit 23ba737 into projectcontour:master Sep 10, 2019
@rochacon rochacon deleted the enable-ready-endpoint branch September 27, 2019 00:13
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.

Envoy v1.11: Investigate using /ready endpoint for probes
5 participants