Merge pull request #1301 from mountkin/keep-custom-bridge

don't delete the bridge interface if it was not created by libnetwork

libnetwork/drivers/bridge/bridge.go

@@ -70,8 +70,19 @@ type networkConfiguration struct {
 	dbIndex            uint64
 	dbExists           bool
 	Internal           bool
+
+	BridgeIfaceCreator ifaceCreator
 }
 
+// ifaceCreator represents how the bridge interface was created
+type ifaceCreator int8
+
+const (
+	ifaceCreatorUnknown ifaceCreator = iota
+	ifaceCreatedByLibnetwork
+	ifaceCreatedByUser
+)
+
 // endpointConfiguration represents the user specified configuration for the sandbox endpoint
 type endpointConfiguration struct {
 	MacAddress net.HardwareAddr
@@ -512,6 +523,17 @@ func parseNetworkOptions(id string, option options.Generic) (*networkConfigurati
 		config.BridgeName = "br-" + id[:12]
 	}
 
+	exists, err := bridgeInterfaceExists(config.BridgeName)
+	if err != nil {
+		return nil, err
+	}
+
+	if !exists {
+		config.BridgeIfaceCreator = ifaceCreatedByLibnetwork
+	} else {
+		config.BridgeIfaceCreator = ifaceCreatedByUser
+	}
+
 	config.ID = id
 	return config, nil
 }
@@ -778,11 +800,17 @@ func (d *driver) DeleteNetwork(nid string) error {
 		return err
 	}
 
