-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
ping timeout of 1s too low in DockerClientProviderStrategy #204
Comments
Gah, that's annoying. Sorry about this. Is there any chance you could debut what's going on inside
|
I'll do some deeper digging, at first glance it looks like it may be more of a docker-java issue. |
What I've found is that testcontainers < 1.1.0 seems to pick up my env fine, which is a docker container jenkins slave running a debian based container (however I'm running docker 1.12 which doesnt work with < 1.0.x) However when using testcontainers 1.1.0+ I get the following
Which is strange because a
These get picked up correctly and displayed in testcontainers 1.0.5 and older. |
Seems to coincide with an update from docker-java 2.2 to 3.x will take a look at that project as well. |
Good news, I've figured out the problems here. @rnorth First one is that in the refactoring that happened for 1.1.0+ the DockerClientProviderStrategy logs strategy.getDescription() before the config is initialized, which is why for Environment strategy the values are empty. It was just a coincidence that this happened at the same time the docker-java client was updated. This is a simple fix to just log this message after strategy.test() or in test() itself if logging is desired in case it fails. The real issue here is that the hardcoded timeout of 1s for the ping command seems to be too low for our setup (docker host thats generally under a lot of load). In testing increasing this slightly resulted in the ping() command not failing. I'll try to put together patches for these items, my only question is about how we could make that ping timeout configurable. Is there a way to allow the user to specify that timeout? Otherwise I may just set it to 5 or 10 seconds max. |
@kunickiaj ah that's great - thank you. I'll accept the PRs. Regarding making the timeout configurable, that's probably a good idea. I think this is the kind of thing we'd want to be configurable via system properties rather than in the test code, though, as it's more a consequence of the environment the tests are being run in. |
Agreed. Let me update the patch to allow configuration. |
@rnorth looking into this further, it looks like the timeout is different in each of the provider strategies ranging from 10s (unix socket) to 30s (docker-machine). I'm wondering why the TCP one was set to a low 1s. How do you feel about standardizing all of them to the same max timeout (e.g. 30s) and having that overridable via system property? |
@kunickiaj thanks. To be honest the reason is just that these timeouts have been 'what works' when each of the strategies was added. Docker Machine support was a bit of an outlier at 30s because testcontainers itself can launch the Machine VM - but the VM takes a while to be ready. We could probably standardise for all to be a long-ish timeout (30s) but only where the strategy will fail fast if it's absent. e.g. if a host doesn't has a unix socket for docker but not TCP, we don't want testcontainers sitting around for 30s waiting for TCP to respond. It's probably not a big deal, but we need to think about each strategy case-by-case! |
hello, I have been having this same issue on and off for a while. I investigated deeper today because it prevented a release from going through. I think a configurable timeout (preferably through a system variable) would be nice. |
Wondering if anyone else has experienced the following...
We upgraded from Docker 1.10 / testcontainers 1.0.5 -> Docker 1.12 / testcontainers 1.1.3.
Since then it seems that the DOCKER_HOST environment variable is not always used by the docker client. I've verified with a debugger that it is in fact set.
Generally this results in an error like the following:
The text was updated successfully, but these errors were encountered: