Skip to content

Commit

Permalink
refactor(cmd): use resty.signal for process signaling (#11382)
Browse files Browse the repository at this point in the history
* refactor(cmd): rename kong.cmd.utils.kill => kong.cmd.utils.process

There's a large refactor of this module in the pipeline, and renaming
from 'kill' to 'process' makes the API more sensible.

* refactor(cmd): rename process.is_running => process.exists

This is another name-only change ahead of some larger refactoring of the
kong.cmd.utils.process module.

* refactor(cmd): use resty.signal for process signaling

This refactors kong.cmd.utils.process to use the resty.signal library
for sending signals instead of dropping to a shell.

This comes with improvements (albeit minor) to performance and security.

* chore(cmd): remove redundant process check

* tests(helpers): fix flaky http_mock PID file test

* chore(cmd): return error on signal sending failures
  • Loading branch information
flrgh authored Sep 20, 2023
1 parent 710489f commit bbdda0b
Show file tree
Hide file tree
Showing 14 changed files with 727 additions and 134 deletions.
5 changes: 2 additions & 3 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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
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,8 +42,7 @@ local function execute(args)

print("")
local pid_file = pl_path.join(prefix, "pids", "nginx.pid")
kill.is_running(pid_file)
assert(kill.is_running(pid_file), "Kong is not running at " .. prefix)
assert(process.exists(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.kill"] = "kong/cmd/utils/kill.lua",
["kong.cmd.utils.process"] = "kong/cmd/utils/process.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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
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 = kill.is_running(v)
local running = process.exists(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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
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 kill.is_running(conf.nginx_pid) then
if not process.exists(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 = kill.is_running(conf.nginx_pid)
running = process.exists(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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
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 = kill.is_running(conf.nginx_pid)
running = process.exists(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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
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 kill.is_running(conf.nginx_pid),
assert(not process.exists(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: 0 additions & 32 deletions kong/cmd/utils/kill.lua

This file was deleted.

20 changes: 11 additions & 9 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 kill = require "kong.cmd.utils.kill"
local process = require "kong.cmd.utils.process"
local meta = require "kong.meta"
local pl_path = require "pl.path"
local version = require "version"
Expand Down Expand Up @@ -55,15 +55,17 @@ end


local function send_signal(kong_conf, signal)
if not kill.is_running(kong_conf.nginx_pid) then
local pid = process.pid(kong_conf.nginx_pid)

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

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

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

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


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


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


function _M.reload(kong_conf)
if not kill.is_running(kong_conf.nginx_pid) then
if not process.exists(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 bbdda0b

@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:bbdda0b0f668c5dbbbebe8af3a92f445c92776c4
Artifacts available https://github.com/Kong/kong/actions/runs/6247316667

Please sign in to comment.