#507 - make docker run handle SIGINT/SIGTERM

[unclejack]

This PR adds SIGTERM and SIGINT handling to docker run. When any of the two signals is received by a docker run process, docker stop will be run to try to stop the container. If the container fails to stop graciously in 4 seconds, it's killed by docker stop.

Observations:

1. docker stop isn't called, the CmdStop function is called instead to avoid duplicating any code.
2. Running things under a process monitoring solution will also require proper return codes, otherwise processes will always seem to be exiting cleanly.
3. This PR doesn't break signal handling for bash running in a container (e.g.: `docker run -i -t ubuntu bash.. ^c).
4. There's an issue with sending a SIGTERM/SIGINT to something which has set up a TTY:

$ docker run -i -t ubuntu bash
root@8158df67a66a:/# 
                     Received signal: interrupt; cleaning up
                                                            bk@bkm:~$

Questions:

1. What SIGKILL timeout should be set for docker stop?
2. How should the TTY output formatting issue be fixed?
3. What other changes should be made?
4. How could we test this?

This PR fixes #507.

[unclejack]

It's time to cc a few persons who might want to review this and provide feedback.

/cc @shykes @creack @vieux @gabrtv @progrium

[vieux]

I know this PR produce the right behaviour, but I think many people, myself included, are used to do ctrl-c to exit the client and not the process. I think we should add a hint somewhere the the keystroke combination to detach. ( I can't remember it, as I always use ctrl+c ;) )

Maybe we could have a longer timeout, but if you press a second time ctrc+c, it call cmdKill ?

[unclejack]

@vieux If you type ctrl-c, the signal will still be sent to the process you're running, e.g.: bash or something else.

ctrl+p ctrl-q detaches and SIGINT is still handled by the process running in the container. SIGTERM/SIGINT has to be sent to the docker process itself in order to stop it.

In my opinion, this PR only changes the behavior of the docker process spawned by docker run, not the behavior of anything else which may be running in the docker container.

Concerning the second signal, yes, we could do that.

[vieux]

ping @shykes @creack

[calavera]

the for { loop is not really necessary, is it?. The goroutine should wait for any of the signals in the line below.

[unclejack]

There's a reason for having the for, we want to ask the container to stop more than just once if SIGINT/SIGTERM is received more than once.

[creack]

LGTM, /cc @vieux @crosbymichael

[crosbymichael]

LGTM

[titanous]

This can be simplified to for sig := range signals {.

[ghost]

I wanted to have signal handling very badly, so I wrote this little wrapper script, which handles INT/TERM/HUP and translates it to stop and restart. Have a look: https://github.com/lgierth/docker-exec

I knew this PR here is in the works, but solving the problem with a wrapping shell script seemed like a fun thing to do. It might also take a bit of pressure off this PR, since anybody who badly wants this feature can just use the script, and then switch to the official support once this PR is released.

[vieux]

LGTM

[crosbymichael]

LGTM