Merge pull request #202 from aboch/mn

Bridge driver to support multiple networks

libnetwork/client/network.go

@@ -13,7 +13,7 @@ import (
 )
 
 const (
-	nullNetType = "null"
+	defaultDriverType = "bridge"
 )
 
 type command struct {
@@ -52,7 +52,7 @@ func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error {
 		return err
 	}
 	if *flDriver == "" {
-		*flDriver = nullNetType
+		*flDriver = defaultDriverType
 	}
 
 	// Construct network create request body

libnetwork/drivers/bridge/bridge.go

@@ -86,8 +86,9 @@ type bridgeNetwork struct {
 }
 
 type driver struct {
-	config  *configuration
-	network *bridgeNetwork
+	config   *configuration
+	network  *bridgeNetwork
+	networks map[types.UUID]*bridgeNetwork
 	sync.Mutex
 }
 
@@ -98,7 +99,7 @@ func init() {
 
 // New constructs a new bridge driver
 func newDriver() driverapi.Driver {
-	return &driver{}
+	return &driver{networks: map[types.UUID]*bridgeNetwork{}}
 }
 
 // Init registers a new instance of bridge driver
@@ -146,6 +147,27 @@ func (c *networkConfiguration) Validate() error {
 	return nil
 }
 
+// Conflict check if two NetworkConfiguration objects overlap in the multinetwork
+func (c *networkConfiguration) Conflict(o *networkConfiguration) bool {
+	if o == nil {
+		return false
+	}
+
+	// Also empty, becasue only one network with empty name is allowed
+	if c.BridgeName == o.BridgeName {
+		return true
+	}
+
+	// They must be in different subnets
+
+	if (c.AddressIPv4 != nil && o.AddressIPv4.IP != nil) &&
+		(c.AddressIPv4.Contains(o.AddressIPv4.IP) || o.AddressIPv4.Contains(c.AddressIPv4.IP)) {
+		return true
+	}
+
+	return false
+}
+
 // FromMap retrieve the configuration data from the map form.
 func (c *networkConfiguration) FromMap(data map[string]interface{}) error {
 	if i, ok := data["BridgeName"]; ok && i != nil {
@@ -340,11 +362,18 @@ func (d *driver) Config(option map[string]interface{}) error {
 }
 
 func (d *driver) getNetwork(id types.UUID) (*bridgeNetwork, error) {
-	// Just a dummy function to return the only network managed by Bridge driver.
-	// But this API makes the caller code unchanged when we move to support multiple networks.
 	d.Lock()
 	defer d.Unlock()
-	return d.network, nil
+
+	if id == "" {
+		return nil, types.BadRequestErrorf("invalid network id: %s", id)
+	}
+
+	if nw, ok := d.networks[id]; ok {
+		return nw, nil
+	}
+
+	return nil, nil
 }
 
 func parseNetworkOptions(option options.Generic) (*networkConfiguration, error) {
@@ -387,35 +416,71 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err
 	d.Lock()
 
 	// Sanity checks
-	if d.network != nil {
+	if _, ok := d.networks[id]; ok {
+		d.Unlock()
+		return types.ForbiddenErrorf("network %s exists", id)
+	}
+
+	// Parse and validate the config. It should not conflict with existing networks' config
+	config, err := parseNetworkOptions(option)
+	if err != nil {
 		d.Unlock()
-		return &ErrNetworkExists{}
+		return err
+	}
+	for _, nw := range d.networks {
+		if nw.config.Conflict(config) {
+			d.Unlock()
+			return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName)
+		}
 	}
 
 	// Create and set network handler in driver
-	d.network = &bridgeNetwork{id: id, endpoints: make(map[types.UUID]*bridgeEndpoint)}
-	network := d.network
+	network := &bridgeNetwork{id: id, endpoints: make(map[types.UUID]*bridgeEndpoint), config: config}
+	d.networks[id] = network
 	d.Unlock()
 
 	// On failure make sure to reset driver network handler to nil
 	defer func() {
 		if err != nil {
 			d.Lock()
-			d.network = nil
+			delete(d.networks, id)
 			d.Unlock()
 		}
 	}()
 
-	config, err := parseNetworkOptions(option)
-	if err != nil {
-		return err
-	}
-	network.config = config
-
 	// Create or retrieve the bridge L3 interface
 	bridgeIface := newInterface(config)
 	network.bridge = bridgeIface
 
+	// Verify network does not conflict with previously configured networks
+	// on parameters that were chosen by the driver.
+	d.Lock()
+	for _, nw := range d.networks {
+		if nw.id == id {
+			continue
+		}
+		// Verify the name (which may have been set by newInterface()) does not conflict with
+		// existing bridge interfaces. Ironically the system chosen name gets stored in the config...
+		// Basically we are checking if the two original configs were both empty.
+		if nw.config.BridgeName == config.BridgeName {
+			d.Unlock()
+			return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName)
+		}
+		// If this network config specifies the AddressIPv4, we need
+		// to make sure it does not conflict with any previously allocated
+		// bridges. This could not be completely caught by the config conflict
+		// check, because networks which config does not specify the AddressIPv4
+		// get their address and subnet selected by the driver (see electBridgeIPv4())
+		if config.AddressIPv4 != nil {
+			if nw.bridge.bridgeIPv4.Contains(config.AddressIPv4.IP) ||
+				config.AddressIPv4.Contains(nw.bridge.bridgeIPv4.IP) {
+				d.Unlock()
+				return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName)
+			}
+		}
+	}
+	d.Unlock()
+
 	// Prepare the bridge setup configuration
 	bridgeSetup := newBridgeSetup(config, bridgeIface)
 
@@ -485,8 +550,12 @@ func (d *driver) DeleteNetwork(nid types.UUID) error {
 
 	// Get network handler and remove it from driver
 	d.Lock()
-	n := d.network
-	d.network = nil
+	n, ok := d.networks[nid]
+	if !ok {
+		d.Unlock()
+		return types.InternalMaskableErrorf("network %s does not exist", nid)
+	}
+	delete(d.networks, nid)
 	d.Unlock()
 
 	// On failure set network handler back in driver, but
@@ -494,8 +563,8 @@ func (d *driver) DeleteNetwork(nid types.UUID) error {
 	defer func() {
 		if err != nil {
 			d.Lock()
-			if d.network == nil {
-				d.network = n
+			if _, ok := d.networks[nid]; !ok {
+				d.networks[nid] = n
 			}
 			d.Unlock()
 		}
@@ -535,7 +604,11 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn
 
 	// Get the network handler and make sure it exists
 	d.Lock()
-	n := d.network
+	n, ok := d.networks[nid]
+	if !ok {
+		d.Unlock()
+		return types.NotFoundErrorf("network %s does not exist", nid)
+	}
 	config := n.config
 	d.Unlock()
 	if n == nil {
@@ -716,7 +789,11 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error {
 
 	// Get the network handler and make sure it exists
 	d.Lock()
-	n := d.network
+	n, ok := d.networks[nid]
+	if !ok {
+		d.Unlock()
+		return types.NotFoundErrorf("network %s does not exist", nid)
+	}
 	config := n.config
 	d.Unlock()
 	if n == nil {
@@ -787,7 +864,11 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error {
 func (d *driver) EndpointOperInfo(nid, eid types.UUID) (map[string]interface{}, error) {
 	// Get the network handler and make sure it exists
 	d.Lock()
-	n := d.network
+	n, ok := d.networks[nid]
+	if !ok {
+		d.Unlock()
+		return nil, types.NotFoundErrorf("network %s does not exist", nid)
+	}
 	d.Unlock()
 	if n == nil {
 		return nil, driverapi.ErrNoNetwork(nid)

libnetwork/drivers/bridge/bridge_test.go

@@ -194,8 +194,12 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
 		t.Fatalf("Failed to create an endpoint : %s", err.Error())
 	}
 
-	ep, _ := dd.network.endpoints["ep1"]
-	data, err := d.EndpointOperInfo(dd.network.id, ep.id)
+	network, ok := dd.networks["net1"]
+	if !ok {
+		t.Fatalf("Cannot find network %s inside driver", "net1")
+	}
+	ep, _ := network.endpoints["ep1"]
+	data, err := d.EndpointOperInfo(network.id, ep.id)
 	if err != nil {
 		t.Fatalf("Failed to ask for endpoint operational data:  %v", err)
 	}

libnetwork/drivers/bridge/network_test.go

@@ -79,7 +79,10 @@ func TestLinkCreate(t *testing.T) {
 		t.Fatalf("Could not find source link %s: %v", te.ifaces[0].srcName, err)
 	}
 
-	n := dr.network
+	n, ok := dr.networks["dummy"]
+	if !ok {
+		t.Fatalf("Cannot find network %s inside driver", "dummy")
+	}
 	ip := te.ifaces[0].addr.IP
 	if !n.bridge.bridgeIPv4.Contains(ip) {
 		t.Fatalf("IP %s is not a valid ip in the subnet %s", ip.String(), n.bridge.bridgeIPv4.String())

libnetwork/drivers/bridge/port_mapping_test.go

@@ -47,7 +47,11 @@ func TestPortMappingConfig(t *testing.T) {
 	}
 
 	dd := d.(*driver)
-	ep, _ := dd.network.endpoints["ep1"]
+	network, ok := dd.networks["dummy"]
+	if !ok {
+		t.Fatalf("Cannot find network %s inside driver", "dummy")
+	}
+	ep, _ := network.endpoints["ep1"]
 	if len(ep.portMapping) != 2 {
 		t.Fatalf("Failed to store the port bindings into the sandbox info. Found: %v", ep.portMapping)
 	}

libnetwork/drivers/bridge/setup_ipv4.go

@@ -19,27 +19,21 @@ func init() {
 	// In theory this shouldn't matter - in practice there's bound to be a few scripts relying
 	// on the internal addressing or other stupid things like that.
 	// They shouldn't, but hey, let's not break them unless we really have to.
-	for _, addr := range []string{
-		"172.17.42.1/16", // Don't use 172.16.0.0/16, it conflicts with EC2 DNS 172.16.0.23
-		"10.0.42.1/16",   // Don't even try using the entire /8, that's too intrusive
-		"10.1.42.1/16",
-		"10.42.42.1/16",
-		"172.16.42.1/24",
-		"172.16.43.1/24",
-		"172.16.44.1/24",
-		"10.0.42.1/24",
-		"10.0.43.1/24",
-		"192.168.42.1/24",
-		"192.168.43.1/24",
-		"192.168.44.1/24",
-	} {
-		ip, net, err := net.ParseCIDR(addr)
-		if err != nil {
-			log.Errorf("Failed to parse address %s", addr)
-			continue
-		}
-		net.IP = ip.To4()
-		bridgeNetworks = append(bridgeNetworks, net)
+	// Don't use 172.16.0.0/16, it conflicts with EC2 DNS 172.16.0.23
+
+	// 172.[17-31].42.1/16
+	mask := []byte{255, 255, 0, 0}
+	for i := 17; i < 32; i++ {
+		bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{172, byte(i), 42, 1}, Mask: mask})
+	}
+	// 10.[0-255].42.1/16
+	for i := 0; i < 256; i++ {
+		bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{10, byte(i), 42, 1}, Mask: mask})
+	}
+	// 192.168.[42-44].1/24
+	mask[2] = 255
+	for i := 42; i < 45; i++ {
+		bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{192, 168, byte(i), 1}, Mask: mask})
 	}
 }