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

dataclients/kubernetes: use healthcheck based on Shutdown predicate #1918

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

AlexanderYastrebov
Copy link
Member

Updates #1899

Signed-off-by: Alexander Yastrebov [email protected]

@@ -470,7 +446,7 @@ func (c *Client) LoadUpdate() ([]*eskip.Route, []string, error) {
// TODO: use eskip.Eq()
if r, ok := next[id]; ok && r.String() != c.current[id].String() {
updatedRoutes = append(updatedRoutes, r)
} else if !ok && id != healthcheckRouteID && id != httpRedirectRouteID {
} else if !ok {
Copy link
Member Author

@AlexanderYastrebov AlexanderYastrebov Nov 30, 2021

Choose a reason for hiding this comment

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

Since healthcheck and globalredirect routes are all static we can move them into loadAndConvert() and simplify update diff logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Special handling of healthcheck route had caused problems in the past.
Previously healthcheck route was added to updatedRoutes after loadAndConvert() and until the #618 it happened on every poll cycle which caused constant routing table recreation #614 (comment)

Static healthcheck and globalredirect routes means they do not change after initial LoadAll() and therefore will not end up neither in updatedRoutes nor in deletedIDs. They are also treated just like normal routes which simplifies the overall code flow.

@AlexanderYastrebov AlexanderYastrebov force-pushed the kube-shutdown-healthcheck branch 2 times, most recently from 83b0424 to 0078558 Compare December 1, 2021 01:45
@szuecs
Copy link
Member

szuecs commented Dec 1, 2021

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member Author

👍

@AlexanderYastrebov AlexanderYastrebov merged commit 7643317 into master Dec 1, 2021
@AlexanderYastrebov AlexanderYastrebov deleted the kube-shutdown-healthcheck branch December 1, 2021 16:51
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.

2 participants