Skip to content

[FEATURE] Add opt-in heartbeat freshness enforcement for continuous Node conditions #357

Description

@alanhuangch

Is your feature request related to a problem or existing issue?

This is a design request rather than a claim that the current API violates an existing contract.

Custom Node conditions are persisted last-known state. When a condition writer last reports a healthy status and then becomes unable to update Node status, Kubernetes does not remove or invalidate that condition. For a continuous NodeReadinessRule, NRC currently has no way to express how long that observation remains trustworthy.

The in-tree readiness condition Reporter already supports a heartbeat contract: it periodically refreshes LastHeartbeatTime even when status, reason, and message remain unchanged. However, NRC currently:

  • filters heartbeat-only Node updates because conditionsEqual compares only condition type and status:
    // conditionsEqual checks if two condition slices are equal.
    func conditionsEqual(a, b []corev1.NodeCondition) bool {
    if len(a) != len(b) {
    return false
    }
    // Create map for quick lookup
    aMap := make(map[corev1.NodeConditionType]corev1.ConditionStatus)
    for _, cond := range a {
    aMap[cond.Type] = cond.Status
    }
    for _, cond := range b {
    if status, exists := aMap[cond.Type]; !exists || status != cond.Status {
    return false
    }
    }
    return true
  • evaluates a requirement using only effectiveStatus == requiredStatus:
    for _, condReq := range rule.Spec.Conditions {
    effectiveStatus, conditionFound := r.getConditionStatus(
    node,
    condReq.Type,
    condReq.GetDefaultStatus(),
    )
    satisfied := effectiveStatus == condReq.RequiredStatus
    // observedStatus is the condition status of a node without applying the default
    // fallback in case the condition is not found.
    observedStatus := effectiveStatus
    if !conditionFound {
    observedStatus = corev1.ConditionUnknown
    }
    if !satisfied {
    allConditionsSatisfied = false
    metrics.ConditionEvaluationFailures.WithLabelValues(rule.Name, condReq.Type).Inc()
    }
    conditionResults = append(conditionResults, readinessv1alpha1.ConditionEvaluationResult{
    Type: condReq.Type,
    CurrentStatus: observedStatus,
    RequiredStatus: condReq.RequiredStatus,
    DefaultStatus: condReq.GetDefaultStatus(),
    })
    log.V(1).Info("Condition evaluation", "node", node.Name, "rule", rule.Name,
    "conditionType", condReq.Type, "observed", observedStatus,
    "effective", effectiveStatus, "required", condReq.RequiredStatus,
    "satisfied", satisfied)
  • returns no time-based RequeueAfter from NodeReconciler:
    func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
    log := ctrl.LoggerFrom(ctx)
    log.Info("Reconciling node", "node", req.Name)
    // Fetch the node
    node := &corev1.Node{}
    if err := r.Get(ctx, req.NamespacedName, node); err != nil {
    return ctrl.Result{}, client.IgnoreNotFound(err)
    }
    // Process node against all applicable rules
    if err := r.Controller.processNodeAgainstAllRules(ctx, node); err != nil {
    return ctrl.Result{}, err
    }
    return ctrl.Result{}, nil

The Reporter heartbeat writer is here:

// If the semantic state is completely unchanged, bypass the API write
// to prevent etcd write amplification and control plane flooding.
needsUpdate := true
if existingCondition != nil && existingCondition.Status == status && existingCondition.Reason == health.Reason && existingCondition.Message == health.Message {
needsUpdate = false
/*
NOTE: Skipping the write stops refreshing the LastHeartbeatTime on every tick.
To mitigate this, force an update every 5 minutes even if the state is unchanged.
*/
if time.Since(existingCondition.LastHeartbeatTime.Time) >= heartbeatPeriod {
needsUpdate = true
}
}
if !needsUpdate {
// state has not changed for specified period, skip the write
klog.V(4).InfoS("Condition state unchanged, skipping node status update", "node", nodeName, "condition", conditionType)
return nil
}
if transitionTime.IsZero() {
transitionTime = now
}
// Create condition
condition := corev1.NodeCondition{
Type: corev1.NodeConditionType(conditionType),
Status: status,
LastHeartbeatTime: now,
LastTransitionTime: transitionTime,

The resulting failure mode is:

condition writer reports AgentReady=True
→ NRC removes the readiness taint
→ writer stops without first reporting False or Unknown
→ the old True remains in Node.status.conditions
→ no new Node event occurs and NRC has no deadline timer
→ the old healthy observation remains trusted indefinitely

This matters for continuously required node-local components such as security agents, CNI or storage helpers, GPU or driver readiness agents, and other condition writers that promise periodic heartbeats.

Reproduction

I reproduced the current behavior in a dedicated kind cluster.

Environment:

  • NRC controller source: main@021cd1d4cb4f571d7917a19228ece5394153738e
  • latest main checked after reproduction: 55dbc33a6d5a8d0b6830b6a7a80bcd9681894dd9
  • Kubernetes server: v1.36.1, linux/arm64
  • kind: v0.32.0
  • Reporter CHECK_INTERVAL=2s
  • Reporter HEARTBEAT_PERIOD=10s
  • one writer for example.com/AgentReady

The controller runtime code is unchanged between the reproduction commit and latest main; the intervening merge fixes the Reporter image builder version in #355.

Rule:

apiVersion: readiness.node.x-k8s.io/v1alpha1
kind: NodeReadinessRule
metadata:
  name: agent-ready-freshness
spec:
  nodeSelector:
    matchLabels:
      freshness-test: "true"
  conditions:
    - type: example.com/AgentReady
      requiredStatus: "True"
  taint:
    key: readiness.k8s.io/NetworkReady
    value: pending
    effect: NoSchedule
  enforcementMode: continuous

Observed timeline, in UTC:

05:03:41  Reporter last refreshes AgentReady=True
05:04:08  Reporter Deployment is scaled to zero
05:12:47  AgentReady is still True, heartbeat is unchanged, taint is absent
05:27:15  NRC is restarted and explicitly re-evaluates the Node
05:27:15  evaluator logs observed=True, required=True, satisfied=true
05:27:22  heartbeat is still 05:03:41 and the taint remains absent

Restarting NRC proves that this is not only a missed watch event or a one-time cache artifact. On restart the controller reads the old condition again and still considers it satisfied.

Control experiment:

  1. Restore the Reporter and verify that heartbeat refresh resumes.
  2. Keep the Reporter running but point its health check at a closed port.
  3. Reporter writes AgentReady=False with EndpointConnectionError.
  4. NRC immediately restores the NoSchedule taint.
  5. Restore the healthy endpoint; Reporter writes True and NRC removes the taint.

The control demonstrates that explicit failure transitions work. The uncovered path is specifically writer silence after a last healthy report.

Describe the solution you would like

I would like to discuss an optional, per-condition freshness contract for continuous rules. A candidate API shape is:

spec:
  enforcementMode: continuous
  conditions:
    - type: example.com/AgentReady
      requiredStatus: "True"
      maxHeartbeatAge: 15m

The field name is intentionally not proposed as final. Alternatives include staleAfter and heartbeatTimeout.

Suggested compatibility and scope:

  • optional and unset by default;
  • unset preserves all current behavior;
  • configured per condition because different writers have different heartbeat periods;
  • initially valid only with continuous enforcement;
  • represented as metav1.Duration with CRD/CEL validation;
  • documented as a condition writer heartbeat contract, not proof that every underlying probe is still functioning.

A possible evaluation model is:

staleAt = LastHeartbeatTime + maxHeartbeatAge
fresh = now < staleAt
satisfied = fresh && effectiveStatus == requiredStatus

Staleness should remain independent from the Kubernetes Unknown status. Mapping stale observations to Unknown would allow a requirement whose requiredStatus is Unknown to become satisfied by an expired observation.

Semantics that need maintainer agreement

Before implementation, I would like guidance on:

  1. Should a missing condition with freshness enabled always fail closed?
  2. Should a zero LastHeartbeatTime fail closed?
  3. Should explicit defaultStatus be rejected when freshness is configured, or should missing observations have separately defined behavior?
  4. Should now == staleAt be considered stale?
  5. How should a heartbeat timestamp in the future be handled so clock skew cannot extend trust indefinitely?
  6. Is restricting the first version to continuous rules the right scope?
  7. Which API name best communicates writer heartbeat age rather than probe health?

Scheduling considerations

Listening to heartbeat-only updates is not sufficient. Once the writer stops, there is no later watch event at the expiration time.

A possible first implementation is Node-level deadline scheduling:

  1. condition evaluation returns its next freshness deadline;
  2. NodeReconciler selects the earliest deadline across matching rules;
  3. successful reconciliation returns RequeueAfter;
  4. at wake-up, NRC reads the current Node again;
  5. a refreshed heartbeat schedules the next deadline;
  6. an unchanged expired heartbeat fails the requirement and restores the taint.

Heartbeat-only events could remain filtered. An existing earlier timer may wake once, observe the refreshed heartbeat, and schedule the new deadline.

The design also needs a Rule-to-Node enqueue path. Rule creation, controller restart, and Node/rule cache ordering must establish deadlines even when the initial Node add happens before the rule cache is ready.

Alternatives considered

Include LastHeartbeatTime in the Node predicate

This causes reconciliation while heartbeats arrive but does not create an event after the writer stops. It therefore cannot enforce an expiration deadline by itself and may add substantial reconcile load.

Convert stale to Unknown

This is unsafe when requiredStatus: Unknown, because an expired observation could satisfy the rule.

Require writers to report False or Unknown before shutdown

Graceful shutdown can improve behavior but cannot cover OOM kills, SIGKILL, process deadlock, RBAC loss, API network failure, node power loss, or a deleted or misconfigured DaemonSet.

Periodically scan every Rule and Node

A fixed Rule-level scan is simpler, but it risks O(rules × nodes) repeated evaluation and status write amplification. A deadline-driven Node requeue appears more targeted, subject to scale testing.

Dedicated global deadline heap

This may reduce wake-ups at large scale, but it introduces lifecycle complexity around restart, leader changes, Rule updates, Node replacement, stale heap entries, and UID handling. It seems unnecessary for a first version unless benchmarks show Node-level scheduling is insufficient.

Scope and non-goals

The first version should not attempt to:

  • detect a writer that continues refreshing an incorrect healthy status;
  • detect every internal NPD monitor stall while its Kubernetes exporter still refreshes cached conditions;
  • add stabilization or grace-period policy;
  • change NoExecute behavior;
  • change Reporter heartbeat defaults;
  • introduce a global deadline scheduler before scale evidence requires one.

For NPD specifically, this feature would detect loss of the condition writer or its API write path. NPD exporter heartbeat is not an independent per-probe observation heartbeat.

Relationship to active work

This design should coordinate with:

A freshness timer should not create an unconditional Rule status patch at every deadline. Events or metrics should be transition-oriented so normal heartbeat operation does not create status write amplification.

Proposed validation

An implementation should cover at least:

  • heartbeat just before, exactly at, and after the deadline using an injected clock;
  • missing and zero heartbeat;
  • future heartbeat and clock skew;
  • requiredStatus: Unknown not bypassing stale;
  • multiple conditions and multiple rules selecting the earliest deadline;
  • heartbeat refresh before expiry;
  • writer recovery after stale;
  • Rule created before or after Node;
  • controller restart and leader change;
  • label match and unmatch;
  • scale behavior at 1k, 2k, and 5k Nodes;
  • no repeated status patches or Events without a freshness transition.

I searched open and closed Issues, PRs, and Discussions for condition freshness, heartbeat timeout, stale condition, LastHeartbeatTime, maxHeartbeatAge, and staleAfter and did not find an existing proposal or implementation.

/kind feature

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/featureCategorizes issue or PR as related to a new feature.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions