I think this will break in the following scenario (but not sure how valid is the scenario though) :

docker network create xxx
docker network create yyy
docker network create zzz
docker run --net=xxx --name=foo ...
docker network connect yyy foo
docker network connect zzz foo
docker network disconnect xxx foo
docker restart foo

This will result in the container losing connectivity to networks yyy and zzz.

I have to think through other cases or possible fix. But am not even sure if we have to handle this case.

So it'll break if the first network that the container was connected to, is disconnected?

correct. I think so. I have to try it out to make sure.

@mavenugo Oh yes, the scenario you describe will be break ...

In the case of docker create, we call SetDefaultNetModeIfBlank(hc) to setup default network mode if it is blank.
This was done as part of the windows support to provide backward compatibility.

With your fix, if the newNetworkMode is empty in ContainerStart (which I assume means use the one setup in docker create). Hence, I think we should first check if the newNetworkMode != "" before performing the above check.

WDYT ?

@mavenugo newNetworkMode couldn't be empty, since the hostConfig that ContainerStart passed is from runconfig.DecodeHostConfig (https://github.com/docker/docker/blob/master/api/server/router/container/container_routes.go#L174 ) and DecodeHostConfig will also call SetDefaultNetModeIfBlank(called in getHostConfig)

Cool. Thanks for confirming.

I don't think we want to support changing the network mode, we only need to support setting the network mode on start (in cases where hostconfig was not passed in on create, but was on start). This should be a one time allowance.

@cpuguy83 wont that break backward compatibility ?
(I have to try how it behaves in 1.9.x and before)

@mavenugo If it was supported before I would consider it a bug... and not likely very used if at all.

sure. I agree.
But regardless of that, it will be considered as an API breakage, which we have to document.