RetryOnFailure should support a bounded retries config (currently retries forever, never reaches Stalled)

[pulkits15]

This is a follow-up to #1278 / #1281, which introduced the RetryOnFailure strategy for .spec.install.strategy and .spec.upgrade.strategy.

Today, RetryOnFailure retries a failed install/upgrade indefinitely at the fixed .retryInterval, with no way to bound the number of attempts. Unlike the RemediateOnFailure strategy — which uses .remediation.retries (0 = no retries, negative = infinite) and transitions the HelmRelease to Stalled with reason RetriesExceeded once retries are exhausted — a HelmRelease using RetryOnFailure can never reach a terminal Stalled state. For a release that is failing for a permanent reason (bad values, invalid chart, misconfigured cluster), this means the controller reconciles forever with no signal that human intervention is needed.

This has real operational cost: #1359 showed that unbounded RetryOnFailure loops were already filling up .status.history (and etcd) before that specific growth bug was fixed — a symptom of retrying forever with no upper bound or off-ramp.

Proposal

Add an optional .retries field to .spec.install.strategy and .spec.upgrade.strategy (alongside the existing .retryInterval), with the same semantics already used by .remediation.retries elsewhere in this API:

• A non-negative value caps the number of retry attempts before the controller stops and marks the HelmRelease Stalled (reason RetriesExceeded, consistent with how RemediateOnFailure already reports this).
• A negative value (default: -1) preserves today's behavior of infinite retries, so this is fully backward compatible.

API Changes

apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
spec:
  install:
    strategy:
      name: RetryOnFailure
      retryInterval: 5m
      retries: -1 # defaults to -1 (infinite, current behavior); a non-negative value caps attempts before Stalled
  upgrade:
    strategy:
      name: RetryOnFailure
      retryInterval: 5m
      retries: -1 # same semantics as install.strategy.retries

Note on naming: I've named this field .retries to match the existing .remediation.retries convention (negative = infinite). Since .remediation.retries is documented as ignored when the strategy is RetryOnFailure, open to alternative names (e.g. maxRetries) if maintainers feel a second field literally named retries in a different location would be confusing — happy to go either way.

Why this matters

This brings parity between RetryOnFailure and RemediateOnFailure: both strategies would support bounded retries with a consistent, discoverable terminal state (Stalled / RetriesExceeded), instead of RetryOnFailure being the only strategy that can never surface "this needs attention."

[matheuscscp]

Nope, RetryOnFailure was designed specifically not to have a limit of retries, to implement a behavior similar to kustomize-controller of continuous reconciliation. A Kubernetes controller has to converge, this is what RetryOnFailure achieves. If you need a bounded number of retries, you are looking for RemediateOnFailure.

[pulkits15]

Nope, RetryOnFailure was designed specifically not to have a limit of retries, to implement a behavior similar to kustomize-controller of continuous reconciliation. A Kubernetes controller has to converge, this is what RetryOnFailure achieves. If you need a bounded number of retries, you are looking for RemediateOnFailure.

@matheuscscp I can't use RemediateOnFailure because I don't want to remediate (rollback or uninstall) the release

[stefanprodan]

How are you using the rest of Flux then? All our controllers are designed for continuous reconciliation, helm-controller was the outlier, we finally fixed it with RetryOnFailure.

[matheuscscp]

I don't see how a retry limit surfaces "this needs attention" more than something that is constantly failing. If the observability of RetryOnFailure is not good enough then we need to improve it. Limiting the number of retries is not a solution.

[pulkits15]

I don't see how a retry limit surfaces "this needs attention" more than something that is constantly failing. If the observability of RetryOnFailure is not good enough then we need to improve it. Limiting the number of retries is not a solution.

