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 istio demo: Internal request route not base on weight but replica count #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StyleTang
Copy link

Notes

Hi Team, I find the Istio demo (both istio and istio-subset) fine-grained, weighted traffic shifting feature has issue for internal request.

A lot of uses are learning from this demo and they maybe not expert in istio, so we'd better to fix it and provide best practice for them, otherwise we will misleading them.
(It is not easy for beginners to fix it by themselves, like me...)

Could you take some time to review it.
Thanks

Architecture

The green path is external request, istio-rollout.apps.argoproj.io/color works well. because

  • VirtualService istio-rollout-vsvc apply to istio-rollout-gateway, and external request go through istio-ingressgateway(istio-rollout-gateway)
  • Host istio-rollout.apps.argoproj.io can be matched, so it can use http.route rule

The red path is internal request, because the VirtualService istio-rollout-vsvc not apply to service mesh, and istio-rollout.rollouts-demo-istio.svc.cluster.local go through mesh but not gateway, it only uses Service istio-rollout's endpoints, http.route rule doesn't work for this request.
image

How to fix

The fix contains 2 parts.

  • VirtualService istio-rollout-vsvc apply to mesh
  • match hosts {servie}.{namespace}.svc.cluster.local, e.g. istio-rollout.rollouts-demo-istio.svc.cluster.local

@StyleTang
Copy link
Author

Link issue: Internal request route not base on weight but replica count

@zachaller
Hi bro, sorry to bother you.
I didn't get notification from your previous replies, thanks for your comment.
Could you help me review this (or anyone else can help)?

Thanks

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.

1 participant