Merge pull request #425 from mavenugo/r0.4

Cherry-picking commits from master to release-0.4

drivers/bridge/bridge.go

@@ -596,21 +596,18 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err
 	// networks. This step is needed now because driver might have now set the bridge
 	// name on this config struct. And because we need to check for possible address
 	// conflicts, so we need to check against operationa lnetworks.
-	if err := config.conflictsWithNetworks(id, networkList); err != nil {
+	if err = config.conflictsWithNetworks(id, networkList); err != nil {
 		return err
 	}
 
 	setupNetworkIsolationRules := func(config *networkConfiguration, i *bridgeInterface) error {
-		defer func() {
-			if err != nil {
-				if err := network.isolateNetwork(networkList, false); err != nil {
-					logrus.Warnf("Failed on removing the inter-network iptables rules on cleanup: %v", err)
-				}
+		if err := network.isolateNetwork(networkList, true); err != nil {
+			if err := network.isolateNetwork(networkList, false); err != nil {
+				logrus.Warnf("Failed on removing the inter-network iptables rules on cleanup: %v", err)
 			}
-		}()
-
-		err := network.isolateNetwork(networkList, true)
-		return err
+			return err
+		}
+		return nil
 	}
 
 	// Prepare the bridge setup configuration
@@ -766,17 +763,26 @@ func (d *driver) DeleteNetwork(nid types.UUID) error {
 }
 
 func addToBridge(ifaceName, bridgeName string) error {
-	iface, err := net.InterfaceByName(ifaceName)
+	link, err := netlink.LinkByName(ifaceName)
 	if err != nil {
 		return fmt.Errorf("could not find interface %s: %v", ifaceName, err)
 	}
+	if err = netlink.LinkSetMaster(link,
+		&netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}); err != nil {
+		logrus.Debugf("Failed to add %s to bridge via netlink.Trying ioctl: %v", ifaceName, err)
+		iface, err := net.InterfaceByName(ifaceName)
+		if err != nil {
+			return fmt.Errorf("could not find network interface %s: %v", ifaceName, err)
+		}
 
-	master, err := net.InterfaceByName(bridgeName)
-	if err != nil {
-		return fmt.Errorf("could not find bridge %s: %v", bridgeName, err)
+		master, err := net.InterfaceByName(bridgeName)
+		if err != nil {
+			return fmt.Errorf("could not find bridge %s: %v", bridgeName, err)
+		}
+
+		return ioctlAddToBridge(iface, master)
 	}
-
-	return ioctlAddToBridge(iface, master)
+	return nil
 }
 
 func setHairpinMode(link netlink.Link, enable bool) error {
@@ -947,15 +953,14 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn
 	}
 
 	// v4 address for the sandbox side pipe interface
-	sub := types.GetIPNetCanonical(n.bridge.bridgeIPv4)
-	ip4, err := ipAllocator.RequestIP(sub, nil)
+	ip4, err := ipAllocator.RequestIP(n.bridge.bridgeIPv4, nil)
 	if err != nil {
 		return err
 	}
 	ipv4Addr := &net.IPNet{IP: ip4, Mask: n.bridge.bridgeIPv4.Mask}
 
 	// Down the interface before configuring mac address.
-	if err := netlink.LinkSetDown(sbox); err != nil {
+	if err = netlink.LinkSetDown(sbox); err != nil {
 		return fmt.Errorf("could not set link down for container interface %s: %v", containerIfName, err)
 	}
 
@@ -968,7 +973,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn
 	endpoint.macAddress = mac
 
 	// Up the host interface after finishing all netlink configuration
-	if err := netlink.LinkSetUp(host); err != nil {
+	if err = netlink.LinkSetUp(host); err != nil {
 		return fmt.Errorf("could not set link up for host interface %s: %v", hostIfName, err)
 	}
 
@@ -1074,8 +1079,7 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error {
 	n.releasePorts(ep)
 
 	// Release the v4 address allocated to this endpoint's sandbox interface
-	sub := types.GetIPNetCanonical(n.bridge.bridgeIPv4)
-	err = ipAllocator.ReleaseIP(sub, ep.addr.IP)
+	err = ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, ep.addr.IP)
 	if err != nil {
 		return err
 	}

drivers/bridge/bridge_test.go

@@ -54,6 +54,17 @@ func TestCreateFullOptions(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to create bridge: %v", err)
 	}
+
+	// Verify the IP address allocated for the endpoint belongs to the container network
+	epOptions := make(map[string]interface{})
+	te := &testEndpoint{ifaces: []*testInterface{}}
+	err = d.CreateEndpoint("dummy", "ep1", te, epOptions)
+	if err != nil {
+		t.Fatalf("Failed to create an endpoint : %s", err.Error())
+	}
+	if !cnw.Contains(te.Interfaces()[0].Address().IP) {
+		t.Fatalf("endpoint got assigned address outside of container network(%s): %s", cnw.String(), te.Interfaces()[0].Address())
+	}
 }
 
 func TestCreate(t *testing.T) {

drivers/bridge/setup_device.go

@@ -1,7 +1,11 @@
 package bridge
 
 import (
+	"fmt"
+
+	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/parsers/kernel"
+	"github.com/docker/libnetwork/netutils"
 	"github.com/vishvananda/netlink"
 )
 
@@ -25,11 +29,25 @@ func setupDevice(config *networkConfiguration, i *bridgeInterface) error {
 	// Only set the bridge's MAC address if the kernel version is > 3.3, as it
 	// was not supported before that.
 	kv, err := kernel.GetKernelVersion()
-	if err == nil && (kv.Kernel >= 3 && kv.Major >= 3) {
-		setMac = true
+	if err != nil {
+		logrus.Errorf("Failed to check kernel versions: %v. Will not assign a MAC address to the bridge interface", err)
+	} else {
+		setMac = kv.Kernel > 3 || (kv.Kernel == 3 && kv.Major >= 3)
 	}
 
-	return ioctlCreateBridge(config.BridgeName, setMac)
+	if err = netlink.LinkAdd(i.Link); err != nil {
+		logrus.Debugf("Failed to create bridge %s via netlink. Trying ioctl", config.BridgeName)
+		return ioctlCreateBridge(config.BridgeName, setMac)
+	}
+
+	if setMac {
+		hwAddr := netutils.GenerateRandomMAC()
+		if err = netlink.LinkSetHardwareAddr(i.Link, hwAddr); err != nil {
+			return fmt.Errorf("failed to set bridge mac-address %s : %s", hwAddr, err.Error())
+		}
+		logrus.Debugf("Setting bridge mac address to %s", hwAddr)
+	}
+	return err
 }
 
 // SetupDeviceUp ups the given bridge interface.

drivers/bridge/setup_ipv4.go

@@ -8,7 +8,6 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/libnetwork/netutils"
-	"github.com/docker/libnetwork/types"
 	"github.com/vishvananda/netlink"
 )
 
@@ -76,8 +75,12 @@ func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error {
 }
 
 func allocateBridgeIP(config *networkConfiguration, i *bridgeInterface) error {
-	sub := types.GetIPNetCanonical(i.bridgeIPv4)
-	ipAllocator.RequestIP(sub, i.bridgeIPv4.IP)
+	// Because of the way ipallocator manages the container address space,
+	// reserve bridge address only if it belongs to the container network
+	// (if defined), no need otherwise
+	if config.FixedCIDR == nil || config.FixedCIDR.Contains(i.bridgeIPv4.IP) {
+		ipAllocator.RequestIP(i.bridgeIPv4, i.bridgeIPv4.IP)
+	}
 	return nil
 }
 
@@ -112,10 +115,13 @@ func setupGatewayIPv4(config *networkConfiguration, i *bridgeInterface) error {
 		return &ErrInvalidGateway{}
 	}
 
-	// Pass the real network subnet to ip allocator (no host bits set)
-	sub := types.GetIPNetCanonical(i.bridgeIPv4)
-	if _, err := ipAllocator.RequestIP(sub, config.DefaultGatewayIPv4); err != nil {
-		return err
+	// Because of the way ipallocator manages the container address space,
+	// reserve default gw address only if it belongs to the container network
+	// (if defined), no need otherwise
+	if config.FixedCIDR == nil || config.FixedCIDR.Contains(config.DefaultGatewayIPv4) {
+		if _, err := ipAllocator.RequestIP(i.bridgeIPv4, config.DefaultGatewayIPv4); err != nil {
+			return err
+		}
 	}
 
 	// Store requested default gateway

drivers/bridge/setup_ipv4_test.go

@@ -82,7 +82,7 @@ func TestSetupGatewayIPv4(t *testing.T) {
 
 	ip, nw, _ := net.ParseCIDR("192.168.0.24/16")
 	nw.IP = ip
-	gw := net.ParseIP("192.168.0.254")
+	gw := net.ParseIP("192.168.2.254")
 
 	config := &networkConfiguration{
 		BridgeName:         DefaultBridgeName,

ipallocator/allocator.go

@@ -66,7 +66,8 @@ func (a *IPAllocator) RegisterSubnet(network *net.IPNet, subnet *net.IPNet) erro
 	a.mutex.Lock()
 	defer a.mutex.Unlock()
 
-	key := network.String()
+	nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask}
+	key := nw.String()
 	if _, ok := a.allocatedIPs[key]; ok {
 		return ErrNetworkAlreadyRegistered
 	}
@@ -90,10 +91,11 @@ func (a *IPAllocator) RequestIP(network *net.IPNet, ip net.IP) (net.IP, error) {
 	a.mutex.Lock()
 	defer a.mutex.Unlock()
 
-	key := network.String()
+	nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask}
+	key := nw.String()
 	allocated, ok := a.allocatedIPs[key]
 	if !ok {
-		allocated = newAllocatedMap(network)
+		allocated = newAllocatedMap(nw)
 		a.allocatedIPs[key] = allocated
 	}
 
@@ -109,7 +111,8 @@ func (a *IPAllocator) ReleaseIP(network *net.IPNet, ip net.IP) error {
 	a.mutex.Lock()
 	defer a.mutex.Unlock()
 
-	if allocated, exists := a.allocatedIPs[network.String()]; exists {
+	nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask}
+	if allocated, exists := a.allocatedIPs[nw.String()]; exists {
 		delete(allocated.p, ip.String())
 	}
 	return nil

libnetwork_test.go

@@ -1919,14 +1919,30 @@ func createGlobalInstance(t *testing.T) {
 			"AllowNonDefaultBridge": true,
 		},
 	}
-	net, err := createTestNetwork(bridgeNetType, "network", netOption)
+
+	net1, err := controller.NetworkByName("testhost")
 	if err != nil {
-		t.Fatal("new network")
+		t.Fatal(err)
 	}
 
-	_, err = net.CreateEndpoint("ep1")
+	net2, err := createTestNetwork("bridge", "network2", netOption)
 	if err != nil {
-		t.Fatal("createendpoint")
+		t.Fatal(err)
+	}
+
+	_, err = net1.CreateEndpoint("pep1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = net2.CreateEndpoint("pep2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = net2.CreateEndpoint("pep3")
+	if err != nil {
+		t.Fatal(err)
 	}
 }
 
@@ -1940,12 +1956,18 @@ func debugf(format string, a ...interface{}) (int, error) {
 
 func parallelJoin(t *testing.T, ep libnetwork.Endpoint, thrNumber int) {
 	debugf("J%d.", thrNumber)
-	err := ep.Join("racing_container")
+	var err error
+	if thrNumber == first {
+		err = ep.Join(fmt.Sprintf("%drace", thrNumber), libnetwork.JoinOptionUseDefaultSandbox())
+	} else {
+		err = ep.Join(fmt.Sprintf("%drace", thrNumber))
+	}
+
 	runtime.LockOSThread()
 	if err != nil {
 		if _, ok := err.(libnetwork.ErrNoContainer); !ok {
 			if _, ok := err.(libnetwork.ErrInvalidJoin); !ok {
-				t.Fatal(err)
+				t.Fatalf("thread %d: %v", thrNumber, err)
 			}
 		}
 		debugf("JE%d(%v).", thrNumber, err)
@@ -1955,12 +1977,18 @@ func parallelJoin(t *testing.T, ep libnetwork.Endpoint, thrNumber int) {
 
 func parallelLeave(t *testing.T, ep libnetwork.Endpoint, thrNumber int) {
 	debugf("L%d.", thrNumber)
-	err := ep.Leave("racing_container")
+	var err error
+	if thrNumber == first {
+		err = ep.Leave(fmt.Sprintf("%drace", thrNumber))
+	} else {
+		err = controller.LeaveAll(fmt.Sprintf("%drace", thrNumber))
+	}
+
 	runtime.LockOSThread()
 	if err != nil {
 		if _, ok := err.(libnetwork.ErrNoContainer); !ok {
 			if _, ok := err.(libnetwork.ErrInvalidJoin); !ok {
-				t.Fatal(err)
+				t.Fatalf("thread %d: %v", thrNumber, err)
 			}
 		}
 		debugf("LE%d(%v).", thrNumber, err)
@@ -2012,15 +2040,33 @@ func runParallelTests(t *testing.T, thrNumber int) {
 	}
 	defer netns.Set(origns)
 
-	net, err := controller.NetworkByName("network")
+	net1, err := controller.NetworkByName("testhost")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if net == nil {
-		t.Fatal("Could not find network")
+	if net1 == nil {
+		t.Fatal("Could not find network1")
+	}
+
+	net2, err := controller.NetworkByName("network2")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if net2 == nil {
+		t.Fatal("Could not find network2")
+	}
+
+	epName := fmt.Sprintf("pep%d", thrNumber)
+
+	//var err error
+	var ep libnetwork.Endpoint
+
+	if thrNumber == first {
+		ep, err = net1.EndpointByName(epName)
+	} else {
+		ep, err = net2.EndpointByName(epName)
 	}
 
-	ep, err := net.EndpointByName("ep1")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -2035,6 +2081,11 @@ func runParallelTests(t *testing.T, thrNumber int) {
 
 	debugf("\n")
 
+	err = ep.Delete()
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	if thrNumber == first {
 		for thrdone := range done {
 			select {
@@ -2043,12 +2094,8 @@ func runParallelTests(t *testing.T, thrNumber int) {
 		}
 
 		testns.Close()
-		err = ep.Delete()
-		if err != nil {
-			t.Fatal(err)
-		}
 
-		if err := net.Delete(); err != nil {
+		if err := net2.Delete(); err != nil {
 			t.Fatal(err)
 		}
 	}

netutils/utils.go

@@ -74,20 +74,22 @@ func NetworkOverlaps(netX *net.IPNet, netY *net.IPNet) bool {
 
 // NetworkRange calculates the first and last IP addresses in an IPNet
 func NetworkRange(network *net.IPNet) (net.IP, net.IP) {
-	var netIP net.IP
-	if network.IP.To4() != nil {
-		netIP = network.IP.To4()
-	} else if network.IP.To16() != nil {
-		netIP = network.IP.To16()
-	} else {
+	if network == nil {
 		return nil, nil
 	}
 
-	lastIP := make([]byte, len(netIP), len(netIP))
-	for i := 0; i < len(netIP); i++ {
-		lastIP[i] = netIP[i] | ^network.Mask[i]
+	firstIP := network.IP.Mask(network.Mask)
+	lastIP := types.GetIPCopy(firstIP)
+	for i := 0; i < len(firstIP); i++ {
+		lastIP[i] = firstIP[i] | ^network.Mask[i]
 	}
-	return netIP.Mask(network.Mask), net.IP(lastIP)
+
+	if network.IP.To4() != nil {
+		firstIP = firstIP.To4()
+		lastIP = lastIP.To4()
+	}
+
+	return firstIP, lastIP
 }
 
 // GetIfaceAddr returns the first IPv4 address and slice of IPv6 addresses for the specified network interface

sandboxdata.go

@@ -139,10 +139,15 @@ func (s *sandboxData) rmEndpoint(ep *endpoint) {
 		}
 	}
 
-	// We don't check if s.endpoints is empty here because
-	// it should never be empty during a rmEndpoint call and
-	// if it is we will rightfully panic here
 	s.Lock()
+	if len(s.endpoints) == 0 {
+		// s.endpoints should never be empty and this is unexpected error condition
+		// We log an error message to note this down for debugging purposes.
+		logrus.Errorf("No endpoints in sandbox while trying to remove endpoint %s", ep.Name())
+		s.Unlock()
+		return
+	}
+
 	highEpBefore := s.endpoints[0]
 	var (
 		i int
@@ -245,7 +250,10 @@ func (c *controller) LeaveAll(id string) error {
 	}
 
 	sData.sandbox().Destroy()
+
+	c.Lock()
 	delete(c.sandboxes, sandbox.GenerateKey(id))
+	c.Unlock()
 
 	return nil
 }