Change ip allocation logic

Now IP reuses only after all IPs from network was allocated
Fixes #5729

Docker-DCO-1.1-Signed-off-by: Alexandr Morozov <lk4d4math@gmail.com> (github: LK4D4)

daemon/networkdriver/ipallocator/allocator.go

@@ -7,9 +7,19 @@ import (
 	"github.com/dotcloud/docker/pkg/collections"
 	"net"
 	"sync"
+	"sync/atomic"
 )
 
-type networkSet map[string]*collections.OrderedIntSet
+type allocatedMap struct {
+	*collections.OrderedIntSet
+	last int32
+}
+
+func newAllocatedMap() *allocatedMap {
+	return &allocatedMap{OrderedIntSet: collections.NewOrderedIntSet()}
+}
+
+type networkSet map[string]*allocatedMap
 
 var (
 	ErrNoAvailableIPs     = errors.New("no available ip addresses on network")
@@ -19,7 +29,6 @@ var (
 var (
 	lock         = sync.Mutex{}
 	allocatedIPs = networkSet{}
-	availableIPS = networkSet{}
 )
 
 // RequestIP requests an available ip from the given network.  It
@@ -55,13 +64,11 @@ func ReleaseIP(address *net.IPNet, ip *net.IP) error {
 	checkAddress(address)
 
 	var (
-		existing  = allocatedIPs[address.String()]
-		available = availableIPS[address.String()]
+		allocated = allocatedIPs[address.String()]
 		pos       = getPosition(address, ip)
 	)
 
-	existing.Remove(int(pos))
-	available.Push(int(pos))
+	allocated.Remove(int(pos))
 
 	return nil
 }
@@ -82,29 +89,19 @@ func getPosition(address *net.IPNet, ip *net.IP) int32 {
 func getNextIp(address *net.IPNet) (*net.IP, error) {
 	var (
 		ownIP     = ipToInt(&address.IP)
-		available = availableIPS[address.String()]
 		allocated = allocatedIPs[address.String()]
 		first, _  = networkdriver.NetworkRange(address)
 		base      = ipToInt(&first)
 		size      = int(networkdriver.NetworkSize(address.Mask))
 		max       = int32(size - 2) // size -1 for the broadcast address, -1 for the gateway address
-		pos       = int32(available.Pop())
+		pos       = atomic.LoadInt32(&allocated.last)
 	)
 
-	// We pop and push the position not the ip
-	if pos != 0 {
-		ip := intToIP(int32(base + pos))
-		allocated.Push(int(pos))
-
-		return ip, nil
-	}
-
 	var (
 		firstNetIP = address.IP.To4().Mask(address.Mask)
 		firstAsInt = ipToInt(&firstNetIP) + 1
 	)
 
-	pos = int32(allocated.PullBack())
 	for i := int32(0); i < max; i++ {
 		pos = pos%max + 1
 		next := int32(base + pos)
@@ -116,6 +113,7 @@ func getNextIp(address *net.IPNet) (*net.IP, error) {
 		if !allocated.Exists(int(pos)) {
 			ip := intToIP(next)
 			allocated.Push(int(pos))
+			atomic.StoreInt32(&allocated.last, pos)
 			return ip, nil
 		}
 	}
@@ -124,15 +122,14 @@ func getNextIp(address *net.IPNet) (*net.IP, error) {
 
 func registerIP(address *net.IPNet, ip *net.IP) error {
 	var (
-		existing  = allocatedIPs[address.String()]
-		available = availableIPS[address.String()]
+		allocated = allocatedIPs[address.String()]
 		pos       = getPosition(address, ip)
 	)
 
-	if existing.Exists(int(pos)) {
+	if allocated.Exists(int(pos)) {
 		return ErrIPAlreadyAllocated
 	}
-	available.Remove(int(pos))
+	atomic.StoreInt32(&allocated.last, pos)
 
 	return nil
 }
@@ -153,7 +150,6 @@ func intToIP(n int32) *net.IP {
 func checkAddress(address *net.IPNet) {
 	key := address.String()
 	if _, exists := allocatedIPs[key]; !exists {
-		allocatedIPs[key] = collections.NewOrderedIntSet()
-		availableIPS[key] = collections.NewOrderedIntSet()
+		allocatedIPs[key] = newAllocatedMap()
 	}
 }

daemon/networkdriver/ipallocator/allocator_test.go

@@ -8,7 +8,6 @@ import (
 
 func reset() {
 	allocatedIPs = networkSet{}
-	availableIPS = networkSet{}
 }
 
 func TestRequestNewIps(t *testing.T) {
@@ -18,8 +17,10 @@ func TestRequestNewIps(t *testing.T) {
 		Mask: []byte{255, 255, 255, 0},
 	}
 
+	var ip *net.IP
+	var err error
 	for i := 2; i < 10; i++ {
-		ip, err := RequestIP(network, nil)
+		ip, err = RequestIP(network, nil)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -28,6 +29,17 @@ func TestRequestNewIps(t *testing.T) {
 			t.Fatalf("Expected ip %s got %s", expected, ip.String())
 		}
 	}
+	value := intToIP(ipToInt(ip) + 1).String()
+	if err := ReleaseIP(network, ip); err != nil {
+		t.Fatal(err)
+	}
+	ip, err = RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if ip.String() != value {
+		t.Fatalf("Expected to receive the next ip %s got %s", value, ip.String())
+	}
 }
 
 func TestReleaseIp(t *testing.T) {
@@ -64,6 +76,17 @@ func TestGetReleasedIp(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	for i := 0; i < 252; i++ {
+		_, err = RequestIP(network, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = ReleaseIP(network, ip)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
 	ip, err = RequestIP(network, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -185,24 +208,6 @@ func TestIPAllocator(t *testing.T) {
 
 		newIPs[i] = ip
 	}
-	// Before loop begin
-	// 2(u) - 3(u) - 4(f) - 5(f) - 6(f)
-	//                       ↑
-
-	// After i = 0
-	// 2(u) - 3(u) - 4(f) - 5(u) - 6(f)
-	//                              ↑
-
-	// After i = 1
-	// 2(u) - 3(u) - 4(f) - 5(u) - 6(u)
-	//                ↑
-
-	// After i = 2
-	// 2(u) - 3(u) - 4(u) - 5(u) - 6(u)
-	//                       ↑
-
-	// Reordered these because the new set will always return the
-	// lowest ips first and not in the order that they were released
 	assertIPEquals(t, &expectedIPs[2], newIPs[0])
 	assertIPEquals(t, &expectedIPs[3], newIPs[1])
 	assertIPEquals(t, &expectedIPs[4], newIPs[2])
@@ -234,6 +239,86 @@ func TestAllocateFirstIP(t *testing.T) {
 	}
 }
 
+func TestAllocateAllIps(t *testing.T) {
+	defer reset()
+	network := &net.IPNet{
+		IP:   []byte{192, 168, 0, 1},
+		Mask: []byte{255, 255, 255, 0},
+	}
+
+	var (
+		current, first *net.IP
+		err            error
+		isFirst        = true
+	)
+
+	for err == nil {
+		current, err = RequestIP(network, nil)
+		if isFirst {
+			first = current
+			isFirst = false
+		}
+	}
+
+	if err != ErrNoAvailableIPs {
+		t.Fatal(err)
+	}
+
+	if _, err := RequestIP(network, nil); err != ErrNoAvailableIPs {
+		t.Fatal(err)
+	}
+
+	if err := ReleaseIP(network, first); err != nil {
+		t.Fatal(err)
+	}
+
+	again, err := RequestIP(network, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertIPEquals(t, first, again)
+}
+
+func TestAllocateDifferentSubnets(t *testing.T) {
+	defer reset()
+	network1 := &net.IPNet{
+		IP:   []byte{192, 168, 0, 1},
+		Mask: []byte{255, 255, 255, 0},
+	}
+	network2 := &net.IPNet{
+		IP:   []byte{127, 0, 0, 1},
+		Mask: []byte{255, 255, 255, 0},
+	}
+	expectedIPs := []net.IP{
+		0: net.IPv4(192, 168, 0, 2),
+		1: net.IPv4(192, 168, 0, 3),
+		2: net.IPv4(127, 0, 0, 2),
+		3: net.IPv4(127, 0, 0, 3),
+	}
+
+	ip11, err := RequestIP(network1, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ip12, err := RequestIP(network1, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ip21, err := RequestIP(network2, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ip22, err := RequestIP(network2, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertIPEquals(t, &expectedIPs[0], ip11)
+	assertIPEquals(t, &expectedIPs[1], ip12)
+	assertIPEquals(t, &expectedIPs[2], ip21)
+	assertIPEquals(t, &expectedIPs[3], ip22)
+}
+
 func assertIPEquals(t *testing.T, ip1, ip2 *net.IP) {
 	if !ip1.Equal(*ip2) {
 		t.Fatalf("Expected IP %s, got %s", ip1, ip2)