From cd3fbc2a95be1e233c9373a031be705edabb24d6 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Thu, 6 Aug 2015 15:45:38 -0700 Subject: [PATCH] Fixes ip allocation for multi bridge networks - Do not discard errors on ip allocation for gw and bridge - Release addresses on network delete - Add some context on top of ipallocator returned error - Create ip allocator instance at driver creation, not at package init, otherwise this affects bridge test code where ip db is carried over test functions Signed-off-by: Alessandro Boch --- libnetwork/drivers/bridge/bridge.go | 17 +++++++++++++---- libnetwork/drivers/bridge/bridge_test.go | 14 +++++++++++++- libnetwork/drivers/bridge/setup_ipv4.go | 8 +++++--- libnetwork/drivers/bridge/setup_ipv4_test.go | 15 +++++++++++++-- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index faf9eb60d7..24d75ae200 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -104,12 +104,9 @@ type driver struct { sync.Mutex } -func init() { - ipAllocator = ipallocator.New() -} - // New constructs a new bridge driver func newDriver() driverapi.Driver { + ipAllocator = ipallocator.New() return &driver{networks: map[string]*bridgeNetwork{}} } @@ -786,6 +783,18 @@ func (d *driver) DeleteNetwork(nid string) error { // Programming err = netlink.LinkDel(n.bridge.Link) + // Release ip addresses (ignore errors) + if config.FixedCIDR == nil || config.FixedCIDR.Contains(config.DefaultGatewayIPv4) { + if e := ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, n.bridge.gatewayIPv4); e != nil { + logrus.Warnf("Failed to release default gateway address %s: %v", n.bridge.gatewayIPv4.String(), e) + } + } + if config.FixedCIDR == nil || config.FixedCIDR.Contains(n.bridge.bridgeIPv4.IP) { + if e := ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, n.bridge.bridgeIPv4.IP); e != nil { + logrus.Warnf("Failed to release bridge IP %s: %v", n.bridge.bridgeIPv4.IP.String(), e) + } + } + return err } diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index 60f025700d..c58d50b4a8 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -637,7 +637,19 @@ func TestSetDefaultGw(t *testing.T) { } _, subnetv6, _ := net.ParseCIDR("2001:db8:ea9:9abc:b0c4::/80") - gw4 := bridgeNetworks[0].IP.To4() + + var nw *net.IPNet + for _, n := range bridgeNetworks { + if err := netutils.CheckRouteOverlaps(n); err == nil { + nw = n + break + } + } + if nw == nil { + t.Skipf("Skip as no more automatic networks available") + } + + gw4 := types.GetIPCopy(nw.IP).To4() gw4[3] = 254 gw6 := net.ParseIP("2001:db8:ea9:9abc:b0c4::254") diff --git a/libnetwork/drivers/bridge/setup_ipv4.go b/libnetwork/drivers/bridge/setup_ipv4.go index c72e71e9a6..85073932c4 100644 --- a/libnetwork/drivers/bridge/setup_ipv4.go +++ b/libnetwork/drivers/bridge/setup_ipv4.go @@ -62,7 +62,7 @@ func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error { return err } - log.Debugf("Creating bridge interface %q with network %s", config.BridgeName, bridgeIPv4) + log.Debugf("Creating bridge interface %s with network %s", config.BridgeName, bridgeIPv4) if err := netlink.AddrAdd(i.Link, &netlink.Addr{IPNet: bridgeIPv4}); err != nil { return &IPv4AddrAddError{IP: bridgeIPv4, Err: err} } @@ -79,7 +79,9 @@ func allocateBridgeIP(config *networkConfiguration, i *bridgeInterface) error { // 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) + if _, err := ipAllocator.RequestIP(i.bridgeIPv4, i.bridgeIPv4.IP); err != nil { + return fmt.Errorf("failed to reserve bridge IP %s: %v", i.bridgeIPv4.IP.String(), err) + } } return nil } @@ -120,7 +122,7 @@ func setupGatewayIPv4(config *networkConfiguration, i *bridgeInterface) error { // (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 + return fmt.Errorf("failed to reserve default gateway %s: %v", config.DefaultGatewayIPv4.String(), err) } } diff --git a/libnetwork/drivers/bridge/setup_ipv4_test.go b/libnetwork/drivers/bridge/setup_ipv4_test.go index e9f26d797e..e95d751ac8 100644 --- a/libnetwork/drivers/bridge/setup_ipv4_test.go +++ b/libnetwork/drivers/bridge/setup_ipv4_test.go @@ -54,6 +54,17 @@ func TestSetupBridgeIPv4Fixed(t *testing.T) { func TestSetupBridgeIPv4Auto(t *testing.T) { defer netutils.SetupTestNetNS(t)() + var toBeChosen *net.IPNet + for _, n := range bridgeNetworks { + if err := netutils.CheckRouteOverlaps(n); err == nil { + toBeChosen = n + break + } + } + if toBeChosen == nil { + t.Skipf("Skip as no more automatic networks available") + } + config, br := setupTestInterface(t) if err := setupBridgeIPv4(config, br); err != nil { t.Fatalf("Failed to setup bridge IPv4: %v", err) @@ -66,14 +77,14 @@ func TestSetupBridgeIPv4Auto(t *testing.T) { var found bool for _, addr := range addrsv4 { - if bridgeNetworks[0].String() == addr.IPNet.String() { + if toBeChosen.String() == addr.IPNet.String() { found = true break } } if !found { - t.Fatalf("Bridge device does not have the automatic IPv4 address %v", bridgeNetworks[0].String()) + t.Fatalf("Bridge device does not have the automatic IPv4 address %s", toBeChosen.String()) } }