don't delete the bridge interface if it was not created by libnetwork#1301
don't delete the bridge interface if it was not created by libnetwork#1301chenchun merged 1 commit intomoby:masterfrom
Conversation
43bc56f to
2ab4c96
Compare
drivers/bridge/bridge.go
Outdated
| bridgeAlreadyExists := bridgeIface.exists() | ||
| if !bridgeAlreadyExists { | ||
| bridgeSetup.queueStep(setupDevice) | ||
| config.ifaceManaged = true |
There was a problem hiding this comment.
Wouldn't this code mark as managed also the libnetwork created docker0 bridge ?
There was a problem hiding this comment.
Oh yes. I updated the code to exclude the docker0 bridge.
7ee6d78 to
6f620ef
Compare
|
You didn't persist the flag, what if daemon restarts? |
|
@chenchun That's a good question. Thanks! 👍 Edit: |
|
Design LGTM, ping @aboch |
drivers/bridge/bridge.go
Outdated
| dbIndex uint64 | ||
| dbExists bool | ||
| Internal bool | ||
| ifaceManaged bool |
There was a problem hiding this comment.
This is not being persisted. I think that if we reload the daemon, the existing bridge networks will all be marked as non-managed. So if we delete one, the respective linux bridge will not be removed.
6f620ef to
26274d6
Compare
drivers/bridge/bridge.go
Outdated
| bridgeAlreadyExists := bridgeIface.exists() | ||
| if !bridgeAlreadyExists { | ||
| bridgeSetup.queueStep(setupDevice) | ||
| config.IfaceManaged = true |
There was a problem hiding this comment.
On daemon reload, this will cause all existing networks' bridges to be kept as non-managed.
Somehow we need to make sure the existing networks in store are marked as managed, but after this feature they should not.
Maybe we can do this by adding a different flag instead, which is true only for network for which com.docker.network.bridge.name was specified at creation time. And the flag needs to be saved to store.
WDYT ?
|
@mountkin We are good with the feature, design. |
|
ping @mountkin |
|
@aboch sorry for the delay. |
23c1eb6 to
ea778ce
Compare
|
@aboch I updated the PR. PTAL again. Thanks |
| const ( | ||
| ifaceCreatorUnknown ifaceCreator = iota | ||
| ifaceCreatedByLibnetwork | ||
| ifaceCreatedByUser |
There was a problem hiding this comment.
Just a minor comment, I would be less verbose
const (
unknown ifaceCreator = iota
libnetwork
user
)
but that's just my opinion, it already looks good to me.
Signed-off-by: Shijiang Wei <[email protected]>
ea778ce to
34628de
Compare
|
@aboch PR updated |
|
LGTM |
1 similar comment
|
LGTM |
Given that I have a bridge interface named br0, when I run docker network create -d bridge -o com.docker.network.bridge.name=br0 mybr and then run docker network rm mybr, the br0 interface on my host will be removed. I think in such case the pre-configured bridge interface should not be delete by docker. cherry-pick from moby/libnetwork#1301 fix DTS2017062908750 Signed-off-by: Shijiang Wei <[email protected]> Signed-off-by: Lei Jitang <[email protected]>
don't delete the bridge interface if it was not created by libnetwork Given that I have a bridge interface named br0, when I run docker network create -d bridge -o com.docker.network.bridge.name=br0 mybr and then run docker network rm mybr, the br0 interface on my host will be removed. I think in such case the pre-configured bridge interface should not be delete by docker. cherry-pick from moby/libnetwork#1301 fix DTS2017062908750 Signed-off-by: Shijiang Wei <[email protected]> Signed-off-by: Lei Jitang <[email protected]> See merge request docker/docker!619
Given that I have a bridge interface named br0, when I run
docker network create -d bridge -o com.docker.network.bridge.name=br0 mybrand then rundocker network rm mybr, the br0 interface on my host will be removed.I think in such case the pre-configured bridge interface should not be delete by docker.
Signed-off-by: Shijiang Wei [email protected]