From 29ea5076ea83246ebf0cff65088e50728ebc000e Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Tue, 16 Aug 2022 15:44:41 +0200 Subject: [PATCH] refactor current ipallocator - rename files to match the allocator backend - use t.Run for tests and cover large ranges - add benchmarks - check that thebitmap ip allocator satisfies the interface goos: linux goarch: amd64 pkg: k8s.io/kubernetes/pkg/registry/core/service/ipallocator cpu: Intel(R) Xeon(R) CPU E5-2678 v3 @ 2.50GHz BenchmarkAllocateNextIPv4Size1048574 BenchmarkAllocateNextIPv4Size1048574-24 1517683 7373 ns/op 135 B/op 8 allocs/op BenchmarkAllocateNextIPv6Size65535 BenchmarkAllocateNextIPv6Size65535-24 5607438 193.9 ns/op 18 B/op 2 allocs/op PASS --- .../ipallocator/{allocator.go => bitmap.go} | 35 +-- .../{allocator_test.go => bitmap_test.go} | 225 +++++++++++------- .../core/service/ipallocator/interfaces.go | 57 +++++ 3 files changed, 195 insertions(+), 122 deletions(-) rename pkg/registry/core/service/ipallocator/{allocator.go => bitmap.go} (92%) rename pkg/registry/core/service/ipallocator/{allocator_test.go => bitmap_test.go} (82%) create mode 100644 pkg/registry/core/service/ipallocator/interfaces.go diff --git a/pkg/registry/core/service/ipallocator/allocator.go b/pkg/registry/core/service/ipallocator/bitmap.go similarity index 92% rename from pkg/registry/core/service/ipallocator/allocator.go rename to pkg/registry/core/service/ipallocator/bitmap.go index df472979502..038487e7de8 100644 --- a/pkg/registry/core/service/ipallocator/allocator.go +++ b/pkg/registry/core/service/ipallocator/bitmap.go @@ -17,7 +17,6 @@ limitations under the License. package ipallocator import ( - "errors" "fmt" "math/big" "net" @@ -27,38 +26,6 @@ import ( netutils "k8s.io/utils/net" ) -// Interface manages the allocation of IP addresses out of a range. Interface -// should be threadsafe. -type Interface interface { - Allocate(net.IP) error - AllocateNext() (net.IP, error) - Release(net.IP) error - ForEach(func(net.IP)) - CIDR() net.IPNet - IPFamily() api.IPFamily - Has(ip net.IP) bool - Destroy() - EnableMetrics() - - // DryRun offers a way to try operations without persisting them. - DryRun() Interface -} - -var ( - ErrFull = errors.New("range is full") - ErrAllocated = errors.New("provided IP is already allocated") - ErrMismatchedNetwork = errors.New("the provided network does not match the current range") -) - -type ErrNotInRange struct { - IP net.IP - ValidRange string -} - -func (e *ErrNotInRange) Error() string { - 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. // // The internal structure of the range is: @@ -89,6 +56,8 @@ type Range struct { metrics metricsRecorderInterface } +var _ Interface = (*Range)(nil) + // New creates a Range over a net.IPNet, calling allocatorFactory to construct the backing store. func New(cidr *net.IPNet, allocatorFactory allocator.AllocatorWithOffsetFactory) (*Range, error) { max := netutils.RangeSize(cidr) diff --git a/pkg/registry/core/service/ipallocator/allocator_test.go b/pkg/registry/core/service/ipallocator/bitmap_test.go similarity index 82% rename from pkg/registry/core/service/ipallocator/allocator_test.go rename to pkg/registry/core/service/ipallocator/bitmap_test.go index fa580e68761..7978a0e03a2 100644 --- a/pkg/registry/core/service/ipallocator/allocator_test.go +++ b/pkg/registry/core/service/ipallocator/bitmap_test.go @@ -51,6 +51,19 @@ func TestAllocate(t *testing.T) { }, alreadyAllocated: "192.168.1.1", }, + { + name: "IPv4 large", + cidr: "10.0.0.0/15", + family: api.IPv4Protocol, + free: 131070, + released: "10.0.0.5", + outOfRange: []string{ + "10.0.0.0", // reserved (base address) + "10.15.255.255", // reserved (broadcast address) + "10.255.255.2", // not in range + }, + alreadyAllocated: "10.0.0.1", + }, { name: "IPv6", cidr: "2001:db8:1::/48", @@ -67,96 +80,97 @@ func TestAllocate(t *testing.T) { }, } for _, tc := range testCases { - _, cidr, err := netutils.ParseCIDRSloppy(tc.cidr) - if err != nil { - t.Fatal(err) - } - r, err := NewInMemory(cidr) - if err != nil { - t.Fatal(err) - } - t.Logf("base: %v", r.base.Bytes()) - if f := r.Free(); f != tc.free { - t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f) - } - - rCIDR := r.CIDR() - if rCIDR.String() != tc.cidr { - t.Errorf("[%s] wrong CIDR: expected %v, got %v", tc.name, tc.cidr, rCIDR.String()) - } - - if r.IPFamily() != tc.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("[%s]: wrong used: expected %d, got %d", tc.name, 0, f) - } - found := sets.NewString() - count := 0 - for r.Free() > 0 { - ip, err := r.AllocateNext() + t.Run(tc.name, func(t *testing.T) { + _, cidr, err := netutils.ParseCIDRSloppy(tc.cidr) if err != nil { - t.Fatalf("[%s] error @ %d: %v", tc.name, count, err) - } - count++ - if !cidr.Contains(ip) { - t.Fatalf("[%s] allocated %s which is outside of %s", tc.name, ip, cidr) - } - if found.Has(ip.String()) { - t.Fatalf("[%s] allocated %s twice @ %d", tc.name, ip, count) - } - found.Insert(ip.String()) - } - if _, err := r.AllocateNext(); err != ErrFull { - t.Fatal(err) - } - - released := netutils.ParseIPSloppy(tc.released) - if err := r.Release(released); err != nil { - t.Fatal(err) - } - if f := r.Free(); f != 1 { - t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f) - } - if f := r.Used(); f != (tc.free - 1) { - 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("[%s] unexpected %s : %s", tc.name, ip, released) - } - - if err := r.Release(released); err != nil { - t.Fatal(err) - } - for _, outOfRange := range tc.outOfRange { - err = r.Allocate(netutils.ParseIPSloppy(outOfRange)) - if _, ok := err.(*ErrNotInRange); !ok { t.Fatal(err) } - } - if err := r.Allocate(netutils.ParseIPSloppy(tc.alreadyAllocated)); err != ErrAllocated { - t.Fatal(err) - } - if f := r.Free(); f != 1 { - t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f) - } - if f := r.Used(); f != (tc.free - 1) { - 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("[%s] wrong free: expected %d, got %d", tc.name, 0, f) - } - if f := r.Used(); f != tc.free { - t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f) - } + r, err := NewInMemory(cidr) + if err != nil { + t.Fatal(err) + } + if f := r.Free(); f != tc.free { + t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f) + } + + rCIDR := r.CIDR() + if rCIDR.String() != tc.cidr { + t.Errorf("[%s] wrong CIDR: expected %v, got %v", tc.name, tc.cidr, rCIDR.String()) + } + + if r.IPFamily() != tc.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("[%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("[%s] error @ %d: %v", tc.name, count, err) + } + count++ + if !cidr.Contains(ip) { + t.Fatalf("[%s] allocated %s which is outside of %s", tc.name, ip, cidr) + } + if found.Has(ip.String()) { + t.Fatalf("[%s] allocated %s twice @ %d", tc.name, ip, count) + } + found.Insert(ip.String()) + } + if _, err := r.AllocateNext(); err != ErrFull { + t.Fatal(err) + } + + released := netutils.ParseIPSloppy(tc.released) + if err := r.Release(released); err != nil { + t.Fatal(err) + } + if f := r.Free(); f != 1 { + t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f) + } + if f := r.Used(); f != (tc.free - 1) { + 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("[%s] unexpected %s : %s", tc.name, ip, released) + } + + if err := r.Release(released); err != nil { + t.Fatal(err) + } + for _, outOfRange := range tc.outOfRange { + err = r.Allocate(netutils.ParseIPSloppy(outOfRange)) + if _, ok := err.(*ErrNotInRange); !ok { + t.Fatal(err) + } + } + if err := r.Allocate(netutils.ParseIPSloppy(tc.alreadyAllocated)); err != ErrAllocated { + t.Fatal(err) + } + if f := r.Free(); f != 1 { + t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, 1, f) + } + if f := r.Used(); f != (tc.free - 1) { + 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("[%s] wrong free: expected %d, got %d", tc.name, 0, f) + } + if f := r.Used(); f != tc.free { + t.Errorf("[%s] wrong free: expected %d, got %d", tc.name, tc.free, f) + } + }) } } @@ -270,8 +284,6 @@ func TestAllocateSmall(t *testing.T) { if r.max != 2 { t.Fatalf("expected range equal to 2, got: %v", r) } - - t.Logf("allocated: %v", found) } func TestForEach(t *testing.T) { @@ -827,3 +839,38 @@ func Test_calculateRangeOffset(t *testing.T) { }) } } + +// cpu: Intel(R) Xeon(R) CPU E5-2678 v3 @ 2.50GHz +// BenchmarkAllocateNextIPv4 +// BenchmarkAllocateNextIPv4-24 1175304 870.9 ns/op 1337 B/op 11 allocs/op +func BenchmarkAllocateNextIPv4Size1048574(b *testing.B) { + _, cidr, err := netutils.ParseCIDRSloppy("10.0.0.0/12") + if err != nil { + b.Fatal(err) + } + r, err := NewInMemory(cidr) + if err != nil { + b.Fatal(err) + } + for n := 0; n < b.N; n++ { + r.AllocateNext() + } +} + +// This is capped to 65535 +// cpu: Intel(R) Xeon(R) CPU E5-2678 v3 @ 2.50GHz +// BenchmarkAllocateNextIPv6 +// BenchmarkAllocateNextIPv6-24 5779431 194.0 ns/op 18 B/op 2 allocs/op +func BenchmarkAllocateNextIPv6Size65535(b *testing.B) { + _, cidr, err := netutils.ParseCIDRSloppy("fd00::/24") + if err != nil { + b.Fatal(err) + } + r, err := NewInMemory(cidr) + if err != nil { + b.Fatal(err) + } + for n := 0; n < b.N; n++ { + r.AllocateNext() + } +} diff --git a/pkg/registry/core/service/ipallocator/interfaces.go b/pkg/registry/core/service/ipallocator/interfaces.go new file mode 100644 index 00000000000..0319b315e45 --- /dev/null +++ b/pkg/registry/core/service/ipallocator/interfaces.go @@ -0,0 +1,57 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ipallocator + +import ( + "errors" + "fmt" + "net" + + api "k8s.io/kubernetes/pkg/apis/core" +) + +// Interface manages the allocation of IP addresses out of a range. Interface +// should be threadsafe. +type Interface interface { + Allocate(net.IP) error + AllocateNext() (net.IP, error) + Release(net.IP) error + ForEach(func(net.IP)) + CIDR() net.IPNet + IPFamily() api.IPFamily + Has(ip net.IP) bool + Destroy() + EnableMetrics() + + // DryRun offers a way to try operations without persisting them. + DryRun() Interface +} + +var ( + ErrFull = errors.New("range is full") + ErrAllocated = errors.New("provided IP is already allocated") + ErrMismatchedNetwork = errors.New("the provided network does not match the current range") +) + +type ErrNotInRange struct { + IP net.IP + ValidRange string +} + +func (e *ErrNotInRange) Error() string { + return fmt.Sprintf("the provided IP (%v) is not in the valid range. The range of valid IPs is %s", e.IP, e.ValidRange) +}