From 81c256d8d9a4b7adc8efa48e0b534a1e615cca54 Mon Sep 17 00:00:00 2001 From: Rawad Hossain Date: Mon, 24 Aug 2026 14:03:32 +0600 Subject: [PATCH] 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.