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.

Sign up for GitHub

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

Jump to bottom

fix(traffic-split): binding upstream via upstream_id is invalid #3842

Merged
merged 3 commits into from
Mar 17, 2021
Merged

fix(traffic-split): binding upstream via upstream_id is invalid #3842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af18e6a
fix(traffic-split): binding upstream via upstream_id is invalid
Firstsawyou Mar 16, 2021
df578be
update ctx.upstream_id
Firstsawyou Mar 17, 2021
4cff581
update.
Firstsawyou Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apisix/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,12 @@ function _M.http_access_phase()
end

local up_id = route.value.upstream_id

-- used for the traffic-split plugin
if api_ctx.upstream_id then
up_id = api_ctx.upstream_id
end

if up_id then
local upstreams = core.config.fetch_created_obj("/upstreams")
if upstreams then
Expand Down
17 changes: 6 additions & 11 deletions apisix/plugins/traffic-split.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ local function set_upstream(upstream_info, ctx)
end


local function new_rr_obj(weighted_upstreams, route_upstream_id)
local function new_rr_obj(weighted_upstreams)
local server_list = {}
for i, upstream_obj in ipairs(weighted_upstreams) do
if upstream_obj.upstream_id then
Expand All @@ -239,12 +239,8 @@ local function new_rr_obj(weighted_upstreams, route_upstream_id)
-- If the upstream object has only the weight value, it means
-- that the upstream weight value on the default route has been reached.
-- Mark empty upstream services in the plugin.
if route_upstream_id then
server_list[route_upstream_id] = upstream_obj.weight
else
upstream_obj.upstream = "plugin#upstream#is#empty"
server_list[upstream_obj.upstream] = upstream_obj.weight
end
upstream_obj.upstream = "plugin#upstream#is#empty"
server_list[upstream_obj.upstream] = upstream_obj.weight

end
end
Expand Down Expand Up @@ -285,9 +281,8 @@ function _M.access(conf, ctx)
return
end

local route_upstream_id = ctx.matched_route.value.upstream_id
local rr_up, err = core.lrucache.plugin_ctx(lrucache, ctx, nil, new_rr_obj,
weighted_upstreams,route_upstream_id)
weighted_upstreams)
if not rr_up then
core.log.error("lrucache roundrobin failed: ", err)
return 500
Expand All @@ -298,13 +293,13 @@ function _M.access(conf, ctx)
core.log.info("upstream: ", core.json.encode(upstream))
return set_upstream(upstream, ctx)
elseif upstream and upstream ~= "plugin#upstream#is#empty" then
ctx.matched_route.value.upstream_id = upstream
ctx.upstream_id = upstream
core.log.info("upstream_id: ", upstream)
return
end

ctx.upstream_id = nil
core.log.info("route_up: ", upstream)
ctx.matched_route.value.upstream_id = nil
return
end

