Skip to content

Commit

Permalink
GRPC policy: Fix routing upstream issue.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
eloycoto committed Mar 11, 2020
1 parent a17158a commit da4eaec
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 50 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Fixed naming issues in policies [THREESCALE-4150](https://issues.jboss.org/browse/THREESCALE-4150) [PR #1167](https://github.com/3scale/APIcast/pull/1167)
- Fixed issues on invalid config in logging policy [THREESCALE-4605](https://issues.jboss.org/browse/THREESCALE-4605) [PR #1168](https://github.com/3scale/APIcast/pull/1168)
- Fixed issues with routing policy and GRPC one [THREESCALE-4684](https://issues.jboss.org/browse/THREESCALE-4684) [PR #1177](https://github.com/3scale/APIcast/pull/1177)
- Fixed issues with routing policy and GRPC one [THREESCALE-4684](https://issues.jboss.org/browse/THREESCALE-4684) [PR #1177](https://github.com/3scale/APIcast/pull/1177) [PR #1179](https://github.com/3scale/APIcast/pull/1179)

## [3.7.0-alpha2]

Expand Down
51 changes: 3 additions & 48 deletions gateway/src/apicast/policy/grpc/grpc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,21 @@

local policy = require('apicast.policy')
local _M = policy.new('grpc', "builtin")
local resty_url = require('resty.url')
local round_robin = require 'resty.balancer.round_robin'

local apicast_balancer = require('apicast.balancer')
local new = _M.new

local balancer = round_robin.new()

function _M.new(config)
local self = new(config)
return self
end

function _M:rewrite(context)
if ngx.var.server_protocol == "HTTP/2.0" then
-- upstream defined in gateway/conf.d/http2.conf
context.upstream_location_name = "@grpc_upstream"
end
end

function _M:content(context)
-- This is needed within the combination of the routing policy, if not the
-- upstream got overwritten and balancer phase is called before.
if not context.upstream_location_name then
return
end

if ngx.var.server_protocol ~= "HTTP/2.0" then
ngx.var.host = context.upstream_location_name
end

ngx.var.proxy_host = "upstream"
end

function _M:balancer(context)
if not context.upstream_location_name then
return
end

-- balancer need to be used due to grpc_pass does not support variables and
-- upstream block need to be in place.
local upstream = context:get_upstream()
if not upstream then
ngx.log(ngx.WARN, "Upstream is not present in the balancer")
return
end

local peers = balancer:peers(upstream.servers)
local peer, err = balancer:select_peer(peers)
if err then
ngx.log(ngx.WARN, "Cannot get a peer for the given upstream: ", err)
return
end

local ip = peer[1]
local port = peer[2] or upstream.uri.port or resty_url.default_port(upstream.uri.scheme)
local _, err = balancer:set_current_peer(ip, port)

if err then
ngx.log(ngx.WARN, "Cannot set balancer IP and port '", ip, ":", port, "'")
return
end
end
_M.balancer = apicast_balancer.call

return _M
4 changes: 3 additions & 1 deletion gateway/src/apicast/upstream.lua
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ function _M:call(context)
end

if not self.servers then self:resolve() end

if context.upstream_location_name then
self.location_name = context.upstream_location_name
end
context[self.upstream_name] = self

return exec(self)
Expand Down

0 comments on commit da4eaec

Please sign in to comment.