diff --git a/internal/controller/node_controller.go b/internal/controller/node_controller.go index 8119ac9..ec46407 100644 --- a/internal/controller/node_controller.go +++ b/internal/controller/node_controller.go @@ -393,6 +393,7 @@ func (r *RuleReadinessController) markBootstrapCompleted(ctx context.Context, no switch { case err != nil: log.Error(err, "Failed to mark bootstrap completed", "node", nodeName, "rule", ruleName) + metrics.BootstrapCompletionErrors.WithLabelValues(ruleName).Inc() case marked: log.Info("Marked bootstrap completed", "node", nodeName, "rule", ruleName) metrics.BootstrapCompleted.WithLabelValues(ruleName).Inc() diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index d73cf4a..d292e48 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -135,6 +135,15 @@ func (r *RuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. // Update rule cache (after cleanup) r.Controller.updateRuleCache(ctx, rule) + // Update selector_matched_nodes_total from the node list we already fetched. + var matchedCount int + for i := range nodeList.Items { + if r.Controller.ruleAppliesTo(ctx, rule, &nodeList.Items[i]) { + matchedCount++ + } + } + metrics.SelectorMatchedNodes.WithLabelValues(rule.Name).Set(float64(matchedCount)) + // Handle dry run if rule.Spec.DryRun { if err := r.Controller.processDryRun(ctx, rule, nodeList); err != nil { @@ -209,6 +218,9 @@ func (r *RuleReconciler) reconcileDelete(ctx context.Context, rule *readinessv1a metrics.RuleLastReconciliationTime.DeleteLabelValues(rule.Name) metrics.BootstrapCompleted.DeleteLabelValues(rule.Name) metrics.BootstrapDuration.DeleteLabelValues(rule.Name) + metrics.SelectorMatchedNodes.DeleteLabelValues(rule.Name) + metrics.BootstrapCompletionErrors.DeleteLabelValues(rule.Name) + metrics.BootstrapNRCDuration.DeleteLabelValues(rule.Name) // For multi-label metrics, use DeletePartialMatch to wipe all combinations metrics.NodesByState.DeletePartialMatch(ruleLabel) @@ -419,6 +431,8 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule if duration > 0 { metrics.BootstrapDuration.WithLabelValues(rule.Name).Observe(duration) } + // TODO: pending decision — record BootstrapNRCDuration from readiness.k8s.io/taint-applied- to time.Now(); skip if missing. + // Alternative: dedicated status condition (needs nodes/status RBAC). } else { log.V(4).Info("Skipping bootstrap duration metric for legacy node or missing transition", "node", node.Name, @@ -437,8 +451,12 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule // Record add taint latency and taint operation counter metrics.TaintOperations.WithLabelValues(rule.Name, "add").Inc() recordLatency("add_taint") + // TODO: pending decision — on first taint add, patch readiness.k8s.io/taint-applied- with time.Now() when absent (same r.Patch(), no extra RBAC). + // Alternative: dedicated status condition (needs nodes/status RBAC). case !shouldRemoveTaint && currentlyHasTaint: + // Adopted taints were already on the node — we never wrote taint-applied-, + // so BootstrapNRCDuration is skipped at completion. if isFirstEvaluation { log.Info("Adopting pre-existing taint", "node", node.Name, "rule", rule.Name, "taint", rule.Spec.Taint.Key) diff --git a/internal/controller/nodereadinessrule_controller_test.go b/internal/controller/nodereadinessrule_controller_test.go index f16d109..3e2ed84 100644 --- a/internal/controller/nodereadinessrule_controller_test.go +++ b/internal/controller/nodereadinessrule_controller_test.go @@ -55,6 +55,12 @@ func histogramSampleCount(histogram interface{ Write(*dto.Metric) error }) uint6 return metric.GetHistogram().GetSampleCount() } +func gaugeValue(gauge interface{ Write(*dto.Metric) error }) float64 { + metric := &dto.Metric{} + Expect(gauge.Write(metric)).To(Succeed()) + return metric.GetGauge().GetValue() +} + // errorInjectingClient forces Patch to fail for selected nodes. type errorInjectingClient struct { client.Client @@ -2229,4 +2235,136 @@ var _ = Describe("NodeReadinessRule Controller", func() { Expect(failedNames).NotTo(ContainElement("stale-recovery-node")) }) }) + + Context("Metric: selector_matched_nodes_total", func() { + It("should set gauge to matched node count after reconcile", func() { + ruleName := "sel-gauge-match-rule" + matchLabel := "sel-gauge-match" + + nodeA := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "sel-gauge-node-a", Labels: map[string]string{matchLabel: "true"}}} + nodeB := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "sel-gauge-node-b", Labels: map[string]string{matchLabel: "true"}}} + nodeC := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "sel-gauge-node-c", Labels: map[string]string{"other": "label"}}} + for _, n := range []*corev1.Node{nodeA, nodeB, nodeC} { + Expect(k8sClient.Create(ctx, n)).To(Succeed()) + } + defer func() { + for _, n := range []*corev1.Node{nodeA, nodeB, nodeC} { + _ = k8sClient.Delete(ctx, n) + } + }() + + rule := &nodereadinessiov1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{Name: ruleName, Finalizers: []string{finalizerName}}, + Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{ + Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "Ready", RequiredStatus: corev1.ConditionTrue}}, + Taint: corev1.Taint{Key: "readiness.k8s.io/sel-gauge-taint", Effect: corev1.TaintEffectNoSchedule}, + NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{matchLabel: "true"}}, + EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous, + }, + } + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rule) }() + + _, err := ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: ruleName}}) + Expect(err).NotTo(HaveOccurred()) + + gauge, err := metrics.SelectorMatchedNodes.GetMetricWith(prometheus.Labels{"rule": ruleName}) + Expect(err).NotTo(HaveOccurred()) + Expect(gaugeValue(gauge)).To(Equal(2.0)) + }) + + It("should set gauge to 0 when no nodes match the selector", func() { + ruleName := "sel-gauge-nomatch-rule" + + rule := &nodereadinessiov1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{Name: ruleName, Finalizers: []string{finalizerName}}, + Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{ + Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "Ready", RequiredStatus: corev1.ConditionTrue}}, + Taint: corev1.Taint{Key: "readiness.k8s.io/sel-nomatch-taint", Effect: corev1.TaintEffectNoSchedule}, + NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"definitely-no-such-label": "true"}}, + EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous, + }, + } + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, rule) }() + + _, err := ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: ruleName}}) + Expect(err).NotTo(HaveOccurred()) + + gauge, err := metrics.SelectorMatchedNodes.GetMetricWith(prometheus.Labels{"rule": ruleName}) + Expect(err).NotTo(HaveOccurred()) + Expect(gaugeValue(gauge)).To(Equal(0.0)) + }) + + It("should clean up label values on rule deletion", func() { + ruleName := "sel-gauge-del-rule" + matchLabel := "sel-gauge-del" + + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "sel-gauge-del-node", Labels: map[string]string{matchLabel: "true"}}} + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, node) }() + + rule := &nodereadinessiov1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{Name: ruleName, Finalizers: []string{finalizerName}}, + Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{ + Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "Ready", RequiredStatus: corev1.ConditionTrue}}, + Taint: corev1.Taint{Key: "readiness.k8s.io/sel-del-taint", Effect: corev1.TaintEffectNoSchedule}, + NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{matchLabel: "true"}}, + EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous, + }, + } + Expect(k8sClient.Create(ctx, rule)).To(Succeed()) + + _, err := ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: ruleName}}) + Expect(err).NotTo(HaveOccurred()) + + gauge, err := metrics.SelectorMatchedNodes.GetMetricWith(prometheus.Labels{"rule": ruleName}) + Expect(err).NotTo(HaveOccurred()) + Expect(gaugeValue(gauge)).To(Equal(1.0), "gauge should be 1 before deletion") + + Expect(k8sClient.Delete(ctx, rule)).To(Succeed()) + + // Deletion reconcile triggers reconcileDelete, which calls DeleteLabelValues. + Eventually(func() bool { + _, err := ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: ruleName}}) + Expect(err).NotTo(HaveOccurred()) + readinessController.ruleCacheMutex.RLock() + _, exists := readinessController.ruleCache[ruleName] + readinessController.ruleCacheMutex.RUnlock() + return !exists + }).Should(BeTrue()) + + // After DeleteLabelValues, GetMetricWith allocates a fresh gauge at 0 (old value removed). + freshGauge, err := metrics.SelectorMatchedNodes.GetMetricWith(prometheus.Labels{"rule": ruleName}) + Expect(err).NotTo(HaveOccurred()) + Expect(gaugeValue(freshGauge)).To(Equal(0.0)) + }) + }) + + Context("Metric: bootstrap_completion_errors_total", func() { + It("should increment counter when annotation write fails", func() { + ruleName := "bce-error-rule" + + before := counterValue(metrics.BootstrapCompletionErrors.WithLabelValues(ruleName)) + + readinessController.markBootstrapCompleted(ctx, "nonexistent-node-for-bce-test", ruleName) + + Expect(counterValue(metrics.BootstrapCompletionErrors.WithLabelValues(ruleName))).To(Equal(before + 1)) + }) + + It("should not increment counter on successful annotation write", func() { + nodeName := "bce-success-node" + ruleName := "bce-success-rule" + + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}} + Expect(k8sClient.Create(ctx, node)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, node) }() + + before := counterValue(metrics.BootstrapCompletionErrors.WithLabelValues(ruleName)) + + readinessController.markBootstrapCompleted(ctx, nodeName, ruleName) + + Expect(counterValue(metrics.BootstrapCompletionErrors.WithLabelValues(ruleName))).To(Equal(before)) + }) + }) }) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 1cfafc5..526008d 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -117,6 +117,35 @@ var ( }, []string{"rule"}, ) + + // SelectorMatchedNodes tracks how many nodes match a rule's NodeSelector. + SelectorMatchedNodes = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "node_readiness_selector_matched_nodes_total", + Help: "Number of nodes matched by a rule's NodeSelector", + }, + []string{"rule"}, + ) + + // BootstrapCompletionErrors tracks failures writing the bootstrap-completion annotation. + BootstrapCompletionErrors = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "node_readiness_bootstrap_completion_errors_total", + Help: "Total failures writing the bootstrap-completion annotation", + }, + []string{"rule"}, + ) + + // BootstrapNRCDuration tracks time from NRC taint application to bootstrap completion. + // Unlike bootstrap_duration_seconds, this excludes pre-NRC node boot time. + BootstrapNRCDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "node_readiness_bootstrap_nrc_duration_seconds", + Help: "Time from NRC taint application to bootstrap completion for bootstrap-only rules", + Buckets: []float64{1, 5, 10, 30, 60, 120, 300, 600, 1200}, // 1s to 20min + }, + []string{"rule"}, + ) ) func init() { @@ -131,4 +160,7 @@ func init() { metrics.Registry.MustRegister(NodesByState) metrics.Registry.MustRegister(ConditionEvaluationFailures) metrics.Registry.MustRegister(RuleLastReconciliationTime) + metrics.Registry.MustRegister(SelectorMatchedNodes) + metrics.Registry.MustRegister(BootstrapCompletionErrors) + metrics.Registry.MustRegister(BootstrapNRCDuration) }