Expand Down
137 changes: 118 additions & 19 deletions t/plugin/traffic-split.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,14 +1623,14 @@ passed
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[=[{
"uri": "/hello*",
"uri": "/server_port",
"plugins": {
"traffic-split": {
"rules": [
{
"match": [
{
"vars": [["uri", "==", "/hello"]]
"vars": [["arg_name", "==", "James"]]
}
],
"weighted_upstreams": [
Expand Down Expand Up @@ -1660,16 +1660,115 @@ passed


=== TEST 47: when `match` rule passed, use the `upstream_id` in plugin, and when it failed, use the `upstream_id` in route
--- pipelined_requests eval
["GET /hello", "GET /hello1", "GET /hello", "GET /hello1", "GET /hello", "GET /hello1"]
--- response_body eval
["hello world\n", "hello1 world\n", "hello world\n", "hello1 world\n", "hello world\n", "hello1 world\n"]
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local bodys = {}

for i = 1, 5, 2 do
-- match rule passed
local _, _, body = t('/server_port?name=James', ngx.HTTP_GET)
bodys[i] = body

-- match rule failed
local _, _, body = t('/server_port', ngx.HTTP_GET)
bodys[i+1] = body
end

table.sort(bodys)
ngx.say(table.concat(bodys, ", "))
}
}
--- request
GET /t
--- response_body
1981, 1981, 1981, 1982, 1982, 1982
--- no_error_log
[error]



=== TEST 48: set route(use upstream for route and upstream_id for plugin), and `weighted_upstreams` does not have a structure with only `weight`
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[=[{
"uri": "/server_port",
"plugins": {
"traffic-split": {
"rules": [
{
"match": [
{
"vars": [["arg_name", "==", "James"]]
}
],
"weighted_upstreams": [
{"upstream_id": 1}
]
}
]
}
},
"upstream": {
"type": "roundrobin",
"nodes": {
"127.0.0.1:1980": 1
}
}
}]=]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 49: when `match` rule passed, use the `upstream_id` in plugin, and when it failed, use the `upstream` in route
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local bodys = {}

for i = 1, 5, 2 do
-- match rule passed
local _, _, body = t('/server_port?name=James', ngx.HTTP_GET)
bodys[i] = body

-- match rule failed
local _, _, body = t('/server_port', ngx.HTTP_GET)
bodys[i+1] = body
end

table.sort(bodys)
ngx.say(table.concat(bodys, ", "))
}
}
--- request
GET /t
--- response_body
1980, 1980, 1980, 1981, 1981, 1981
--- no_error_log
[error]



=== TEST 48: set route(id: 1, upstream_id: 1, upstream_id in plugin: 2), and `weighted_upstreams` has a structure with only `weight`
=== TEST 50: set route(id: 1, upstream_id: 1, upstream_id in plugin: 2), and `weighted_upstreams` has a structure with only `weight`
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -1713,7 +1812,7 @@ passed



=== TEST 49: all requests `match` rule passed, proxy requests to the upstream of route based on the structure with only `weight` in `weighted_upstreams`
=== TEST 51: all requests `match` rule passed, proxy requests to the upstream of route based on the structure with only `weight` in `weighted_upstreams`
--- config
location /t {
content_by_lua_block {
Expand All @@ -1736,7 +1835,7 @@ GET /t



=== TEST 50: the upstream_id is used in the plugin
=== TEST 52: the upstream_id is used in the plugin
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -1786,7 +1885,7 @@ passed



=== TEST 51: `match` rule passed(upstream_id)
=== TEST 53: `match` rule passed(upstream_id)
--- config
location /t {
content_by_lua_block {
Expand All @@ -1810,7 +1909,7 @@ GET /t



=== TEST 52: only use upstream_id in the plugin
=== TEST 54: only use upstream_id in the plugin
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -1859,7 +1958,7 @@ passed



=== TEST 53: `match` rule passed(only use upstream_id)
=== TEST 55: `match` rule passed(only use upstream_id)
--- config
location /t {
content_by_lua_block {
Expand All @@ -1882,7 +1981,7 @@ GET /t



=== TEST 54: use upstream and upstream_id in the plugin
=== TEST 56: use upstream and upstream_id in the plugin
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -1932,7 +2031,7 @@ passed



=== TEST 55: `match` rule passed(upstream + upstream_id)
=== TEST 57: `match` rule passed(upstream + upstream_id)
--- config
location /t {
content_by_lua_block {
Expand All @@ -1957,7 +2056,7 @@ GET /t



=== TEST 56: set route + upstream (two upstream node: one healthy + one unhealthy)
=== TEST 58: set route + upstream (two upstream node: one healthy + one unhealthy)
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -2032,7 +2131,7 @@ passed



=== TEST 57: hit routes, ensure the checker is bound to the upstream
=== TEST 59: hit routes, ensure the checker is bound to the upstream
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -2086,7 +2185,7 @@ qr/\([^)]+\) unhealthy .* for '.*'/



=== TEST 58: set upstream(id: 1), by default retries count = number of nodes
=== TEST 60: set upstream(id: 1), by default retries count = number of nodes
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -2118,7 +2217,7 @@ passed



=== TEST 59: set route(id: 1, upstream_id: 1)
=== TEST 61: set route(id: 1, upstream_id: 1)
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -2162,7 +2261,7 @@ passed



=== TEST 60: hit routes
=== TEST 62: hit routes
--- request
GET /hello
--- error_code: 502
Expand Down