spacewander commented on a change in pull request #3955:
URL: https://github.com/apache/apisix/pull/3955#discussion_r604679235
##########
File path: docs/zh/latest/plugins/traffic-split.md
##########
@@ -47,7 +47,7 @@ traffic-split 插件使用户可以逐步引导各个上游之间的流量百分
| rules.weighted_upstreams | array[object] | 可选 | | |
上游配置规则列表。 |
| weighted_upstreams.upstream_id | string / integer | 可选 | | |
通过上游 id 绑定对应上游。 |
| weighted_upstreams.upstream | object | 可选 | | | 上游配置信息。 |
-| upstream.type | enum | 可选 | roundrobin |
[roundrobin, chash] | roundrobin 支持权重的负载，chash 一致性哈希，两者是二选一的(目前只支持
`roundrobin`)。 |
+| upstream.type | enum | 可选 | roundrobin |
[roundrobin, chash] | roundrobin 支持权重的负载，chash 一致性哈希，两者是二选一。 |
Review comment:
Can we add a separate passage for the features that haven't been
implemented yet, like retry and service discovery?
By the way, we should also update the English version.
##########
File path: t/plugin/traffic-split2.t
##########
@@ -311,3 +311,93 @@ host: localhost
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 10: the upstream.type is `chash` and `key` is header
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ [=[{
+ "uri": "/server_port",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "chash_test",
+ "type": "chash",
+ "hash_on": "header",
+ "key": "custom_header",
+ "nodes": {
+ "127.0.0.1:1981":1,
+ "127.0.0.1:1982":1
+ }
+ },
+ "weight": 1
+ }
+ ]
+ }
+ ]
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+ }]=]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
Review comment:
There is no need to set `--- request`, we already set it in the
add_block_preprocessor at the beginning of this file.
##########
File path: apisix/plugins/traffic-split.lua
##########
@@ -200,6 +200,11 @@ local function set_upstream(upstream_info, ctx)
}
}
+ if upstream_info.type == "chash" then
+ up_conf.hash_on = upstream_info.hash_on
+ up_conf.key = upstream_info.key
Review comment:
We can directly copy those fields without checking their type.
##########
File path: t/plugin/traffic-split2.t
##########
@@ -311,3 +311,93 @@ host: localhost
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 10: the upstream.type is `chash` and `key` is header
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ [=[{
+ "uri": "/server_port",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "chash_test",
+ "type": "chash",
+ "hash_on": "header",
+ "key": "custom_header",
+ "nodes": {
+ "127.0.0.1:1981":1,
+ "127.0.0.1:1982":1
+ }
+ },
+ "weight": 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 11: hit routes, hash_on custom header
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["custom_header"] = "custom-one"
+ for i = 1, 4 do
+ local _, _, body = t('/server_port', ngx.HTTP_GET, "", nil,
headers)
+ bodys[i] = body
+ end
+
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body eval
+qr/1981, 1981, 1981, 1981|1982, 1982, 1982, 1982/
Review comment:
Since we only use `custom-one` as the key, in what situation 1982 will
be used?
##########
File path: t/plugin/traffic-split2.t
##########
@@ -311,3 +311,93 @@ host: localhost
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 10: the upstream.type is `chash` and `key` is header
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ [=[{
+ "uri": "/server_port",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "chash_test",
+ "type": "chash",
+ "hash_on": "header",
+ "key": "custom_header",
+ "nodes": {
+ "127.0.0.1:1981":1,
+ "127.0.0.1:1982":1
+ }
+ },
+ "weight": 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
Review comment:
Ditto
##########
File path: t/plugin/traffic-split2.t
##########
@@ -311,3 +311,93 @@ host: localhost
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 10: the upstream.type is `chash` and `key` is header
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PATCH,
+ [=[{
+ "uri": "/server_port",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "chash_test",
+ "type": "chash",
+ "hash_on": "header",
+ "key": "custom_header",
+ "nodes": {
+ "127.0.0.1:1981":1,
+ "127.0.0.1:1982":1
+ }
+ },
+ "weight": 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 11: hit routes, hash_on custom header
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["custom_header"] = "custom-one"
+ for i = 1, 4 do
+ local _, _, body = t('/server_port', ngx.HTTP_GET, "", nil,
headers)
+ bodys[i] = body
+ end
+
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body eval
+qr/1981, 1981, 1981, 1981|1982, 1982, 1982, 1982/
Review comment:
Since we only use `custom-one` as the key, in what situation 1982 will
be used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
