extend histogram buckets

Author: rawadhossain

api/v1alpha1/nodereadinessrule_types.go

@@ -275,6 +275,16 @@ type NodeEvaluation struct {
 	//
 	// +required
 	LastEvaluationTime metav1.Time `json:"lastEvaluationTime,omitempty,omitzero"`
+
+	// taintAppliedAt is the timestamp when the controller applied the readiness taint to this Node.
+	//
+	// +optional
+	TaintAppliedAt metav1.Time `json:"taintAppliedAt,omitempty,omitzero"`
+
+	// taintObservedAt is the timestamp when the readiness taint was first observed on this Node.
+	//
+	// +optional
+	TaintObservedAt metav1.Time `json:"taintObservedAt,omitempty,omitzero"`
 }
 
 // ConditionEvaluationResult provides a detailed report of the comparison between

api/v1alpha1/zz_generated.deepcopy.go

@@ -440,6 +440,16 @@ spec:
                       minLength: 1
                       pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                       type: string
+                    taintAppliedAt:
+                      description: taintAppliedAt is the timestamp when the controller
+                        applied the readiness taint to this Node.
+                      format: date-time
+                      type: string
+                    taintObservedAt:
+                      description: taintObservedAt is the timestamp when the readiness
+                        taint was first observed on this Node.
+                      format: date-time
+                      type: string
                     taintStatus:
                       description: taintStatus represents the taint status on the
                         Node, one of Present, Absent.

config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml

@@ -69,6 +69,25 @@ Total number of failure events recorded by the controller.
 | `rule` | `NodeReadinessRule` name | Any rule name |
 | `reason` | Failure label recorded by the controller | `EvaluationError`, `AddTaintError`, `RemoveTaintError` |
 
+### `node_readiness_bootstrap_hold_duration_seconds`
+
+Time from readiness taint application or observation to bootstrap completion.
+
+| Property | Value |
+| --- | --- |
+| Type | `histogram` |
+| Labels | `rule`, `taint_origin` |
+| Buckets | `1, 5, 10, 30, 60, 120, 300, 600, 1200, 1800, 3600` |
+| Recorded when | The controller marks bootstrap as completed for a node under a bootstrap-only rule. |
+
+#### Labels
+
+| Label | Description | Values |
+| --- | --- | --- |
+| `rule` | `NodeReadinessRule` name | Any rule name |
+| `taint_origin` | Origin of the readiness taint's anchor timestamp | `controller`, `adopted` |
+
+
 ### `node_readiness_build_info`
 
 *Available starting from the v0.6.0 release.*

docs/book/src/operations/monitoring.md

@@ -25,17 +25,21 @@ import (
 	readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
 )
 
-//nolint:godot
 const (
 	// bootstrapAnnotationPrefix is the common prefix for all bootstrap completion
 	// annotations on a Node. The suffix is the rule's metadata.uid (RFC 4122 UUID,
 	// ~36 chars), which is immutable for the object's lifetime and globally unique.
 	//
 	// Full key format: readiness.k8s.io/bootstrap-completed-<ruleUID>
-	// Value format:    {"rule-name":"<ruleName>"}   (for human readability)
+	// Value format:    {"rule-name":"<ruleName>"}   (for human readability).
 	bootstrapAnnotationPrefix = "readiness.k8s.io/bootstrap-completed-"
 )
 
