Use gateway instead of 0.0.0.0 to allow docker in docker #8
Conversation
| @@ -6,6 +6,16 @@ | |||
| from docker.errors import ImageNotFound | |||
| import docker as docker_module | |||
|
|
|||
|
|
|||
| def escape_env_var(varname): | |||
There was a problem hiding this comment.
Can you either add a test with realistic examples (preferred) or a docstring explaining why this is necessary?
There was a problem hiding this comment.
I will do both.
tox_docker.py
Outdated
| @@ -54,6 +64,9 @@ def tox_runtest_pre(venv): | |||
| conf._docker_containers.append(container) | |||
|
|
|||
| container.reload() | |||
| gateway_ip = "0.0.0.0" | |||
| if conf.docker_use_gateway and container.attrs["NetworkSettings"]["Gateway"]: | |||
There was a problem hiding this comment.
Is the gateway sometimes not available? I'd love for this to be the default way to do things, rather than an option, in order to keep the interface smaller, but if it's sometimes not possible, that's a bummer.
There was a problem hiding this comment.
I will check if it is possible that the gateway is unavailable.
There was a problem hiding this comment.
It seems to always be set. So I will remove the parameter docker_use_gateway.
GuillaumeSmaha
force-pushed
the
use_gateway
branch
2 times, most recently
from
c476f66
to
d9b7022
Compare
|
@dcrosta It is ok, Thank for your answer :) |
|
|
||
| class ToxDockerRegistryTest(unittest.TestCase): | ||
| def test_it_sets_specific_env_vars(self): | ||
| self.assertEqual("env-var-value", os.environ["ENV_VAR"]) |
There was a problem hiding this comment.
We don't need this test case replicated in the other test env.
| def test_it_sets_automatic_env_vars(self): | ||
| # the nginx image we use exposes port 80 | ||
| self.assertIn("DOCKER_IO_LIBRARY_NGINX_HOST", os.environ) | ||
| self.assertIn("DOCKER_IO_LIBRARY_NGINX_80_TCP", os.environ) |
There was a problem hiding this comment.
It's not clear to me that this should be what you have here vs. eg NGINX_80_TCP. What's the use case for having multiple containers with the same name running (if they come from different regstries)?
@tox-dev/maintainers any opinions?
There was a problem hiding this comment.
Sorry for the delay, I miss the notification for your replies.
The aim is to replace all invalid characters in the environment variable name.
Without the fix, an image from docker.io/library/nginx:1.13-alpine (or another private registry) will create an env var with the name DOCKER.IO_LIBRARY_NGINX_80_TCP which is not valid for a environment var.
| name.upper(), | ||
| containerport.replace("/", "_").upper(), | ||
| ) | ||
| envvar = escape_env_var("{}_HOST".format( |
There was a problem hiding this comment.
Could you add documentation on this to README.md?
GuillaumeSmaha
force-pushed
the
use_gateway
branch
from
ec255c3
to
5810ea9
Compare
|
Since the commit for UDP check (63b80bf), my test fails and currently, I don't know why it works with the |
I want to use tox-docker inside docker (docker in docker). But using
0.0.0.0orlocalhostdoestn't work in this case.That why I made this PR to add an option
docker_use_gatewaywhich use docker gateway ip to test connection with the container. I also add a new environment varXXX_HOSTto save the gateway ip.Finally, I replace
name.upper()by a call toescape_env_var()because when you use a private registry, the name with the URL is not correctly escaped. Example:my.registry.com/myimagebecameMY.REGISTRY.COM/MYIMAGEinstead ofMY_REGISTRY_COM_MYIMAGE.To test the failure case:
docker run --rm -ti -v $(pwd):/src -w /src -v /var/run/docker.sock:/var/run/docker.sock -v /etc/passwd:/etc/passwd -v /etc/group:/etc/group docker sh -c 'apk add python py-pip ; pip install . ; tox'In
tox.ini, if you setdocker_use_gatewaytoTrue, the previous command will success.