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(kuma-cp) locality-aware lb for external-services #2903

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented Oct 6, 2021

Summary

If locality-aware load balancing is enabled, we should respect kuma.io/zone tags on external services.

Full changelog

  • add e2e test
  • set different priorities depending on kuma.io/zone

Issues resolved

N/A

Documentation

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a comment

Choose a reason for hiding this comment

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

Code is ok. What about docs?

@@ -57,9 +60,12 @@ networking:
// External Service non-Kuma Cluster
external = clusters.GetCluster(Kuma3)

// todo(lobkovilya): use test-server as an external service
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a lot of work? Would it be possible to do it now? If now, can we create a card for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We deploy test-server always with a sidecar, so it's not as easy as just using TestServerUniversal(...). I'd better create a new issue for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lobkovilya
Copy link
Contributor Author

Docs are in progress

Copy link
Contributor

@bartsmykla bartsmykla left a comment

Choose a reason for hiding this comment

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

lgtm

@lobkovilya lobkovilya merged commit ec05784 into master Oct 7, 2021
@lobkovilya lobkovilya deleted the fix/lalb-for-es branch October 7, 2021 14:10
mergify bot pushed a commit that referenced this pull request Oct 7, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
(cherry picked from commit ec05784)
lobkovilya added a commit that referenced this pull request Oct 14, 2021
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
(cherry picked from commit ec05784)

Co-authored-by: Ilya Lobkov <ilya.lobkov@konghq.com>
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

3 participants