Merge pull request #8699 from shuai-z/fix-portalloc

Fix corner cases in ipallocator and portallocator

daemon/networkdriver/ipallocator/allocator.go

@@ -129,7 +129,10 @@ func (allocated *allocatedMap) checkIP(ip net.IP) (net.IP, error) {
 // return an available ip if one is currently available.  If not,
 // return the next available ip for the nextwork
 func (allocated *allocatedMap) getNextIP() (net.IP, error) {
-	for pos := big.NewInt(0).Add(allocated.last, big.NewInt(1)); pos.Cmp(allocated.last) != 0; pos.Add(pos, big.NewInt(1)) {
+	pos := big.NewInt(0).Set(allocated.last)
+	allRange := big.NewInt(0).Sub(allocated.end, allocated.begin)
+	for i := big.NewInt(0); i.Cmp(allRange) <= 0; i.Add(i, big.NewInt(1)) {
+		pos.Add(pos, big.NewInt(1))
 		if pos.Cmp(allocated.end) == 1 {
 			pos.Set(allocated.begin)
 		}

daemon/networkdriver/ipallocator/allocator_test.go

@@ -432,6 +432,65 @@ func TestAllocateAllIps(t *testing.T) {
 	}
 
 	assertIPEquals(t, first, again)
+
+	// ensure that alloc.last == alloc.begin won't result in dead loop
+	if _, err := RequestIP(network, nil); err != ErrNoAvailableIPs {
+		t.Fatal(err)
+	}
+
+	// Test by making alloc.last the only free ip and ensure we get it back
+	// #1. first of the range, (alloc.last == ipToInt(first) already)
+	if err := ReleaseIP(network, first); err != nil {
+		t.Fatal(err)
+	}
+
+	ret, err := RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, first, ret)
+
+	// #2. last of the range, note that current is the last one
+	last := net.IPv4(192, 168, 0, 254)
+	setLastTo(t, network, last)
+
+	ret, err = RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, last, ret)
+
+	// #3. middle of the range
+	mid := net.IPv4(192, 168, 0, 7)
+	setLastTo(t, network, mid)
+
+	ret, err = RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, mid, ret)
+}
+
+// make sure the pool is full when calling setLastTo.
+// we don't cheat here
+func setLastTo(t *testing.T, network *net.IPNet, ip net.IP) {
+	if err := ReleaseIP(network, ip); err != nil {
+		t.Fatal(err)
+	}
+
+	ret, err := RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, ip, ret)
+
+	if err := ReleaseIP(network, ip); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestAllocateDifferentSubnets(t *testing.T) {

daemon/networkdriver/portallocator/portallocator.go

@@ -136,7 +136,9 @@ func ReleaseAll() error {
 }
 
 func (pm *portMap) findPort() (int, error) {
-	for port := pm.last + 1; port != pm.last; port++ {
+	port := pm.last
+	for i := 0; i <= EndPortRange-BeginPortRange; i++ {
+		port++
 		if port > EndPortRange {
 			port = BeginPortRange
 		}

daemon/networkdriver/portallocator/portallocator_test.go

@@ -134,6 +134,19 @@ func TestAllocateAllPorts(t *testing.T) {
 	if newPort != port {
 		t.Fatalf("Expected port %d got %d", port, newPort)
 	}
+
+	// now pm.last == newPort, release it so that it's the only free port of
+	// the range, and ensure we get it back
+	if err := ReleasePort(defaultIP, "tcp", newPort); err != nil {
+		t.Fatal(err)
+	}
+	port, err = RequestPort(defaultIP, "tcp", 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if newPort != port {
+		t.Fatalf("Expected port %d got %d", newPort, port)
+	}
 }
 
 func BenchmarkAllocatePorts(b *testing.B) {