[Bug]: the Kubernetes connector drops health events on write failure while the store and grpcsink connectors retry, so a lost recovery event latches a node condition forever

[lfriedman-netllama]

What happened

The Kubernetes connector is the only one of the three platform connectors that drops a health event when processing fails. grpcsink and store both requeue with backoff. Because the Kubernetes connector is the one whose writes carry recovery semantics, a single terminal write failure can leave a node condition True permanently, with no in-band path to correct it.

The asymmetry

// platform-connectors/pkg/connectors/kubernetes/k8s_connector.go:136
r.ringBuffer.HealthMetricEleProcessingFailed(queuedHealthEvents)

// platform-connectors/pkg/ringbuffer/ring_buffer.go:126-129
func (rb *RingBuffer) HealthMetricEleProcessingFailed(data *QueuedHealthEvents) {
	rb.healthMetricQueue.Forget(data)   // reset backoff
	rb.healthMetricQueue.Done(data)     // and discard
}

Forget + Done with no re-add means the event is gone. Compare the other two connectors, which use the requeue method that already exists beside it (AddRateLimited, ring_buffer.go:131-134):

• platform-connectors/pkg/connectors/store/store_connector.go:135 — requeues with exponential backoff up to maxRetries, then logs "Max retries exceeded, dropping health events permanently" (store_connector.go:138).
• platform-connectors/pkg/connectors/grpcsink/grpc_sink_connector.go:116 — same pattern.

So the bounded-retry-then-drop pattern is already established in this codebase; the Kubernetes connector simply skips the retry half.

Why the in-process retry does not cover it

processNodeEvents does wrap the read-modify-write in retry.OnError (platform-connectors/pkg/connectors/kubernetes/process_node_events.go:72-84), classifying conflicts and temporary errors as retriable:

err := retry.OnError(retry.DefaultRetry, func(err error) bool {
	isRetriable := apierrors.IsConflict(err) || isTemporaryError(err)
	...

That handles the common Conflict case well. But once retry.DefaultRetry is exhausted, or the error is classified non-retriable, processHealthEvents returns an error and the event is discarded at k8s_connector.go:136. An API server that is unavailable for longer than DefaultRetry's short window (five attempts, tens of milliseconds of backoff) exhausts it easily.

Why it matters asymmetrically for recovery events

For an unhealthy event, dropping is mostly self-correcting: the fault is usually still present, so the next event re-adds the condition.

For a healthy event it is not. Nothing re-sends a recovery, because recovery is an edge rather than a state that keeps being reported. The node condition stays True after the hardware is fine, and the only remedy is a manual kubectl patch of node status. This is the "lost clear" failure mode operators hit and attribute to the analyzer.

This gets more important with #1553 / #1706. Today health-events-analyzer never publishes healthy events at all (health-events-analyzer/pkg/publisher/publisher.go:140 hardcodes IsHealthy = false), so there are few clears available to lose. Once derived-condition recovery lands, every derived condition depends on a healthy event surviving exactly one node-status write. A transport that drops on failure would silently undercut that feature: the recovery logic would be correct and the condition would still latch.

Suggested fix

Match the other two connectors: requeue retriable failures with AddRateLimited up to a bounded retry count, then drop with the same explicit "permanently dropping" log the store connector already emits. Two details worth deciding deliberately:

1. Distinguish permanent from transient. A malformed event or a NotFound node should not be retried forever; store-client/pkg/client/permanent_error.go suggests a precedent for that classification already exists in-tree.
2. Preserve ordering expectations, or document that they do not hold. Requeuing one batch behind newer ones can reorder a fault and its recovery for the same condition. aggregateEventMessages is last-write-wins per condition, so a reordered stale fault could re-assert a cleared condition. Whichever way this goes, it deserves a test.

Verification

Confirmed against v1.21.0 and current main. Observed in a 288-node GB200 fleet where the analyzer's derived events are deliberately kept STORE_ONLY specifically because analyzer-derived conditions never clear, which is the workaround #1553 describes.