MindFlavor/prometheus_wireguard_exporter

Signals support in Docker

arisudesu opened this issue ยท 8 comments

It is noticeable that exporter app doesn't process signals well when running in Docker. It doesn't exit immediately on docker stop, instead, it continues working until containerd-init kills it forcefully.

It should respect SIGTERM signal to shutdown gracefully. And maybe SIGINT too, if anyone tries to run exporter in interactive mode and stop it with Ctrl+C.

qdm12 commented

I have the same problem. But is it limited to Docker? I believe it fails to exit even when run in a terminal interactively.

mmm I can't seem to be able to repro it (at least interactively):

p

Can you please help me pinpoint the problem?

Tried to look into problem and I noticed that my container wasn't running with --init flag, as stated in README. Exporter was started as PID1 inside container and of course it did not have default signal handling for INT, TERM. Adding the --init flag solves shutdown problem.

Sorry!

Ok cool!
There's nothing better than an issue that resolves itself without touching the code ๐Ÿ˜ƒ !

qdm12 commented

Well actually that --init flag is a bit of hack, why can't the rust binary handle the os signals properly?

We might also want to exit the program cleanly when receiving a signal, instead of having another program killing the rust program when receiving the signal.

EDIT: I used --init when running multiple programs in parallel in the same container to avoid zombie processes, until I wrote a proper entrypoint (in go) which would manage the life cycles of each program and process os signals. Here since we only have one program running, running with the init system feels a bit wrong, although, yes, it works.

A bit off-topic, but this is not a Rust problem, but rather the consequences of what we got by processing pid 1 signals in the kernel in linux differently from other processes. Rust just does what the OS tells it by default (or doesn't, lol).

In Go, for example, it works a little differently. Runtime registers signal handlers explicitly at startup and does internal processing, so SIGTERM works even in a process with pid 1 because it has a registered handler, and in order not to process it, you need to explicitly catch it in the go-code and ignore it. As a result, processes in containers behave correctly when sending signals, as if it were a normal OS process. IMO the well-behaving init process should do the same, and the kernel should not make exceptions for pid 1, but now we have what we had for years, and we just have to live with it. Or add explicit signal handlers to all dockerized programs.

qdm12 commented

Ah very much indeed, this article explains it well. PID 1 is indeed special, TIL!

Although Rust's doc is bit confusing in that regard ๐Ÿ˜„

If your applications does not need to gracefully shutdown, the default handling is fine (i.e. exit immediately and let the OS cleanup resources like open file handles). In that case: No need to do what this chapter tells you!

From https://rust-cli.github.io/book/in-depth/signals.html

Now we could handle a bunch of signals in the program and exit gracefully, maybe that would be interesting... or don't change anything that's fine too

Note however that init is built-in recent docker versions (19.xx i think) and some users might still be using older versions (๐Ÿ‘€ NAS), so not relying on it would a be a tiny plus. Saying this as I had the issue on one of my containers with NAS users having un-upgradable docker daemons.

qdm12 commented

We can use

apk add tini
#...
ENTRYPOINT ["/sbin/tini", "--", "/usr/local/bin/prometheus_wireguard_exporter"]

In the final image stage so --init will no longer be required. I'll address this shortly.