You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/07/07 02:02:44 UTC
[GitHub] [apisix] spacewander opened a new pull request #4549: feat: enable balancer phase for plugins
spacewander opened a new pull request #4549:
URL: https://github.com/apache/apisix/pull/4549
Signed-off-by: spacewander <sp...@gmail.com>
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#issuecomment-875787466
cc @dmsolr
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4549:
URL: https://github.com/apache/apisix/pull/4549
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#discussion_r669242229
##########
File path: apisix/balancer.lua
##########
@@ -308,15 +309,24 @@ function _M.run(route, ctx)
return core.response.exit(502)
end
+ plugin_funcs("balancer")
+
local pass_host = ctx.pass_host
if pass_host == "node" and balancer.recreate_request then
local host = server.domain or server.host
if host ~= ctx.var.upstream_host then
-- retried node has a different host
ctx.var.upstream_host = host
- balancer.recreate_request()
+ header_changed = true
end
end
+
+ end
+
+ local _, run = plugin_funcs("balancer")
Review comment:
Fixed in the new commit
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#discussion_r669267826
##########
File path: t/plugin/serverless.t
##########
@@ -679,3 +679,159 @@ GET /hello
--- error_log
default phase: access
match uri /hello
+
+
+
+=== TEST 23: run in the balancer phase
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.req.set_header('X-SERVERLESS', ctx.balancer_ip) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 24: check plugin
+--- request
+GET /log_request
+--- skip_nginx: 4: < 1.19.3
+--- grep_error_log eval
+qr/(proxy request to \S+|x-serverless: [\d.]+)/
+--- grep_error_log_out
+proxy request to 127.0.0.2:1979
+proxy request to 127.0.0.1:1980
+x-serverless: 127.0.0.1
+
+
+
+=== TEST 25: exit in the balancer phase
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.exit(403) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 26: check plugin
+--- request
+GET /log_request
+--- error_code: 403
+--- no_error_log
+[error]
+
+
+
+=== TEST 27: ensure balancer phase run correct time
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.log(ngx.WARN, 'run balancer phase with ', ctx.balancer_ip) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 28: check plugin
+--- request
+GET /log_request
+--- grep_error_log eval
+qr/(run balancer phase with [\d.]+)/
+--- grep_error_log_out
+run balancer phase with 127.0.0.2
Review comment:
The last one is for retry
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#discussion_r669230828
##########
File path: apisix/balancer.lua
##########
@@ -308,15 +309,24 @@ function _M.run(route, ctx)
return core.response.exit(502)
end
+ plugin_funcs("balancer")
+
local pass_host = ctx.pass_host
if pass_host == "node" and balancer.recreate_request then
local host = server.domain or server.host
if host ~= ctx.var.upstream_host then
-- retried node has a different host
ctx.var.upstream_host = host
- balancer.recreate_request()
+ header_changed = true
end
end
+
+ end
+
+ local _, run = plugin_funcs("balancer")
Review comment:
We will run balancer twice?
Line 312 and line 326 ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#discussion_r669261787
##########
File path: t/plugin/serverless.t
##########
@@ -679,3 +679,159 @@ GET /hello
--- error_log
default phase: access
match uri /hello
+
+
+
+=== TEST 23: run in the balancer phase
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.req.set_header('X-SERVERLESS', ctx.balancer_ip) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 24: check plugin
+--- request
+GET /log_request
+--- skip_nginx: 4: < 1.19.3
+--- grep_error_log eval
+qr/(proxy request to \S+|x-serverless: [\d.]+)/
+--- grep_error_log_out
+proxy request to 127.0.0.2:1979
+proxy request to 127.0.0.1:1980
+x-serverless: 127.0.0.1
+
+
+
+=== TEST 25: exit in the balancer phase
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.exit(403) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 26: check plugin
+--- request
+GET /log_request
+--- error_code: 403
+--- no_error_log
+[error]
+
+
+
+=== TEST 27: ensure balancer phase run correct time
+--- 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,
+ [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "balancer",
+ "functions" : ["return function(conf, ctx) ngx.log(ngx.WARN, 'run balancer phase with ', ctx.balancer_ip) end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.2:1979": 100000,
+ "127.0.0.1:1980": 1
+ },
+ "type": "chash",
+ "key": "remote_addr"
+ },
+ "uri": "/log_request"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 28: check plugin
+--- request
+GET /log_request
+--- grep_error_log eval
+qr/(run balancer phase with [\d.]+)/
+--- grep_error_log_out
+run balancer phase with 127.0.0.2
Review comment:
I don't understand why there are two `run balancer phases with ……`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #4549: feat: enable balancer phase for plugins
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #4549:
URL: https://github.com/apache/apisix/pull/4549#discussion_r669230828
##########
File path: apisix/balancer.lua
##########
@@ -308,15 +309,24 @@ function _M.run(route, ctx)
return core.response.exit(502)
end
+ plugin_funcs("balancer")
+
local pass_host = ctx.pass_host
if pass_host == "node" and balancer.recreate_request then
local host = server.domain or server.host
if host ~= ctx.var.upstream_host then
-- retried node has a different host
ctx.var.upstream_host = host
- balancer.recreate_request()
+ header_changed = true
end
end
+
+ end
+
+ local _, run = plugin_funcs("balancer")
Review comment:
We will return balancer twice?
Line 312 and line 326 ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org