Skip to content

Commit

Permalink
Revert "refactor(cmd): use resty.signal for process signaling (#11382)…
Browse files Browse the repository at this point in the history
…" (#11620)

This reverts commit bbdda0b.

For posterity's sake, there are actually _two_ reasons for us to merge this:

1. commit history cleanup (as noted in the PR desc for #11620)
2. we discovered a subtle bug in the way that `resty.signal` traverses `LUA_CPATH` in order to discover and load its shared object dependency ([link](https://github.com/openresty/lua-resty-signal/blob/7834226610e2df36f8e69641c4ca0d5ea75a9535/lib/resty/signal.lua#L21-L58)) and _need_ to revert this to un-break master
  • Loading branch information
hanshuebner authored Sep 20, 2023
1 parent e0d53a8 commit bfc8ef7
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 727 deletions.
5 changes: 3 additions & 2 deletions bin/kong-health
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
setmetatable(_G, nil)
package.path = (os.getenv("KONG_LUA_PATH_OVERRIDE") or "") .. "./?.lua;./?/init.lua;" .. package.path

local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local kong_default_conf = require "kong.templates.kong_defaults"
local pl_app = require "pl.lapp"
local pl_config = require "pl.config"
Expand Down Expand Up @@ -42,7 +42,8 @@ local function execute(args)

print("")
local pid_file = pl_path.join(prefix, "pids", "nginx.pid")
assert(process.exists(pid_file), "Kong is not running at " .. prefix)
kill.is_running(pid_file)
assert(kill.is_running(pid_file), "Kong is not running at " .. prefix)
print("Kong is healthy at ", prefix)
end

Expand Down
2 changes: 1 addition & 1 deletion kong-3.5.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ build = {
["kong.cmd.version"] = "kong/cmd/version.lua",
["kong.cmd.hybrid"] = "kong/cmd/hybrid.lua",
["kong.cmd.utils.log"] = "kong/cmd/utils/log.lua",
["kong.cmd.utils.process"] = "kong/cmd/utils/process.lua",
["kong.cmd.utils.kill"] = "kong/cmd/utils/kill.lua",
["kong.cmd.utils.env"] = "kong/cmd/utils/env.lua",
["kong.cmd.utils.migrations"] = "kong/cmd/utils/migrations.lua",
["kong.cmd.utils.tty"] = "kong/cmd/utils/tty.lua",
Expand Down
4 changes: 2 additions & 2 deletions kong/cmd/health.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
local log = require "kong.cmd.utils.log"
local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local pl_path = require "pl.path"
local pl_tablex = require "pl.tablex"
local pl_stringx = require "pl.stringx"
Expand All @@ -26,7 +26,7 @@ local function execute(args)

local count = 0
for k, v in pairs(pids) do
local running = process.exists(v)
local running = kill.is_running(v)
local msg = pl_stringx.ljust(k, 12, ".") .. (running and "running" or "not running")
if running then
count = count + 1
Expand Down
6 changes: 3 additions & 3 deletions kong/cmd/quit.lua
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
local nginx_signals = require "kong.cmd.utils.nginx_signals"
local conf_loader = require "kong.conf_loader"
local pl_path = require "pl.path"
local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local log = require "kong.cmd.utils.log"

local function execute(args)
Expand All @@ -24,7 +24,7 @@ local function execute(args)
log.verbose("waiting %s seconds before quitting", args.wait)
while twait > ngx.now() do
ngx.sleep(0.2)
if not process.exists(conf.nginx_pid) then
if not kill.is_running(conf.nginx_pid) then
log.error("Kong stopped while waiting (unexpected)")
return
end
Expand All @@ -41,7 +41,7 @@ local function execute(args)
local texp, running = tstart + math.max(args.timeout, 1) -- min 1s timeout
repeat
ngx.sleep(0.2)
running = process.exists(conf.nginx_pid)
running = kill.is_running(conf.nginx_pid)
ngx.update_time()
until not running or ngx.now() >= texp

Expand Down
4 changes: 2 additions & 2 deletions kong/cmd/restart.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
local log = require "kong.cmd.utils.log"
local stop = require "kong.cmd.stop"
local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local start = require "kong.cmd.start"
local pl_path = require "pl.path"
local conf_loader = require "kong.conf_loader"
Expand All @@ -27,7 +27,7 @@ local function execute(args)
local running
repeat
ngx.sleep(0.1)
running = process.exists(conf.nginx_pid)
running = kill.is_running(conf.nginx_pid)
until not running or ngx.time() >= texp

start.execute(args)
Expand Down
4 changes: 2 additions & 2 deletions kong/cmd/start.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ local prefix_handler = require "kong.cmd.utils.prefix_handler"
local nginx_signals = require "kong.cmd.utils.nginx_signals"
local conf_loader = require "kong.conf_loader"
local kong_global = require "kong.global"
local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local log = require "kong.cmd.utils.log"
local DB = require "kong.db"
local lfs = require "lfs"
Expand Down Expand Up @@ -53,7 +53,7 @@ local function execute(args)

conf.pg_timeout = args.db_timeout -- connect + send + read

assert(not process.exists(conf.nginx_pid),
assert(not kill.is_running(conf.nginx_pid),
"Kong is already running in " .. conf.prefix)

assert(prefix_handler.prepare_prefix(conf, args.nginx_conf, nil, nil,
Expand Down
32 changes: 32 additions & 0 deletions kong/cmd/utils/kill.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
local pl_path = require "pl.path"
local pl_utils = require "pl.utils"
local log = require "kong.cmd.utils.log"

local cmd_tmpl = [[kill %s `cat %s 2>&1` >/dev/null 2>&1]]

local function kill(pid_file, args)
log.debug("sending signal to pid at: %s", pid_file)
local cmd = string.format(cmd_tmpl, args or "-0", pid_file)
if pl_path.exists(pid_file) then
log.debug(cmd)
local _, code = pl_utils.execute(cmd)
return code
else
log.debug("no pid file at: %s", pid_file)
return 0
end
end

local function is_running(pid_file)
-- we do our own pid_file exists check here because
-- we want to return `nil` in case of NOT running,
-- and not `0` like `kill` would return.
if pl_path.exists(pid_file) then
return kill(pid_file) == 0
end
end

return {
kill = kill,
is_running = is_running
}
20 changes: 9 additions & 11 deletions kong/cmd/utils/nginx_signals.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
local ffi = require "ffi"
local log = require "kong.cmd.utils.log"
local process = require "kong.cmd.utils.process"
local kill = require "kong.cmd.utils.kill"
local meta = require "kong.meta"
local pl_path = require "pl.path"
local version = require "version"
Expand Down Expand Up @@ -55,17 +55,15 @@ end


local function send_signal(kong_conf, signal)
local pid = process.pid(kong_conf.nginx_pid)

if not pid or not process.exists(pid) then
if not kill.is_running(kong_conf.nginx_pid) then
return nil, fmt("nginx not running in prefix: %s", kong_conf.prefix)
end

log.verbose("sending %s signal to nginx running at %s", signal, kong_conf.nginx_pid)

local ok, err = process.signal(pid, signal)
if not ok then
return nil, fmt("could not send signal: %s", err or "unknown error")
local code = kill.kill(kong_conf.nginx_pid, "-s " .. signal)
if code ~= 0 then
return nil, "could not send signal"
end

return true
Expand Down Expand Up @@ -145,7 +143,7 @@ function _M.start(kong_conf)
return nil, err
end

if process.exists(kong_conf.nginx_pid) then
if kill.is_running(kong_conf.nginx_pid) then
return nil, "nginx is already running in " .. kong_conf.prefix
end

Expand Down Expand Up @@ -207,17 +205,17 @@ end


function _M.stop(kong_conf)
return send_signal(kong_conf, process.SIG_TERM)
return send_signal(kong_conf, "TERM")
end


function _M.quit(kong_conf)
return send_signal(kong_conf, process.SIG_QUIT)
return send_signal(kong_conf, "QUIT")
end


function _M.reload(kong_conf)
if not process.exists(kong_conf.nginx_pid) then
if not kill.is_running(kong_conf.nginx_pid) then
return nil, fmt("nginx not running in prefix: %s", kong_conf.prefix)
end

Expand Down
Loading

1 comment on commit bfc8ef7

@khcp-gha-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:bfc8ef79c8496bf025315ec9d014ba7cb915b4eb
Artifacts available https://github.com/Kong/kong/actions/runs/6252950017

Please sign in to comment.