nic-6443 commented on code in PR #13375:
URL: https://github.com/apache/apisix/pull/13375#discussion_r3393128718
##########
t/plugin/log-rotate.t:
##########
@@ -216,3 +222,116 @@ true
}
--- response_body
passed
+
+
+
+=== TEST 6: reopen plugin and access logs even if one configured log file is
missing
+# Log-phase plugins run after the response is sent to the client. Keep the
+# rotation setup, request trigger, and assertions in separate requests so the
+# previous request's log-phase output is flushed before checking the log files.
+--- timeout: 30
+--- config
+ location /setup {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local prefix = ngx.config.prefix()
+ local access_log = prefix .. "/logs/access.log"
+ local function fail(msg)
+ ngx.status = 500
+ ngx.say(msg)
+ ngx.exit(ngx.HTTP_OK)
+ end
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "serverless-post-function": {
+ "phase": "log",
+ "functions" : ["return function(conf, ctx)
require('apisix.core').log.info('serverless post-rotation marker') end"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ fail(body)
+ end
+
+ os.remove(access_log)
+
+ ngx.sleep(2.5)
+
+ local data = [[
+apisix:
+ node_listen: 1984
+ admin_key: null
+plugins:
+ - serverless-post-function
+nginx_config:
+ error_log_level: info
+ http:
+ access_log_buffer: 1
+ ]]
+ require("lib.test_admin").set_config_yaml(data)
+ code, _, body = t('/apisix/admin/plugins/reload',
Review Comment:
The fix itself looks right to me, but I think this disable step can flake on
slow runners: `plugins/reload` just posts an event and returns
(`post_reload_plugins` in `admin/init.lua`), it does not wait for the
privileged agent — where the log-rotate timer actually runs — to process the
event and unregister the timer. With `interval: 1`, a late rotation tick can
still fire after `/setup` returns; if it lands between `/hello`'s log-phase
write and `/verify`'s read (that window spans a 1s boundary thanks to the 0.5s
sleep), both logs get rotated away to fresh empty files and `/verify` fails on
the missing marker. TEST 4 in this file tolerates exactly this uncertainty with
a `<= 1` assertion. Maybe make `/setup` wait until rotation has provably
stopped before returning, e.g. record the set of `__error.log` files after the
reload, sleep just past one interval, and loop until no new rotated file shows
up.
##########
t/plugin/log-rotate.t:
##########
@@ -28,6 +28,12 @@ add_block_preprocessor(sub {
my $extra_yaml_config = <<_EOC_;
plugins: # plugin list
- log-rotate
+ - serverless-post-function
+
+nginx_config:
Review Comment:
These two `nginx_config` entries do not actually take effect in tests: the
test server's nginx.conf is generated by `t/APISIX.pm`, which hardcodes an
unbuffered `access_log logs/access.log main;` and already sets
`log_level('info')`. Nothing at runtime reads `error_log_level` /
`access_log_buffer` from config.yaml either — they are only consumed by the CLI
template, which the test framework bypasses. The test works because the
framework's access log is unbuffered to begin with. I would drop them (here and
from the `set_config_yaml` payload in `/setup`) so they do not read as
load-bearing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
