custom_build_with_restart does not pass signals to wrapped process

[jscheel]

I'm using custom_build_with_restart in conjunction with a k8s_resource. I've been testing graceful shutdowns, and noticed that the custom_build that is created does not actually forward signals to the wrapped process. Docker relies on the entrypoint having pid=1, but with the wrapper, the process that actually needs to handle the signals no longer has that pid.

[nicks]

Hmm... sorry to hear you're having trouble. Can you provide more specific repro steps on what you're trying to do?

When you say "the custom_build that is created", do you mean the process building the image or the entry point of the result image?

When you say "forward signals", which signals? the signals from the kubelet as part of pod shutdown ? The signals that the cancel button in the tilt UI sends? A manual process that sends signals some other way?

[jscheel]

@nicks hey, sorry, I was in a hurry to write that report earlier, realize now it was pretty ambiguous :) So, I think this is pretty straightforward when I explain it better:

Docker entrypoint processes run with pid=1. This means that your entrypoint process should handle SIGTERM and other signals for graceful shutdowns. I don't think it necessarily matters what is sending those signals, it could be the tilt interface, it could be a pod shutdown, or anything else. I may have muddied my initial report by talking about k8s.

tilt-restart-wrapper.go wraps the entrypoint, and therefore assumes pid=1. However, this executable does not handle signals. Functionally, this means that when using custom_build_with_restart, the restart wrapper executable swallows the signals and does not forward them to the original entrypoint process (which now has a different pid). Here is some additional context from a blog post from Yelp's engineering team: https://engineeringblog.yelp.com/2016/01/dumb-init-an-init-for-docker.html

I think solution would be for the wrapper to listen for signals and forward them to the subprocess it is wrapping. I haven't tested this code, but possibly something like this:

sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM, syscall.SIGKILL, syscall.SIGHUP)
go func() {
for {
    sig := <-sigs
    if cmd.Process != nil {
        cmd.Process.Signal(sig) // Forward the signal to the child process
    }
}
}()

I can submit a PR if that makes sense.

[nicks]

ya, that seems reasonable to me

[jscheel]

@nicks just added a PR for it