don't delete the bridge interface if it was not created by libnetwork #1301
Conversation
43bc56f to
2ab4c96
Compare
| @@ -674,6 +675,7 @@ func (d *driver) createNetwork(config *networkConfiguration) error { | |||
| 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 |
| @@ -70,6 +70,7 @@ type networkConfiguration struct { | |||
| 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
| @@ -674,6 +675,7 @@ func (d *driver) createNetwork(config *networkConfiguration) error { | |||
| 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 <mountkin@gmail.com>
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 <mountkin@gmail.com> Signed-off-by: Lei Jitang <leijitang@huawei.com>
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 <mountkin@gmail.com> Signed-off-by: Lei Jitang <leijitang@huawei.com> 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 mountkin@gmail.com