You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by sp...@apache.org on 2021/06/11 05:04:49 UTC
[apisix] branch master updated: feat(ext-plugin): stop the runner
with SIGTERM (#4367)
This is an automated email from the ASF dual-hosted git repository.
spacewander pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new 68711e4 feat(ext-plugin): stop the runner with SIGTERM (#4367)
68711e4 is described below
commit 68711e443dbeae90a6171d000506f3ee2041b2b3
Author: 罗泽轩 <sp...@gmail.com>
AuthorDate: Fri Jun 11 13:04:42 2021 +0800
feat(ext-plugin): stop the runner with SIGTERM (#4367)
---
apisix/cli/ngx_tpl.lua | 6 +++
apisix/cli/ops.lua | 10 +++-
apisix/core/os.lua | 34 +++++++++++++
apisix/init.lua | 5 ++
apisix/plugins/ext-plugin/init.lua | 30 +++++++++--
docs/en/latest/external-plugin.md | 11 ++++
t/APISIX.pm | 10 ++++
t/plugin/ext-plugin/runner_can_not_terminated.sh | 23 +++++++++
t/plugin/ext-plugin/sanity-openresty-1-19.t | 65 ++++++++++++++++++++++++
t/plugin/ext-plugin/sanity.t | 10 ++++
10 files changed, 198 insertions(+), 6 deletions(-)
diff --git a/apisix/cli/ngx_tpl.lua b/apisix/cli/ngx_tpl.lua
index f237648..eeb36f4 100644
--- a/apisix/cli/ngx_tpl.lua
+++ b/apisix/cli/ngx_tpl.lua
@@ -291,6 +291,12 @@ http {
apisix.http_init_worker()
}
+ {% if not use_openresty_1_17 then %}
+ exit_worker_by_lua_block {
+ apisix.http_exit_worker()
+ }
+ {% end %}
+
{% if enable_control then %}
server {
listen {* control_server_addr *};
diff --git a/apisix/cli/ops.lua b/apisix/cli/ops.lua
index 83229b4..8f8716b 100644
--- a/apisix/cli/ops.lua
+++ b/apisix/cli/ops.lua
@@ -65,7 +65,7 @@ version: print the version of apisix
end
-local function check_version(cur_ver_s, need_ver_s)
+local function version_greater_equal(cur_ver_s, need_ver_s)
local cur_vers = util.split(cur_ver_s, [[.]])
local need_vers = util.split(need_ver_s, [[.]])
local len = max(#cur_vers, #need_vers)
@@ -357,10 +357,15 @@ Please modify "admin_key" in conf/config.yaml .
end
local need_ver = "1.17.3"
- if not check_version(or_ver, need_ver) then
+ if not version_greater_equal(or_ver, need_ver) then
util.die("openresty version must >=", need_ver, " current ", or_ver, "\n")
end
+ local use_openresty_1_17 = false
+ if not version_greater_equal(or_ver, "1.19.3") then
+ use_openresty_1_17 = true
+ end
+
local or_info = util.execute_cmd("openresty -V 2>&1")
local with_module_status = true
if or_info and not or_info:find("http_stub_status_module", 1, true) then
@@ -446,6 +451,7 @@ Please modify "admin_key" in conf/config.yaml .
-- Using template.render
local sys_conf = {
+ use_openresty_1_17 = use_openresty_1_17,
lua_path = env.pkg_path_org,
lua_cpath = env.pkg_cpath_org,
os_name = util.trim(util.execute_cmd("uname")),
diff --git a/apisix/core/os.lua b/apisix/core/os.lua
index b8476ec..0edc319 100644
--- a/apisix/core/os.lua
+++ b/apisix/core/os.lua
@@ -23,11 +23,18 @@ local type = type
local _M = {}
+local WNOHANG = 1
ffi.cdef[[
+ typedef int32_t pid_t;
+ typedef unsigned int useconds_t;
+
int setenv(const char *name, const char *value, int overwrite);
char *strerror(int errnum);
+
+ int usleep(useconds_t usec);
+ pid_t waitpid(pid_t pid, int *wstatus, int options);
]]
@@ -52,4 +59,31 @@ function _M.setenv(name, value)
end
+local function waitpid_nohang(pid)
+ local res = C.waitpid(pid, nil, WNOHANG)
+ if res == -1 then
+ return nil, err()
+ end
+ return res > 0
+end
+
+
+function _M.waitpid(pid, timeout)
+ local count = 0
+ local step = 1000 * 10
+ local total = timeout * 1000 * 1000
+ while step * count < total do
+ count = count + 1
+ C.usleep(step)
+ local ok, err = waitpid_nohang(pid)
+ if err then
+ return nil, err
+ end
+ if ok then
+ return true
+ end
+ end
+end
+
+
return _M
diff --git a/apisix/init.lua b/apisix/init.lua
index d325bdb..f3b53f0 100644
--- a/apisix/init.lua
+++ b/apisix/init.lua
@@ -134,6 +134,11 @@ function _M.http_init_worker()
end
+function _M.http_exit_worker()
+ require("apisix.plugins.ext-plugin.init").exit_worker()
+end
+
+
function _M.http_ssl_phase()
local ngx_ctx = ngx.ctx
local api_ctx = ngx_ctx.api_ctx
diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua
index 53e1d1c..4c263cd 100644
--- a/apisix/plugins/ext-plugin/init.lua
+++ b/apisix/plugins/ext-plugin/init.lua
@@ -36,6 +36,7 @@ if is_http then
ngx_pipe = require("ngx.pipe")
events = require("resty.worker.events")
end
+local resty_signal = require "resty.signal"
local bit = require("bit")
local band = bit.band
local lshift = bit.lshift
@@ -588,8 +589,10 @@ local function spawn_proc(cmd)
end
+local runner
local function setup_runner(cmd)
- local proc = spawn_proc(cmd)
+ runner = spawn_proc(cmd)
+
ngx_timer_at(0, function(premature)
if premature then
return
@@ -599,7 +602,7 @@ local function setup_runner(cmd)
while true do
-- drain output
local max = 3800 -- smaller than Nginx error log length limit
- local data, err = proc:stdout_read_any(max)
+ local data, err = runner:stdout_read_any(max)
if not data then
if exiting() then
return
@@ -615,11 +618,13 @@ local function setup_runner(cmd)
end
end
- local ok, reason, status = proc:wait()
+ local ok, reason, status = runner:wait()
if not ok then
core.log.warn("runner exited with reason: ", reason, ", status: ", status)
end
+ runner = nil
+
local ok, err = events.post(events_list._source, events_list.runner_exit)
if not ok then
core.log.error("post event failure with ", events_list._source, ", error: ", err)
@@ -628,7 +633,7 @@ local function setup_runner(cmd)
core.log.warn("respawn runner 3 seconds later with cmd: ", core.json.encode(cmd))
core.utils.sleep(3)
core.log.warn("respawning new runner...")
- proc = spawn_proc(cmd)
+ runner = spawn_proc(cmd)
end
end)
end
@@ -656,4 +661,21 @@ function _M.init_worker()
end
+function _M.exit_worker()
+ if process.type() == "privileged agent" and runner then
+ -- We need to send SIGTERM in the exit_worker phase, as:
+ -- 1. privileged agent doesn't support graceful exiting when I write this
+ -- 2. better to make it work without graceful exiting
+ local pid = runner:pid()
+ core.log.notice("terminate runner ", pid, " with SIGTERM")
+ local num = resty_signal.signum("TERM")
+ runner:kill(num)
+
+ -- give 1s to clean up the mess
+ core.os.waitpid(pid, 1)
+ -- then we KILL it via gc finalizer
+ end
+end
+
+
return _M
diff --git a/docs/en/latest/external-plugin.md b/docs/en/latest/external-plugin.md
index 318e38e..6312055 100644
--- a/docs/en/latest/external-plugin.md
+++ b/docs/en/latest/external-plugin.md
@@ -103,3 +103,14 @@ nginx_config:
envs:
- MY_ENV_VAR
```
+
+### APISIX terminates my runner with SIGKILL but not SIGTERM!
+
+Since `v2.7`, APISIX will stop the runner with SIGTERM when it is running on
+OpenResty 1.19+.
+
+However, APISIX needs to wait the runner to quit so that we can ensure the resource
+for the process group is freed.
+
+Therefore, we send SIGTERM first. And then after 1 second, if the runner is still
+running, we will send SIGKILL.
diff --git a/t/APISIX.pm b/t/APISIX.pm
index e1cd16b..f8bd235 100644
--- a/t/APISIX.pm
+++ b/t/APISIX.pm
@@ -402,7 +402,17 @@ _EOC_
require("apisix").http_init_worker()
$extra_init_worker_by_lua
}
+_EOC_
+ if ($version !~ m/\/1.17.8/) {
+ $http_config .= <<_EOC_;
+ exit_worker_by_lua_block {
+ require("apisix").http_exit_worker()
+ }
+_EOC_
+ }
+
+ $http_config .= <<_EOC_;
log_format main escape=default '\$remote_addr - \$remote_user [\$time_local] \$http_host "\$request" \$status \$body_bytes_sent \$request_time "\$http_referer" "\$http_user_agent" \$upstream_addr \$upstream_status \$upstream_response_time "\$upstream_scheme://\$upstream_host\$upstream_uri"';
# fake server, only for test
diff --git a/t/plugin/ext-plugin/runner_can_not_terminated.sh b/t/plugin/ext-plugin/runner_can_not_terminated.sh
new file mode 100755
index 0000000..f58956c
--- /dev/null
+++ b/t/plugin/ext-plugin/runner_can_not_terminated.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+term() {
+ eval sleep 1800
+}
+trap term SIGTERM
+sleep 1800
diff --git a/t/plugin/ext-plugin/sanity-openresty-1-19.t b/t/plugin/ext-plugin/sanity-openresty-1-19.t
new file mode 100644
index 0000000..c33d86e
--- /dev/null
+++ b/t/plugin/ext-plugin/sanity-openresty-1-19.t
@@ -0,0 +1,65 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX;
+
+my $nginx_binary = $ENV{'TEST_NGINX_BINARY'} || 'nginx';
+my $version = eval { `$nginx_binary -V 2>&1` };
+
+if ($version =~ m/\/1.17.8/) {
+ plan(skip_all => "require OpenResty 1.19+");
+} else {
+ plan('no_plan');
+}
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ my $cmd = $block->ext_plugin_cmd // "['sleep', '5s']";
+ my $extra_yaml_config = <<_EOC_;
+ext-plugin:
+ cmd: $cmd
+_EOC_
+ $block->set_value("extra_yaml_config", $extra_yaml_config);
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: terminate spawn runner
+--- ext_plugin_cmd
+["t/plugin/ext-plugin/runner.sh", "3600"]
+--- config
+ location /t {
+ return 200;
+ }
+--- shutdown_error_log eval
+qr/terminate runner \d+ with SIGTERM/
diff --git a/t/plugin/ext-plugin/sanity.t b/t/plugin/ext-plugin/sanity.t
index 1a3a233..8889313 100644
--- a/t/plugin/ext-plugin/sanity.t
+++ b/t/plugin/ext-plugin/sanity.t
@@ -453,3 +453,13 @@ MY_ENV_VAR foo
{"error_msg":"failed to check the configuration of plugin ext-plugin-pre-req err: property \"conf\" validation failed: failed to validate item 1: property \"name\" is required"}
{"error_msg":"failed to check the configuration of plugin ext-plugin-post-req err: property \"conf\" validation failed: failed to validate item 1: property \"value\" is required"}
+
+
+
+=== TEST 15: spawn runner which can't be terminated, ensure APISIX won't be blocked
+--- ext_plugin_cmd
+["t/plugin/ext-plugin/runner_can_not_terminated.sh"]
+--- config
+ location /t {
+ return 200;
+ }