Skip to content

Commit

Permalink
balancer: raise error when trying to set the peer twice
Browse files Browse the repository at this point in the history
With the latest changes, the balancer phase can be retried, and the
upstream might be already set. This change is needed to only raise an
error when the peer has not been set for the current retry.
  • Loading branch information
davidor committed Jun 6, 2019
1 parent afd2542 commit f81442a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
4 changes: 2 additions & 2 deletions gateway/src/apicast/balancer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ local function get_upstream(context)
return nil, 'missing upstream'
end

if context[upstream] then
if context.peer_set_in_current_balancer_try then
return nil, 'already set peer'
end

Expand Down Expand Up @@ -114,7 +114,7 @@ function _M.call(_, context, bal)

-- I wish there would be a nicer way, but unfortunately ngx.exit(ngx.OK) does not
-- terminate the current phase handler and will evaluate all remaining balancer phases.
context[upstream] = peer
context.peer_set_in_current_balancer_try = true
return peer
else
ngx.log(ngx.ERR, 'failed to set current backend peer: ', err)
Expand Down
1 change: 1 addition & 0 deletions gateway/src/apicast/executor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ do
function _M:balancer()
local context = self:context('balancer')
context.balancer_retries = (context.balancer_retries and context.balancer_retries + 1) or 0
context.peer_set_in_current_balancer_try = false
return self.policy_chain.balancer(self.policy_chain, context)
end

Expand Down
6 changes: 6 additions & 0 deletions spec/executor_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ describe('executor', function()
assert.same(3, ngx.ctx.context.balancer_retries)
end)

it('sets a var in the context that marks that the peer has not been set yet in the current try', function()
executor:balancer()

assert.is_false(ngx.ctx.context.peer_set_in_current_balancer_try)
end)

it('forwards the call to the policy chain', function()
executor:balancer()

Expand Down

0 comments on commit f81442a

Please sign in to comment.