Skip to content

Commit 53bf987

Browse files
yongtangyongtang
committed
Fix issue for --fixed-cidr when bridge has multiple addresses
This fix tries to address the issue raised in: moby#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>
1 parent 0b2dd7c commit 53bf987

File tree

11 files changed

+126
-57
lines changed

11 files changed

+126
-57
lines changed

‎libnetwork/cmd/dnet/dnet.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -507,11 +507,11 @@ func encodeData(data interface{}) (*bytes.Buffer, error) {
507507
}
508508

509509
func ipamOption(bridgeName string) libnetwork.NetworkOption {
510-
if nw, _, err := netutils.ElectInterfaceAddresses(bridgeName); err == nil {
511-
ipamV4Conf := &libnetwork.IpamConf{PreferredPool: nw.String()}
512-
hip, _ := types.GetHostPartIP(nw.IP, nw.Mask)
510+
if nws, _, err := netutils.ElectInterfaceAddresses(bridgeName); err == nil {
511+
ipamV4Conf := &libnetwork.IpamConf{PreferredPool: nws[0].String()}
512+
hip, _ := types.GetHostPartIP(nws[0].IP, nws[0].Mask)
513513
if hip.IsGlobalUnicast() {
514-
ipamV4Conf.Gateway = nw.IP.String()
514+
ipamV4Conf.Gateway = nws[0].IP.String()
515515
}
516516
return libnetwork.NetworkOptionIpam("default", "", []*libnetwork.IpamConf{ipamV4Conf}, nil, nil)
517517
}

‎libnetwork/drivers/bridge/bridge_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,13 @@ func compareBindings(a, b []types.PortBinding) bool {
169169

170170
func getIPv4Data(t *testing.T, iface string) []driverapi.IPAMData {
171171
ipd := driverapi.IPAMData{AddressSpace: "full"}
172-
nw, _, err := netutils.ElectInterfaceAddresses(iface)
172+
nws, _, err := netutils.ElectInterfaceAddresses(iface)
173173
if err != nil {
174174
t.Fatal(err)
175175
}
176-
ipd.Pool = nw
176+
ipd.Pool = nws[0]
177177
// Set network gateway to X.X.X.1
178-
ipd.Gateway = types.GetIPNetCopy(nw)
178+
ipd.Gateway = types.GetIPNetCopy(nws[0])
179179
ipd.Gateway.IP[len(ipd.Gateway.IP)-1] = 1
180180
return []driverapi.IPAMData{ipd}
181181
}
@@ -1054,12 +1054,12 @@ func TestCreateWithExistingBridge(t *testing.T) {
10541054
t.Fatalf("Failed to getNetwork(%s): %v", brName, err)
10551055
}
10561056

1057-
addr4, _, err := nw.bridge.addresses()
1057+
addrs4, _, err := nw.bridge.addresses()
10581058
if err != nil {
10591059
t.Fatalf("Failed to get the bridge network's address: %v", err)
10601060
}
10611061

1062-
if !addr4.IP.Equal(ip) {
1062+
if !addrs4[0].IP.Equal(ip) {
10631063
t.Fatal("Creating bridge network with existing bridge interface unexpectedly modified the IP address of the bridge")
10641064
}
10651065

‎libnetwork/drivers/bridge/interface.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,22 @@ func (i *bridgeInterface) exists() bool {
5252
return i.Link != nil
5353
}
5454

55-
// addresses returns a single IPv4 address and all IPv6 addresses for the
56-
// bridge interface.
57-
func (i *bridgeInterface) addresses() (netlink.Addr, []netlink.Addr, error) {
55+
// addresses returns all IPv4 addresses and all IPv6 addresses for the bridge interface.
56+
func (i *bridgeInterface) addresses() ([]netlink.Addr, []netlink.Addr, error) {
5857
v4addr, err := i.nlh.AddrList(i.Link, netlink.FAMILY_V4)
5958
if err != nil {
60-
return netlink.Addr{}, nil, fmt.Errorf("Failed to retrieve V4 addresses: %v", err)
59+
return nil, nil, fmt.Errorf("Failed to retrieve V4 addresses: %v", err)
6160
}
6261

6362
v6addr, err := i.nlh.AddrList(i.Link, netlink.FAMILY_V6)
6463
if err != nil {
65-
return netlink.Addr{}, nil, fmt.Errorf("Failed to retrieve V6 addresses: %v", err)
64+
return nil, nil, fmt.Errorf("Failed to retrieve V6 addresses: %v", err)
6665
}
6766

6867
if len(v4addr) == 0 {
69-
return netlink.Addr{}, v6addr, nil
68+
return nil, v6addr, nil
7069
}
71-
return v4addr[0], v6addr, nil
70+
return v4addr, v6addr, nil
7271
}
7372

7473
func (i *bridgeInterface) programIPv6Address() error {

‎libnetwork/drivers/bridge/interface_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ func TestAddressesEmptyInterface(t *testing.T) {
3737
t.Fatalf("newInterface() failed: %v", err)
3838
}
3939

40-
addrv4, addrsv6, err := inf.addresses()
40+
addrsv4, addrsv6, err := inf.addresses()
4141
if err != nil {
4242
t.Fatalf("Failed to get addresses of default interface: %v", err)
4343
}
44-
if expected := (netlink.Addr{}); addrv4 != expected {
45-
t.Fatalf("Default interface has unexpected IPv4: %s", addrv4)
44+
if len(addrsv4) != 0 {
45+
t.Fatalf("Default interface has unexpected IPv4: %s", addrsv4)
4646
}
4747
if len(addrsv6) != 0 {
4848
t.Fatalf("Default interface has unexpected IPv6: %v", addrsv6)

‎libnetwork/drivers/bridge/setup_ipv4.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,36 @@ package bridge
33
import (
44
"fmt"
55
"io/ioutil"
6+
"net"
67
"path/filepath"
78

89
log "github.com/Sirupsen/logrus"
910
"github.com/docker/libnetwork/types"
1011
"github.com/vishvananda/netlink"
1112
)
1213

14+
func selectIPv4Address(addresses []netlink.Addr, selector *net.IPNet) (netlink.Addr, error) {
15+
if len(addresses) == 0 {
16+
return netlink.Addr{}, fmt.Errorf("unable to select an address as the address pool is empty")
17+
}
18+
if selector != nil {
19+
for _, addr := range addresses {
20+
if selector.Contains(addr.IP) {
21+
return addr, nil
22+
}
23+
}
24+
}
25+
return addresses[0], nil
26+
}
27+
1328
func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error {
14-
addrv4, _, err := i.addresses()
29+
addrv4List, _, err := i.addresses()
1530
if err != nil {
1631
return fmt.Errorf("failed to retrieve bridge interface addresses: %v", err)
1732
}
1833

34+
addrv4, _ := selectIPv4Address(addrv4List, config.AddressIPv4)
35+
1936
if !types.CompareIPNet(addrv4.IPNet, config.AddressIPv4) {
2037
if addrv4.IPNet != nil {
2138
if err := i.nlh.AddrDel(i.Link, &addrv4); err != nil {

‎libnetwork/drivers/bridge/setup_verify.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ import (
1111
)
1212

1313
func setupVerifyAndReconcile(config *networkConfiguration, i *bridgeInterface) error {
14-
// Fetch a single IPv4 and a slice of IPv6 addresses from the bridge.
15-
addrv4, addrsv6, err := i.addresses()
14+
// Fetch a slice of IPv4 addresses and a slice of IPv6 addresses from the bridge.
15+
addrsv4, addrsv6, err := i.addresses()
1616
if err != nil {
1717
return fmt.Errorf("Failed to verify ip addresses: %v", err)
1818
}
1919

20+
addrv4, _ := selectIPv4Address(addrsv4, config.AddressIPv4)
21+
2022
// Verify that the bridge does have an IPv4 address.
2123
if addrv4.IPNet == nil {
2224
return &ErrNoIPAddr{}

‎libnetwork/netutils/utils_freebsd.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import (
77
)
88

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

‎libnetwork/netutils/utils_linux.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,15 @@ func GenerateIfaceName(nlh *netlink.Handle, prefix string, len int) (string, err
6262
}
6363

6464
// ElectInterfaceAddresses looks for an interface on the OS with the
65-
// specified name and returns its IPv4 and IPv6 addresses in CIDR
66-
// form. If the interface does not exist, it chooses from a predefined
65+
// specified name and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
66+
// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
67+
// If the interface does not exist, it chooses from a predefined
6768
// list the first IPv4 address which does not conflict with other
6869
// interfaces on the system.
69-
func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
70+
func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
7071
var (
71-
v4Net *net.IPNet
72+
v4Nets []*net.IPNet
7273
v6Nets []*net.IPNet
73-
err error
7474
)
7575

7676
defer osl.InitOSContext()()
@@ -85,23 +85,24 @@ func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
8585
if err != nil {
8686
return nil, nil, err
8787
}
88-
if len(v4addr) > 0 {
89-
v4Net = v4addr[0].IPNet
88+
for _, nlAddr := range v4addr {
89+
v4Nets = append(v4Nets, nlAddr.IPNet)
9090
}
9191
for _, nlAddr := range v6addr {
9292
v6Nets = append(v6Nets, nlAddr.IPNet)
9393
}
9494
}
9595

96-
if link == nil || v4Net == nil {
96+
if link == nil || len(v4Nets) == 0 {
9797
// Choose from predefined broad networks
98-
v4Net, err = FindAvailableNetwork(ipamutils.PredefinedBroadNetworks)
98+
v4Net, err := FindAvailableNetwork(ipamutils.PredefinedBroadNetworks)
9999
if err != nil {
100100
return nil, nil, err
101101
}
102+
v4Nets = append(v4Nets, v4Net)
102103
}
103104

104-
return v4Net, v6Nets, nil
105+
return v4Nets, v6Nets, nil
105106
}
106107

107108
// FindAvailableNetwork returns a network from the passed list which does not

‎libnetwork/netutils/utils_solaris.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@ func CheckRouteOverlaps(toCheck *net.IPNet) error {
2222
}
2323

2424
// ElectInterfaceAddresses looks for an interface on the OS with the specified name
25-
// and returns its IPv4 and IPv6 addresses in CIDR form. If the interface does not exist,
26-
// it chooses from a predifined list the first IPv4 address which does not conflict
27-
// with other interfaces on the system.
28-
func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
25+
// and returns returns all its IPv4 and IPv6 addresses in CIDR notation.
26+
// If a failure in retrieving the addresses or no IPv4 address is found, an error is returned.
27+
// If the interface does not exist, it chooses from a predefined
28+
// list the first IPv4 address which does not conflict with other
29+
// interfaces on the system.
30+
func ElectInterfaceAddresses(name string) ([]*net.IPNet, []*net.IPNet, error) {
2931
var (
3032
v4Net *net.IPNet
3133
)
@@ -63,7 +65,7 @@ func ElectInterfaceAddresses(name string) (*net.IPNet, []*net.IPNet, error) {
6365
return nil, nil, err
6466
}
6567
}
66-
return v4Net, nil, nil
68+
return []*net.IPNet{v4Net}, nil, nil
6769
}
6870

6971
// FindAvailableNetwork returns a network from the passed list which does not

‎libnetwork/netutils/utils_test.go‎

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package netutils
55
import (
66
"bytes"
77
"net"
8+
"sort"
89
"testing"
910

1011
"github.com/docker/libnetwork/ipamutils"
@@ -265,6 +266,43 @@ func TestNetworkRequest(t *testing.T) {
265266
}
266267
}
267268

269+
func TestElectInterfaceAddressMultipleAddresses(t *testing.T) {
270+
defer testutils.SetupTestOSContext(t)()
271+
ipamutils.InitNetworks()
272+
273+
nws := []string{"172.101.202.254/16", "172.102.202.254/16"}
274+
createInterface(t, "test", nws...)
275+
276+
ipv4NwList, ipv6NwList, err := ElectInterfaceAddresses("test")
277+
if err != nil {
278+
t.Fatal(err)
279+
}
280+
281+
if len(ipv4NwList) == 0 {
282+
t.Fatalf("unexpected empty ipv4 network addresses")
283+
}
284+
285+
if len(ipv6NwList) == 0 {
286+
t.Fatalf("unexpected empty ipv6 network addresses")
287+
}
288+
289+
nwList := []string{}
290+
for _, ipv4Nw := range ipv4NwList {
291+
nwList = append(nwList, ipv4Nw.String())
292+
}
293+
sort.Strings(nws)
294+
sort.Strings(nwList)
295+
296+
if len(nws) != len(nwList) {
297+
t.Fatalf("expected %v. got %v", nws, nwList)
298+
}
299+
for i, nw := range nws {
300+
if nw != nwList[i] {
301+
t.Fatalf("expected %v. got %v", nw, nwList[i])
302+
}
303+
}
304+
}
305+
268306
func TestElectInterfaceAddress(t *testing.T) {
269307
defer testutils.SetupTestOSContext(t)()
270308
ipamutils.InitNetworks()
@@ -277,37 +315,43 @@ func TestElectInterfaceAddress(t *testing.T) {
277315
t.Fatal(err)
278316
}
279317

280-
if ipv4Nw == nil {
318+
if len(ipv4Nw) == 0 {
281319
t.Fatalf("unexpected empty ipv4 network addresses")
282320
}
283321

284322
if len(ipv6Nw) == 0 {
285-
t.Fatalf("unexpected empty ipv4 network addresses")
323+
t.Fatalf("unexpected empty ipv6 network addresses")
286324
}
287325

288-
if nws != ipv4Nw.String() {
289-
t.Fatalf("expected %s. got %s", nws, ipv4Nw)
326+
if nws != ipv4Nw[0].String() {
327+
t.Fatalf("expected %s. got %s", nws, ipv4Nw[0])
290328
}
291329
}
292330

293-
func createInterface(t *testing.T, name, nw string) {
331+
func createInterface(t *testing.T, name string, nws ...string) {
294332
// Add interface
295333
link := &netlink.Bridge{
296334
LinkAttrs: netlink.LinkAttrs{
297335
Name: "test",
298336
},
299337
}
300-
bip, err := types.ParseCIDR(nw)
301-
if err != nil {
302-
t.Fatal(err)
338+
bips := []*net.IPNet{}
339+
for _, nw := range nws {
340+
bip, err := types.ParseCIDR(nw)
341+
if err != nil {
342+
t.Fatal(err)
343+
}
344+
bips = append(bips, bip)
303345
}
304-
if err = netlink.LinkAdd(link); err != nil {
346+
if err := netlink.LinkAdd(link); err != nil {
305347
t.Fatalf("Failed to create interface via netlink: %v", err)
306348
}
307-
if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
308-
t.Fatal(err)
349+
for _, bip := range bips {
350+
if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
351+
t.Fatal(err)
352+
}
309353
}
310-
if err = netlink.LinkSetUp(link); err != nil {
354+
if err := netlink.LinkSetUp(link); err != nil {
311355
t.Fatal(err)
312356
}
313357
}

0 commit comments

Comments
 (0)