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

[flrgh]

Primary improvements:

• Removes usage of os.execute() for sending signals
• Decouples reading a pidfile from sending a signal
• Adds quality-of-life APIs for sending sig-(term|kill|quit)

Some months ago I needed to look over the code in kong/cmd/utils/kill.lua and thought to myself "we can do better than os.execute() here." At long last, here are the improvements that stemmed from that thought.

Along the way I renamed the file to kong/cmd/utils/process.lua and changed the name of the is_running() function to exists() for aesthetic reasons. This and the other API changes to the file make the PR somewhat large, but these changes were done in separate commits, so review commit-by-commit if you want to have an easier read-through of the changes.

There was a flaky test in spec/02-integration/01-helpers/03-http_mock_spec.lua that was giving me trouble. It was actually related to this problem domain (waiting for a pidfile to exist and contain a PID), so I fixed it here as well.

[flrgh]

NOTE: When the time comes, I'd prefer we Rebase and merge this one rather than squashing.

[flrgh]

Question for reviewers on this edge case: should we handle EPERM ("Operation not permitted") in a special way?

The kill(2) man page has this to say about errors:

EINVAL - An invalid signal was specified.

EPERM - The calling process does not have permission to send the signal to any of the target processes.

ESRCH - The target process or process group does not exist. Note that an existing process might be a zombie, a process that has terminated execution, but has not yet been wait(2)ed for.

I take that to mean that the process definitely exists which might imply that we should return true. However, this module seems primarily oriented towards managing our own processes (i.e. process.exists(pid) is often followed by process.signal(pid, some_signal)), so maybe it would be surprising to callers for process.exists() to return true for a process that we are not allowed to otherwise manage.

As of right now, the behavior is to return true:

$ echo 1 > ./pidfile
$ resty \
> -e 'setmetatable(_G, nil)' \
> -e 'print(require("kong.cmd.utils.process").exists("./pidfile"))'
true

The behavior of the old kong.cmd.utils.kill.is_running() function would be to return false:

$ echo 1 > ./pidfile
$ resty \
> -e 'setmetatable(_G, nil)' \
> -e 'print(require("kong.cmd.utils.kill").is_running("./pidfile"))'
false

[hanshuebner]

I would find it surprising for an exists function to error if the process exists, but cannot be managed. I would expect it to return true instead.

[flrgh]

We talked about this outside of GH, but I'm continuing the discussion here for the sake of anyone else who wants to chime in. I'd like some consensus before merging.

---

In a vacuum, returning true seems to be the most correct. In our case, this does have user-facing side effects.

The primary example is in the kong-health script. Start kong as root and then run kong-health as an unprivileged user.

In master, kong-health reports that kong is not running because of the current behavior of the is_running() API:

$ kong-health

Error: Kong is not running at /home/michaelm/git/kong/kong/eperm

  Run with --v (verbose) or --vv (debug) for more details

Whereas changing is_running() (now exists()) to return true in this condition means that this branch will report that kong is running:

$ kong-health

Kong is healthy at /home/michaelm/git/kong/kong/eperm

I would say this is arguably more "correct", but it may be a surprising change to introduce.

need re-review

[hanshuebner]

@flrgh crap, i squashed the commits, sorry!