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

GRPC policy: Fix routing upstream issue. #1179

Merged
merged 1 commit into from
Mar 11, 2020

Conversation

eloycoto
Copy link
Contributor

This commit makes things super simpler in GRPC, and avoid the
miss-configuration that was made previously around upstream&balancers.

Before on GRPC we set the balancer, and all the balancer logic happened
on the policy itself, instead of using the apicast.upstream and
apicast.balancer.

With this change, the location is changed on the upstream.lua, so it
does not matter if was set in the apicast.policy, or in another policy,
in this case the routing policy.

This finally fix THREESCALE-4684

Signed-off-by: Eloy Coto eloy.coto@gmail.com

@eloycoto eloycoto requested a review from a team as a code owner March 11, 2020 11:56
@eloycoto
Copy link
Contributor Author

This issue has test coverage on QE framework. @jsmadis

This commit makes things super simpler in GRPC, and avoid the
miss-configuration that was made previously around upstream&balancers.

Before on GRPC we set the balancer, and all the balancer logic happened
on the policy itself, instead of using the apicast.upstream and
apicast.balancer.

With this change, the location is changed on the upstream.lua, so it
does not matter if was set in the apicast.policy, or in another policy,
in this case the routing policy.

This finally fix THREESCALE-4684

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
@jsmadis
Copy link

jsmadis commented Mar 11, 2020

This issue has test coverage on QE framework.
@eloycoto Right now I am testing it manually. I will add a regression test asap I can.

@eloycoto eloycoto merged commit 35046b5 into 3scale:master Mar 11, 2020
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.

3 participants