-	// We only delete the bridge when it's not the default bridge. This is keep the backward compatible behavior.
-	if !config.DefaultBridge {
-		if err := d.nlh.LinkDel(n.bridge.Link); err != nil {
-			logrus.Warnf("Failed to remove bridge interface %s on network %s delete: %v", config.BridgeName, nid, err)
+	switch config.BridgeIfaceCreator {
+	case ifaceCreatedByLibnetwork, ifaceCreatorUnknown:
+		// We only delete the bridge if it was created by the bridge driver and
+		// it is not the default one (to keep the backward compatible behavior.)
+		if !config.DefaultBridge {
+			if err := d.nlh.LinkDel(n.bridge.Link); err != nil {
+				logrus.Warnf("Failed to remove bridge interface %s on network %s delete: %v", config.BridgeName, nid, err)
+			}
 		}
+	case ifaceCreatedByUser:
+		// Don't delete the bridge interface if it was not created by libnetwork.
 	}
 
 	// clean all relevant iptables rules

libnetwork/drivers/bridge/bridge_store.go

@@ -143,6 +143,7 @@ func (ncfg *networkConfiguration) MarshalJSON() ([]byte, error) {
 	nMap["DefaultBindingIP"] = ncfg.DefaultBindingIP.String()
 	nMap["DefaultGatewayIPv4"] = ncfg.DefaultGatewayIPv4.String()
 	nMap["DefaultGatewayIPv6"] = ncfg.DefaultGatewayIPv6.String()
+	nMap["BridgeIfaceCreator"] = ncfg.BridgeIfaceCreator
 
 	if ncfg.AddressIPv4 != nil {
 		nMap["AddressIPv4"] = ncfg.AddressIPv4.String()
@@ -191,6 +192,10 @@ func (ncfg *networkConfiguration) UnmarshalJSON(b []byte) error {
 		ncfg.Internal = v.(bool)
 	}
 
+	if v, ok := nMap["BridgeIfaceCreator"]; ok {
+		ncfg.BridgeIfaceCreator = ifaceCreator(v.(float64))
+	}
+
 	return nil
 }
 

libnetwork/drivers/bridge/bridge_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/libnetwork/options"
 	"github.com/docker/libnetwork/testutils"
 	"github.com/docker/libnetwork/types"
+	"github.com/vishvananda/netlink"
 )
 
 func init() {
@@ -166,9 +167,9 @@ func compareBindings(a, b []types.PortBinding) bool {
 	return true
 }
 
-func getIPv4Data(t *testing.T) []driverapi.IPAMData {
+func getIPv4Data(t *testing.T, iface string) []driverapi.IPAMData {
 	ipd := driverapi.IPAMData{AddressSpace: "full"}
-	nw, _, err := netutils.ElectInterfaceAddresses("")
+	nw, _, err := netutils.ElectInterfaceAddresses(iface)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -243,7 +244,7 @@ func TestCreateNoConfig(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = netconfig
 
-	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 }
@@ -282,7 +283,7 @@ func TestCreateFullOptionsLabels(t *testing.T) {
 	netOption[netlabel.EnableIPv6] = true
 	netOption[netlabel.GenericData] = labels
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	ipd6List := []driverapi.IPAMData{
 		{
 			Pool: nwV6,
@@ -364,11 +365,11 @@ func TestCreate(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = netconfig
 
-	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 
-	err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t), nil)
+	err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t, ""), nil)
 	if err == nil {
 		t.Fatalf("Expected bridge driver to refuse creation of second network with default name")
 	}
@@ -392,7 +393,7 @@ func TestCreateFail(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = netconfig
 
-	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t), nil); err == nil {
+	if err := d.CreateNetwork("dummy", genericOption, nil, getIPv4Data(t, ""), nil); err == nil {
 		t.Fatal("Bridge creation was expected to fail")
 	}
 }
@@ -417,13 +418,13 @@ func TestCreateMultipleNetworks(t *testing.T) {
 	config1 := &networkConfiguration{BridgeName: "net_test_1"}
 	genericOption = make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config1
-	if err := d.CreateNetwork("1", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("1", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 
 	config2 := &networkConfiguration{BridgeName: "net_test_2"}
 	genericOption[netlabel.GenericData] = config2
-	if err := d.CreateNetwork("2", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("2", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 
@@ -432,7 +433,7 @@ func TestCreateMultipleNetworks(t *testing.T) {
 
 	config3 := &networkConfiguration{BridgeName: "net_test_3"}
 	genericOption[netlabel.GenericData] = config3
-	if err := d.CreateNetwork("3", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("3", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 
@@ -441,7 +442,7 @@ func TestCreateMultipleNetworks(t *testing.T) {
 
 	config4 := &networkConfiguration{BridgeName: "net_test_4"}
 	genericOption[netlabel.GenericData] = config4
-	if err := d.CreateNetwork("4", genericOption, nil, getIPv4Data(t), nil); err != nil {
+	if err := d.CreateNetwork("4", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
 
@@ -625,7 +626,7 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
 	genericOption = make(map[string]interface{})
 	genericOption[netlabel.GenericData] = netconfig
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("net1", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
@@ -728,7 +729,7 @@ func TestLinkContainers(t *testing.T) {
 	genericOption = make(map[string]interface{})
 	genericOption[netlabel.GenericData] = netconfig
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("net1", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
@@ -945,7 +946,7 @@ func TestSetDefaultGw(t *testing.T) {
 
 	_, subnetv6, _ := net.ParseCIDR("2001:db8:ea9:9abc:b0c4::/80")
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	gw4 := types.GetIPCopy(ipdList[0].Pool.IP).To4()
 	gw4[3] = 254
 	gw6 := net.ParseIP("2001:db8:ea9:9abc:b0c4::254")
@@ -1008,3 +1009,65 @@ func TestCleanupIptableRules(t *testing.T) {
 		}
 	}
 }
+
+func TestCreateWithExistingBridge(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	d := newDriver()
+
+	if err := d.configure(nil); err != nil {
+		t.Fatalf("Failed to setup driver config: %v", err)
+	}
+
+	brName := "br111"
+	br := &netlink.Bridge{
+		LinkAttrs: netlink.LinkAttrs{
+			Name: brName,
+		},
+	}
+	if err := netlink.LinkAdd(br); err != nil {
+		t.Fatalf("Failed to create bridge interface: %v", err)
+	}
+	defer netlink.LinkDel(br)
+	if err := netlink.LinkSetUp(br); err != nil {
+		t.Fatalf("Failed to set bridge interface up: %v", err)
+	}
+
+	ip := net.IP{192, 168, 122, 1}
+	addr := &netlink.Addr{IPNet: &net.IPNet{
+		IP:   ip,
+		Mask: net.IPv4Mask(255, 255, 255, 0),
+	}}
+	if err := netlink.AddrAdd(br, addr); err != nil {
+		t.Fatalf("Failed to add IP address to bridge: %v", err)
+	}
+
+	netconfig := &networkConfiguration{BridgeName: brName}
+	genericOption := make(map[string]interface{})
+	genericOption[netlabel.GenericData] = netconfig
+
+	if err := d.CreateNetwork(brName, genericOption, nil, getIPv4Data(t, brName), nil); err != nil {
+		t.Fatalf("Failed to create bridge network: %v", err)
+	}
+
+	nw, err := d.getNetwork(brName)
+	if err != nil {
+		t.Fatalf("Failed to getNetwork(%s): %v", brName, err)
+	}
+
+	addr4, _, err := nw.bridge.addresses()
+	if err != nil {
+		t.Fatalf("Failed to get the bridge network's address: %v", err)
+	}
+
+	if !addr4.IP.Equal(ip) {
+		t.Fatal("Creating bridge network with existing bridge interface unexpectedly modified the IP address of the bridge")
+	}
+
+	if err := d.DeleteNetwork(brName); err != nil {
+		t.Fatalf("Failed to delete network %s: %v", brName, err)
+	}
+
+	if _, err := netlink.LinkByName(brName); err != nil {
+		t.Fatalf("Deleting bridge network that using existing bridge interface unexpectedly deleted the bridge interface")
+	}
+}

libnetwork/drivers/bridge/network_test.go

@@ -26,7 +26,7 @@ func TestLinkCreate(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
@@ -118,7 +118,7 @@ func TestLinkCreateTwo(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
@@ -154,7 +154,7 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
@@ -189,7 +189,7 @@ func TestLinkDelete(t *testing.T) {
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)

libnetwork/drivers/bridge/port_mapping_test.go

@@ -44,7 +44,7 @@ func TestPortMappingConfig(t *testing.T) {
 	netOptions := make(map[string]interface{})
 	netOptions[netlabel.GenericData] = netConfig
 
-	ipdList := getIPv4Data(t)
+	ipdList := getIPv4Data(t, "")
 	err := d.CreateNetwork("dummy", netOptions, nil, ipdList, nil)
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)

libnetwork/drivers/bridge/setup_verify.go

@@ -2,8 +2,10 @@ package bridge
 
 import (
 	"fmt"
+	"strings"
 
 	log "github.com/Sirupsen/logrus"
+	"github.com/docker/libnetwork/ns"
 	"github.com/docker/libnetwork/types"
 	"github.com/vishvananda/netlink"
 )
@@ -51,3 +53,19 @@ func findIPv6Address(addr netlink.Addr, addresses []netlink.Addr) bool {
 	}
 	return false
 }
+
+func bridgeInterfaceExists(name string) (bool, error) {
+	nlh := ns.NlHandle()
+	link, err := nlh.LinkByName(name)
+	if err != nil {
+		if strings.Contains(err.Error(), "Link not found") {
+			return false, nil
+		}
+		return false, fmt.Errorf("failed to check bridge interface existence: %v", err)
+	}
+
+	if link.Type() == "bridge" {
+		return true, nil
+	}
+	return false, fmt.Errorf("existing interface %s is not a bridge", name)
+}