Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ClusterCIDR name to MultiCIDRSet to track metrics correctly. #47

Merged
merged 3 commits into from
Jan 3, 2025
Merged

Add ClusterCIDR name to MultiCIDRSet to track metrics correctly. #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f71d44
Add ClusterCIDR name to MultiCIDRSet to track metrics correctly.
mneverov Jan 2, 2025
3a0ac58
Register new metric label.
mneverov Jan 2, 2025
5658476
Fix tests.
mneverov Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/controller/ipam/multi_cidr_priority_queue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestLess(t *testing.T) {

func createTestPriorityQueueItem(name, cidr, selectorString string, labelMatchCount, perNodeHostBits int) *PriorityQueueItem {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(cidr)
cidrSet, _ := multicidrset.NewMultiCIDRSet(clusterCIDR, perNodeHostBits)
cidrSet, _ := multicidrset.NewMultiCIDRSet(name, clusterCIDR, perNodeHostBits)

return &PriorityQueueItem{
clusterCIDR: &multicidrset.ClusterCIDR{
Expand Down
8 changes: 6 additions & 2 deletions pkg/controller/ipam/multi_cidr_range_allocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,9 @@ func (r *multiCIDRRangeAllocator) createClusterCIDRSet(clusterCIDR *v1.ClusterCI
if err != nil {
return nil, fmt.Errorf("unable to parse provided IPv4 CIDR: %w", err)
}
clusterCIDRSet.IPv4CIDRSet, err = cidrset.NewMultiCIDRSet(ipv4CIDR, int(clusterCIDR.Spec.PerNodeHostBits))
clusterCIDRSet.IPv4CIDRSet, err = cidrset.NewMultiCIDRSet(
clusterCIDR.Name, ipv4CIDR, int(clusterCIDR.Spec.PerNodeHostBits),
)
if err != nil {
return nil, fmt.Errorf("unable to create IPv4 cidrSet: %w", err)
}
Expand All @@ -1192,7 +1194,9 @@ func (r *multiCIDRRangeAllocator) createClusterCIDRSet(clusterCIDR *v1.ClusterCI
if err != nil {
return nil, fmt.Errorf("unable to parse provided IPv6 CIDR: %w", err)
}
clusterCIDRSet.IPv6CIDRSet, err = cidrset.NewMultiCIDRSet(ipv6CIDR, int(clusterCIDR.Spec.PerNodeHostBits))
clusterCIDRSet.IPv6CIDRSet, err = cidrset.NewMultiCIDRSet(
clusterCIDR.Name, ipv6CIDR, int(clusterCIDR.Spec.PerNodeHostBits),
)
if err != nil {
return nil, fmt.Errorf("unable to create IPv6 cidrSet: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/ipam/multi_cidr_range_allocator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ func getTestCidrMap(testClusterCIDRMap map[string][]*testClusterCIDR) map[string

if testClusterCIDR.ipv4CIDR != "" {
_, testCIDR, _ := utilnet.ParseCIDRSloppy(testClusterCIDR.ipv4CIDR)
testCIDRSet, _ := multicidrset.NewMultiCIDRSet(testCIDR, int(testClusterCIDR.perNodeHostBits))
testCIDRSet, _ := multicidrset.NewMultiCIDRSet(clusterCIDR.Name, testCIDR, int(testClusterCIDR.perNodeHostBits))
clusterCIDR.IPv4CIDRSet = testCIDRSet
}
if testClusterCIDR.ipv6CIDR != "" {
_, testCIDR, _ := utilnet.ParseCIDRSloppy(testClusterCIDR.ipv6CIDR)
testCIDRSet, _ := multicidrset.NewMultiCIDRSet(testCIDR, int(testClusterCIDR.perNodeHostBits))
testCIDRSet, _ := multicidrset.NewMultiCIDRSet(clusterCIDR.Name, testCIDR, int(testClusterCIDR.perNodeHostBits))
clusterCIDR.IPv6CIDRSet = testCIDRSet
}
clusterCIDRList = append(clusterCIDRList, clusterCIDR)
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/ipam/multicidrset/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ var (
Name: "multicidrset_cidrs_allocations_total",
Help: "Counter measuring total number of CIDR allocations.",
},
[]string{"clusterCIDR"},
[]string{"clusterCIDR", "clusterCIDRName"},
)
cidrSetReleases = prometheus.NewCounterVec(
prometheus.CounterOpts{
Subsystem: nodeIpamSubsystem,
Name: "multicidrset_cidrs_releases_total",
Help: "Counter measuring total number of CIDR releases.",
},
[]string{"clusterCIDR"},
[]string{"clusterCIDR", "clusterCIDRName"},
)
// This is a gauge, as in theory, a limit can increase or decrease.
cidrSetMaxCidrs = prometheus.NewGaugeVec(
Expand All @@ -46,15 +46,15 @@ var (
Name: "multicidrset_max_cidrs",
Help: "Maximum number of CIDRs that can be allocated.",
},
[]string{"clusterCIDR"},
[]string{"clusterCIDR", "clusterCIDRName"},
)
cidrSetUsage = prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Subsystem: nodeIpamSubsystem,
Name: "multicidrset_usage_cidrs",
Help: "Gauge measuring percentage of allocated CIDRs.",
},
[]string{"clusterCIDR"},
[]string{"clusterCIDR", "clusterCIDRName"},
)
cidrSetAllocationTriesPerRequest = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Expand All @@ -63,7 +63,7 @@ var (
Help: "Histogram measuring CIDR allocation tries per request.",
Buckets: prometheus.ExponentialBuckets(1, 5, 5),
},
[]string{"clusterCIDR"},
[]string{"clusterCIDR", "clusterCIDRName"},
)
)

Expand Down
17 changes: 10 additions & 7 deletions pkg/controller/ipam/multicidrset/multi_cidr_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ type MultiCIDRSet struct {
// 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
// clusterCIDRName is a name of the ClusterCIDR this set belongs to.
clusterCIDRName string
// nodeMask is the network mask assigned to the nodes.
nodeMask net.IPMask
// allocatedCIDRs counts the number of CIDRs allocated.
Expand Down Expand Up @@ -112,7 +114,7 @@ func (err *CIDRSetSubNetTooBigErr) Error() string {
}

// NewMultiCIDRSet creates a new MultiCIDRSet.
func NewMultiCIDRSet(cidrConfig *net.IPNet, perNodeHostBits int) (*MultiCIDRSet, error) {
func NewMultiCIDRSet(clusterCIDRName string, cidrConfig *net.IPNet, perNodeHostBits int) (*MultiCIDRSet, error) {
clusterMask := cidrConfig.Mask
clusterMaskSize, bits := clusterMask.Size()

Expand All @@ -135,14 +137,15 @@ func NewMultiCIDRSet(cidrConfig *net.IPNet, perNodeHostBits int) (*MultiCIDRSet,
maxCIDRs := getMaxCIDRs(subNetMaskSize, clusterMaskSize)
multiCIDRSet := &MultiCIDRSet{
ClusterCIDR: cidrConfig,
clusterCIDRName: clusterCIDRName,
nodeMask: net.CIDRMask(subNetMaskSize, bits),
clusterMaskSize: clusterMaskSize,
MaxCIDRs: maxCIDRs,
NodeMaskSize: subNetMaskSize,
Label: cidrConfig.String(),
AllocatedCIDRMap: make(map[string]bool, 0),
}
cidrSetMaxCidrs.WithLabelValues(multiCIDRSet.Label).Set(float64(maxCIDRs))
cidrSetMaxCidrs.WithLabelValues(multiCIDRSet.Label, clusterCIDRName).Set(float64(maxCIDRs))

return multiCIDRSet, nil
}
Expand Down Expand Up @@ -287,11 +290,11 @@ func (s *MultiCIDRSet) Release(cidr *net.IPNet) error {
if _, ok := s.AllocatedCIDRMap[currCIDR.String()]; ok {
delete(s.AllocatedCIDRMap, currCIDR.String())
s.allocatedCIDRs--
cidrSetReleases.WithLabelValues(s.Label).Inc()
cidrSetReleases.WithLabelValues(s.Label, s.clusterCIDRName).Inc()
}
}

cidrSetUsage.WithLabelValues(s.Label).Set(float64(s.allocatedCIDRs) / float64(s.MaxCIDRs))
cidrSetUsage.WithLabelValues(s.Label, s.clusterCIDRName).Set(float64(s.allocatedCIDRs) / float64(s.MaxCIDRs))

return nil
}
Expand All @@ -315,11 +318,11 @@ func (s *MultiCIDRSet) Occupy(cidr *net.IPNet) (err error) {
}
if _, ok := s.AllocatedCIDRMap[currCIDR.String()]; !ok {
s.AllocatedCIDRMap[currCIDR.String()] = true
cidrSetAllocations.WithLabelValues(s.Label).Inc()
cidrSetAllocations.WithLabelValues(s.Label, s.clusterCIDRName).Inc()
s.allocatedCIDRs++
}
}
cidrSetUsage.WithLabelValues(s.Label).Set(float64(s.allocatedCIDRs) / float64(s.MaxCIDRs))
cidrSetUsage.WithLabelValues(s.Label, s.clusterCIDRName).Set(float64(s.allocatedCIDRs) / float64(s.MaxCIDRs))

return nil
}
Expand Down Expand Up @@ -348,7 +351,7 @@ func (s *MultiCIDRSet) getIndexForIP(ip net.IP) (int, error) {

// UpdateEvaluatedCount increments the evaluated count.
func (s *MultiCIDRSet) UpdateEvaluatedCount(evaluated int) {
cidrSetAllocationTriesPerRequest.WithLabelValues(s.Label).Observe(float64(evaluated))
cidrSetAllocationTriesPerRequest.WithLabelValues(s.Label, s.clusterCIDRName).Observe(float64(evaluated))
}

// getMaxCIDRs returns the max number of CIDRs that can be obtained by subdividing a mask of size `clusterMaskSize`
Expand Down
42 changes: 22 additions & 20 deletions pkg/controller/ipam/multicidrset/multi_cidr_set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestCIDRSetFullyAllocated(t *testing.T) {
}
for _, tc := range cases {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(tc.clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, tc.perNodeHostBits)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, tc.perNodeHostBits)
if err != nil {
t.Fatalf("unexpected error: %v for %v", err, tc.description)
}
Expand Down Expand Up @@ -210,7 +210,7 @@ func TestIndexToCIDRBlock(t *testing.T) {
}
for _, tc := range cases {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(tc.clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, tc.perNodeHostBits)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, tc.perNodeHostBits)
if err != nil {
t.Fatalf("error for %v ", tc.description)
}
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestCIDRSet_RandomishAllocation(t *testing.T) {
}
for _, tc := range cases {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(tc.clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, 8)
if err != nil {
t.Fatalf("Error allocating CIDRSet for %v", tc.description)
}
Expand Down Expand Up @@ -300,7 +300,7 @@ func TestCIDRSet_AllocationOccupied(t *testing.T) {
}
for _, tc := range cases {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(tc.clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, 8)
if err != nil {
t.Fatalf("Error allocating CIDRSet for %v", tc.description)
}
Expand Down Expand Up @@ -412,7 +412,7 @@ func TestDoubleOccupyRelease(t *testing.T) {
numAllocatable24s := (1 << 8) - 3

_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, 8)
if err != nil {
t.Fatalf("Error allocating CIDRSet")
}
Expand Down Expand Up @@ -575,7 +575,7 @@ func TestGetBitforCIDR(t *testing.T) {
t.Fatalf("unexpected error: %v for %v", err, tc.description)
}

cs, err := NewMultiCIDRSet(clusterCIDR, tc.perNodeHostBits)
cs, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, tc.perNodeHostBits)
if err != nil {
t.Fatalf("Error allocating CIDRSet for %v", tc.description)
}
Expand Down Expand Up @@ -636,7 +636,7 @@ func TestCIDRSetv6(t *testing.T) {
for _, tc := range cases {
t.Run(tc.description, func(t *testing.T) {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(tc.clusterCIDRStr)
a, err := NewMultiCIDRSet(clusterCIDR, tc.perNodeHostBits)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, tc.perNodeHostBits)
if gotErr := err != nil; gotErr != tc.expectErr {
t.Fatalf("NewMultiCIDRSet(%v, %v) = %v, %v; gotErr = %t, want %t", clusterCIDR, tc.perNodeHostBits, a, err, gotErr, tc.expectErr)
}
Expand Down Expand Up @@ -677,7 +677,7 @@ func TestMultiCIDRSetMetrics(t *testing.T) {
clearMetrics(map[string]string{"clusterCIDR": cidr})

// We have 256 free cidrs
a, err := NewMultiCIDRSet(clusterCIDR, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, 8)
if err != nil {
t.Fatalf("unexpected error creating MultiCIDRSet: %v", err)
}
Expand Down Expand Up @@ -734,10 +734,10 @@ func TestMultiCIDRSetMetrics(t *testing.T) {
func TestMultiCIDRSetMetricsHistogram(t *testing.T) {
cidr := "10.0.0.0/16"
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(cidr)
clearMetrics(map[string]string{"clusterCIDR": cidr})
clearMetrics(map[string]string{"clusterCIDR": cidr, "clusterCIDRName": "test-cluster-cidr"})

// We have 256 free cidrs.
a, err := NewMultiCIDRSet(clusterCIDR, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, 8)
if err != nil {
t.Fatalf("unexpected error creating MultiCIDRSet: %v", err)
}
Expand Down Expand Up @@ -783,9 +783,9 @@ func TestMultiCIDRSetMetricsDual(t *testing.T) {
// create IPv4 cidrSet.
cidrIPv4 := "10.0.0.0/16"
_, clusterCIDRv4, _ := utilnet.ParseCIDRSloppy(cidrIPv4)
clearMetrics(map[string]string{"clusterCIDR": cidrIPv4})
clearMetrics(map[string]string{"clusterCIDR": cidrIPv4, "clusterCIDRName": "test-cluster-cidr"})

a, err := NewMultiCIDRSet(clusterCIDRv4, 8)
a, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDRv4, 8)
if err != nil {
t.Fatalf("unexpected error creating MultiCIDRSet: %v", err)
}
Expand All @@ -804,9 +804,9 @@ func TestMultiCIDRSetMetricsDual(t *testing.T) {
// create IPv6 cidrSet.
cidrIPv6 := "2001:db8::/48"
_, clusterCIDRv6, _ := utilnet.ParseCIDRSloppy(cidrIPv6)
clearMetrics(map[string]string{"clusterCIDR": cidrIPv6})
clearMetrics(map[string]string{"clusterCIDR": cidrIPv6, "clusterCIDRName": "test-cluster-cidr"})

b, err := NewMultiCIDRSet(clusterCIDRv6, 64)
b, err := NewMultiCIDRSet("test-cluster-cidr", clusterCIDRv6, 64)
if err != nil {
t.Fatalf("unexpected error creating MultiCIDRSet: %v", err)
}
Expand Down Expand Up @@ -922,23 +922,25 @@ type testMetrics struct {
func expectMetrics(t *testing.T, label string, em testMetrics) {
var m testMetrics
var err error
m.usage, err = testutil.GetGaugeMetricValue(cidrSetUsage.WithLabelValues(label))
m.usage, err = testutil.GetGaugeMetricValue(cidrSetUsage.WithLabelValues(label, "test-cluster-cidr"))
if err != nil {
t.Errorf("failed to get cidr usage value, err: %v", err)
}
m.allocs, err = testutil.GetCounterMetricValue(cidrSetAllocations.WithLabelValues(label))
m.allocs, err = testutil.GetCounterMetricValue(cidrSetAllocations.WithLabelValues(label, "test-cluster-cidr"))
if err != nil {
t.Errorf("failed to get cidr allocations value, err: %v", err)
}
m.releases, err = testutil.GetCounterMetricValue(cidrSetReleases.WithLabelValues(label))
m.releases, err = testutil.GetCounterMetricValue(cidrSetReleases.WithLabelValues(label, "test-cluster-cidr"))
if err != nil {
t.Errorf("failed to get cidr releases value, err: %v", err)
}
m.allocTries, err = testutil.GetHistogramMetricValue(cidrSetAllocationTriesPerRequest.WithLabelValues(label))
m.allocTries, err = testutil.GetHistogramMetricValue(
cidrSetAllocationTriesPerRequest.WithLabelValues(label, "test-cluster-cidr"),
)
if err != nil {
t.Errorf("failed to get cidr allocation tries value, err: %v", err)
}
m.max, err = testutil.GetGaugeMetricValue(cidrSetMaxCidrs.WithLabelValues(label))
m.max, err = testutil.GetGaugeMetricValue(cidrSetMaxCidrs.WithLabelValues(label, "test-cluster-cidr"))
if err != nil {
t.Errorf("failed to get max cidrs value, err: %v", err)
}
Expand All @@ -951,7 +953,7 @@ func expectMetrics(t *testing.T, label string, em testMetrics) {
// Benchmarks
func benchmarkAllocateAllIPv6(cidr string, perNodeHostBits int, b *testing.B) {
_, clusterCIDR, _ := utilnet.ParseCIDRSloppy(cidr)
a, _ := NewMultiCIDRSet(clusterCIDR, perNodeHostBits)
a, _ := NewMultiCIDRSet("test-cluster-cidr", clusterCIDR, perNodeHostBits)
for n := 0; n < b.N; n++ {
// Allocate the whole range + 1.
for i := 0; i <= a.MaxCIDRs; i++ {
Expand Down
Loading