-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix Multiple Network Support #23 #40
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
Conversation
With insights from moby/moby#29265 the behaviour is the same as the one from docker-compose * connect to 1 network (at random) at start * disconnect from that network * reconnect to all the network from the previous configuration
container/client.go
Outdated
| return err | ||
| } | ||
|
|
||
| for k, _ := range simpleNetworkConfig.EndpointsConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should omit 2nd value from range; this loop is equivalent to for k := range ...
|
Any news on this? |
|
This will be merged soon. I'm currently refactoring the overall build process. As soon as this is finished, new updated images will be published. |
|
Fixes #23 |
|
Thanks @dolanor for your contribution. |
| log.Debugf("Starting container %s (%s)", name, creation.ID) | ||
|
|
||
| return client.api.ContainerStart(bg, creation.ID, types.ContainerStartOptions{}) | ||
| err = client.api.ContainerStart(bg, creation.ID, types.ContainerStartOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dolanor I've just seen that you reattach the networks after the container has already been started. I think it would be better if we first reattach the networks and afterwards start the container again. Or did you have some problems and made it in this order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation of this behaviour is detailed in the git message 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm aware of the behaviour that we can't directly create the container with all attached networks. But my question was more if we could do it this way.
- ContainerCreate(simpleNetworkConfig)
- NetworkDisconnect(simpleNetworkConfig)
- NetworkConnect(networkConfig.EndpointsConfig)
- ContainerStart()
I think starting the container with only one attached network and afterwards disconnect it and reconnecting the correct ones, may result in strange behaviors for the containers.
It seems like docker-compose also first creates the container, then connects/disconnect the networks and as last step starts the container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see.
Good point. In fact I didn't understand the answer from thaJetzah like that. But you're right.
So I guess my current implementation doesn't work like docker-compose then.
I will fix it tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that would be great.
Following the discussion on #23 and the patch from @rosscado in #26 (which greatly helped me), I think I fixed the problem watchtower had with restarting a container previously connected to multiple networks.
I can say that It works here ™️ with docker compose.
Hope I'll help other people too.