fix: the traffic-split plugin is invalid to bind upstream via upstrea…

…m_id (#3758) fix #3740
apache · Mar 15, 2021 · 210ca46 · 210ca46
1 parent 4ebccaf
commit 210ca46
Show file tree

Hide file tree

Showing 2 changed files with 143 additions and 15 deletions.
diff --git a/apisix/plugins/traffic-split.lua b/apisix/plugins/traffic-split.lua
@@ -255,7 +255,7 @@ local function set_upstream(upstream_info, ctx)
 end
 
 
-local function new_rr_obj(weighted_upstreams)
+local function new_rr_obj(weighted_upstreams, route_upstream_id)
     local server_list = {}
     for i, upstream_obj in ipairs(weighted_upstreams) do
         if upstream_obj.upstream_id then
@@ -268,8 +268,13 @@ local function new_rr_obj(weighted_upstreams)
             -- 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.
-            upstream_obj.upstream = "plugin#upstream#is#empty"
-            server_list[upstream_obj.upstream] = upstream_obj.weight
+            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
+
         end
     end
 
@@ -309,7 +314,9 @@ function _M.access(conf, ctx)
         return
     end
 
-    local rr_up, err = lrucache(weighted_upstreams, nil, new_rr_obj, weighted_upstreams)
+    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)
     if not rr_up then
         core.log.error("lrucache roundrobin failed: ", err)
         return 500

diff --git a/t/plugin/traffic-split.t b/t/plugin/traffic-split.t
@@ -1615,7 +1615,128 @@ passed
 
 
 
-=== TEST 46: the upstream_id is used in the plugin
+=== TEST 46: set route(id: 1, upstream_id: 1, upstream_id in plugin: 2), 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": "/hello*",
+                    "plugins": {
+                        "traffic-split": {
+                            "rules": [
+                                {
+                                    "match": [
+                                        {
+                                            "vars": [["uri", "==", "/hello"]]
+                                        }
+                                    ],
+                                    "weighted_upstreams": [
+                                        {"upstream_id": 2}
+                                    ]
+                                }
+                            ]
+                        }
+                    },
+                    "upstream_id":"1"
+                }]=]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== 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"]
+--- 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`
+--- 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": [["uri", "==", "/server_port"]]
+                                        }
+                                    ],
+                                    "weighted_upstreams": [
+                                        {"upstream_id": 2, "weight": 1},
+                                        {"weight": 1}
+                                    ]
+                                }
+                            ]
+                        }
+                    },
+                    "upstream_id":"1"
+                }]=]
+            )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 49: 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 {
+        local t = require("lib.test_admin").test
+        local bodys = {}
+        for i = 1, 6 do
+            local _, _, body = t('/server_port', ngx.HTTP_GET)
+            bodys[i] = 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 50: the upstream_id is used in the plugin
 --- config
     location /t {
         content_by_lua_block {
@@ -1665,7 +1786,7 @@ passed
 
 
 
-=== TEST 47: `match` rule passed(upstream_id)
+=== TEST 51: `match` rule passed(upstream_id)
 --- config
 location /t {
     content_by_lua_block {
@@ -1689,7 +1810,7 @@ GET /t
 
 
 
-=== TEST 48: only use upstream_id in the plugin
+=== TEST 52: only use upstream_id in the plugin
 --- config
     location /t {
         content_by_lua_block {
@@ -1738,7 +1859,7 @@ passed
 
 
 
-=== TEST 49: `match` rule passed(only use upstream_id)
+=== TEST 53: `match` rule passed(only use upstream_id)
 --- config
 location /t {
     content_by_lua_block {
@@ -1761,7 +1882,7 @@ GET /t
 
 
 
-=== TEST 50: use upstream and upstream_id in the plugin
+=== TEST 54: use upstream and upstream_id in the plugin
 --- config
     location /t {
         content_by_lua_block {
@@ -1811,7 +1932,7 @@ passed
 
 
 
-=== TEST 51: `match` rule passed(upstream + upstream_id)
+=== TEST 55: `match` rule passed(upstream + upstream_id)
 --- config
 location /t {
     content_by_lua_block {
@@ -1836,7 +1957,7 @@ GET /t
 
 
 
-=== TEST 52: set route + upstream (two upstream node: one healthy + one unhealthy)
+=== TEST 56: set route + upstream (two upstream node: one healthy + one unhealthy)
 --- config
     location /t {
         content_by_lua_block {
@@ -1911,7 +2032,7 @@ passed
 
 
 
-=== TEST 53: hit routes, ensure the checker is bound to the upstream
+=== TEST 57: hit routes, ensure the checker is bound to the upstream
 --- config
 location /t {
     content_by_lua_block {
@@ -1965,7 +2086,7 @@ qr/\([^)]+\) unhealthy .* for '.*'/
 
 
 
-=== TEST 54: set upstream(id: 1), by default retries count = number of nodes
+=== TEST 58: set upstream(id: 1), by default retries count = number of nodes
 --- config
     location /t {
         content_by_lua_block {
@@ -1997,7 +2118,7 @@ passed
 
 
 
-=== TEST 55: set route(id: 1, upstream_id: 1)
+=== TEST 59: set route(id: 1, upstream_id: 1)
 --- config
     location /t {
         content_by_lua_block {
@@ -2041,7 +2162,7 @@ passed
 
 
 
-=== TEST 56: hit routes
+=== TEST 60: hit routes
 --- request
 GET /hello
 --- error_code: 502