-
Notifications
You must be signed in to change notification settings - Fork 327
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(kuma-cp) scaling-up issue #1282
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…er status in Kubernetes Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
jakubdyszkiewicz
approved these changes
Dec 8, 2020
@@ -67,6 +67,10 @@ message Dataplane { | |||
// e.g. kuma.io/service=web, version=1.0. | |||
// `kuma.io/service` tag is mandatory. | |||
map<string, string> tags = 2 [ (validate.rules).map.min_pairs = 1 ]; | |||
|
|||
message Health { bool ready = 1; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs, please indicate that it is not required and if null, then assumed that it is healthy
@@ -73,6 +73,9 @@ func fillDataplaneOutbounds(outbound core_xds.EndpointMap, dataplanes []*mesh_co | |||
iface := dataplane.Spec.Networking.ToInboundInterface(inbound) | |||
// TODO(yskopets): do we need to dedup? | |||
// TODO(yskopets): sort ? | |||
if inbound.Health != nil && !inbound.Health.Ready { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop a comment why inbound.Health == nil
is assumed healthy
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
mergify bot
pushed a commit
that referenced
this pull request
Dec 9, 2020
(cherry picked from commit 8417151)
nickolaev
pushed a commit
that referenced
this pull request
Dec 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
After #1036 Kuma supports application probes on Kubernetes. But we didn't take advantage of it and didn't take probes into account when generating EDS. That means during scaling up some requests might return 503. This PR extends Dataplane model with the new field
health
:And then during generating EDS we don't include endpoints that are not ready yet.
Caution
This PR doesn't solve the scaling-down issue. That means during scaling down you can observe some 503. The scaling-down issue will be solved when we implement a retry policy.
Issues resolved
Fix #1132
Documentation
NOT READY