Merge pull request #91606 from danwinship/service-ipallocator-cleanups

Service IPAllocator cleanups

pkg/registry/core/service/ipallocator/allocator.go

@@ -82,14 +82,28 @@ type Range struct {
 
 // NewAllocatorCIDRRange creates a Range over a net.IPNet, calling allocatorFactory to construct the backing store.
 func NewAllocatorCIDRRange(cidr *net.IPNet, allocatorFactory allocator.AllocatorFactory) (*Range, error) {
-	max := RangeSize(cidr)
+	max := utilnet.RangeSize(cidr)
-	base := bigForIP(cidr.IP)
+	base := utilnet.BigForIP(cidr.IP)
 	rangeSpec := cidr.String()
 
+	if utilnet.IsIPv6CIDR(cidr) {
+		// Limit the max size, since the allocator keeps a bitmap of that size.
+		if max > 65536 {
+			max = 65536
+		}
+	} else {
+		// Don't use the IPv4 network's broadcast address.
+		max--
+	}
+
+	// Don't use the network's ".0" address.
+	base.Add(base, big.NewInt(1))
+	max--
+
 	r := Range{
 		net:  cidr,
-		base: base.Add(base, big.NewInt(1)), // don't use the network base
+		base: base,
-		max:  maximum(0, int(max-2)),        // don't use the network broadcast,
+		max:  maximum(0, int(max)),
 	}
 	var err error
 	r.alloc, err = allocatorFactory(r.max, rangeSpec)
@@ -171,7 +185,7 @@ func (r *Range) AllocateNext() (net.IP, error) {
 	if !ok {
 		return nil, ErrFull
 	}
-	return addIPOffset(r.base, offset), nil
+	return utilnet.AddIPOffset(r.base, offset), nil
 }
 
 // Release releases the IP back to the pool. Releasing an
@@ -247,40 +261,8 @@ func (r *Range) contains(ip net.IP) (bool, int) {
 	return true, offset
 }
 
-// bigForIP creates a big.Int based on the provided net.IP
-func bigForIP(ip net.IP) *big.Int {
-	b := ip.To4()
-	if b == nil {
-		b = ip.To16()
-	}
-	return big.NewInt(0).SetBytes(b)
-}
-
-// addIPOffset adds the provided integer offset to a base big.Int representing a
-// net.IP
-func addIPOffset(base *big.Int, offset int) net.IP {
-	return net.IP(big.NewInt(0).Add(base, big.NewInt(int64(offset))).Bytes())
-}
-
 // calculateIPOffset calculates the integer offset of ip from base such that
 // base + offset = ip. It requires ip >= base.
 func calculateIPOffset(base *big.Int, ip net.IP) int {
-	return int(big.NewInt(0).Sub(bigForIP(ip), base).Int64())
+	return int(big.NewInt(0).Sub(utilnet.BigForIP(ip), base).Int64())
-}
-
-// RangeSize returns the size of a range in valid addresses.
-func RangeSize(subnet *net.IPNet) int64 {
-	ones, bits := subnet.Mask.Size()
-	if bits == 32 && (bits-ones) >= 31 || bits == 128 && (bits-ones) >= 127 {
-		return 0
-	}
-	// For IPv6, the max size will be limited to 65536
-	// This is due to the allocator keeping track of all the
-	// allocated IP's in a bitmap. This will keep the size of
-	// the bitmap to 64k.
-	if bits == 128 && (bits-ones) >= 16 {
-		return int64(1) << uint(16)
-	} else {
-		return int64(1) << uint(bits-ones)
-	}
 }

pkg/registry/core/service/ipallocator/allocator_test.go

@@ -30,9 +30,7 @@ func TestAllocate(t *testing.T) {
 		cidr             string
 		free             int
 		released         string
-		outOfRange1      string
+		outOfRange       []string
-		outOfRange2      string
-		outOfRange3      string
 		alreadyAllocated string
 	}{
 		{
@@ -40,19 +38,25 @@ func TestAllocate(t *testing.T) {
 			cidr:     "192.168.1.0/24",
 			free:     254,
 			released: "192.168.1.5",
-			outOfRange1:      "192.168.0.1",
+			outOfRange: []string{
-			outOfRange2:      "192.168.1.0",
+				"192.168.0.1",   // not in 192.168.1.0/24
-			outOfRange3:      "192.168.1.255",
+				"192.168.1.0",   // reserved (base address)
+				"192.168.1.255", // reserved (broadcast address)
+				"192.168.2.2",   // not in 192.168.1.0/24
+			},
 			alreadyAllocated: "192.168.1.1",
 		},
 		{
 			name:     "IPv6",
 			cidr:     "2001:db8:1::/48",
-			free:             65534,
+			free:     65535,
 			released: "2001:db8:1::5",
-			outOfRange1:      "2001:db8::1",
+			outOfRange: []string{
-			outOfRange2:      "2001:db8:1::",
+				"2001:db8::1",     // not in 2001:db8:1::/48
-			outOfRange3:      "2001:db8:1::ffff",
+				"2001:db8:1::",    // reserved (base address)
+				"2001:db8:1::1:0", // not in the low 16 bits of 2001:db8:1::/48
+				"2001:db8:2::2",   // not in 2001:db8:1::/48
+			},
 			alreadyAllocated: "2001:db8:1::1",
 		},
 	}
@@ -119,21 +123,15 @@ func TestAllocate(t *testing.T) {
 		if err := r.Release(released); err != nil {
 			t.Fatal(err)
 		}
-		err = r.Allocate(net.ParseIP(tc.outOfRange1))
+		for _, outOfRange := range tc.outOfRange {
+			err = r.Allocate(net.ParseIP(outOfRange))
 			if _, ok := err.(*ErrNotInRange); !ok {
 				t.Fatal(err)
 			}
+		}
 		if err := r.Allocate(net.ParseIP(tc.alreadyAllocated)); err != ErrAllocated {
 			t.Fatal(err)
 		}
-		err = r.Allocate(net.ParseIP(tc.outOfRange2))
-		if _, ok := err.(*ErrNotInRange); !ok {
-			t.Fatal(err)
-		}
-		err = r.Allocate(net.ParseIP(tc.outOfRange3))
-		if _, ok := err.(*ErrNotInRange); !ok {
-			t.Fatal(err)
-		}
 		if f := r.Free(); f != 1 {
 			t.Errorf("Test %s unexpected free %d", tc.name, f)
 		}
@@ -213,51 +211,6 @@ func TestAllocateSmall(t *testing.T) {
 	t.Logf("allocated: %v", found)
 }
 
-func TestRangeSize(t *testing.T) {
-	testCases := []struct {
-		name  string
-		cidr  string
-		addrs int64
-	}{
-		{
-			name:  "supported IPv4 cidr",
-			cidr:  "192.168.1.0/24",
-			addrs: 256,
-		},
-		{
-			name:  "supported large IPv4 cidr",
-			cidr:  "10.96.0.0/12",
-			addrs: 1048576,
-		},
-		{
-			name:  "unsupported IPv4 cidr",
-			cidr:  "192.168.1.0/1",
-			addrs: 0,
-		},
-		{
-			name:  "supported IPv6 cidr",
-			cidr:  "2001:db8::/48",
-			addrs: 65536,
-		},
-		{
-			name:  "unsupported IPv6 mask",
-			cidr:  "2001:db8::/1",
-			addrs: 0,
-		},
-	}
-
-	for _, tc := range testCases {
-		_, cidr, err := net.ParseCIDR(tc.cidr)
-		if err != nil {
-			t.Errorf("failed to parse cidr for test %s, unexpected error: '%s'", tc.name, err)
-		}
-		if size := RangeSize(cidr); size != tc.addrs {
-			t.Errorf("test %s failed. %s should have a range size of %d, got %d",
-				tc.name, tc.cidr, tc.addrs, size)
-		}
-	}
-}
-
 func TestForEach(t *testing.T) {
 	_, cidr, err := net.ParseCIDR("192.168.1.0/24")
 	if err != nil {

pkg/registry/core/service/ipallocator/controller/repair_test.go

@@ -529,7 +529,7 @@ func TestRepairWithExistingDualStack(t *testing.T) {
 	if !secondaryAfter.Has(net.ParseIP("2000::1")) || !secondaryAfter.Has(net.ParseIP("2000::2")) {
 		t.Errorf("unexpected ipallocator state: %#v", secondaryAfter)
 	}
-	if free := secondaryAfter.Free(); free != 65532 {
+	if free := secondaryAfter.Free(); free != 65533 {
 		t.Errorf("unexpected ipallocator state: %d free (number of free ips is not 65532)", free)
 	}