fix(traffix-split): multiple rules with multiple weighted_upstreams under each rule cause upstream_key duplicate

apisix/plugins/traffic-split.lua

@@ -24,6 +24,7 @@ local pairs      = pairs
 local ipairs     = ipairs
 local type       = type
 local table_insert = table.insert
+local tostring   = tostring
 
 local lrucache = core.lrucache.new({
     ttl = 0, count = 512
@@ -187,7 +188,10 @@ local function set_upstream(upstream_info, ctx)
     local matched_route = ctx.matched_route
     up_conf.parent = matched_route
     local upstream_key = up_conf.type .. "#route_" ..
-                         matched_route.value.id .. "_" ..upstream_info.vid
+                         matched_route.value.id .. "_" .. upstream_info.vid
+    if upstream_info.node_tid then
+        upstream_key = upstream_key .. "_" .. upstream_info.node_tid
+    end
     core.log.info("upstream_key: ", upstream_key)
     upstream.set(ctx, upstream_key, ctx.conf_version, up_conf)
 
@@ -203,6 +207,12 @@ local function new_rr_obj(weighted_upstreams)
         elseif upstream_obj.upstream then
             -- Add a virtual id field to uniquely identify the upstream key.
             upstream_obj.upstream.vid = i
+            -- Get the table id of the nodes as part of the upstream_key,
+            -- avoid upstream_key duplicate because vid is the same in the loop
+            -- when multiple rules with multiple weighted_upstreams under each rule.
+            -- see https://github.com/apache/apisix/issues/5276
+            local node_tid = tostring(upstream_obj.upstream.nodes):sub(#"table: " + 1)
+            upstream_obj.upstream.node_tid = node_tid
             server_list[upstream_obj.upstream] = upstream_obj.weight
         else
             -- If the upstream object has only the weight value, it means

t/plugin/traffic-split5.t

@@ -0,0 +1,313 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('info');
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->error_log && !$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]");
+    }
+
+    my $http_config = $block->http_config // <<_EOC_;
+    # fake server, only for test
+    server {
+        listen 1970;
+        location / {
+            content_by_lua_block {
+                ngx.say(1970)
+            }
+        }
+    }
+
+    server {
+        listen 1971;
+        location / {
+            content_by_lua_block {
+                ngx.say(1971)
+            }
+        }
+    }
+
+    server {
+        listen 1972;
+        location / {
+            content_by_lua_block {
+                ngx.say(1972)
+            }
+        }
+    }
+
+    server {
+        listen 1973;
+        location / {
+            content_by_lua_block {
+                ngx.say(1973)
+            }
+        }
+    }
+
+    server {
+        listen 1974;
+        location / {
+            content_by_lua_block {
+                ngx.say(1974)
+            }
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set upstream(multiple rules, multiple nodes under each weighted_upstreams) and add route
+--- config
+    location /t {
+        content_by_lua_block {
+            local json = require("toolkit.json")
+            local t = require("lib.test_admin").test
+            local data = {
+                uri = "/hello",
+                plugins = {
+                    ["traffic-split"] = {
+                        rules = {
+                            {
+                                match = { {
+                                    vars = { { "arg_id", "==", "1" } }
+                                } },
+                                weighted_upstreams = {
+                                    {
+                                        upstream = {
+                                            name = "upstream_A",
+                                            type = "roundrobin",
+                                            nodes = {
+                                                ["127.0.0.1:1970"] = 1,
+                                                ["127.0.0.1:1971"] = 1
+                                            }
+                                        },
+                                        weight = 1
+                                    }
+                               }
+                            },
+                            {
+                                match = { {
+                                    vars = { { "arg_id", "==", "2" } }
+                                } },
+                                weighted_upstreams = {
+                                    {
+                                        upstream = {
+                                            name = "upstream_B",
+                                            type = "roundrobin",
+                                            nodes = {
+                                                ["127.0.0.1:1972"] = 1,
+                                                ["127.0.0.1:1973"] = 1
+                                            }
+                                        },
+                                        weight = 1
+                                    }
+                               }
+                            }
+                        }
+                    }
+                },
+                upstream = {
+                    type = "roundrobin",
+                    nodes = {
+                        ["127.0.0.1:1974"] = 1
+                    }
+                }
+            }
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                json.encode(data)
+            )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: hit different weighted_upstreams by rules
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+            local res, err = httpc:request_uri(uri)
+            local port = tonumber(res.body)
+            if port ~= 1974 then
+                ngx.status = ngx.HTTP_INTERNAL_SERVER_ERROR
+                ngx.say("failed while no arg_id")
+                return
+            end
+
+            uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello?id=1"
+            res, err = httpc:request_uri(uri)
+            port = tonumber(res.body)
+            if port ~= 1970 and port ~= 1971 then
+                ngx.status = ngx.HTTP_INTERNAL_SERVER_ERROR
+                ngx.say("failed while arg_id = 1")
+                return
+            end
+
+            uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello?id=2"
+            res, err = httpc:request_uri(uri)
+            port = tonumber(res.body)
+            if port ~= 1972 and port ~= 1973 then
+                ngx.status = ngx.HTTP_INTERNAL_SERVER_ERROR
+                ngx.say("failed while arg_id = 2")
+                return
+            end
+
+            ngx.say("passed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: set upstream(multiple rules, multiple nodes with different weight under each weighted_upstreams) and add route
+--- config
+    location /t {
+        content_by_lua_block {
+            local json = require("toolkit.json")
+            local t = require("lib.test_admin").test
+            local data = {
+                uri = "/hello",
+                plugins = {
+                    ["traffic-split"] = {
+                        rules = {
+                            {
+                                match = { {
+                                    vars = { { "arg_id", "==", "1" } }
+                                } },
+                                weighted_upstreams = {
+                                    {
+                                        upstream = {
+                                            name = "upstream_A",
+                                            type = "roundrobin",
+                                            nodes = {
+                                                ["127.0.0.1:1970"] = 2,
+                                                ["127.0.0.1:1971"] = 1
+                                            }
+                                        },
+                                        weight = 1
+                                    }
+                               }
+                            },
+                            {
+                                match = { {
+                                    vars = { { "arg_id", "==", "2" } }
+                                } },
+                                weighted_upstreams = {
+                                    {
+                                        upstream = {
+                                            name = "upstream_B",
+                                            type = "roundrobin",
+                                            nodes = {
+                                                ["127.0.0.1:1972"] = 2,
+                                                ["127.0.0.1:1973"] = 1
+                                            }
+                                        },
+                                        weight = 1
+                                    }
+                               }
+                            }
+                        }
+                    }
+                },
+                upstream = {
+                    type = "roundrobin",
+                    nodes = {
+                        ["127.0.0.1:1974"] = 1
+                    }
+                }
+            }
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                json.encode(data)
+            )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: pick different nodes by weight
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello?id=1"
+            local ports = {}
+            local res, err
+            for i = 1, 3 do
+                res, err = httpc:request_uri(uri)
+                local port = tonumber(res.body)
+                ports[i] = port
+            end
+            table.sort(ports)
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello?id=2"
+            for i = 4, 6 do
+                res, err = httpc:request_uri(uri)
+                local port = tonumber(res.body)
+                ports[i] = port
+            end
+            table.sort(ports)
+
+            ngx.say(table.concat(ports, ", "))
+        }
+    }
+--- response_body
+1970, 1970, 1971, 1972, 1972, 1973