tokers commented on a change in pull request #3512:
URL: https://github.com/apache/apisix/pull/3512#discussion_r579896320
##########
File path: apisix/plugins/traffic-split.lua
##########
@@ -97,7 +97,7 @@ local upstreams_schema = {
items = {
type = "object",
properties = {
- upstream_id = schema_def.id_schema, -- todo: support
upstream_id method
+ upstream_id = schema_def.id_schema,
Review comment:
What about making `upstream_id` and `upstream` exclusive in the schema?
##########
File path: doc/plugins/traffic-split.md
##########
@@ -111,6 +113,40 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H
'X-API-KEY: edd1c9f034335f13
}'
```
+2. Use the `upstream_id` attribute in the plugin to bind upstream.
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "uri": "/index.html",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream_id": 1,
+ "weight": 1
+ },
+ {
+ "weight": 1
+ }
+ ]
+ }
+ ]
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+}'
+```
+
+>Note: **1.** Use the `upstream_id` to bind the defined upstream, it can reuse
upstream health detection, retry and other functions. **2.** Support the two
configuration methods of `upstream` and `upstream_id` to be used together.
Review comment:
It seems that you don't refer to any external literatures, so the `>`
symbol is not necessary.
##########
File path: t/plugin/traffic-split.t
##########
@@ -1550,3 +1550,508 @@ GET /t
1980, 1981, 1981, 1982, 1982
--- no_error_log
[error]
+
+
+
+=== TEST 44: set upstream(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1981": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 45: set upstream(id: 2)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/2',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1982": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 46: the upstream_id is used in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 2},
+ {"upstream_id": 2, "weight": 1},
+ {"weight": 2}
+ ]
+ }
+ ]
+ }
+ },
+ "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 47: `match` rule passed(upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
+ for i = 1, 5 do
+ local _, _, body = t('/server_port?x-api-name=jack', ngx.HTTP_GET,
"", nil, headers)
+ bodys[i] = body
+ end
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body
+1980, 1980, 1981, 1981, 1982
+--- no_error_log
+[error]
+
+
+
+=== TEST 48: only use upstream_id in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 1},
+ {"upstream_id": 2, "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 49: `match` rule passed(only use upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
+ for i = 1, 4 do
+ local _, _, body = t('/server_port?x-api-name=jack', ngx.HTTP_GET,
"", nil, headers)
+ bodys[i] = body
+ end
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body
+1981, 1981, 1982, 1982
+--- no_error_log
+[error]
+
+
+
+=== TEST 50: use upstream and upstream_id in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 2},
+ {"upstream": {"type": "roundrobin",
"nodes": [{"host":"127.0.0.1", "port":1982, "weight": 1}]}, "weight": 1},
+ {"weight": 2}
+ ]
+ }
+ ]
+ }
+ },
+ "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 51: `match` rule passed(upstream + upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
+ for i = 1, 5 do
+ local _, _, body = t('/server_port?x-api-name=jack', ngx.HTTP_GET,
"", nil, headers)
+ bodys[i] = body
+ end
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body
+1980, 1980, 1981, 1981, 1982
+--- no_error_log
+[error]
+
+
+
+=== TEST 52: set route + upstream (two upstream node: one healthy + one
unhealthy)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1981": 1,
+ "127.0.0.1:1970": 1
+ },
+ "checks": {
+ "active": {
+ "http_path": "/status",
+ "host": "foo.com",
+ "healthy": {
+ "interval": 1,
+ "successes": 1
+ },
+ "unhealthy": {
+ "interval": 1,
+ "http_failures": 2
+ }
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [=[{
+ "uri": "/server_port",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {"upstream_id": 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
+--- grep_error_log eval
+qr/^.*?\[error\](?!.*process exiting).*/
+--- grep_error_log_out
Review comment:
Empty section?
##########
File path: t/plugin/traffic-split.t
##########
@@ -1550,3 +1550,508 @@ GET /t
1980, 1981, 1981, 1982, 1982
--- no_error_log
[error]
+
+
+
+=== TEST 44: set upstream(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1981": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 45: set upstream(id: 2)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/2',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1982": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 46: the upstream_id is used in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 2},
+ {"upstream_id": 2, "weight": 1},
+ {"weight": 2}
+ ]
+ }
+ ]
+ }
+ },
+ "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 47: `match` rule passed(upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
Review comment:
It seems this header was not used.
##########
File path: doc/zh-cn/plugins/traffic-split.md
##########
@@ -111,6 +113,40 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H
'X-API-KEY: edd1c9f034335f13
}'
```
+2、通过插件中的 `upstream_id` 属性绑定上游服务。
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+ "uri": "/index.html",
+ "plugins": {
+ "traffic-split": {
+ "rules": [
+ {
+ "weighted_upstreams": [
+ {
+ "upstream_id": 1,
+ "weight": 1
+ },
+ {
+ "weight": 1
+ }
+ ]
+ }
+ ]
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+}'
+```
+
+>注：1、通过 `upstream_id` 方式来绑定已定义的上游，它可以复用上游具有的健康检测、重试等功能。2、支持 `upstream` 和
`upstream_id` 的两种配置方式一起使用。
Review comment:
Ditto.
##########
File path: apisix/init.lua
##########
@@ -409,6 +409,33 @@ function _M.http_access_phase()
api_ctx.route_id = route.value.id
api_ctx.route_name = route.value.name
+ if route.value.script then
Review comment:
Got it, but be sure no plugins depend on the upstream values or your
change will break these plugins.
##########
File path: t/plugin/traffic-split.t
##########
@@ -1550,3 +1550,508 @@ GET /t
1980, 1981, 1981, 1982, 1982
--- no_error_log
[error]
+
+
+
+=== TEST 44: set upstream(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1981": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 45: set upstream(id: 2)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/2',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1982": 1
+ },
+ "type": "roundrobin",
+ "desc": "new upstream"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 46: the upstream_id is used in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 2},
+ {"upstream_id": 2, "weight": 1},
+ {"weight": 2}
+ ]
+ }
+ ]
+ }
+ },
+ "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 47: `match` rule passed(upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
+ for i = 1, 5 do
+ local _, _, body = t('/server_port?x-api-name=jack', ngx.HTTP_GET,
"", nil, headers)
+ bodys[i] = body
+ end
+ table.sort(bodys)
+ ngx.say(table.concat(bodys, ", "))
+ }
+}
+--- request
+GET /t
+--- response_body
+1980, 1980, 1981, 1981, 1982
+--- no_error_log
+[error]
+
+
+
+=== TEST 48: only use upstream_id in the plugin
+--- 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_x-api-name", "==",
"jack"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {"upstream_id": 1, "weight": 1},
+ {"upstream_id": 2, "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 49: `match` rule passed(only use upstream_id)
+--- config
+location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local bodys = {}
+ local headers = {}
+ headers["x-api-appkey"] = "hello"
Review comment:
Where does this header use?
----------------------------------------------------------------
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:
us...@infra.apache.org
