Fix bug in initializeNetwork() #17558
Conversation
| @@ -1007,13 +1013,15 @@ func (container *Container) connectToNetwork(idOrName string, updateSettings boo | |||
| } | |||
| }() | |||
|
|
|||
| if err := container.updateEndpointNetworkSettings(n, ep); err != nil { | |||
| if err = container.updateEndpointNetworkSettings(n, ep); err != nil { | |||
There was a problem hiding this comment.
Uh-oh, that's why I'm fan of named return handling in defers. Because someone new will write := 100%
There was a problem hiding this comment.
Yes. defers with := is a bad combination and it has tripped us once in the past.
There was a problem hiding this comment.
@LK4D4
I agree that naked return would also help. But by experience we noticed that from a code clarity standpoint, it is not optimal to use naked return in functions bigger than an handful of lines.
|
@mavenugo Addressed your comment. PTAL. |
|
@LK4D4 Taken care of your comment. PTAL when you get a chance. |
|
@aboch
|
|
Thanks @vdemeester. Taken care of the golint issue. PTAL. |
|
LGTM. Janky error is unrelated. |
| @@ -914,6 +914,16 @@ func (container *Container) allocateNetwork() error { | |||
| if mode.IsDefault() { | |||
| networkName = controller.Config().Daemon.DefaultNetwork | |||
| } | |||
| if mode.IsUserDefined() { | |||
| var ( | |||
There was a problem hiding this comment.
Can we get rid of this var block and instead do
n, err := condainer.daemon...;
if err != nil {
return err
}
networkName = n.Name()There was a problem hiding this comment.
Good point.
Let me take care of it.
Thanks
- On `docker run --net <network id> ...` the bug would cause the container to attempt to connect to the network two times - Also made sure endpoint creation rollback will be executed on failures in `func (container *Container) connectToNetwork()` Signed-off-by: Alessandro Boch <aboch@docker.com>
|
LGTM |
Fix bug in initializeNetwork()
tiborvass
added
status/5-cherry-pick
impact/changelog
and removed
status/5-cherry-pick
status/4-merge
labels
docker run --net <network id> ...the bug would cause the container to attempt
to connect to the network two times
be executed on failures in
func (container *Container) connectToNetwork()Fixes #17543
Signed-off-by: Alessandro Boch aboch@docker.com