Add dry-run support to the IP allocator subsystem

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

@@ -37,6 +37,9 @@ type Interface interface {
 	CIDR() net.IPNet
 	IPFamily() api.IPFamily
 	Has(ip net.IP) bool
+
+	// DryRun offers a way to try operations without persisting them.
+	DryRun() Interface
 }
 
 var (
@@ -46,11 +49,12 @@ var (
 )
 
 type ErrNotInRange struct {
+	IP         net.IP
 	ValidRange string
 }
 
 func (e *ErrNotInRange) Error() string {
-	return fmt.Sprintf("provided IP is not in the valid range. The range of valid IPs is %s", e.ValidRange)
+	return fmt.Sprintf("the provided IP (%v) is not in the valid range. The range of valid IPs is %s", e.IP, e.ValidRange)
 }
 
 // Range is a contiguous block of IPs that can be allocated atomically.
@@ -98,11 +102,13 @@ func New(cidr *net.IPNet, allocatorFactory allocator.AllocatorFactory) (*Range,
 		}
 	} else {
 		family = api.IPv4Protocol
-		// Don't use the IPv4 network's broadcast address.
+		// Don't use the IPv4 network's broadcast address, but don't just
+		// Allocate() it - we don't ever want to be able to release it.
 		max--
 	}
 
-	// Don't use the network's ".0" address.
+	// Don't use the network's ".0" address, but don't just Allocate() it - we
+	// don't ever want to be able to release it.
 	base.Add(base, big.NewInt(1))
 	max--
 
@@ -114,6 +120,7 @@ func New(cidr *net.IPNet, allocatorFactory allocator.AllocatorFactory) (*Range,
 	}
 	var err error
 	r.alloc, err = allocatorFactory(r.max, rangeSpec)
+
 	return &r, err
 }
 
@@ -162,18 +169,35 @@ func (r *Range) CIDR() net.IPNet {
 	return *r.net
 }
 
+// DryRun returns a non-persisting form of this Range.
+func (r *Range) DryRun() Interface {
+	return dryRunRange{r}
+}
+
+// For clearer code.
+const dryRunTrue = true
+const dryRunFalse = false
+
 // Allocate attempts to reserve the provided IP. ErrNotInRange or
 // ErrAllocated will be returned if the IP is not valid for this range
 // or has already been reserved.  ErrFull will be returned if there
 // are no addresses left.
 func (r *Range) Allocate(ip net.IP) error {
+	return r.allocate(ip, dryRunFalse)
+}
+
+func (r *Range) allocate(ip net.IP, dryRun bool) error {
 	label := r.CIDR()
 	ok, offset := r.contains(ip)
 	if !ok {
 		// update metrics
 		clusterIPAllocationErrors.WithLabelValues(label.String()).Inc()
-
+		return &ErrNotInRange{ip, r.net.String()}
-		return &ErrNotInRange{r.net.String()}
+	}
+	if dryRun {
+		// Don't bother to check whether the IP is actually free. It's racy and
+		// not worth the effort to plumb any further.
+		return nil
 	}
 
 	allocated, err := r.alloc.Allocate(offset)
@@ -200,7 +224,17 @@ func (r *Range) Allocate(ip net.IP) error {
 // AllocateNext reserves one of the IPs from the pool. ErrFull may
 // be returned if there are no addresses left.
 func (r *Range) AllocateNext() (net.IP, error) {
+	return r.allocateNext(dryRunFalse)
+}
+
+func (r *Range) allocateNext(dryRun bool) (net.IP, error) {
 	label := r.CIDR()
+	if dryRun {
+		// Don't bother finding a free value. It's racy and not worth the
+		// effort to plumb any further.
+		return r.CIDR().IP, nil
+	}
+
 	offset, ok, err := r.alloc.AllocateNext()
 	if err != nil {
 		// update metrics
@@ -226,10 +260,17 @@ func (r *Range) AllocateNext() (net.IP, error) {
 // unallocated IP or an IP out of the range is a no-op and
 // returns no error.
 func (r *Range) Release(ip net.IP) error {
+	return r.release(ip, dryRunFalse)
+}
+
+func (r *Range) release(ip net.IP, dryRun bool) error {
 	ok, offset := r.contains(ip)
 	if !ok {
 		return nil
 	}
+	if dryRun {
+		return nil
+	}
 
 	err := r.alloc.Release(offset)
 	if err == nil {
@@ -312,3 +353,40 @@ func (r *Range) contains(ip net.IP) (bool, int) {
 func calculateIPOffset(base *big.Int, ip net.IP) int {
 	return int(big.NewInt(0).Sub(netutils.BigForIP(ip), base).Int64())
 }
+
+// dryRunRange is a shim to satisfy Interface without persisting state.
+type dryRunRange struct {
+	real *Range
+}
+
+func (dry dryRunRange) Allocate(ip net.IP) error {
+	return dry.real.allocate(ip, dryRunTrue)
+}
+
+func (dry dryRunRange) AllocateNext() (net.IP, error) {
+	return dry.real.allocateNext(dryRunTrue)
+}
+
+func (dry dryRunRange) Release(ip net.IP) error {
+	return dry.real.release(ip, dryRunTrue)
+}
+
+func (dry dryRunRange) ForEach(cb func(net.IP)) {
+	dry.real.ForEach(cb)
+}
+
+func (dry dryRunRange) CIDR() net.IPNet {
+	return dry.real.CIDR()
+}
+
+func (dry dryRunRange) IPFamily() api.IPFamily {
+	return dry.real.IPFamily()
+}
+
+func (dry dryRunRange) DryRun() Interface {
+	return dry
+}
+
+func (dry dryRunRange) Has(ip net.IP) bool {
+	return dry.real.Has(ip)
+}

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

@@ -76,34 +76,34 @@ func TestAllocate(t *testing.T) {
 		}
 		t.Logf("base: %v", r.base.Bytes())
 		if f := r.Free(); f != tc.free {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f)
 		}
 
 		rCIDR := r.CIDR()
 		if rCIDR.String() != tc.cidr {
-			t.Errorf("allocator returned a different cidr")
+			t.Errorf("[%s] wrong CIDR: expected %v, got %v", tc.name, tc.cidr, rCIDR.String())
 		}
 
 		if r.IPFamily() != tc.family {
-			t.Errorf("allocator returned wrong IP family")
+			t.Errorf("[%s] wrong IP family: expected %v, got %v", tc.name, tc.family, r.IPFamily())
 		}
 
 		if f := r.Used(); f != 0 {
-			t.Errorf("Test %s unexpected used %d", tc.name, f)
+			t.Errorf("[%s]: wrong used: expected %d, got %d", tc.name, 0, f)
 		}
 		found := sets.NewString()
 		count := 0
 		for r.Free() > 0 {
 			ip, err := r.AllocateNext()
 			if err != nil {
-				t.Fatalf("Test %s error @ %d: %v", tc.name, count, err)
+				t.Fatalf("[%s] error @ %d: %v", tc.name, count, err)
 			}
 			count++
 			if !cidr.Contains(ip) {
-				t.Fatalf("Test %s allocated %s which is outside of %s", tc.name, ip, cidr)
+				t.Fatalf("[%s] allocated %s which is outside of %s", tc.name, ip, cidr)
 			}
 			if found.Has(ip.String()) {
-				t.Fatalf("Test %s allocated %s twice @ %d", tc.name, ip, count)
+				t.Fatalf("[%s] allocated %s twice @ %d", tc.name, ip, count)
 			}
 			found.Insert(ip.String())
 		}
@@ -116,17 +116,17 @@ func TestAllocate(t *testing.T) {
 			t.Fatal(err)
 		}
 		if f := r.Free(); f != 1 {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f)
 		}
 		if f := r.Used(); f != (tc.free - 1) {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free-1, f)
 		}
 		ip, err := r.AllocateNext()
 		if err != nil {
 			t.Fatal(err)
 		}
 		if !released.Equal(ip) {
-			t.Errorf("Test %s unexpected %s : %s", tc.name, ip, released)
+			t.Errorf("[%s] unexpected %s : %s", tc.name, ip, released)
 		}
 
 		if err := r.Release(released); err != nil {
@@ -142,19 +142,19 @@ func TestAllocate(t *testing.T) {
 			t.Fatal(err)
 		}
 		if f := r.Free(); f != 1 {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f)
 		}
 		if f := r.Used(); f != (tc.free - 1) {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free-1, f)
 		}
 		if err := r.Allocate(released); err != nil {
 			t.Fatal(err)
 		}
 		if f := r.Free(); f != 0 {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 0, f)
 		}
 		if f := r.Used(); f != tc.free {
-			t.Errorf("Test %s unexpected free %d", tc.name, f)
+			t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f)
 		}
 	}
 }
