Stop container health check also on kill event

[nevalla]

When container is stopped (docker stop <container>), Docker will send container event with status kill. Currently agent will stop container health check worker only with die event.

[nevalla]

@jakolehm @jnummelin Do you see any side effects because of this?

[nevalla]

Hmm... or should we only track kill event?

[jnummelin]

Hmm, stopping health check on both die and kill does not really make sense to me. At least according to Docker event state here moby/moby#12164 (comment), all containers should get die event when they are being stopped. I.e. when you stop a container, you should get events kill-> die -> stop.

[SpComb]

It definitely makes more sense to stop health-checking on kill, because the process may stop responding to health checks while shutting down, and we don't want to restart it while it's shutting down... Consider a process that traps the SIGTERM signal, closes its listening socket, waits for any pending transactions to complete, and flushes out any write buffers before exiting... it may take an arbitrarily long time between the Docker kill and the die events (assuming that die = the container pid=1 process has exited).

Even with the kill coming before the die, the Kontena::Workers::ContainerHealthCheckWorker is still racy with the Kontena::ServicePods::Stopper/Terminator, though... the agent should not be health-checking once it has stopped the container, but this process currently relies on the Docker::Container.stop -> Docker event -> Celluloid::Notifications container:event -> Celluloid::Actor.kill chain completing before the ContainerHealthCheckWorker periodic interval kicks in, and that's just a question of timing.

[SpComb]

Mmm stale copy-pasta :)