Fix issue for `--fixed-cidr` when bridge has multiple addresses

This fix tries to address the issue raised in:
https://github.com/docker/docker/issues/26341
where multiple addresses in a bridge may cause `--fixed-cidr` to
not have the correct addresses.

The issue is that `netutils.ElectInterfaceAddresses(bridgeName)`
only returns the first IPv4 address.

This fix changes `ElectInterfaceAddresses()` and `addresses()`
so that all IPv4 addresses are returned. This will allow the
possibility of selectively choose the address needed.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

libnetwork/cmd/dnet/dnet.go

@@ -507,11 +507,11 @@ func encodeData(data interface{}) (*bytes.Buffer, error) {
 }
 
 func ipamOption(bridgeName string) libnetwork.NetworkOption {
-	if nw, _, err := netutils.ElectInterfaceAddresses(bridgeName); err == nil {
-		ipamV4Conf := &libnetwork.IpamConf{PreferredPool: nw.String()}
-		hip, _ := types.GetHostPartIP(nw.IP, nw.Mask)
+	if nws, _, err := netutils.ElectInterfaceAddresses(bridgeName); err == nil {
+		ipamV4Conf := &libnetwork.IpamConf{PreferredPool: nws[0].String()}
+		hip, _ := types.GetHostPartIP(nws[0].IP, nws[0].Mask)
 		if hip.IsGlobalUnicast() {
-			ipamV4Conf.Gateway = nw.IP.String()
+			ipamV4Conf.Gateway = nws[0].IP.String()
 		}
 		return libnetwork.NetworkOptionIpam("default", "", []*libnetwork.IpamConf{ipamV4Conf}, nil, nil)
 	}

libnetwork/drivers/bridge/bridge_test.go

@@ -169,13 +169,13 @@ func compareBindings(a, b []types.PortBinding) bool {
 
 func getIPv4Data(t *testing.T, iface string) []driverapi.IPAMData {
 	ipd := driverapi.IPAMData{AddressSpace: "full"}
-	nw, _, err := netutils.ElectInterfaceAddresses(iface)
+	nws, _, err := netutils.ElectInterfaceAddresses(iface)
 	if err != nil {
 		t.Fatal(err)
 	}
-	ipd.Pool = nw
+	ipd.Pool = nws[0]
 	// Set network gateway to X.X.X.1
-	ipd.Gateway = types.GetIPNetCopy(nw)
+	ipd.Gateway = types.GetIPNetCopy(nws[0])
 	ipd.Gateway.IP[len(ipd.Gateway.IP)-1] = 1
 	return []driverapi.IPAMData{ipd}
 }
@@ -1054,12 +1054,12 @@ func TestCreateWithExistingBridge(t *testing.T) {
 		t.Fatalf("Failed to getNetwork(%s): %v", brName, err)
 	}
 
-	addr4, _, err := nw.bridge.addresses()
+	addrs4, _, err := nw.bridge.addresses()
 	if err != nil {
 		t.Fatalf("Failed to get the bridge network's address: %v", err)
 	}
 
-	if !addr4.IP.Equal(ip) {
+	if !addrs4[0].IP.Equal(ip) {
 		t.Fatal("Creating bridge network with existing bridge interface unexpectedly modified the IP address of the bridge")
 	}
 

libnetwork/drivers/bridge/interface.go

@@ -52,23 +52,22 @@ func (i *bridgeInterface) exists() bool {
 	return i.Link != nil
 }
 
-// addresses returns a single IPv4 address and all IPv6 addresses for the
-// bridge interface.
-func (i *bridgeInterface) addresses() (netlink.Addr, []netlink.Addr, error) {
+// addresses returns all IPv4 addresses and all IPv6 addresses for the bridge interface.
+func (i *bridgeInterface) addresses() ([]netlink.Addr, []netlink.Addr, error) {
 	v4addr, err := i.nlh.AddrList(i.Link, netlink.FAMILY_V4)
 	if err != nil {
-		return netlink.Addr{}, nil, fmt.Errorf("Failed to retrieve V4 addresses: %v", err)
+		return nil, nil, fmt.Errorf("Failed to retrieve V4 addresses: %v", err)
 	}
 
 	v6addr, err := i.nlh.AddrList(i.Link, netlink.FAMILY_V6)
 	if err != nil {
-		return netlink.Addr{}, nil, fmt.Errorf("Failed to retrieve V6 addresses: %v", err)
+		return nil, nil, fmt.Errorf("Failed to retrieve V6 addresses: %v", err)
 	}
 
 	if len(v4addr) == 0 {
-		return netlink.Addr{}, v6addr, nil
+		return nil, v6addr, nil
 	}
-	return v4addr[0], v6addr, nil
+	return v4addr, v6addr, nil
 }
 
 func (i *bridgeInterface) programIPv6Address() error {

libnetwork/drivers/bridge/interface_test.go

@@ -37,12 +37,12 @@ func TestAddressesEmptyInterface(t *testing.T) {
 		t.Fatalf("newInterface() failed: %v", err)
 	}
 
-	addrv4, addrsv6, err := inf.addresses()
+	addrsv4, addrsv6, err := inf.addresses()
 	if err != nil {
 		t.Fatalf("Failed to get addresses of default interface: %v", err)
 	}
-	if expected := (netlink.Addr{}); addrv4 != expected {
-		t.Fatalf("Default interface has unexpected IPv4: %s", addrv4)
+	if len(addrsv4) != 0 {
+		t.Fatalf("Default interface has unexpected IPv4: %s", addrsv4)
 	}
 	if len(addrsv6) != 0 {
 		t.Fatalf("Default interface has unexpected IPv6: %v", addrsv6)

libnetwork/drivers/bridge/setup_ipv4.go

@@ -3,6 +3,7 @@ package bridge
 import (
 	"fmt"
 	"io/ioutil"
+	"net"
 	"path/filepath"
 
 	log "github.com/Sirupsen/logrus"
@@ -10,12 +11,28 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
+func selectIPv4Address(addresses []netlink.Addr, selector *net.IPNet) (netlink.Addr, error) {
+	if len(addresses) == 0 {
+		return netlink.Addr{}, fmt.Errorf("unable to select an address as the address pool is empty")
+	}
+	if selector != nil {
+		for _, addr := range addresses {
+			if selector.Contains(addr.IP) {
+				return addr, nil
+			}
+		}
+	}
+	return addresses[0], nil
+}
+
 func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error {
-	addrv4, _, err := i.addresses()
+	addrv4List, _, err := i.addresses()
 	if err != nil {
 		return fmt.Errorf("failed to retrieve bridge interface addresses: %v", err)
 	}
 
+	addrv4, _ := selectIPv4Address(addrv4List, config.AddressIPv4)
+
 	if !types.CompareIPNet(addrv4.IPNet, config.AddressIPv4) {
 		if addrv4.IPNet != nil {
 			if err := i.nlh.AddrDel(i.Link, &addrv4); err != nil {

libnetwork/drivers/bridge/setup_verify.go

@@ -11,12 +11,14 @@ import (
 )
 
 func setupVerifyAndReconcile(config *networkConfiguration, i *bridgeInterface) error {
-	// Fetch a single IPv4 and a slice of IPv6 addresses from the bridge.
-	addrv4, addrsv6, err := i.addresses()
+	// Fetch a slice of IPv4 addresses and a slice of IPv6 addresses from the bridge.
+	addrsv4, addrsv6, err := i.addresses()
 	if err != nil {
 		return fmt.Errorf("Failed to verify ip addresses: %v", err)
 	}
 
+	addrv4, _ := selectIPv4Address(addrsv4, config.AddressIPv4)
+
 	// Verify that the bridge does have an IPv4 address.
 	if addrv4.IPNet == nil {
 		return &ErrNoIPAddr{}

libnetwork/netutils/utils_freebsd.go

@@ -7,10 +7,12 @@ import (
 )
 
 // ElectInterfaceAddresses looks for an interface on the OS with the specified name
-// and returns its IPv4 and IPv6 addresses in CIDR form. If the interface does not exist,
-// it chooses from a predifined list the first IPv4 address which does not conflict
-// with other interfaces on the system.
-func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
+// and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
+// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
+// If the interface does not exist, it chooses from a predefined
+// list the first IPv4 address which does not conflict with other
+// interfaces on the system.
+func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
 	return nil, nil, types.NotImplementedErrorf("not supported on freebsd")
 }
 

libnetwork/netutils/utils_linux.go

@@ -62,15 +62,15 @@ func GenerateIfaceName(nlh *netlink.Handle, prefix string, len int) (string, err
 }
 
 // ElectInterfaceAddresses looks for an interface on the OS with the
-// specified name and returns its IPv4 and IPv6 addresses in CIDR
-// form. If the interface does not exist, it chooses from a predefined
+// specified name and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
+// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
+// If the interface does not exist, it chooses from a predefined
 // list the first IPv4 address which does not conflict with other
 // interfaces on the system.
-func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
+func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
 	var (
-		v4Net  *net.IPNet
+		v4Nets []*net.IPNet
 		v6Nets []*net.IPNet
-		err    error
 	)
 
 	defer osl.InitOSContext()()
@@ -85,23 +85,24 @@ func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
 		if err != nil {
 			return nil, nil, err
 		}
-		if len(v4addr) > 0 {
-			v4Net = v4addr[0].IPNet
+		for _, nlAddr := range v4addr {
+			v4Nets = append(v4Nets, nlAddr.IPNet)
 		}
 		for _, nlAddr := range v6addr {
 			v6Nets = append(v6Nets, nlAddr.IPNet)
 		}
 	}
 
-	if link == nil || v4Net == nil {
+	if link == nil || len(v4Nets) == 0 {
 		// Choose from predefined broad networks
-		v4Net, err = FindAvailableNetwork(ipamutils.PredefinedBroadNetworks)
+		v4Net, err := FindAvailableNetwork(ipamutils.PredefinedBroadNetworks)
 		if err != nil {
 			return nil, nil, err
 		}
+		v4Nets = append(v4Nets, v4Net)
 	}
 
-	return v4Net, v6Nets, nil
+	return v4Nets, v6Nets, nil
 }
 
 // FindAvailableNetwork returns a network from the passed list which does not

libnetwork/netutils/utils_solaris.go

@@ -22,10 +22,12 @@ func CheckRouteOverlaps(toCheck *net.IPNet) error {
 }
 
 // ElectInterfaceAddresses looks for an interface on the OS with the specified name
-// and returns its IPv4 and IPv6 addresses in CIDR form. If the interface does not exist,
-// it chooses from a predifined list the first IPv4 address which does not conflict
-// with other interfaces on the system.
-func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
+// and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
+// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
+// If the interface does not exist, it chooses from a predefined
+// list the first IPv4 address which does not conflict with other
+// interfaces on the system.
+func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
 	var (
 		v4Net *net.IPNet
 	)
@@ -63,7 +65,7 @@ func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
 			return nil, nil, err
 		}
 	}
-	return v4Net, nil, nil
+	return []*net.IPNet{v4Net}, nil, nil
 }
 
 // FindAvailableNetwork returns a network from the passed list which does not

libnetwork/netutils/utils_test.go

@@ -5,6 +5,7 @@ package netutils
 import (
 	"bytes"
 	"net"
+	"sort"
 	"testing"
 
 	"github.com/docker/libnetwork/ipamutils"
@@ -265,6 +266,43 @@ func TestNetworkRequest(t *testing.T) {
 	}
 }
 
+func TestElectInterfaceAddressMultipleAddresses(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	ipamutils.InitNetworks()
+
+	nws := []string{"172.101.202.254/16", "172.102.202.254/16"}
+	createInterface(t, "test", nws...)
+
+	ipv4NwList, ipv6NwList, err := ElectInterfaceAddresses("test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(ipv4NwList) == 0 {
+		t.Fatalf("unexpected empty ipv4 network addresses")
+	}
+
+	if len(ipv6NwList) == 0 {
+		t.Fatalf("unexpected empty ipv6 network addresses")
+	}
+
+	nwList := []string{}
+	for _, ipv4Nw := range ipv4NwList {
+		nwList = append(nwList, ipv4Nw.String())
+	}
+	sort.Strings(nws)
+	sort.Strings(nwList)
+
+	if len(nws) != len(nwList) {
+		t.Fatalf("expected %v. got %v", nws, nwList)
+	}
+	for i, nw := range nws {
+		if nw != nwList[i] {
+			t.Fatalf("expected %v. got %v", nw, nwList[i])
+		}
+	}
+}
+
 func TestElectInterfaceAddress(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
 	ipamutils.InitNetworks()
@@ -277,37 +315,43 @@ func TestElectInterfaceAddress(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if ipv4Nw == nil {
+	if len(ipv4Nw) == 0 {
 		t.Fatalf("unexpected empty ipv4 network addresses")
 	}
 
 	if len(ipv6Nw) == 0 {
-		t.Fatalf("unexpected empty ipv4 network addresses")
+		t.Fatalf("unexpected empty ipv6 network addresses")
 	}
 
-	if nws != ipv4Nw.String() {
-		t.Fatalf("expected %s. got %s", nws, ipv4Nw)
+	if nws != ipv4Nw[0].String() {
+		t.Fatalf("expected %s. got %s", nws, ipv4Nw[0])
 	}
 }
 
-func createInterface(t *testing.T, name, nw string) {
+func createInterface(t *testing.T, name string, nws ...string) {
 	// Add interface
 	link := &netlink.Bridge{
 		LinkAttrs: netlink.LinkAttrs{
 			Name: "test",
 		},
 	}
-	bip, err := types.ParseCIDR(nw)
-	if err != nil {
-		t.Fatal(err)
+	bips := []*net.IPNet{}
+	for _, nw := range nws {
+		bip, err := types.ParseCIDR(nw)
+		if err != nil {
+			t.Fatal(err)
+		}
+		bips = append(bips, bip)
 	}
-	if err = netlink.LinkAdd(link); err != nil {
+	if err := netlink.LinkAdd(link); err != nil {
 		t.Fatalf("Failed to create interface via netlink: %v", err)
 	}
-	if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
-		t.Fatal(err)
+	for _, bip := range bips {
+		if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
+			t.Fatal(err)
+		}
 	}
-	if err = netlink.LinkSetUp(link); err != nil {
+	if err := netlink.LinkSetUp(link); err != nil {
 		t.Fatal(err)
 	}
 }

libnetwork/netutils/utils_windows.go

@@ -7,10 +7,12 @@ import (
 )
 
 // ElectInterfaceAddresses looks for an interface on the OS with the specified name
-// and returns its IPv4 and IPv6 addresses in CIDR form. If the interface does not exist,
-// it chooses from a predifined list the first IPv4 address which does not conflict
-// with other interfaces on the system.
-func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
+// and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
+// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
+// If the interface does not exist, it chooses from a predefined
+// list the first IPv4 address which does not conflict with other
+// interfaces on the system.
+func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
 	return nil, nil, types.NotImplementedErrorf("not supported on windows")
 }