From 52856f3fbed42d463746e8c2d0249b3da2bfa300 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 25 Nov 2020 13:07:36 -0800 Subject: [PATCH] Add dry-run support to the IP allocator subsystem --- .../core/service/ipallocator/allocator.go | 88 +++++++++++++++++- .../service/ipallocator/allocator_test.go | 92 ++++++++++++++++--- 2 files changed, 161 insertions(+), 19 deletions(-) diff --git a/pkg/registry/core/service/ipallocator/allocator.go b/pkg/registry/core/service/ipallocator/allocator.go index df8a2841ac0..820ae4a73e7 100644 --- a/pkg/registry/core/service/ipallocator/allocator.go +++ b/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{r.net.String()} + return &ErrNotInRange{ip, 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) +} diff --git a/pkg/registry/core/service/ipallocator/allocator_test.go b/pkg/registry/core/service/ipallocator/allocator_test.go index 8580303adb4..bf4dec2fa83 100644 --- a/pkg/registry/core/service/ipallocator/allocator_test.go +++ b/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) + }) + } +}