From 069707f75a14b9c98538755b089a123192bb0368 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Sun, 19 Apr 2020 21:54:11 +0200 Subject: [PATCH] refactor and instrument range_allocator cidr_sets refactor and add the following metrics to the cidr_sets used by the range allocator:, under the subsystem: node_ipam_controller cidrset_cidrs_allocations_total cidrset_cidrs_releases_total cidrset_usage_cidrs cidrset_allocation_tries_per_request --- pkg/controller/nodeipam/ipam/cidrset/BUILD | 14 +- .../nodeipam/ipam/cidrset/cidr_set.go | 128 ++++++----- .../nodeipam/ipam/cidrset/cidr_set_test.go | 206 ++++++++++++++++++ .../nodeipam/ipam/cidrset/metrics.go | 78 +++++++ 4 files changed, 371 insertions(+), 55 deletions(-) create mode 100644 pkg/controller/nodeipam/ipam/cidrset/metrics.go diff --git a/pkg/controller/nodeipam/ipam/cidrset/BUILD b/pkg/controller/nodeipam/ipam/cidrset/BUILD index 34fddb93183..f8f4e06454c 100644 --- a/pkg/controller/nodeipam/ipam/cidrset/BUILD +++ b/pkg/controller/nodeipam/ipam/cidrset/BUILD @@ -10,13 +10,23 @@ go_test( name = "go_default_test", srcs = ["cidr_set_test.go"], embed = [":go_default_library"], - deps = ["//vendor/k8s.io/klog/v2:go_default_library"], + deps = [ + "//staging/src/k8s.io/component-base/metrics/testutil:go_default_library", + "//vendor/k8s.io/klog/v2:go_default_library", + ], ) go_library( name = "go_default_library", - srcs = ["cidr_set.go"], + srcs = [ + "cidr_set.go", + "metrics.go", + ], importpath = "k8s.io/kubernetes/pkg/controller/nodeipam/ipam/cidrset", + deps = [ + "//staging/src/k8s.io/component-base/metrics:go_default_library", + "//staging/src/k8s.io/component-base/metrics/legacyregistry:go_default_library", + ], ) filegroup( diff --git a/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go b/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go index cf2295496cd..359f830e3a6 100644 --- a/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go +++ b/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go @@ -30,13 +30,26 @@ import ( // be allocated from. type CidrSet struct { sync.Mutex - clusterCIDR *net.IPNet - clusterIP net.IP + // clusterCIDR is the CIDR assigned to the cluster + clusterCIDR *net.IPNet + // clusterMaskSize is the mask size, in bits, assigned to the cluster + // caches the mask size to avoid the penalty of calling clusterCIDR.Mask.Size() clusterMaskSize int - maxCIDRs int - nextCandidate int - used big.Int - subNetMaskSize int + // nodeMask is the network mask assigned to the nodes + nodeMask net.IPMask + // nodeMaskSize is the mask size, in bits,assigned to the nodes + // caches the mask size to avoid the penalty of calling nodeMask.Size() + nodeMaskSize int + // maxCIDRs is the maximum number of CIDRs that can be allocated + maxCIDRs int + // allocatedCIDRs counts the number of CIDRs allocated + allocatedCIDRs int + // nextCandidate points to the next CIDR that should be free + nextCandidate int + // used is a bitmap used to track the CIDRs allocated + used big.Int + // label is used to identify the metrics + label string } const ( @@ -64,53 +77,55 @@ var ( // NewCIDRSet creates a new CidrSet. func NewCIDRSet(clusterCIDR *net.IPNet, subNetMaskSize int) (*CidrSet, error) { clusterMask := clusterCIDR.Mask - clusterMaskSize, _ := clusterMask.Size() + clusterMaskSize, bits := clusterMask.Size() var maxCIDRs int if (clusterCIDR.IP.To4() == nil) && (subNetMaskSize-clusterMaskSize > clusterSubnetMaxDiff) { return nil, ErrCIDRSetSubNetTooBig } + + // register CidrSet metrics + registerCidrsetMetrics() + maxCIDRs = 1 << uint32(subNetMaskSize-clusterMaskSize) return &CidrSet{ clusterCIDR: clusterCIDR, - clusterIP: clusterCIDR.IP, + nodeMask: net.CIDRMask(subNetMaskSize, bits), clusterMaskSize: clusterMaskSize, maxCIDRs: maxCIDRs, - subNetMaskSize: subNetMaskSize, + nodeMaskSize: subNetMaskSize, + label: clusterCIDR.String(), }, nil } func (s *CidrSet) indexToCIDRBlock(index int) *net.IPNet { var ip []byte - var mask int switch /*v4 or v6*/ { - case s.clusterIP.To4() != nil: + case s.clusterCIDR.IP.To4() != nil: { - j := uint32(index) << uint32(32-s.subNetMaskSize) - ipInt := (binary.BigEndian.Uint32(s.clusterIP)) | j - ip = make([]byte, 4) + j := uint32(index) << uint32(32-s.nodeMaskSize) + ipInt := (binary.BigEndian.Uint32(s.clusterCIDR.IP)) | j + ip = make([]byte, net.IPv4len) binary.BigEndian.PutUint32(ip, ipInt) - mask = 32 - } - case s.clusterIP.To16() != nil: + case s.clusterCIDR.IP.To16() != nil: { // leftClusterIP | rightClusterIP // 2001:0DB8:1234:0000:0000:0000:0000:0000 const v6NBits = 128 const halfV6NBits = v6NBits / 2 - leftClusterIP := binary.BigEndian.Uint64(s.clusterIP[:halfIPv6Len]) - rightClusterIP := binary.BigEndian.Uint64(s.clusterIP[halfIPv6Len:]) + leftClusterIP := binary.BigEndian.Uint64(s.clusterCIDR.IP[:halfIPv6Len]) + rightClusterIP := binary.BigEndian.Uint64(s.clusterCIDR.IP[halfIPv6Len:]) - leftIP, rightIP := make([]byte, halfIPv6Len), make([]byte, halfIPv6Len) + ip = make([]byte, net.IPv6len) - if s.subNetMaskSize <= halfV6NBits { + if s.nodeMaskSize <= halfV6NBits { // We only care about left side IP - leftClusterIP |= uint64(index) << uint(halfV6NBits-s.subNetMaskSize) + leftClusterIP |= uint64(index) << uint(halfV6NBits-s.nodeMaskSize) } else { if s.clusterMaskSize < halfV6NBits { // see how many bits are needed to reach the left side - btl := uint(s.subNetMaskSize - halfV6NBits) + btl := uint(s.nodeMaskSize - halfV6NBits) indexMaxBit := uint(64 - bits.LeadingZeros64(uint64(index))) if indexMaxBit > btl { leftClusterIP |= uint64(index) >> btl @@ -118,18 +133,15 @@ func (s *CidrSet) indexToCIDRBlock(index int) *net.IPNet { } // the right side will be calculated the same way either the // subNetMaskSize affects both left and right sides - rightClusterIP |= uint64(index) << uint(v6NBits-s.subNetMaskSize) + rightClusterIP |= uint64(index) << uint(v6NBits-s.nodeMaskSize) } - binary.BigEndian.PutUint64(leftIP, leftClusterIP) - binary.BigEndian.PutUint64(rightIP, rightClusterIP) - - ip = append(leftIP, rightIP...) - mask = 128 + binary.BigEndian.PutUint64(ip[:halfIPv6Len], leftClusterIP) + binary.BigEndian.PutUint64(ip[halfIPv6Len:], rightClusterIP) } } return &net.IPNet{ IP: ip, - Mask: net.CIDRMask(s.subNetMaskSize, mask), + Mask: s.nodeMask, } } @@ -139,22 +151,27 @@ func (s *CidrSet) AllocateNext() (*net.IPNet, error) { s.Lock() defer s.Unlock() - nextUnused := -1 - for i := 0; i < s.maxCIDRs; i++ { - candidate := (i + s.nextCandidate) % s.maxCIDRs - if s.used.Bit(candidate) == 0 { - nextUnused = candidate - break - } - } - if nextUnused == -1 { + if s.allocatedCIDRs == s.maxCIDRs { return nil, ErrCIDRRangeNoCIDRsRemaining } - s.nextCandidate = (nextUnused + 1) % s.maxCIDRs + candidate := s.nextCandidate + var i int + for i = 0; i < s.maxCIDRs; i++ { + if s.used.Bit(candidate) == 0 { + break + } + candidate = (candidate + 1) % s.maxCIDRs + } - s.used.SetBit(&s.used, nextUnused, 1) + s.nextCandidate = (candidate + 1) % s.maxCIDRs + s.used.SetBit(&s.used, candidate, 1) + s.allocatedCIDRs++ + // Update metrics + cidrSetAllocations.WithLabelValues(s.label).Inc() + cidrSetAllocationTriesPerRequest.WithLabelValues(s.label).Observe(float64(i)) + cidrSetUsage.WithLabelValues(s.label).Set(float64(s.allocatedCIDRs) / float64(s.maxCIDRs)) - return s.indexToCIDRBlock(nextUnused), nil + return s.indexToCIDRBlock(candidate), nil } func (s *CidrSet) getBeginingAndEndIndices(cidr *net.IPNet) (begin, end int, err error) { @@ -176,10 +193,9 @@ func (s *CidrSet) getBeginingAndEndIndices(cidr *net.IPNet) (begin, end int, err if cidr.IP.To4() == nil { ipSize = net.IPv6len } - subNetMask := net.CIDRMask(s.subNetMaskSize, ipSize*8) begin, err = s.getIndexForCIDR(&net.IPNet{ - IP: cidr.IP.Mask(subNetMask), - Mask: subNetMask, + IP: cidr.IP.Mask(s.nodeMask), + Mask: s.nodeMask, }) if err != nil { return -1, -1, err @@ -197,8 +213,8 @@ func (s *CidrSet) getBeginingAndEndIndices(cidr *net.IPNet) (begin, end int, err binary.BigEndian.PutUint64(ip[net.IPv6len/2:], ipIntRight) } end, err = s.getIndexForCIDR(&net.IPNet{ - IP: net.IP(ip).Mask(subNetMask), - Mask: subNetMask, + IP: net.IP(ip).Mask(s.nodeMask), + Mask: s.nodeMask, }) if err != nil { return -1, -1, err @@ -217,7 +233,11 @@ func (s *CidrSet) Release(cidr *net.IPNet) error { defer s.Unlock() for i := begin; i <= end; i++ { s.used.SetBit(&s.used, i, 0) + s.allocatedCIDRs-- + cidrSetReleases.WithLabelValues(s.label).Inc() } + + cidrSetUsage.WithLabelValues(s.label).Set(float64(s.allocatedCIDRs) / float64(s.maxCIDRs)) return nil } @@ -228,13 +248,15 @@ func (s *CidrSet) Occupy(cidr *net.IPNet) (err error) { if err != nil { return err } - s.Lock() defer s.Unlock() for i := begin; i <= end; i++ { s.used.SetBit(&s.used, i, 1) + s.allocatedCIDRs++ + cidrSetAllocations.WithLabelValues(s.label).Inc() } + cidrSetUsage.WithLabelValues(s.label).Set(float64(s.allocatedCIDRs) / float64(s.maxCIDRs)) return nil } @@ -244,19 +266,19 @@ func (s *CidrSet) getIndexForCIDR(cidr *net.IPNet) (int, error) { func (s *CidrSet) getIndexForIP(ip net.IP) (int, error) { if ip.To4() != nil { - cidrIndex := (binary.BigEndian.Uint32(s.clusterIP) ^ binary.BigEndian.Uint32(ip.To4())) >> uint32(32-s.subNetMaskSize) + cidrIndex := (binary.BigEndian.Uint32(s.clusterCIDR.IP) ^ binary.BigEndian.Uint32(ip.To4())) >> uint32(32-s.nodeMaskSize) if cidrIndex >= uint32(s.maxCIDRs) { - return 0, fmt.Errorf("CIDR: %v/%v is out of the range of CIDR allocator", ip, s.subNetMaskSize) + return 0, fmt.Errorf("CIDR: %v/%v is out of the range of CIDR allocator", ip, s.nodeMaskSize) } return int(cidrIndex), nil } if ip.To16() != nil { - bigIP := big.NewInt(0).SetBytes(s.clusterIP) + bigIP := big.NewInt(0).SetBytes(s.clusterCIDR.IP) bigIP = bigIP.Xor(bigIP, big.NewInt(0).SetBytes(ip)) - cidrIndexBig := bigIP.Rsh(bigIP, uint(net.IPv6len*8-s.subNetMaskSize)) + cidrIndexBig := bigIP.Rsh(bigIP, uint(net.IPv6len*8-s.nodeMaskSize)) cidrIndex := cidrIndexBig.Uint64() if cidrIndex >= uint64(s.maxCIDRs) { - return 0, fmt.Errorf("CIDR: %v/%v is out of the range of CIDR allocator", ip, s.subNetMaskSize) + return 0, fmt.Errorf("CIDR: %v/%v is out of the range of CIDR allocator", ip, s.nodeMaskSize) } return int(cidrIndex), nil } diff --git a/pkg/controller/nodeipam/ipam/cidrset/cidr_set_test.go b/pkg/controller/nodeipam/ipam/cidrset/cidr_set_test.go index 9f25a2cfda0..ead684ea159 100644 --- a/pkg/controller/nodeipam/ipam/cidrset/cidr_set_test.go +++ b/pkg/controller/nodeipam/ipam/cidrset/cidr_set_test.go @@ -22,6 +22,7 @@ import ( "reflect" "testing" + "k8s.io/component-base/metrics/testutil" "k8s.io/klog/v2" ) @@ -736,3 +737,208 @@ func TestCIDRSetv6(t *testing.T) { }) } } + +func TestCidrSetMetrics(t *testing.T) { + cidr := "10.0.0.0/16" + _, clusterCIDR, _ := net.ParseCIDR(cidr) + // We have 256 free cidrs + a, err := NewCIDRSet(clusterCIDR, 24) + if err != nil { + t.Fatalf("unexpected error creating CidrSet: %v", err) + } + clearMetrics(map[string]string{"clusterCIDR": cidr}) + + // Allocate next all + for i := 1; i <= 256; i++ { + _, err := a.AllocateNext() + if err != nil { + t.Fatalf("unexpected error allocating a new CIDR: %v", err) + } + em := testMetrics{ + usage: float64(i) / float64(256), + allocs: float64(i), + releases: 0, + allocTries: 0, + } + expectMetrics(t, cidr, em) + } + // Release all + a.Release(clusterCIDR) + em := testMetrics{ + usage: 0, + allocs: 256, + releases: 256, + allocTries: 0, + } + expectMetrics(t, cidr, em) + + // Allocate all + a.Occupy(clusterCIDR) + em = testMetrics{ + usage: 1, + allocs: 512, + releases: 256, + allocTries: 0, + } + expectMetrics(t, cidr, em) + +} + +func TestCidrSetMetricsHistogram(t *testing.T) { + cidr := "10.0.0.0/16" + _, clusterCIDR, _ := net.ParseCIDR(cidr) + // We have 256 free cidrs + a, err := NewCIDRSet(clusterCIDR, 24) + if err != nil { + t.Fatalf("unexpected error creating CidrSet: %v", err) + } + clearMetrics(map[string]string{"clusterCIDR": cidr}) + + // Allocate half of the range + // Occupy does not update the nextCandidate + _, halfClusterCIDR, _ := net.ParseCIDR("10.0.0.0/17") + a.Occupy(halfClusterCIDR) + em := testMetrics{ + usage: 0.5, + allocs: 128, + releases: 0, + allocTries: 0, + } + expectMetrics(t, cidr, em) + // Allocate next should iterate until the next free cidr + // that is exactly the same number we allocated previously + _, err = a.AllocateNext() + if err != nil { + t.Fatalf("unexpected error allocating a new CIDR: %v", err) + } + em = testMetrics{ + usage: float64(129) / float64(256), + allocs: 129, + releases: 0, + allocTries: 128, + } + expectMetrics(t, cidr, em) +} + +func TestCidrSetMetricsDual(t *testing.T) { + // create IPv4 cidrSet + cidrIPv4 := "10.0.0.0/16" + _, clusterCIDRv4, _ := net.ParseCIDR(cidrIPv4) + a, err := NewCIDRSet(clusterCIDRv4, 24) + if err != nil { + t.Fatalf("unexpected error creating CidrSet: %v", err) + } + clearMetrics(map[string]string{"clusterCIDR": cidrIPv4}) + // create IPv6 cidrSet + cidrIPv6 := "2001:db8::/48" + _, clusterCIDRv6, _ := net.ParseCIDR(cidrIPv6) + b, err := NewCIDRSet(clusterCIDRv6, 64) + if err != nil { + t.Fatalf("unexpected error creating CidrSet: %v", err) + } + clearMetrics(map[string]string{"clusterCIDR": cidrIPv6}) + // Allocate all + a.Occupy(clusterCIDRv4) + em := testMetrics{ + usage: 1, + allocs: 256, + releases: 0, + allocTries: 0, + } + expectMetrics(t, cidrIPv4, em) + + b.Occupy(clusterCIDRv6) + em = testMetrics{ + usage: 1, + allocs: 65536, + releases: 0, + allocTries: 0, + } + expectMetrics(t, cidrIPv6, em) + + // Release all + a.Release(clusterCIDRv4) + em = testMetrics{ + usage: 0, + allocs: 256, + releases: 256, + allocTries: 0, + } + expectMetrics(t, cidrIPv4, em) + b.Release(clusterCIDRv6) + em = testMetrics{ + usage: 0, + allocs: 65536, + releases: 65536, + allocTries: 0, + } + expectMetrics(t, cidrIPv6, em) + +} + +// Metrics helpers +func clearMetrics(labels map[string]string) { + cidrSetAllocations.Delete(labels) + cidrSetReleases.Delete(labels) + cidrSetUsage.Delete(labels) + cidrSetAllocationTriesPerRequest.Delete(labels) +} + +type testMetrics struct { + usage float64 + allocs float64 + releases float64 + allocTries float64 +} + +func expectMetrics(t *testing.T, label string, em testMetrics) { + var m testMetrics + var err error + m.usage, err = testutil.GetGaugeMetricValue(cidrSetUsage.WithLabelValues(label)) + if err != nil { + t.Errorf("failed to get %s value, err: %v", cidrSetUsage.Name, err) + } + m.allocs, err = testutil.GetCounterMetricValue(cidrSetAllocations.WithLabelValues(label)) + if err != nil { + t.Errorf("failed to get %s value, err: %v", cidrSetAllocations.Name, err) + } + m.releases, err = testutil.GetCounterMetricValue(cidrSetReleases.WithLabelValues(label)) + if err != nil { + t.Errorf("failed to get %s value, err: %v", cidrSetReleases.Name, err) + } + m.allocTries, err = testutil.GetHistogramMetricValue(cidrSetAllocationTriesPerRequest.WithLabelValues(label)) + if err != nil { + t.Errorf("failed to get %s value, err: %v", cidrSetAllocationTriesPerRequest.Name, err) + } + + if m != em { + t.Fatalf("metrics error: expected %v, received %v", em, m) + } +} + +// Benchmarks +func benchmarkAllocateAllIPv6(cidr string, subnetMaskSize int, b *testing.B) { + _, clusterCIDR, _ := net.ParseCIDR(cidr) + a, _ := NewCIDRSet(clusterCIDR, subnetMaskSize) + for n := 0; n < b.N; n++ { + // Allocate the whole range + 1 + for i := 0; i <= a.maxCIDRs; i++ { + a.AllocateNext() + } + // Release all + a.Release(clusterCIDR) + } +} + +func BenchmarkAllocateAll_48_52(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/48", 52, b) } +func BenchmarkAllocateAll_48_56(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/48", 56, b) } + +func BenchmarkAllocateAll_48_60(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/48", 60, b) } +func BenchmarkAllocateAll_48_64(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/48", 64, b) } + +func BenchmarkAllocateAll_64_68(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/64", 68, b) } + +func BenchmarkAllocateAll_64_72(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/64", 72, b) } +func BenchmarkAllocateAll_64_76(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/64", 76, b) } + +func BenchmarkAllocateAll_64_80(b *testing.B) { benchmarkAllocateAllIPv6("2001:db8::/64", 80, b) } diff --git a/pkg/controller/nodeipam/ipam/cidrset/metrics.go b/pkg/controller/nodeipam/ipam/cidrset/metrics.go new file mode 100644 index 00000000000..df4d2ce6492 --- /dev/null +++ b/pkg/controller/nodeipam/ipam/cidrset/metrics.go @@ -0,0 +1,78 @@ +/* +Copyright 2020 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 cidrset + +import ( + "sync" + + "k8s.io/component-base/metrics" + "k8s.io/component-base/metrics/legacyregistry" +) + +const nodeIpamSubsystem = "node_ipam_controller" + +var ( + cidrSetAllocations = metrics.NewCounterVec( + &metrics.CounterOpts{ + Subsystem: nodeIpamSubsystem, + Name: "cidrset_cidrs_allocations_total", + Help: "Counter measuring total number of CIDR allocations.", + StabilityLevel: metrics.ALPHA, + }, + []string{"clusterCIDR"}, + ) + cidrSetReleases = metrics.NewCounterVec( + &metrics.CounterOpts{ + Subsystem: nodeIpamSubsystem, + Name: "cidrset_cidrs_releases_total", + Help: "Counter measuring total number of CIDR releases.", + StabilityLevel: metrics.ALPHA, + }, + []string{"clusterCIDR"}, + ) + cidrSetUsage = metrics.NewGaugeVec( + &metrics.GaugeOpts{ + Subsystem: nodeIpamSubsystem, + Name: "cidrset_usage_cidrs", + Help: "Gauge measuring percentage of allocated CIDRs.", + StabilityLevel: metrics.ALPHA, + }, + []string{"clusterCIDR"}, + ) + cidrSetAllocationTriesPerRequest = metrics.NewHistogramVec( + &metrics.HistogramOpts{ + Subsystem: nodeIpamSubsystem, + Name: "cidrset_allocation_tries_per_request", + Help: "Number of endpoints added on each Service sync", + StabilityLevel: metrics.ALPHA, + Buckets: metrics.ExponentialBuckets(1, 5, 5), + }, + []string{"clusterCIDR"}, + ) +) + +var registerMetrics sync.Once + +// registerCidrsetMetrics the metrics that are to be monitored. +func registerCidrsetMetrics() { + registerMetrics.Do(func() { + legacyregistry.MustRegister(cidrSetAllocations) + legacyregistry.MustRegister(cidrSetReleases) + legacyregistry.MustRegister(cidrSetUsage) + legacyregistry.MustRegister(cidrSetAllocationTriesPerRequest) + }) +}