@matheuscscp Agreed, but then I would need an authoritative signal, produced by helm-controller itself, that a release isn't making progress — not something I derive on my own from failure counters like .status.failures or elapsed time since HelmRelease was applied. If I build that heuristic myself, my tooling is making its own arbitrary call about "failed" instead of faithfully reflecting what helm-controller has actually determined about the release. That distinction matters for anyone building status reporting on top of HelmRelease: the source of truth for "is this actually stuck" needs to be the controller, not something every consumer reinvents independently.

[stefanprodan]

the source of truth for "is this actually stuck" needs to be the controller, not something every consumer reinvents independently.

I disagree, Flux has no way of telling when something is truly stuck, this condition is very specific to your infra. A classical example: the AWS region runs out of spot instances during a Helm upgrade, it can take hours before capacity is available and pods can be scheduled.

[pulkits15]

the source of truth for "is this actually stuck" needs to be the controller, not something every consumer reinvents independently.

I disagree, Flux has no way of telling when something is truly stuck, this condition is very specific to your infra. A classical example: the AWS region runs out of spot instances during a Helm upgrade, it can take hours before capacity is available and pods can be scheduled.

Fair example, and I should've been clearer — I'm not arguing Flux should hardcode a definition of "stuck." I'm saying the HelmRelease should mark the release as stuck and the rule for when helm-controller marks a HelmRelease as stuck should be user-configurable. For example, flux could expose a field like .spec.upgrade.strategy.progressDeadline, similar to how the k8s Deployment controller exposes spec.progressDeadlineSeconds.

---

Stepping back though: even if we expose more fields to configure when a HelmRelease is considered stuck, one problem remains — helm-controller keeps burning resources on it indefinitely. That's the actual motivation behind my original proposal to cap the number of retries.

Capping retries solves both problems at once:

1. It produces an authoritative signal that the HelmRelease is stuck and needs attention.
2. It prevents helm-controller from wasting resources on indefinite retries for a release that isn't going to recover on its own.

RetryOnFailure was designed specifically not to have a limit of retries, to implement a behavior similar to kustomize-controller of continuous reconciliation

@matheuscscp — that's a reasonable design goal in general, but the issue that introduced RetryOnFailure gives a narrower rationale:

The RetryOnFailure strategy is suitable for statefulsets and other workloads that cannot tolerate rollbacks and have a high rollout duration susceptible to health check timeouts and transient capacity errors.

That's exactly my situation: I have StatefulSets, so RetryOnFailure is my only option — but I don't want helm-controller reconciling forever and burning resources on a release that's genuinely stuck.

Worth noting RemediateOnFailure already breaks the "continuous reconciliation" convention that's being invoked to keep RetryOnFailure unbounded — helm-controller isn't internally consistent on this point today. Given that, I don't see a strong reason not to add an optional retry-limit field to RetryOnFailure as well.

[pulkits15]

How are you using the rest of Flux then? All our controllers are designed for continuous reconciliation, helm-controller was the outlier, we finally fixed it with RetryOnFailure.

@stefanprodan to answer directly: the rest of Flux (kustomize-controller, source-controller, etc.) reconciles cheap, idempotent operations — reapplying a manifest that's already applied is essentially a no-op, so retrying forever costs almost nothing. Retrying a Helm upgrade against a StatefulSet isn't free in the same way: each attempt can trigger further pod churn on workloads that, per the issue that introduced RetryOnFailure, were explicitly called out as unable to tolerate rollbacks in the first place.

So "continuous reconciliation is safe everywhere else in Flux" doesn't automatically carry over to Helm upgrades on stateful workloads — the cost model is different. That gap, where retrying forever is fine for most Flux controllers but can be genuinely wasteful (and disruptive) for Helm upgrades on stateful workloads, is exactly the problem this proposal is trying to close.

[stefanprodan]

Helm v4 is using SSA, their implementation is very close to how kustomize-controller always worked. Having a StatefutSet managed by kustomize-controller or by helm-controller (with SSA enabled) is quite the same. RemediateOnFailure is indeed very costly, as it does a rollback which results in pod churn, RetryOnFailure is what makes reties cheap basically no-op.