-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add new lua-resty-events as events implementation #10550
Changes from 38 commits
7aea90c
692feb8
a1d9c28
11d35ed
bbb2fc1
df17f01
ecee33b
dcdb282
3ca211f
0ee5494
06701ea
827ff8c
14c0117
10999fa
76dc27b
1f68994
2cc1fbc
ee50b1a
bd8af2f
7cf568b
b7549bc
9e2e58f
6ed090a
1f3e632
db2da3a
af3b366
6ffc4ab
8a93fed
4c9b9b9
7645c38
4e9725f
a4141d8
4a1424b
0e97a95
b215d35
ec6ffc7
87ede50
b772ff3
8955d39
fa97305
ac50b44
a3cdf11
7119cb1
2c20df4
8b4282e
b45bb9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,12 +37,18 @@ install_dependencies() { | |
yum install -y openresty-openssl111 openresty-openssl111-devel pcre pcre pcre-devel xz | ||
yum -y install https://repos.apiseven.com/packages/centos/apache-apisix-repo-1.0-1.noarch.rpm | ||
|
||
wget "https://raw.githubusercontent.com/api7/apisix-build-tools/apisix-runtime/${APISIX_RUNTIME}/build-apisix-runtime-debug-centos7.sh" | ||
wget "https://raw.githubusercontent.com/api7/apisix-build-tools/apisix-runtime/${APISIX_RUNTIME}/build-apisix-runtime.sh" | ||
# TODO: disabled temporarily, waiting for APISIX 3.8 to be released to synchronize the apisix-runtime version | ||
#wget "https://raw.githubusercontent.com/api7/apisix-build-tools/apisix-runtime/${APISIX_RUNTIME}/build-apisix-runtime-debug-centos7.sh" | ||
#wget "https://raw.githubusercontent.com/api7/apisix-build-tools/apisix-runtime/${APISIX_RUNTIME}/build-apisix-runtime.sh" | ||
wget "https://raw.githubusercontent.com/api7/apisix-build-tools/master/build-apisix-runtime-debug-centos7.sh" | ||
wget "https://raw.githubusercontent.com/api7/apisix-build-tools/master/build-apisix-runtime.sh" | ||
chmod +x build-apisix-runtime.sh | ||
chmod +x build-apisix-runtime-debug-centos7.sh | ||
./build-apisix-runtime-debug-centos7.sh | ||
|
||
# patch lua-resty-events | ||
sed -i 's/log(ERR, "event worker failed: ", perr)/log(ngx.WARN, "event worker failed: ", perr)/' /usr/local/openresty/lualib/resty/events/worker.lua | ||
|
||
# install luarocks | ||
./utils/linux-install-luarocks.sh | ||
|
||
|
@@ -94,7 +100,7 @@ run_case() { | |
make init | ||
set_coredns | ||
# run test cases | ||
FLUSH_ETCD=1 prove --timer -Itest-nginx/lib -I./ -r ${TEST_FILE_SUB_DIR} | tee /tmp/test.result | ||
FLUSH_ETCD=1 TEST_EVENTS_MODULE=lua-resty-worker-events prove --timer -Itest-nginx/lib -I./ -r ${TEST_FILE_SUB_DIR} | tee /tmp/test.result | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don’t quite understand this part. I see that the code already uses lua-resty-event as the default event library. Why do I need to specify this variable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First, this is the same logic executed in the other test scripts, with the difference that under ubuntu we are executing the tests for both event modules at the same time, where an environment variable controlled by GitHub workflow is used as input. You can see the header of some of the test cases, some of the tests are related to the events library and because of the implementation of the library they have slightly different behaviors, such as proactive health checks, which use different test cases under different events modules. If an events module is not explicitly configured, it will default to the value in config-default, which is lua-resty-events, so simultaneous testing is not possible. But it looks like some of the logic can be ported to APISIX.pm, and I'll be making that change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to lua-resty-events as the default test events module |
||
rerun_flaky_tests /tmp/test.result | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -413,6 +413,11 @@ _EOC_ | |
|
||
require "resty.core" | ||
|
||
local events_sock_path = "$apisix_home/t/servroot/logs/stream_worker_events.sock" | ||
if os.getenv("TEST_NGINX_USE_HUP") ~= "1" then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the test case using the HUP signal need to remove the sock, and other cases do not need to remove the unix sock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Theoretically they will process properly, I can try. Maybe it's not necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed, it can work. |
||
pcall(os.remove, events_sock_path) | ||
end | ||
|
||
$stream_extra_init_by_lua_start | ||
|
||
apisix = require("apisix") | ||
|
@@ -437,6 +442,14 @@ _EOC_ | |
|
||
$extra_stream_config | ||
|
||
server { | ||
listen unix:$apisix_home/t/servroot/logs/stream_worker_events.sock; | ||
access_log off; | ||
content_by_lua_block { | ||
require("resty.events.compat").run() | ||
} | ||
} | ||
|
||
# fake server, only for test | ||
server { | ||
listen 1995; | ||
|
@@ -516,6 +529,11 @@ _EOC_ | |
|
||
require "resty.core" | ||
|
||
local events_sock_path = "$apisix_home/t/servroot/logs/worker_events.sock" | ||
if os.getenv("TEST_NGINX_USE_HUP") ~= "1" then | ||
pcall(os.remove, events_sock_path) | ||
end | ||
|
||
$extra_init_by_lua_start | ||
|
||
apisix = require("apisix") | ||
|
@@ -687,6 +705,18 @@ _EOC_ | |
} | ||
} | ||
|
||
_EOC_ | ||
|
||
$http_config .= <<_EOC_; | ||
server { | ||
listen unix:$apisix_home/t/servroot/logs/worker_events.sock; | ||
access_log off; | ||
location / { | ||
content_by_lua_block { | ||
require("resty.events.compat").run() | ||
} | ||
} | ||
} | ||
_EOC_ | ||
|
||
$block->set_value("http_config", $http_config); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why update this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is impossible to execute a reload without a transient interruption because the worker that was handling the unix socket connection is being restarted, and the other workers will not be able to connect to the broker over the unix socket again until the new worker that replaces it is up and running, so the logs will be some error level logs about connection closed, which will interrupt our tests, so we'll have to change the code here to warn level.
There is no need to apply this patch in production as it has no impact other than causing some error logs, when the worker is available, the worker and broker will reconnect, and event push will be available again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok.