bootstrap failures and use reason constants

Author: rawadhossain

internal/controller/node_controller.go

@@ -156,7 +156,7 @@ func (r *RuleReadinessController) processNodeAgainstAllRules(ctx context.Context
 			log.Error(err, "Failed to evaluate rule for node",
 				"node", node.Name, "rule", rule.Name)
 			// Continue with other rules even if one fails
-			r.recordNodeFailure(rule, node.Name, "EvaluationError", err.Error())
+			r.recordNodeFailure(rule, node.Name, string(metrics.FailureReasonEvaluationError), err.Error())
 			errs = append(errs, err)
 			metrics.Failures.WithLabelValues(rule.Name, string(metrics.FailureReasonEvaluationError)).Inc()
 		}
@@ -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, "uid", ruleUID)
+		metrics.Failures.WithLabelValues(ruleName, string(metrics.FailureReasonAnnotationPatchFailed)).Inc()
 	case marked:
 		log.Info("Marked bootstrap completed", "node", nodeName, "rule", ruleName, "uid", ruleUID)
 		metrics.BootstrapCompleted.WithLabelValues(ruleName).Inc()

internal/controller/nodereadinessrule_controller.go

@@ -138,9 +138,19 @@ func (r *RuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	// Update rule cache (after cleanup)
 	r.Controller.updateRuleCache(ctx, rule)
 
+	// Filter nodes once and update node_readiness_rule_matched_nodes.
+	var matchedNodes []corev1.Node
+	for i := range nodeList.Items {
+		if r.Controller.ruleAppliesTo(ctx, rule, &nodeList.Items[i]) {
+			matchedNodes = append(matchedNodes, nodeList.Items[i])
+		}
+	}
+	metrics.RuleMatchedNodes.WithLabelValues(rule.Name).Set(float64(len(matchedNodes)))
+	filteredList := &corev1.NodeList{Items: matchedNodes}
+
 	// Handle dry run
 	if rule.Spec.DryRun {
-		if err := r.Controller.processDryRun(ctx, rule, nodeList); err != nil {
+		if err := r.Controller.processDryRun(ctx, rule, filteredList); err != nil {
 			log.Error(err, "Failed to process dry run", "rule", rule.Name)
 			return ctrl.Result{RequeueAfter: time.Minute}, err
 		}
@@ -149,7 +159,7 @@ func (r *RuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		rule.Status.DryRunResults = readinessv1alpha1.DryRunResults{}
 
 		// Process all applicable nodes for this rule
-		if err := r.Controller.processAllNodesForRule(ctx, rule, nodeList); err != nil {
+		if err := r.Controller.processAllNodesForRule(ctx, rule, filteredList); err != nil {
 			log.Error(err, "Failed to process nodes for rule", "rule", rule.Name)
 			return ctrl.Result{RequeueAfter: time.Minute}, err
 		}
@@ -213,6 +223,7 @@ func (r *RuleReconciler) reconcileDelete(ctx context.Context, rule *readinessv1a
 	metrics.BootstrapCompleted.DeleteLabelValues(rule.Name)
 	metrics.BootstrapDuration.DeleteLabelValues(rule.Name)
 	metrics.EvaluationDuration.DeleteLabelValues(rule.Name)
+	metrics.RuleMatchedNodes.DeleteLabelValues(rule.Name)
 
 	// For multi-label metrics, use DeletePartialMatch to wipe all combinations
 	metrics.NodesByState.DeletePartialMatch(ruleLabel)
@@ -281,24 +292,26 @@ func (r *RuleReadinessController) cleanupDeletedNodes(ctx context.Context, rule
 func (r *RuleReadinessController) processAllNodesForRule(ctx context.Context, rule *readinessv1alpha1.NodeReadinessRule, nodeList *corev1.NodeList) error {
 	log := ctrl.LoggerFrom(ctx)
 
-	log.Info("Processing all nodes for rule", "rule", rule.Name, "totalNodes", len(nodeList.Items))
+	log.Info("Processing all nodes for rule", "rule", rule.Name, "matchedNodes", len(nodeList.Items))
 
 	var appliedNodes []string
 	for _, node := range nodeList.Items {
 		if r.ruleAppliesTo(ctx, rule, &node) {
 			log.Info("Processing node for rule", "rule", rule.Name, "node", node.Name)
 			if err := r.evaluateRuleForNode(ctx, rule, &node); err != nil {
 				log.Error(err, "Failed to evaluate node for rule", "rule", rule.Name, "node", node.Name)
-				r.recordNodeFailure(rule, node.Name, "EvaluationError", err.Error())
+				r.recordNodeFailure(rule, node.Name, string(metrics.FailureReasonEvaluationError), err.Error())
 				metrics.Failures.WithLabelValues(rule.Name, string(metrics.FailureReasonEvaluationError)).Inc()
 			} else {
 				appliedNodes = append(appliedNodes, node.Name)
+
 				var updatedFailedNodes []readinessv1alpha1.NodeFailure
 				for _, f := range rule.Status.FailedNodes {
 					if f.NodeName != node.Name {
 						updatedFailedNodes = append(updatedFailedNodes, f)
 					}
 				}
+
 				rule.Status.FailedNodes = updatedFailedNodes
 			}
 		}
@@ -600,10 +613,6 @@ func (r *RuleReadinessController) processDryRun(ctx context.Context, rule *readi
 	var summaryParts []string
 
 	for _, node := range nodeList.Items {
-		if !r.ruleAppliesTo(ctx, rule, &node) {
-			continue
-		}
-
 		affectedNodes++
 
 		// Simulate rule evaluation

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
@@ -2235,4 +2241,138 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 			Expect(failedNames).NotTo(ContainElement("stale-recovery-node"))
 		})
 	})
+
+	Context("Metric: node_readiness_rule_matched_nodes", 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.RuleMatchedNodes.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.RuleMatchedNodes.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.RuleMatchedNodes.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.RuleMatchedNodes.GetMetricWith(prometheus.Labels{"rule": ruleName})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(gaugeValue(freshGauge)).To(Equal(0.0))
+		})
+	})
+
+	Context("Metric: failures_total (reason=AnnotationPatchFailed)", func() {
+		It("should increment counter when annotation write fails", func() {
+			ruleName := "bce-error-rule"
+			ruleUID := types.UID("33333333-3333-3333-3333-333333333333")
+
+			before := counterValue(metrics.Failures.WithLabelValues(ruleName, string(metrics.FailureReasonAnnotationPatchFailed)))
+
+			readinessController.markBootstrapCompleted(ctx, "nonexistent-node-for-bce-test", ruleName, ruleUID)
+
+			Expect(counterValue(metrics.Failures.WithLabelValues(ruleName, string(metrics.FailureReasonAnnotationPatchFailed)))).To(Equal(before + 1))
+		})
+
+		It("should not increment counter on successful annotation write", func() {
+			nodeName := "bce-success-node"
+			ruleName := "bce-success-rule"
+			ruleUID := types.UID("44444444-4444-4444-4444-444444444444")
+
+			node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}}
+			Expect(k8sClient.Create(ctx, node)).To(Succeed())
+			defer func() { _ = k8sClient.Delete(ctx, node) }()
+
+			before := counterValue(metrics.Failures.WithLabelValues(ruleName, string(metrics.FailureReasonAnnotationPatchFailed)))
+
+			readinessController.markBootstrapCompleted(ctx, nodeName, ruleName, ruleUID)
+
+			Expect(counterValue(metrics.Failures.WithLabelValues(ruleName, string(metrics.FailureReasonAnnotationPatchFailed)))).To(Equal(before))
+		})
+	})
 })