+// bootstrapAnnotationPayload is the JSON value stored in a bootstrap-completion annotation.
+type bootstrapAnnotationPayload struct {
+	RuleName string `json:"rule-name"`
+}
+
 // bootstrapAnnotationKey returns the annotation key for a rule's bootstrap
 // completion state, using the rule's UID as the suffix.
 func bootstrapAnnotationKey(uid types.UID) string {
@@ -45,9 +49,7 @@ func bootstrapAnnotationKey(uid types.UID) string {
 // bootstrapAnnotationValue returns the JSON-encoded value to store in the
 // bootstrap annotation. It includes the rule name for human readability.
 func bootstrapAnnotationValue(ruleName string) string {
-	v := struct {
-		RuleName string `json:"rule-name"`
-	}{RuleName: ruleName}
+	v := bootstrapAnnotationPayload{RuleName: ruleName}
 	b, err := json.Marshal(v)
 	if err != nil {
 		return `{"rule-name":""}` // should never happen

internal/controller/helper.go

@@ -20,6 +20,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -147,6 +149,16 @@ func (r *RuleReadinessController) processNodeAgainstAllRules(ctx context.Context
 			continue
 		}
 
+		// Recover a missing TaintAppliedAt/TaintObservedAt anchor before evaluating the rule.
+		// Skip repeated recovery checks once attempts are exhausted.
+		if rule.Spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly &&
+			r.hasTaintBySpec(node, rule.Spec.Taint) &&
+			r.taintAnchorMissing(rule, node.Name) &&
+			r.shouldAttemptTaintAppliedAtRecovery(rule.Name, node.Name) {
+			recovered := r.recoverTaintAppliedAtFromAPI(ctx, rule, node.Name)
+			r.recordTaintAppliedAtRecoveryOutcome(rule.Name, node.Name, recovered)
+		}
+
 		log.Info("Evaluating rule for node",
 			"node", node.Name,
 			"rule", rule.Name,
@@ -192,6 +204,12 @@ func (r *RuleReadinessController) processNodeAgainstAllRules(ctx context.Context
 			found := false
 			for i := range latestRule.Status.NodeEvaluations {
 				if latestRule.Status.NodeEvaluations[i].NodeName == node.Name {
+					if currEval.TaintAppliedAt.IsZero() && !latestRule.Status.NodeEvaluations[i].TaintAppliedAt.IsZero() {
+						currEval.TaintAppliedAt = latestRule.Status.NodeEvaluations[i].TaintAppliedAt
+					}
+					if currEval.TaintObservedAt.IsZero() && !latestRule.Status.NodeEvaluations[i].TaintObservedAt.IsZero() {
+						currEval.TaintObservedAt = latestRule.Status.NodeEvaluations[i].TaintObservedAt
+					}
 					latestRule.Status.NodeEvaluations[i] = currEval
 					found = true
 					break
@@ -250,6 +268,127 @@ func (r *RuleReadinessController) processNodeAgainstAllRules(ctx context.Context
 	return errors.Join(errs...)
 }
 
+const maxTaintAnchorRecoveryAttempts = 2
+
+// Reports whether the cached evaluation is missing TaintAppliedAt or TaintObservedAt.
+func (r *RuleReadinessController) taintAnchorMissing(rule *readinessv1alpha1.NodeReadinessRule, nodeName string) bool {
+	r.ruleCacheMutex.Lock()
+	defer r.ruleCacheMutex.Unlock()
+
+	prevEval := r.getPreviousNodeEvaluation(rule, nodeName)
+	return prevEval == nil || prevEval.TaintAppliedAt.IsZero() && prevEval.TaintObservedAt.IsZero()
+}
+
+// Reports whether recovery should still be attempted.
+func (r *RuleReadinessController) shouldAttemptTaintAppliedAtRecovery(ruleName, nodeName string) bool {
+	r.taintAnchorRecoveryMutex.Lock()
+	defer r.taintAnchorRecoveryMutex.Unlock()
+
+	return r.taintAnchorRecoveryAttempts[ruleName+"/"+nodeName] < maxTaintAnchorRecoveryAttempts
+}
+
+// Records the outcome of a recovery attempt.
+func (r *RuleReadinessController) recordTaintAppliedAtRecoveryOutcome(ruleName, nodeName string, recovered bool) {
+	key := ruleName + "/" + nodeName
+
+	r.taintAnchorRecoveryMutex.Lock()
+	defer r.taintAnchorRecoveryMutex.Unlock()
+
+	if recovered {
+		delete(r.taintAnchorRecoveryAttempts, key)
+		return
+	}
+	if r.taintAnchorRecoveryAttempts == nil {
+		r.taintAnchorRecoveryAttempts = make(map[string]int)
+	}
+	r.taintAnchorRecoveryAttempts[key]++
+}
+
+// Clears recovery tracking for a deleted rule.
+func (r *RuleReadinessController) clearTaintAppliedAtRecoveryForRule(ruleName string) {
+	prefix := ruleName + "/"
+
+	r.taintAnchorRecoveryMutex.Lock()
+	defer r.taintAnchorRecoveryMutex.Unlock()
+
+	for key := range r.taintAnchorRecoveryAttempts {
+		if strings.HasPrefix(key, prefix) {
+			delete(r.taintAnchorRecoveryAttempts, key)
+		}
+	}
+}
+
+// Clears recovery tracking for a rule/node pair.
+func (r *RuleReadinessController) clearTaintAppliedAtRecoveryForNode(ruleName, nodeName string) {
+	r.taintAnchorRecoveryMutex.Lock()
+	defer r.taintAnchorRecoveryMutex.Unlock()
+
+	delete(r.taintAnchorRecoveryAttempts, ruleName+"/"+nodeName)
+}
+
+// Recovers a missing TaintAppliedAt/TaintObservedAt from the API and updates the cached rule.
+// Returns true if an existing anchor was found.
+func (r *RuleReadinessController) recoverTaintAppliedAtFromAPI(ctx context.Context, rule *readinessv1alpha1.NodeReadinessRule, nodeName string) bool {
+	log := ctrl.LoggerFrom(ctx)
+
+	const (
+		attempts = 3
+		delay    = 500 * time.Millisecond
+	)
+
+	for i := range attempts {
+		if i > 0 {
+			select {
+			case <-ctx.Done():
+				return false
+			case <-time.After(delay):
+			}
+		}
+
+		latestRule := &readinessv1alpha1.NodeReadinessRule{}
+		if err := r.Get(ctx, client.ObjectKey{Name: rule.Name}, latestRule); err != nil {
+			log.V(4).Info("Failed to refresh rule for TaintAppliedAt recovery",
+				"rule", rule.Name, "node", nodeName, "error", err.Error())
+			continue
+		}
+
+		for _, eval := range latestRule.Status.NodeEvaluations {
+			if eval.NodeName != nodeName {
+				continue
+			}
+			if eval.TaintAppliedAt.IsZero() && eval.TaintObservedAt.IsZero() {
+				continue
+			}
+
+			r.ruleCacheMutex.Lock()
+			nodeEval := r.getOrCreateNodeEvaluation(rule, nodeName)
+			if nodeEval.TaintAppliedAt.IsZero() && !eval.TaintAppliedAt.IsZero() {
+				nodeEval.TaintAppliedAt = eval.TaintAppliedAt
+			}
+			if nodeEval.TaintObservedAt.IsZero() && !eval.TaintObservedAt.IsZero() {
+				nodeEval.TaintObservedAt = eval.TaintObservedAt
+			}
+
+			if cachedRule, ok := r.ruleCache[rule.Name]; ok {
+				cachedNodeEval := r.getOrCreateNodeEvaluation(cachedRule, nodeName)
+				if cachedNodeEval.TaintAppliedAt.IsZero() && !eval.TaintAppliedAt.IsZero() {
+					cachedNodeEval.TaintAppliedAt = eval.TaintAppliedAt
+				}
+				if cachedNodeEval.TaintObservedAt.IsZero() && !eval.TaintObservedAt.IsZero() {
+					cachedNodeEval.TaintObservedAt = eval.TaintObservedAt
+				}
+			}
+			r.ruleCacheMutex.Unlock()
+
+			log.V(4).Info("Recovered taint anchor(s) from API into stale cache entry",
+				"rule", rule.Name, "node", nodeName,
+				"taintAppliedAt", eval.TaintAppliedAt, "taintObservedAt", eval.TaintObservedAt)
+			return true
+		}
+	}
+	return false
+}
+
 // getConditionStatus gets the status of a condition on a node.
 // If the condition is not present, defaultStatus is returned with found=false.
 func (r *RuleReadinessController) getConditionStatus(

internal/controller/node_controller.go

@@ -375,6 +375,97 @@ var _ = Describe("Node Controller", func() {
 					return false
 				}, time.Second*2).Should(BeFalse())
 			})
+
+			It("should not retry recovery for an adopted taint", func() {
+				// The node already has the taint, so the first reconcile takes the adopt path.
+				// TaintObservedAt is set while TaintAppliedAt stays zero.
+				_, err := nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+				Expect(err).NotTo(HaveOccurred())
+
+				updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+				Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ruleName}, updatedRule)).To(Succeed())
+
+				var eval *nodereadinessiov1alpha1.NodeEvaluation
+				for i := range updatedRule.Status.NodeEvaluations {
+					if updatedRule.Status.NodeEvaluations[i].NodeName == nodeName {
+						eval = &updatedRule.Status.NodeEvaluations[i]
+					}
+				}
+				Expect(eval).NotTo(BeNil())
+				Expect(eval.TaintAppliedAt.IsZero()).To(BeTrue(), "TaintAppliedAt must stay zero for an adopted taint")
+				Expect(eval.TaintObservedAt.IsZero()).To(BeFalse(), "TaintObservedAt must be stamped for an adopted taint")
+				Expect(readinessController.taintAnchorMissing(updatedRule, nodeName)).To(BeFalse())
+
+				// Refresh the cache and record the recovery attempts so far.
+				readinessController.ruleCache[ruleName] = updatedRule
+				readinessController.taintAnchorRecoveryMutex.Lock()
+				attemptsBefore := readinessController.taintAnchorRecoveryAttempts[ruleName+"/"+nodeName]
+				readinessController.taintAnchorRecoveryMutex.Unlock()
+
+				_, err = nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+				Expect(err).NotTo(HaveOccurred())
+
+				readinessController.taintAnchorRecoveryMutex.Lock()
+				attemptsAfter := readinessController.taintAnchorRecoveryAttempts[ruleName+"/"+nodeName]
+				readinessController.taintAnchorRecoveryMutex.Unlock()
+				Expect(attemptsAfter).To(Equal(attemptsBefore),
+					"no additional recovery attempt should have been made once TaintObservedAt is known")
+			})
+
+			It("should self-heal the rule cache after recovering the taint anchor", func() {
+				// Reconcile #1: adopt the existing taint and persist TaintObservedAt to the API.
+				_, err := nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+				Expect(err).NotTo(HaveOccurred())
+
+				readCachedEval := func() *nodereadinessiov1alpha1.NodeEvaluation {
+					readinessController.ruleCacheMutex.RLock()
+					defer readinessController.ruleCacheMutex.RUnlock()
+					cachedRule, ok := readinessController.ruleCache[ruleName]
+					if !ok {
+						return nil
+					}
+					for i := range cachedRule.Status.NodeEvaluations {
+						if cachedRule.Status.NodeEvaluations[i].NodeName == nodeName {
+							return &cachedRule.Status.NodeEvaluations[i]
+						}
+					}
+					return nil
+				}
+
+				Expect(readCachedEval()).To(BeNil(),
+					"the persistent cache should still be stale immediately after the adopt reconcile")
+
+				// Reconcile #2: recover TaintObservedAt from the API and update the cache.
+				_, err = nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+				Expect(err).NotTo(HaveOccurred())
+
+				cachedEval := readCachedEval()
+				Expect(cachedEval).NotTo(BeNil(),
+					"the persistent cache should now contain a NodeEvaluation for this node")
+				Expect(cachedEval.TaintObservedAt.IsZero()).To(BeFalse(),
+					"the recovered TaintObservedAt should have been written into r.ruleCache directly")
+				Expect(cachedEval.TaintAppliedAt.IsZero()).To(BeTrue())
+
+				readinessController.ruleCacheMutex.RLock()
+				cachedRule := readinessController.ruleCache[ruleName]
+				readinessController.ruleCacheMutex.RUnlock()
+				Expect(readinessController.taintAnchorMissing(cachedRule, nodeName)).To(BeFalse(),
+					"the self-healed cache entry must no longer report the anchor as missing")
+
+				readinessController.taintAnchorRecoveryMutex.Lock()
+				attemptsBefore := readinessController.taintAnchorRecoveryAttempts[ruleName+"/"+nodeName]
+				readinessController.taintAnchorRecoveryMutex.Unlock()
+
+				// Reconcile #3: the cache is already healed, so no further recovery is needed.
+				_, err = nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+				Expect(err).NotTo(HaveOccurred())
+
+				readinessController.taintAnchorRecoveryMutex.Lock()
+				attemptsAfter := readinessController.taintAnchorRecoveryAttempts[ruleName+"/"+nodeName]
+				readinessController.taintAnchorRecoveryMutex.Unlock()
+				Expect(attemptsAfter).To(Equal(attemptsBefore),
+					"no further recovery attempt should be needed once the cache has self-healed")
+			})
 		})
 
 		When("in continuous mode", func() {