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

fix: don't depend on downstream in readiness check #641

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

enocom
Copy link
Member

@enocom enocom commented May 1, 2024

The readiness probe should not depend on downstream connections. Doing so can cause unwanted downtime. See 1. This commit removes all dialing of downstream database servers as a result.

In addition, after the Proxy receives a SIGTERM or SIGINT, the readiness check will now report an unhealthy status to ensure it is removed from circulation before shutdown completes.

@enocom enocom requested review from nancynh and a team as code owners May 1, 2024 16:17
Copy link
Collaborator

@nancynh nancynh left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for this!

The readiness probe should not depend on downstream connections. Doing
so can cause unwanted downtime. See [1]. This commit removes all dialing
of downstream database servers as a result.

In addition, after the Proxy receives a SIGTERM or SIGINT, the readiness
check will now report an unhealthy status to ensure it is removed from
circulation before shutdown completes.

[1]: https://github.com/zegl/kube-score/blob/master/README_PROBES.md#readinessprobe
@enocom enocom enabled auto-merge (squash) May 1, 2024 18:12
@enocom enocom merged commit 3a7c789 into main May 2, 2024
18 checks passed
@enocom enocom deleted the better-readiness branch May 2, 2024 18:26
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.

None yet

2 participants