internal/metrics/metrics.go

@@ -25,9 +25,10 @@ import (
 type FailureReason string
 
 const (
-	FailureReasonEvaluationError  FailureReason = "EvaluationError"
-	FailureReasonAddTaintError    FailureReason = "AddTaintError"
-	FailureReasonRemoveTaintError FailureReason = "RemoveTaintError"
+	FailureReasonEvaluationError       FailureReason = "EvaluationError"
+	FailureReasonAddTaintError         FailureReason = "AddTaintError"
+	FailureReasonRemoveTaintError      FailureReason = "RemoveTaintError"
+	FailureReasonAnnotationPatchFailed FailureReason = "AnnotationPatchFailed"
 )
 
 // TaintOperation represents a taint operation.
@@ -152,6 +153,15 @@ var (
 		},
 		[]string{"rule"},
 	)
+
+	// RuleMatchedNodes tracks how many nodes match each rule's selector.
+	RuleMatchedNodes = prometheus.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Name: "node_readiness_rule_matched_nodes",
+			Help: "Number of nodes matched by a rule's NodeSelector",
+		},
+		[]string{"rule"},
+	)
 )
 
 func init() {
@@ -166,4 +176,5 @@ func init() {
 	metrics.Registry.MustRegister(NodesByState)
 	metrics.Registry.MustRegister(ConditionEvaluationFailures)
 	metrics.Registry.MustRegister(RuleLastReconciliationTime)
+	metrics.Registry.MustRegister(RuleMatchedNodes)
 }