@@ -514,3 +514,67 @@ func expectMetrics(t *testing.T, label string, em testMetrics) {
 		t.Fatalf("metrics error: expected %v, received %v", em, m)
 	}
 }
+
+func TestDryRun(t *testing.T) {
+	testCases := []struct {
+		name   string
+		cidr   string
+		family api.IPFamily
+	}{{
+		name:   "IPv4",
+		cidr:   "192.168.1.0/24",
+		family: api.IPv4Protocol,
+	}, {
+		name:   "IPv6",
+		cidr:   "2001:db8:1::/48",
+		family: api.IPv6Protocol,
+	}}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, cidr, err := netutils.ParseCIDRSloppy(tc.cidr)
+			if err != nil {
+				t.Fatalf("unexpected failure: %v", err)
+			}
+			r, err := NewInMemory(cidr)
+			if err != nil {
+				t.Fatalf("unexpected failure: %v", err)
+			}
+
+			baseUsed := r.Used()
+
+			rCIDR := r.DryRun().CIDR()
+			if rCIDR.String() != tc.cidr {
+				t.Errorf("allocator returned a different cidr")
+			}
+
+			if r.DryRun().IPFamily() != tc.family {
+				t.Errorf("allocator returned wrong IP family")
+			}
+
+			expectUsed := func(t *testing.T, r *Range, expect int) {
+				t.Helper()
+				if u := r.Used(); u != expect {
+					t.Errorf("unexpected used count: got %d, wanted %d", u, expect)
+				}
+			}
+			expectUsed(t, r, baseUsed)
+
+			err = r.DryRun().Allocate(netutils.AddIPOffset(netutils.BigForIP(cidr.IP), 1))
+			if err != nil {
+				t.Fatalf("unexpected failure: %v", err)
+			}
+			expectUsed(t, r, baseUsed)
+
+			_, err = r.DryRun().AllocateNext()
+			if err != nil {
+				t.Fatalf("unexpected failure: %v", err)
+			}
+			expectUsed(t, r, baseUsed)
+
+			if err := r.DryRun().Release(cidr.IP); err != nil {
+				t.Fatalf("unexpected failure: %v", err)
+			}
+			expectUsed(t, r, baseUsed)
+		})
+	}
+}