From 81c256d8d9a4b7adc8efa48e0b534a1e615cca54 Mon Sep 17 00:00:00 2001 From: Rawad Hossain Date: Mon, 24 Aug 2026 14:03:32 +0600 Subject: [PATCH 1/2] reshape readiness rules metric Signed-off-by: Rawad Hossain --- docs/TEST_README.md | 6 ++ docs/book/src/operations/monitoring.md | 19 ++++++ internal/controller/helper_unit_test.go | 61 +++++++++++++++++++ .../nodereadinessrule_controller.go | 17 ++++++ internal/metrics/metrics.go | 10 +++ internal/metrics/metrics_test.go | 54 ++++++++++++++++ 6 files changed, 167 insertions(+) diff --git a/docs/TEST_README.md b/docs/TEST_README.md index 6eedcf4a..91093d6a 100644 --- a/docs/TEST_README.md +++ b/docs/TEST_README.md @@ -241,6 +241,9 @@ After running the test scenario, you should see the following metrics: ```bash # Number of active rules curl -s http://localhost:8080/metrics | grep "node_readiness_rules_total" + + # Number of active rules by enforcement mode and dry-run state + curl -s http://localhost:8080/metrics | grep "node_readiness_rules{" ``` 2. **Taint Operations:** @@ -296,6 +299,9 @@ After completing Steps 1-9, verify the metrics reflect the test scenario: # Should show 1 rule (network-readiness-rule) curl -s http://localhost:8080/metrics | grep 'node_readiness_rules_total' +# Should show 1 rule under its enforcement_mode/dry_run combination +curl -s http://localhost:8080/metrics | grep 'node_readiness_rules{' + # Should show taint removal operations for worker2, worker3, worker4 curl -s http://localhost:8080/metrics | grep 'node_readiness_taint_operations_total{.*operation="remove"}' diff --git a/docs/book/src/operations/monitoring.md b/docs/book/src/operations/monitoring.md index fe2b6d1b..68b2d37a 100644 --- a/docs/book/src/operations/monitoring.md +++ b/docs/book/src/operations/monitoring.md @@ -10,6 +10,8 @@ The controller serves metrics on `/metrics` only when metrics are explicitly ena ### `node_readiness_rules_total` +***Deprecated:** use [`node_readiness_rules`](#node_readiness_rules) instead. It provides the same rule count with additional `enforcement_mode` and `dry_run` labels. `node_readiness_rules_total` is still published for compatibility.* + Number of `NodeReadinessRule` objects tracked by the controller. | Property | Value | @@ -18,6 +20,23 @@ Number of `NodeReadinessRule` objects tracked by the controller. | Labels | none | | Recorded when | The controller refreshes or removes a tracked rule | +### `node_readiness_rules` + +Number of `NodeReadinessRule` objects tracked by the controller by enforcement mode and dry-run state. + +| Property | Value | +| --- | --- | +| Type | `gauge` | +| Labels | `enforcement_mode`, `dry_run` | +| Recorded when | The controller refreshes or removes a tracked rule | + +#### Labels + +| Label | Description | Values | +| --- | --- | --- | +| `enforcement_mode` | Enforcement mode of the rule | `bootstrap-only`, `continuous` | +| `dry_run` | Whether the rule is in dry-run mode | `true`, `false` | + ### `node_readiness_taint_operations_total` Total number of taint operations performed by the controller. diff --git a/internal/controller/helper_unit_test.go b/internal/controller/helper_unit_test.go index 1d6123d8..2eadd67a 100644 --- a/internal/controller/helper_unit_test.go +++ b/internal/controller/helper_unit_test.go @@ -17,14 +17,17 @@ limitations under the License. package controller import ( + "strings" "testing" . "github.com/onsi/gomega" + "github.com/prometheus/client_golang/prometheus/testutil" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1" + "sigs.k8s.io/node-readiness-controller/internal/metrics" ) func TestBootstrapAnnotationKey(t *testing.T) { @@ -303,3 +306,61 @@ func TestApplyNodeStatusDelta(t *testing.T) { g.Expect(rule.Status.FailedNodes).To(BeEmpty()) }) } + +func TestSyncRulesByModeLocked(t *testing.T) { + g := NewWithT(t) + metrics.RulesByMode.Reset() + t.Cleanup(metrics.RulesByMode.Reset) + + c := &RuleReadinessController{ + ruleCache: make(map[string]*readinessv1alpha1.NodeReadinessRule), + } + ctx := t.Context() + + newRule := func(name string, mode readinessv1alpha1.EnforcementMode, dryRun bool) *readinessv1alpha1.NodeReadinessRule { + return &readinessv1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: readinessv1alpha1.NodeReadinessRuleSpec{ + EnforcementMode: mode, + DryRun: dryRun, + }, + } + } + + ruleA := newRule("rule-a", readinessv1alpha1.EnforcementModeBootstrapOnly, false) + ruleB := newRule("rule-b", readinessv1alpha1.EnforcementModeBootstrapOnly, false) + ruleC := newRule("rule-c", readinessv1alpha1.EnforcementModeContinuous, true) + + c.updateRuleCache(ctx, ruleA) + c.updateRuleCache(ctx, ruleB) + c.updateRuleCache(ctx, ruleC) + + expected := ` +# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state +# TYPE node_readiness_rules gauge +node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 +node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 +` + g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) + + c.removeRuleFromCache(ctx, "rule-c") + + expected = ` +# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state +# TYPE node_readiness_rules gauge +node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 +` + g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) + + ruleB.Spec.EnforcementMode = readinessv1alpha1.EnforcementModeContinuous + ruleB.Spec.DryRun = true + c.updateRuleCache(ctx, ruleB) + + expected = ` +# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state +# TYPE node_readiness_rules gauge +node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 1 +node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 +` + g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) +} diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index eb854306..c3777bfb 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "strconv" "strings" "sync" "time" @@ -709,6 +710,7 @@ func (r *RuleReadinessController) updateRuleCache(ctx context.Context, rule *rea ruleCopy := rule.DeepCopy() r.ruleCache[rule.Name] = ruleCopy metrics.RulesTotal.Set(float64(len(r.ruleCache))) + r.syncRulesByModeLocked() log.V(4).Info("Updated rule cache", "rule", rule.Name, "totalRules", len(r.ruleCache), @@ -723,9 +725,24 @@ func (r *RuleReadinessController) removeRuleFromCache(ctx context.Context, ruleN delete(r.ruleCache, ruleName) metrics.RulesTotal.Set(float64(len(r.ruleCache))) + r.syncRulesByModeLocked() log.Info("Removed rule from cache", "rule", ruleName, "totalRules", len(r.ruleCache)) } +// syncRulesByModeLocked updates RulesByMode from the rule cache. +func (r *RuleReadinessController) syncRulesByModeLocked() { + counts := make(map[[2]string]int) + for _, rule := range r.ruleCache { + key := [2]string{string(rule.Spec.EnforcementMode), strconv.FormatBool(rule.Spec.DryRun)} + counts[key]++ + } + + metrics.RulesByMode.Reset() + for key, count := range counts { + metrics.RulesByMode.WithLabelValues(key[0], key[1]).Set(float64(count)) + } +} + // patchRuleStatusWithOptimisticLock fetches the latest NodeReadinessRule, and apply mutate status // changes to it. It then patches the result to API with an optimistic-locked JSON merge patch. mutate // should return false if it made no changes, to skip an unnecessary Patch call. diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index db418e48..6cc0f56c 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -74,6 +74,15 @@ var ( }, ) + // RulesByMode tracks the number of NodeReadinessRules. + RulesByMode = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "node_readiness_rules", + Help: "Number of NodeReadinessRules by enforcement mode and dry-run state", + }, + []string{"enforcement_mode", "dry_run"}, + ) + // TaintOperations tracks the number of taint operations (add/remove). TaintOperations = prometheus.NewCounterVec( prometheus.CounterOpts{ @@ -179,6 +188,7 @@ var ( func init() { // Register custom metrics with the global prometheus registry metrics.Registry.MustRegister(RulesTotal) + metrics.Registry.MustRegister(RulesByMode) metrics.Registry.MustRegister(TaintOperations) metrics.Registry.MustRegister(EvaluationDuration) metrics.Registry.MustRegister(Failures) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 0a58a880..da68bcc1 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -20,10 +20,64 @@ import ( "strings" "testing" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" "sigs.k8s.io/controller-runtime/pkg/metrics" ) +func TestRulesByMode(t *testing.T) { + RulesByMode.Reset() + t.Cleanup(RulesByMode.Reset) + + RulesByMode.WithLabelValues("bootstrap-only", "false").Set(2) + RulesByMode.WithLabelValues("continuous", "true").Set(1) + + expected := ` +# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state +# TYPE node_readiness_rules gauge +node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 +node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 +` + assertObservationReflected(t, RulesByMode, "node_readiness_rules", expected) + + assertMetricRegistered(t, metrics.Registry, + "node_readiness_rules", "GAUGE", + "Number of NodeReadinessRules by enforcement mode and dry-run state") +} + +// assertMetricRegistered checks that the metric is registered correctly. +func assertMetricRegistered(t *testing.T, registry prometheus.Gatherer, name, wantType, wantHelp string) { + t.Helper() + + gathered, err := registry.Gather() + if err != nil { + t.Fatalf("failed to gather metrics: %v", err) + } + + for _, mf := range gathered { + if mf.GetName() != name { + continue + } + if got := mf.GetType().String(); got != wantType { + t.Fatalf("expected %s to be a %s, got %s", name, wantType, got) + } + if got := mf.GetHelp(); got != wantHelp { + t.Fatalf("unexpected help text for %s: got %q, want %q", name, got, wantHelp) + } + return + } + t.Fatalf("expected %s to be registered with the controller-runtime metrics registry", name) +} + +// assertObservationReflected checks the collected metric. +func assertObservationReflected(t *testing.T, collector prometheus.Collector, name, expectedExposition string) { + t.Helper() + + if err := testutil.CollectAndCompare(collector, strings.NewReader(expectedExposition), name); err != nil { + t.Fatalf("unexpected collecting result:\n%s", err) + } +} + func TestBuildInfo(t *testing.T) { expected := ` # HELP node_readiness_build_info Build information for the node-readiness-controller binary. From a89e0b00d69a5df048d293ec47731f29123e1977 Mon Sep 17 00:00:00 2001 From: Rawad Hossain Date: Wed, 16 Sep 2026 20:56:37 +0600 Subject: [PATCH 2/2] rules metric collector approach --- docs/book/src/operations/monitoring.md | 2 +- internal/controller/helper_unit_test.go | 70 +++++---------- .../nodereadinessrule_controller.go | 33 ++++--- internal/metrics/collector.go | 30 +++++++ internal/metrics/collector_test.go | 88 ++++++++++++++++++- internal/metrics/metrics.go | 10 --- internal/metrics/metrics_test.go | 54 ------------ 7 files changed, 155 insertions(+), 132 deletions(-) diff --git a/docs/book/src/operations/monitoring.md b/docs/book/src/operations/monitoring.md index 68b2d37a..88166e66 100644 --- a/docs/book/src/operations/monitoring.md +++ b/docs/book/src/operations/monitoring.md @@ -28,7 +28,7 @@ Number of `NodeReadinessRule` objects tracked by the controller by enforcement m | --- | --- | | Type | `gauge` | | Labels | `enforcement_mode`, `dry_run` | -| Recorded when | The controller refreshes or removes a tracked rule | +| Recorded when | Computed on each Prometheus scrape from the cached rule list | #### Labels diff --git a/internal/controller/helper_unit_test.go b/internal/controller/helper_unit_test.go index 2eadd67a..393fc5eb 100644 --- a/internal/controller/helper_unit_test.go +++ b/internal/controller/helper_unit_test.go @@ -17,11 +17,9 @@ limitations under the License. package controller import ( - "strings" "testing" . "github.com/onsi/gomega" - "github.com/prometheus/client_golang/prometheus/testutil" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -307,60 +305,40 @@ func TestApplyNodeStatusDelta(t *testing.T) { }) } -func TestSyncRulesByModeLocked(t *testing.T) { +func TestListRuleInventory(t *testing.T) { g := NewWithT(t) - metrics.RulesByMode.Reset() - t.Cleanup(metrics.RulesByMode.Reset) - c := &RuleReadinessController{ - ruleCache: make(map[string]*readinessv1alpha1.NodeReadinessRule), - } + c := &RuleReadinessController{} ctx := t.Context() - newRule := func(name string, mode readinessv1alpha1.EnforcementMode, dryRun bool) *readinessv1alpha1.NodeReadinessRule { - return &readinessv1alpha1.NodeReadinessRule{ + newRule := func(name string, mode readinessv1alpha1.EnforcementMode, dryRun bool, deleting bool) *readinessv1alpha1.NodeReadinessRule { + rule := &readinessv1alpha1.NodeReadinessRule{ ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: readinessv1alpha1.NodeReadinessRuleSpec{ EnforcementMode: mode, DryRun: dryRun, }, } + if deleting { + now := metav1.Now() + rule.DeletionTimestamp = &now + rule.Finalizers = []string{finalizerName} + } + return rule + } + + rules := []*readinessv1alpha1.NodeReadinessRule{ + newRule("rule-a", readinessv1alpha1.EnforcementModeBootstrapOnly, false, false), + newRule("rule-b", readinessv1alpha1.EnforcementModeBootstrapOnly, false, false), + newRule("rule-c", readinessv1alpha1.EnforcementModeContinuous, true, false), + newRule("rule-deleting", readinessv1alpha1.EnforcementModeContinuous, false, true), } - ruleA := newRule("rule-a", readinessv1alpha1.EnforcementModeBootstrapOnly, false) - ruleB := newRule("rule-b", readinessv1alpha1.EnforcementModeBootstrapOnly, false) - ruleC := newRule("rule-c", readinessv1alpha1.EnforcementModeContinuous, true) - - c.updateRuleCache(ctx, ruleA) - c.updateRuleCache(ctx, ruleB) - c.updateRuleCache(ctx, ruleC) - - expected := ` -# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state -# TYPE node_readiness_rules gauge -node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 -node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 -` - g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) - - c.removeRuleFromCache(ctx, "rule-c") - - expected = ` -# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state -# TYPE node_readiness_rules gauge -node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 -` - g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) - - ruleB.Spec.EnforcementMode = readinessv1alpha1.EnforcementModeContinuous - ruleB.Spec.DryRun = true - c.updateRuleCache(ctx, ruleB) - - expected = ` -# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state -# TYPE node_readiness_rules gauge -node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 1 -node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 -` - g.Expect(testutil.CollectAndCompare(metrics.RulesByMode, strings.NewReader(expected), "node_readiness_rules")).To(Succeed()) + counts, err := c.ListRuleInventory(ctx, rules) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(counts).To(Equal(map[metrics.RuleModeKey]float64{ + {EnforcementMode: "bootstrap-only", DryRun: false}: 2, + {EnforcementMode: "continuous", DryRun: true}: 1, + })) } diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index c3777bfb..dda662aa 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -19,7 +19,6 @@ package controller import ( "context" "fmt" - "strconv" "strings" "sync" "time" @@ -683,6 +682,22 @@ func (r *RuleReadinessController) ListBlockedNodes(ctx context.Context, nodes [] return result, nil } +// ListRuleInventory counts rules by enforcement mode and dry-run state. +func (r *RuleReadinessController) ListRuleInventory(_ context.Context, rules []*readinessv1alpha1.NodeReadinessRule) (map[metrics.RuleModeKey]float64, error) { + counts := make(map[metrics.RuleModeKey]float64) + + for _, rule := range rules { + if !rule.DeletionTimestamp.IsZero() { + continue + } + + key := metrics.RuleModeKey{EnforcementMode: string(rule.Spec.EnforcementMode), DryRun: rule.Spec.DryRun} + counts[key]++ + } + + return counts, nil +} + // parseNodeSelector parses a rule's NodeSelector into a labels.Selector. func parseNodeSelector(rule *readinessv1alpha1.NodeReadinessRule) (labels.Selector, error) { return metav1.LabelSelectorAsSelector(&rule.Spec.NodeSelector) @@ -710,7 +725,6 @@ func (r *RuleReadinessController) updateRuleCache(ctx context.Context, rule *rea ruleCopy := rule.DeepCopy() r.ruleCache[rule.Name] = ruleCopy metrics.RulesTotal.Set(float64(len(r.ruleCache))) - r.syncRulesByModeLocked() log.V(4).Info("Updated rule cache", "rule", rule.Name, "totalRules", len(r.ruleCache), @@ -725,24 +739,9 @@ func (r *RuleReadinessController) removeRuleFromCache(ctx context.Context, ruleN delete(r.ruleCache, ruleName) metrics.RulesTotal.Set(float64(len(r.ruleCache))) - r.syncRulesByModeLocked() log.Info("Removed rule from cache", "rule", ruleName, "totalRules", len(r.ruleCache)) } -// syncRulesByModeLocked updates RulesByMode from the rule cache. -func (r *RuleReadinessController) syncRulesByModeLocked() { - counts := make(map[[2]string]int) - for _, rule := range r.ruleCache { - key := [2]string{string(rule.Spec.EnforcementMode), strconv.FormatBool(rule.Spec.DryRun)} - counts[key]++ - } - - metrics.RulesByMode.Reset() - for key, count := range counts { - metrics.RulesByMode.WithLabelValues(key[0], key[1]).Set(float64(count)) - } -} - // patchRuleStatusWithOptimisticLock fetches the latest NodeReadinessRule, and apply mutate status // changes to it. It then patches the result to API with an optimistic-locked JSON merge patch. mutate // should return false if it made no changes, to skip an unnecessary Patch call. diff --git a/internal/metrics/collector.go b/internal/metrics/collector.go index e61d339f..e5649eb7 100644 --- a/internal/metrics/collector.go +++ b/internal/metrics/collector.go @@ -18,6 +18,7 @@ package metrics import ( "context" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -59,12 +60,24 @@ type BlockedNodesLister interface { ListBlockedNodes(ctx context.Context, nodes []corev1.Node, rules []*readinessv1alpha1.NodeReadinessRule) (map[string]RuleBlockedConditions, error) } +// RuleModeKey identifies a bucket of rules sharing the same enforcement mode and dry-run state. +type RuleModeKey struct { + EnforcementMode string + DryRun bool +} + +// RuleInventoryLister counts NodeReadinessRules by enforcement mode and dry-run state. +type RuleInventoryLister interface { + ListRuleInventory(ctx context.Context, rules []*readinessv1alpha1.NodeReadinessRule) (map[RuleModeKey]float64, error) +} + // ReadinessLister aggregates the scrape-time lookups the collector needs. type ReadinessLister interface { NodeLister RuleLister RuleNodeStateLister BlockedNodesLister + RuleInventoryLister } var ruleNodesDesc = prometheus.NewDesc( @@ -81,6 +94,13 @@ var blockedNodesDesc = prometheus.NewDesc( nil, ) +var ruleInventoryByModeDesc = prometheus.NewDesc( + "node_readiness_rules", + "Number of NodeReadinessRules by enforcement mode and dry-run state", + []string{"enforcement_mode", "dry_run"}, + nil, +) + // ReadinessCollector is a prometheus.Collector that reads at scrape time. type ReadinessCollector struct { lister ReadinessLister @@ -94,6 +114,7 @@ func NewReadinessCollector(lister ReadinessLister) *ReadinessCollector { func (c *ReadinessCollector) Describe(ch chan<- *prometheus.Desc) { ch <- ruleNodesDesc ch <- blockedNodesDesc + ch <- ruleInventoryByModeDesc } // Collect implements prometheus.Collector. @@ -133,4 +154,13 @@ func (c *ReadinessCollector) Collect(ch chan<- prometheus.Metric) { } } } + + ruleInventory, err := c.lister.ListRuleInventory(ctx, rules) + if err != nil { + ctrl.Log.V(2).Info("Failed to list rule inventory", "error", err) + } else { + for key, count := range ruleInventory { + ch <- prometheus.MustNewConstMetric(ruleInventoryByModeDesc, prometheus.GaugeValue, count, key.EnforcementMode, strconv.FormatBool(key.DryRun)) + } + } } diff --git a/internal/metrics/collector_test.go b/internal/metrics/collector_test.go index f7c6f295..b9926ae7 100644 --- a/internal/metrics/collector_test.go +++ b/internal/metrics/collector_test.go @@ -46,11 +46,15 @@ type stubLister struct { blocked map[string]RuleBlockedConditions blockedErr error + inventory map[RuleModeKey]float64 + inventoryErr error + mu sync.Mutex gotNodesForRuleStates []corev1.Node gotNodesForBlocked []corev1.Node gotRulesForRuleStates []*readinessv1alpha1.NodeReadinessRule gotRulesForBlocked []*readinessv1alpha1.NodeReadinessRule + gotRulesForInventory []*readinessv1alpha1.NodeReadinessRule } func (s *stubLister) ListNodes(_ context.Context) ([]corev1.Node, error) { @@ -89,6 +93,16 @@ func (s *stubLister) ListBlockedNodes(_ context.Context, nodes []corev1.Node, ru return s.blocked, nil } +func (s *stubLister) ListRuleInventory(_ context.Context, rules []*readinessv1alpha1.NodeReadinessRule) (map[RuleModeKey]float64, error) { + s.mu.Lock() + s.gotRulesForInventory = rules + s.mu.Unlock() + if s.inventoryErr != nil { + return nil, s.inventoryErr + } + return s.inventory, nil +} + func TestReadinessCollector_NoRules(t *testing.T) { c := NewReadinessCollector(&stubLister{counts: map[string]RuleNodeCounts{}}) @@ -220,8 +234,9 @@ func collectAll(t *testing.T, c *ReadinessCollector) map[string][]*dto.Metric { func TestReadinessCollector_RuleNodesErrorDoesNotBlockBlockedNodes(t *testing.T) { c := NewReadinessCollector(&stubLister{ - err: errors.New("cache not synced"), - blocked: map[string]RuleBlockedConditions{"gpu-ready": {"GPUDriverReady": 2}}, + err: errors.New("cache not synced"), + blocked: map[string]RuleBlockedConditions{"gpu-ready": {"GPUDriverReady": 2}}, + inventoryErr: errors.New("rule inventory cache not synced"), }) got := collectAll(t, c) @@ -241,8 +256,9 @@ func TestReadinessCollector_RuleNodesErrorDoesNotBlockBlockedNodes(t *testing.T) func TestReadinessCollector_BlockedNodesErrorDoesNotBlockRuleNodes(t *testing.T) { c := NewReadinessCollector(&stubLister{ - counts: map[string]RuleNodeCounts{"gpu-ready": {Held: 3, Released: 1}}, - blockedErr: errors.New("cache not synced"), + counts: map[string]RuleNodeCounts{"gpu-ready": {Held: 3, Released: 1}}, + blockedErr: errors.New("cache not synced"), + inventoryErr: errors.New("rule inventory cache not synced"), }) got := collectAll(t, c) @@ -349,6 +365,67 @@ func TestReadinessCollector_RulesSharedBetweenBothListers(t *testing.T) { } } +func TestReadinessCollector_RuleInventory_ByModeAndDryRun(t *testing.T) { + c := NewReadinessCollector(&stubLister{ + counts: map[string]RuleNodeCounts{}, + blocked: map[string]RuleBlockedConditions{}, + inventory: map[RuleModeKey]float64{ + {EnforcementMode: "bootstrap-only", DryRun: false}: 2, + {EnforcementMode: "continuous", DryRun: true}: 1, + }, + }) + + expected := ` + # HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state + # TYPE node_readiness_rules gauge + node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 + node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 + ` + if err := testutil.CollectAndCompare(c, strings.NewReader(expected), "node_readiness_rules"); err != nil { + t.Fatalf("unexpected collect mismatch: %v", err) + } +} + +func TestReadinessCollector_RuleInventoryErrorSkipsBothInventoryMetrics(t *testing.T) { + stub := &stubLister{ + counts: map[string]RuleNodeCounts{}, + blocked: map[string]RuleBlockedConditions{}, + inventoryErr: errors.New("cache not synced"), + } + c := NewReadinessCollector(stub) + + ch := make(chan prometheus.Metric, 8) + c.Collect(ch) + close(ch) + + for m := range ch { + if m.Desc() == ruleInventoryByModeDesc { + t.Fatalf("expected no rule inventory metrics when ListRuleInventory fails, got %v", m.Desc()) + } + } +} + +func TestReadinessCollector_RuleInventoryReceivesSharedRules(t *testing.T) { + rules := []*readinessv1alpha1.NodeReadinessRule{{ObjectMeta: metav1.ObjectMeta{Name: "gpu-ready"}}} + stub := &stubLister{ + rules: rules, + counts: map[string]RuleNodeCounts{}, + blocked: map[string]RuleBlockedConditions{}, + inventory: map[RuleModeKey]float64{}, + } + c := NewReadinessCollector(stub) + + ch := make(chan prometheus.Metric, 8) + c.Collect(ch) + close(ch) + for range ch { + } + + if len(stub.gotRulesForInventory) != 1 || stub.gotRulesForInventory[0].Name != "gpu-ready" { + t.Fatalf("ListRuleInventory did not receive the shared rule snapshot: %v", stub.gotRulesForInventory) + } +} + func TestReadinessCollector_CollectAndLint(t *testing.T) { c := NewReadinessCollector(&stubLister{ nodes: []corev1.Node{{}}, @@ -358,6 +435,9 @@ func TestReadinessCollector_CollectAndLint(t *testing.T) { blocked: map[string]RuleBlockedConditions{ "gpu-ready": {"GPUDriverReady": 2, "CNIReady": 0}, }, + inventory: map[RuleModeKey]float64{ + {EnforcementMode: "bootstrap-only", DryRun: false}: 1, + }, }) problems, err := testutil.CollectAndLint(c) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 6cc0f56c..db418e48 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -74,15 +74,6 @@ var ( }, ) - // RulesByMode tracks the number of NodeReadinessRules. - RulesByMode = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "node_readiness_rules", - Help: "Number of NodeReadinessRules by enforcement mode and dry-run state", - }, - []string{"enforcement_mode", "dry_run"}, - ) - // TaintOperations tracks the number of taint operations (add/remove). TaintOperations = prometheus.NewCounterVec( prometheus.CounterOpts{ @@ -188,7 +179,6 @@ var ( func init() { // Register custom metrics with the global prometheus registry metrics.Registry.MustRegister(RulesTotal) - metrics.Registry.MustRegister(RulesByMode) metrics.Registry.MustRegister(TaintOperations) metrics.Registry.MustRegister(EvaluationDuration) metrics.Registry.MustRegister(Failures) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index da68bcc1..0a58a880 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -20,64 +20,10 @@ import ( "strings" "testing" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" "sigs.k8s.io/controller-runtime/pkg/metrics" ) -func TestRulesByMode(t *testing.T) { - RulesByMode.Reset() - t.Cleanup(RulesByMode.Reset) - - RulesByMode.WithLabelValues("bootstrap-only", "false").Set(2) - RulesByMode.WithLabelValues("continuous", "true").Set(1) - - expected := ` -# HELP node_readiness_rules Number of NodeReadinessRules by enforcement mode and dry-run state -# TYPE node_readiness_rules gauge -node_readiness_rules{dry_run="false",enforcement_mode="bootstrap-only"} 2 -node_readiness_rules{dry_run="true",enforcement_mode="continuous"} 1 -` - assertObservationReflected(t, RulesByMode, "node_readiness_rules", expected) - - assertMetricRegistered(t, metrics.Registry, - "node_readiness_rules", "GAUGE", - "Number of NodeReadinessRules by enforcement mode and dry-run state") -} - -// assertMetricRegistered checks that the metric is registered correctly. -func assertMetricRegistered(t *testing.T, registry prometheus.Gatherer, name, wantType, wantHelp string) { - t.Helper() - - gathered, err := registry.Gather() - if err != nil { - t.Fatalf("failed to gather metrics: %v", err) - } - - for _, mf := range gathered { - if mf.GetName() != name { - continue - } - if got := mf.GetType().String(); got != wantType { - t.Fatalf("expected %s to be a %s, got %s", name, wantType, got) - } - if got := mf.GetHelp(); got != wantHelp { - t.Fatalf("unexpected help text for %s: got %q, want %q", name, got, wantHelp) - } - return - } - t.Fatalf("expected %s to be registered with the controller-runtime metrics registry", name) -} - -// assertObservationReflected checks the collected metric. -func assertObservationReflected(t *testing.T, collector prometheus.Collector, name, expectedExposition string) { - t.Helper() - - if err := testutil.CollectAndCompare(collector, strings.NewReader(expectedExposition), name); err != nil { - t.Fatalf("unexpected collecting result:\n%s", err) - } -} - func TestBuildInfo(t *testing.T) { expected := ` # HELP node_readiness_build_info Build information for the node-readiness-controller binary.