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

Avoid container leaks by setting hook before container creation #110

Merged

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Mar 30, 2016

Hi!

If somebody will stop execution during the container creation, especially during the "waitUntilContainerStarted" phase, hook will not be executed, seems like finally doesn't work in case of the SIGTERM.

Symptoms: I see a lot of non-destroyed containers after test execution when we stop our Jenkins job.

This fix will add a hook just before container is being created to make sure that it will be executed afterwards.

Thanks!

@bsideup
Copy link
Member Author

bsideup commented Mar 30, 2016

hey @rnorth,

Could you please take a look at this one for the next release as well? I wouldn't call it critical, but definitely annoying and causing a lot of issues on CI servers and local environments.

Thanks!

@rnorth
Copy link
Member

rnorth commented Mar 30, 2016

Hi,
Sure - this is an important one to fix so I'll try and merge and release
quickly. If you don't mind, I'll prioritise inclusion of this in 1.0.3 over
#106, and perhaps hold that back for the next point release. I hope this is
OK!
Cheers
Rich

On Wed, Mar 30, 2016 at 4:03 PM, Sergei Egorov notifications@github.com
wrote:

hey @rnorth https://github.com/rnorth,

Could you please take a look at this one for the next release as well? I
wouldn't call it critical, but definitely annoying and causing a lot of
issues on CI servers and local environments.

Thanks!


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#110 (comment)

@bsideup
Copy link
Member Author

bsideup commented Mar 30, 2016

@rnorth makes sense :)

BTW CircleCI failed for some reason, can't pull MySQL image. Re-run maybe?

@rnorth
Copy link
Member

rnorth commented Mar 30, 2016

@bsideup that's annoying - re-running!

@rnorth rnorth merged commit ee72e7e into testcontainers:master Mar 30, 2016
@rnorth
Copy link
Member

rnorth commented Mar 30, 2016

Looks good to me, so merged.

rnorth added a commit that referenced this pull request Mar 30, 2016
…y class. This is able to cleanup all container IDs that are created, as opposed to containers that are started.
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

2 participants