Merge pull request #110027 from zlabjp/fix-ipallocator-metrics

Fix cluster IP allocator metrics

pkg/registry/core/rest/storage_core.go

@@ -213,6 +213,7 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(apiResourceConfigSource
 	if err != nil {
 		return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, fmt.Errorf("cannot create cluster IP allocator: %v", err)
 	}
+	serviceClusterIPAllocator.EnableMetrics()
 	restStorage.ServiceClusterIPAllocator = serviceClusterIPRegistry
 
 	// allocator for secondary service ip range
@@ -233,6 +234,7 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(apiResourceConfigSource
 		if err != nil {
 			return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, fmt.Errorf("cannot create cluster secondary IP allocator: %v", err)
 		}
+		secondaryServiceClusterIPAllocator.EnableMetrics()
 		restStorage.SecondaryServiceClusterIPAllocator = secondaryServiceClusterIPRegistry
 	}
 

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

@@ -40,6 +40,7 @@ type Interface interface {
 	IPFamily() api.IPFamily
 	Has(ip net.IP) bool
 	Destroy()
+	EnableMetrics()
 
 	// DryRun offers a way to try operations without persisting them.
 	DryRun() Interface
@@ -86,12 +87,12 @@ type Range struct {
 	family api.IPFamily
 
 	alloc allocator.Interface
+	// metrics is a metrics recorder that can be disabled
+	metrics metricsRecorderInterface
 }
 
 // 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) {
-	registerMetrics()
-
 	max := netutils.RangeSize(cidr)
 	base := netutils.BigForIP(cidr.IP)
 	rangeSpec := cidr.String()
@@ -116,10 +117,11 @@ func New(cidr *net.IPNet, allocatorFactory allocator.AllocatorWithOffsetFactory)
 	max--
 
 	r := Range{
-		net:    cidr,
+		net:     cidr,
-		base:   base,
+		base:    base,
-		max:    maximum(0, int(max)),
+		max:     maximum(0, int(max)),
-		family: family,
+		family:  family,
+		metrics: &emptyMetricsRecorder{}, // disabled by default
 	}
 
 	offset := 0
@@ -201,8 +203,10 @@ func (r *Range) allocate(ip net.IP, dryRun bool) error {
 	label := r.CIDR()
 	ok, offset := r.contains(ip)
 	if !ok {
-		// update metrics
+		if !dryRun {
-		clusterIPAllocationErrors.WithLabelValues(label.String(), "static").Inc()
+			// update metrics
+			r.metrics.incrementAllocationErrors(label.String(), "static")
+		}
 		return &ErrNotInRange{ip, r.net.String()}
 	}
 	if dryRun {
@@ -214,20 +218,20 @@ func (r *Range) allocate(ip net.IP, dryRun bool) error {
 	allocated, err := r.alloc.Allocate(offset)
 	if err != nil {
 		// update metrics
-		clusterIPAllocationErrors.WithLabelValues(label.String(), "static").Inc()
+		r.metrics.incrementAllocationErrors(label.String(), "static")
 
 		return err
 	}
 	if !allocated {
 		// update metrics
-		clusterIPAllocationErrors.WithLabelValues(label.String(), "static").Inc()
+		r.metrics.incrementAllocationErrors(label.String(), "static")
 
 		return ErrAllocated
 	}
 	// update metrics
-	clusterIPAllocations.WithLabelValues(label.String(), "static").Inc()
+	r.metrics.incrementAllocations(label.String(), "static")
-	clusterIPAllocated.WithLabelValues(label.String()).Set(float64(r.Used()))
+	r.metrics.setAllocated(label.String(), r.Used())
-	clusterIPAvailable.WithLabelValues(label.String()).Set(float64(r.Free()))
+	r.metrics.setAvailable(label.String(), r.Free())
 
 	return nil
 }
@@ -249,20 +253,20 @@ func (r *Range) allocateNext(dryRun bool) (net.IP, error) {
 	offset, ok, err := r.alloc.AllocateNext()
 	if err != nil {
 		// update metrics
-		clusterIPAllocationErrors.WithLabelValues(label.String(), "dynamic").Inc()
+		r.metrics.incrementAllocationErrors(label.String(), "dynamic")
 
 		return nil, err
 	}
 	if !ok {
 		// update metrics
-		clusterIPAllocationErrors.WithLabelValues(label.String(), "dynamic").Inc()
+		r.metrics.incrementAllocationErrors(label.String(), "dynamic")
 
 		return nil, ErrFull
 	}
 	// update metrics
-	clusterIPAllocations.WithLabelValues(label.String(), "dynamic").Inc()
+	r.metrics.incrementAllocations(label.String(), "dynamic")
-	clusterIPAllocated.WithLabelValues(label.String()).Set(float64(r.Used()))
+	r.metrics.setAllocated(label.String(), r.Used())
-	clusterIPAvailable.WithLabelValues(label.String()).Set(float64(r.Free()))
+	r.metrics.setAvailable(label.String(), r.Free())
 
 	return netutils.AddIPOffset(r.base, offset), nil
 }
@@ -287,8 +291,8 @@ func (r *Range) release(ip net.IP, dryRun bool) error {
 	if err == nil {
 		// update metrics
 		label := r.CIDR()
-		clusterIPAllocated.WithLabelValues(label.String()).Set(float64(r.Used()))
+		r.metrics.setAllocated(label.String(), r.Used())
-		clusterIPAvailable.WithLabelValues(label.String()).Set(float64(r.Free()))
+		r.metrics.setAvailable(label.String(), r.Free())
 	}
 	return err
 }
@@ -364,6 +368,12 @@ func (r *Range) Destroy() {
 	r.alloc.Destroy()
 }
 
+// EnableMetrics enables metrics recording.
+func (r *Range) EnableMetrics() {
+	registerMetrics()
+	r.metrics = &metricsRecorder{}
+}
+
 // 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 {
@@ -436,3 +446,6 @@ func (dry dryRunRange) Has(ip net.IP) bool {
 
 func (dry dryRunRange) Destroy() {
 }
+
+func (dry dryRunRange) EnableMetrics() {
+}

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

@@ -434,10 +434,12 @@ func TestClusterIPMetrics(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error creating CidrSet: %v", err)
 	}
+	a.EnableMetrics()
 	// create IPv6 allocator
 	cidrIPv6 := "2001:db8::/112"
 	_, clusterCIDRv6, _ := netutils.ParseCIDRSloppy(cidrIPv6)
 	b, err := NewInMemory(clusterCIDRv6)
+	b.EnableMetrics()
 	if err != nil {
 		t.Fatalf("unexpected error creating CidrSet: %v", err)
 	}
@@ -546,6 +548,7 @@ func TestClusterIPAllocatedMetrics(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error creating CidrSet: %v", err)
 	}
+	a.EnableMetrics()
 
 	em := testMetrics{
 		free:      0,
@@ -595,6 +598,58 @@ func TestClusterIPAllocatedMetrics(t *testing.T) {
 	}
 }
 
+func TestMetricsDisabled(t *testing.T) {
+	// create metrics enabled allocator
+	cidrIPv4 := "10.0.0.0/24"
+	_, clusterCIDRv4, _ := netutils.ParseCIDRSloppy(cidrIPv4)
+	a, err := NewInMemory(clusterCIDRv4)
+	if err != nil {
+		t.Fatalf("unexpected error creating CidrSet: %v", err)
+	}
+	a.EnableMetrics()
+
+	// create metrics disabled allocator with same CIDR
+	// this metrics should be ignored
+	b, err := NewInMemory(clusterCIDRv4)
+	if err != nil {
+		t.Fatalf("unexpected error creating CidrSet: %v", err)
+	}
+
+	// Check initial state
+	em := testMetrics{
+		free:      0,
+		used:      0,
+		allocated: 0,
+		errors:    0,
+	}
+	expectMetrics(t, cidrIPv4, em)
+
+	// allocate in metrics enabled allocator
+	for i := 0; i < 100; i++ {
+		_, err := a.AllocateNext()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	em = testMetrics{
+		free:      154,
+		used:      100,
+		allocated: 100,
+		errors:    0,
+	}
+	expectMetrics(t, cidrIPv4, em)
+
+	// allocate in metrics disabled allocator
+	for i := 0; i < 200; i++ {
+		_, err := b.AllocateNext()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	// the metrics should not be changed
+	expectMetrics(t, cidrIPv4, em)
+}
+
 // Metrics helpers
 func clearMetrics() {
 	clusterIPAllocated.Reset()

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

@@ -85,3 +85,38 @@ func registerMetrics() {
 		legacyregistry.MustRegister(clusterIPAllocationErrors)
 	})
 }
+
+// metricsRecorderInterface is the interface to record metrics.
+type metricsRecorderInterface interface {
+	setAllocated(cidr string, allocated int)
+	setAvailable(cidr string, available int)
+	incrementAllocations(cidr, scope string)
+	incrementAllocationErrors(cidr, scope string)
+}
+
+// metricsRecorder implements metricsRecorderInterface.
+type metricsRecorder struct{}
+
+func (m *metricsRecorder) setAllocated(cidr string, allocated int) {
+	clusterIPAllocated.WithLabelValues(cidr).Set(float64(allocated))
+}
+
+func (m *metricsRecorder) setAvailable(cidr string, available int) {
+	clusterIPAvailable.WithLabelValues(cidr).Set(float64(available))
+}
+
+func (m *metricsRecorder) incrementAllocations(cidr, scope string) {
+	clusterIPAllocations.WithLabelValues(cidr, scope).Inc()
+}
+
+func (m *metricsRecorder) incrementAllocationErrors(cidr, scope string) {
+	clusterIPAllocationErrors.WithLabelValues(cidr, scope).Inc()
+}
+
+// emptyMetricsRecorder is a null object implements metricsRecorderInterface.
+type emptyMetricsRecorder struct{}
+
+func (*emptyMetricsRecorder) setAllocated(cidr string, allocated int)      {}
+func (*emptyMetricsRecorder) setAvailable(cidr string, available int)      {}
+func (*emptyMetricsRecorder) incrementAllocations(cidr, scope string)      {}
+func (*emptyMetricsRecorder) incrementAllocationErrors(cidr, scope string) {}