docker exec should cleanup its orphaned processes when it exits

[dqminh]

Some background on this is at:

• Kill docker exec command will not terminate the spawned process #9098
• https://github.com/docker/libcontainer/issues/286
• add spec for exec a new process inside a container docker-archive/libcontainer#290

TLDR: docker exec process when exits can leave behind orphaned/unwanted processes, this patch put all processes belongs to a single docker exec session into its own subcgroup and removes them when the main process finishes. This will not affect processes spawned in the container and/or other exec sessions.

[SvenDowideit]

nice - can you please add a sentence or two to this effect into the run.md - that way anyone that is curious how and why this happens will have something to refer to

I'm interested to see how (or if) users will want to start new daemon processes using docker exec

[phemmer]

I think this should have a flag to control the behavior. While I somewhat agree with the idea in this change, I do not think it that unreasonable for some more complex applications & systems to use docker exec for lauching long running commands inside a container. Perhaps as part of some nightly maintenance scripts, to kick off a background task.

[dqminh]

@phemmer @SvenDowideit in that case, would it make more sense to run each long-running process in a different docker exec -d session instead of having short-lived script launching multiple long-running daemon ?

The purpose of the patch is to prevent that exact situation as ideally an exec session should not leave orphaned process inside the PID namespace when it finishes.

[LK4D4]

@dqminh I think that libcontainer was bumped and now you can rebase against master.

[SvenDowideit]

yes, I guess docker attach $(docker exec -dit ...) makes sense, compared to docker exec -it nohup stuff - does it work that way?

yes, I realised talking to some people last night, that you might have a long running upgrade / transform task you might start with docke exec, and having it reaped due to network failure to the Docker client may make them cry too.

no-easy-answer

[SvenDowideit]

I'd document what the PR does now, as it seems like a useful addition as is.

[dqminh]

yes, I realised talking to some people last night, that you might have a long running upgrade / transform task you might start with docke exec, and having it reaped due to network failure to the Docker client may make them cry too.

ah no, it currently doesnt behave like that. The behavior of how docker exec client affects the remote exec session on the server when the client exits is currently undefined.

Starting a long running exec session with docker exec -d will work as is ( i.e, the remote exec process will not be killed if the client is killed ). This patch only affects cleanup of docker exec processes when the main process exits. For example:

# before this patch
> docker exec -it my-precious sh
/ # sleep 1000 &
/ # exit
# now there's a dangling `sleep 1000` process inside the parent container after main `sh` process exits and user has to clean it up manually

# after this patch
> docker exec -it my-precious sh
/ # sleep 1000 &
/ # exit
# main `sh` process and its `sleep 1000` child process will be stopped.

@SvenDowideit i will add some docs to cli and the HTTP api to document the new behavior for this patch.

[dqminh]

Actually i think it probably makes more sense to have the cgroup setup as an optional part. Some of our users do want to launch background processes from an interactive exec session and dont want them to be shut down automatically.

I will close this and open again when it's ready.

[thefallentree]

@dqminh Is there followup on this PR?