ttlStrategy: secondsAfterFailure does not match phase=Error, causing silent workflow leaks

Summary

ttlStrategy.secondsAfterFailure only matches workflows whose phase is exactly Failed. Workflows whose phase is Error are never garbage collected on that clock. Any cluster that treats Error and Failed alike (which most operators intuitively do) leaks Error-phase workflows indefinitely unless secondsAfterCompletion is also set, which forces successful workflows onto the same expiry clock as unsuccessful ones.

Source references on main (HEAD 084009d56):

• workflow/gccontroller/gc_controller.go:261-274: the ttl() switch has three arms: Failed && SecondsAfterFailure, Successful && SecondsAfterSuccess, Completed && SecondsAfterCompletion. No arm covers Failed || Error on its own clock.
• pkg/apis/workflow/v1alpha1/workflow_types.go:2708-2710: Status.Failed() returns Phase == WorkflowFailed only.
• pkg/apis/workflow/v1alpha1/workflow_phase.go:6-22: WorkflowError is a distinct phase; Completed() covers Succeeded|Failed|Error.

Proposal

Add a fourth field to TTLStrategy:

// SecondsAfterUnsuccessful is the number of seconds to live after the workflow
// finished in phase Failed or Error.
SecondsAfterUnsuccessful *int32

Behavior: matches when Status.Failed() || Status.Phase == WorkflowError.

Precedence: preserved as first-match. The new case would be inserted between the SecondsAfterSuccess case and the existing SecondsAfterCompletion case in ttl(). If a user sets both SecondsAfterFailure and SecondsAfterUnsuccessful, SecondsAfterFailure still wins for Failed workflows. Rewriting the switch to a min-collector (shortest matching TTL always wins) would change existing behavior for configurations combining per-phase and completion fields. However, I think it makes sense, would like to hear opinions on it.

Naming

• secondsAfterUnsuccessful (proposed): parallels the existing outcome-based fields (AfterSuccess, AfterFailure), reads naturally as the antonym of Success.
• secondsAfterError: narrower; would leave Failed still needing its own field and doesn't help the ergonomic problem operators actually hit.

Happy to change the name if a different form is preferred.

Prior art

• #1883 : the PR that originally introduced TTLStrategy (merged 2020-01-07). The title mentions "Succeeded, Failed or Error" but the merged struct only shipped SecondsAfterSuccess / SecondsAfterFailure / SecondsAfterCompletion. Error was in the design intent but did not survive to the schema. This proposal fills that gap.
• #12135 : retentionPolicy.completed conflates Failed and Errored; users expect distinct buckets. Same class of problem on the retentionPolicy side.
• #14475 : open PR adding succeeded to retentionPolicy in gc_controller; confirms maintainers already recognise the Failed-vs-Errored gap in one GC surface.
• #15174 : real-world user configuring per-phase retention with distinct errored and failed values.

Open questions

1. Final field name (see naming section).
2. Precedence: preserve first-match (this proposal) or refactor ttl() to a min-collector as part of this change?
3. Docs: is the auto-generated docs/fields.md table sufficient, or would you like a new prose page on retention/GC?

Use Cases

Any Argo installation that wants unsuccessful workflows cleaned up on a different clock from successful ones. Today the only way to expire Error-phase workflows is secondsAfterCompletion, which also expires Succeeded workflows on the same clock. Operators frequently want to retain successes for audit while GC'ing unsuccessful runs quickly; there is no way to express that today.

Concrete trigger seen in practice: apiserver / etcd transient blips kill the wait sidecar mid-workflow, dropping the workflow into phase Error rather than Failed. Operators who set secondsAfterFailure reasonably expect these to be cleaned up on the same clock, however they are not, and the workflow CRs can accumulate silently.

Minimal reproduction

Deterministic Error trigger via an invalid outputs.parameters.path (does not rely on the activeDeadlineSeconds race, which lands the workflow in Failed on some runs and Error on others):

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: ttl-error-repro-
spec:
  ttlStrategy:
    secondsAfterFailure: 10   # note: only Failure, no Completion
  entrypoint: main
  templates:
  - name: main
    container:
      image: alpine:3.20
      command: [sh, -c, "echo hi"]
    outputs:
      parameters:
      - name: missing
        valueFrom:
          path: /does/not/exist   # forces phase Error, not Failed

Expected today: workflow phase becomes Error and the CR is never garbage-collected. Expected with the proposal (using secondsAfterUnsuccessful: 10 instead of secondsAfterFailure): the CR is GC'd ~10s after finishedAt.

Happy to send a PR once naming and precedence are agreed.

/area gc /area controller

Message from the maintainers:

Love this feature request? Give it a . We prioritise the proposals with the most .