Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix delay when stopping the docker container #1

Merged
merged 3 commits into from Jul 17, 2017

Conversation

mjeri
Copy link
Contributor

@mjeri mjeri commented Jul 14, 2017

Hi @yaman,

while setting up integration tests for my project I stumbled over your repository and found it quite useful and would like to use it there. Anyway I had small problems with getting it to work, as the docker image does not listen properly to the SIGTERM signal, because it was using the shell form of the ENTRYPOINT syntax. Here is a quite good article how docker stops and why it doesn't work properly with this shell form.

As the setup and the go version 1.4.2 is coming to age, I changed the docker setup to the version 1.8 in the way it is recommended on the golang docker image here(Section: Start a Go instance in your app). To get this working I had to extend the code a bit, so that it can read also the proto from the environment and did a minor optimisation with the for-loop.

This way the change is completely compatible, the same docker commands work and from the command line nothing changed. I hope you like the change and you are up to merging this. I would be very happy when you can publish the new version on dockerhub as well. This would make it most easy for me to integrate it in my integration setup.

Let me know about feedback :)

the most efficient way to block the go-routine of exiting is to simply
select{} on nothing.
with EXEC syntax.

This change was actually done because of the problem that when using
ENTRYPOINT with shell syntax, in the Docker container first a shell
command is launched which in turn executes the go command afterwards.

The problem with that is when stopping the container the shell command
would receive the SIGTERM signal and it does not pass it somehow to the
running go-process. This means the container does not exit after
receiving the SIGTERM but docker will invoke, usually after 10 seconds,
a SIGKILL. This is somewhat unclean.
@mjeri
Copy link
Contributor Author

mjeri commented Jul 14, 2017

As a minor side-note: at the moment the environment variables are preferred over command line flags. I think this is a bit unintuitive. Usually it is the other way around, but I didn't want to include this in my PR as it possibly would break things. What do you think about this?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 99.296% when pulling 7d4d81d on marco-jantke:fix-docker-stop into 5bd2007 on yaman:master.

@yaman yaman merged commit 6f77774 into yaman:master Jul 17, 2017
@yaman
Copy link
Owner

yaman commented Jul 17, 2017

Hi @marco-jantke,
Sorry for the late response. I was re-installing my laptop, so I missed. Thanks for the pull request. I think your changes improve things a lot. And feel free to break things. I feel like I have been holding back about radical changes for a while.

@mjeri
Copy link
Contributor Author

mjeri commented Jul 19, 2017

Hehe great, thanks for merging and publishing the image so quickly :)

@mjeri mjeri deleted the fix-docker-stop branch July 19, 2017 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants