argo-workflows #14615

Issue Analysis

The issue #14615 describes a scenario where a Workflow using spec.workflowTemplateRef points to a WorkflowTemplate that either doesn't exist yet or hasn't been synchronized in the informer cache. In this case, the controller immediately marks the Workflow with a final Error status.

xyz-init-workflow   Error   2m3s   workflowtemplates.argoproj.io "xyz-workflow-template" not found

The user provides additional context:

This is because the workflow is deployed before the workflowtemplate. But even if we deploy it in the correct order, ArgoWF actually takes some time to process the workflowtemplate and know about it. So it still failed.

The user intuitively reports that the retryStrategy logic is broken, since the configured retryStrategy never triggers:

Naturally, the retryStrategy should make the workflow retry in such scenarios. But it doesn't do retries.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: xyz-init-workflow
spec:
  workflowTemplateRef:
    name: xyz-workflow-template
  retryStrategy:
    limit: 3
    retryPolicy: "Always"

After deep investigation of the call chain, the real problem is not that the retryStrategy logic is broken - rather, the workflow never reaches the retry logic at all. The workflow operate() function already determines the entire workflow has an error and reports it directly.

Call Chain Analysis

operate()                          operator.go:194   ← reconcile entry
│ defer persistUpdates()           operator.go:199   ← always writes status back to k8s
│
├─ setExecWorkflow(ctx)            operator.go:217   ← ★ template parsing happens here, very early
│  └─ setStoredWfSpec(ctx)         operator.go:4343
│     └─ fetchWorkflowSpec(ctx)    operator.go:4454 → 4299
│        └─ Lister().Get(name)     operator.go:4316  ← ★★ error source: NotFound
│           │
│           〔if err〕→ markWorkflowError  operator.go:4345  ← ★★★ unconditionally marks as Error (terminal)
│           〔if err〕→ return (early exit) operator.go:220
│
└─ (never reaches) executeTemplate() → processNodeRetries()  ← retryStrategy lives here, never reached

Detailed Call Chain

1. Error source - fetchWorkflowSpec, operator.go:4316

We find the source of the error workflowtemplates.argoproj.io "xyz" not found:

// operator.go:4314-4320
} else {
    woc.controller.metrics.CountWorkflowTemplate(...)
    specHolder, err = woc.controller.wftmplInformer.Lister().
        WorkflowTemplates(woc.wf.Namespace).
        Get(woc.wf.Spec.WorkflowTemplateRef.Name) // :4316
}
if err != nil {
    return nil, err // :4319 ← passed up unchanged, no classification
}

• Lister() is the client-go informer cache lister (not a direct API server call). When the cache doesn't find the name, it returns *apierrors.StatusError{Reason: NotFound}.
• This reflects the user's real scenario: either the WFT hasn't been deployed yet, or it has been deployed but the informer cache hasn't synchronized.
• At line 4318, if err != nil { return nil, err } makes no transient classification, treating NotFound the same as template content errors.

2. setStoredWfSpec, operator.go:4454-4456

// operator.go:4453-4457
if woc.wf.Status.StoredWorkflowSpec == nil {
    wftHolder, err := woc.fetchWorkflowSpec(ctx) // :4454
    if err != nil {
        return err // :4456 ← also passed up unchanged
    }
    ...
}

This is a check during the first time the system processes a Workflow. The safeguard logic is: if the task definition (Spec) cannot be found, the system won't waste resources creating an execution environment (execWf) - instead, it reports the error and ends. So NotFound occurs at the very beginning of the lifecycle, when woc.execWf is still nil.

3. Critical location - setExecWorkflow, operator.go:4343-4346

// operator.go:4333-4348 (workflowTemplateRef branch)
case woc.wf.Spec.WorkflowTemplateRef != nil:
    ...
    // Strict/Secure mode check
    err := woc.setStoredWfSpec(ctx) // :4343
    if err != nil {
        ctx = woc.markWorkflowError(ctx, err) // :4345 ★★★ unconditional
        return ctx, err                       // :4346
    }
    woc.execWf = &wfv1.Workflow{Spec: *woc.wf.Status.StoredWorkflowSpec.DeepCopy()} // :4348 ← never reached

Line 4345 unconditionally calls markWorkflowError - without checking whether the error is transient or whether it's a NotFound:

// operator.go:2800-2802
func (woc *wfOperationCtx) markWorkflowError(ctx context.Context, err error) context.Context {
    return woc.markWorkflowPhase(ctx, wfv1.WorkflowError, err.Error()) // Phase = "Error"
}

This results in WorkflowError being a terminal state:

// workflow_phase.go:15-22
func (p WorkflowPhase) Completed() bool {
    switch p {
    case WorkflowSucceeded, WorkflowFailed, WorkflowError: // Error counts as Completed
        return true
    ...
}

Once in Error, the controller only performs cleanup and will not re-operate.

4. Cleanup - early return + defer persistence, operator.go:218-220 / 199

// operator.go:217-221
ctx, err := woc.setExecWorkflow(ctx)
if err != nil {
    woc.log.WithError(err).Error(ctx, "Unable to set ExecWorkflow") // :219
    return // :220 ← entire operate ends early
}

// operator.go:198-200
defer func() {
    woc.persistUpdates(ctx) // :199 ← even with early return at :220, defer still writes Error to k8s
}()

At this point, the workflow is sentenced: status.message = workflowtemplates.argoproj.io "xyz" not found.

User's confusion: Why doesn't retryStrategy participate at all?

The retryStrategy logic at operator.go:387-407 only runs after woc.execWf has been built:

// operator.go:387-407
node, err := woc.executeTemplate(ctx, woc.wf.Name, ..., woc.execWf.Spec.Arguments, ...) // :387
if err != nil {
    ...
    default:
        // ★ This path DOES check transient: transient errors don't become Error, they stay for requeue
        if !errorsutil.IsTransientErr(ctx, err) && !woc.wf.Status.Phase.Completed() && ... {
            woc.markWorkflowError(ctx, x) // :399
        }
}

The actual retry logic lives in executeTemplate() → processNodeRetries() (operator.go:972), which only applies to nodes that have already started execution and failed. WFT parsing failure happens before any node execution - execWf hasn't even been built yet - so retryStrategy is never reached. This explains why the behavior differs from the user's expectations.

From Root Cause to Solution

From the source code analysis, we can summarize the root cause:

• Layer mismatch: WFT parsing happens at the very beginning of workflow startup, before any node execution.
• Unconditional failure path: NotFound on this path is unconditionally handled by markWorkflowError, resulting in immediate terminal state.

The issue is not that "retryStrategy is broken" - it's a combination of layer mismatch and an unconditional failure path.

How to Solve?

The unconditional markWorkflowError at setStoredWfSpec (operator.go:4345) is the first place to address.

The original behavior: when a workflow points to a WorkflowTemplate that isn't ready, the controller immediately marks the workflow as Error. This is too binary.

The proposed fix: change it to "wait first (mark as Pending within a grace period, retry every 10 seconds), then give up and mark as Error only if the template doesn't appear within the timeout." This provides a buffer for the resolution process.

Problem 1: Who should this new behavior apply to?

Two options: opt-in (users must enable it) or default-on (applies to everyone automatically).

Option 1: Opt-in

Add a new field to the CRD (Workflow YAML spec):

spec:
  retryOnTemplateNotFound: true  # user must write this line for the new behavior to activate

Users who don't write this line keep the old immediate-failure behavior. Only those who know about and set this option get the buffer time.

This option has significant cost: although it's just one new CRD field, it triggers:

• Running make codegen (regenerating many Go types)
• Updating swagger (API documentation)
• Updating user docs

This would result in a large PR touching many files, making review more difficult.

Option 2: Default-on

No new fields. All workflows automatically get the new behavior. Users don't need to change anything.

The final choice is default-on: enable it for everyone. There's no real risk, and it saves the large CRD/codegen engineering effort, keeping the PR minimal.

Problem 2: When does the "grace period" start? How long should it be?

A new question arises: retrying can't continue indefinitely. We need an upper limit, and to calculate that limit, we need a starting point.

Intuitively: "When we mark it as Pending, start counting from that moment." This causes problems because the controller runs every 10 seconds, and each run re-marks it as Pending, resetting the timer every 10 seconds - resulting in never giving up!

The better choice: CreationTimestamp - the timestamp when the workflow was created. Kubernetes already records this, and it never changes after creation. This fixed anchor point gives us a clear retry upper limit that will definitely expire.

The final decision: use CreationTimestamp as the starting point.

Action: In Detail

The core concept of this fix: change the current unconditional markWorkflowError at setExecWorkflow (operator.go:4343-4346) to:

• Only apply bounded retries for WorkflowTemplate NotFound
• Mark as Pending within the grace period and requeue, waiting for the WorkflowTemplate to become ready
• Fall back to Error only after timeout

Detailed implementation

err := woc.setStoredWfSpec(ctx)
if err != nil {
    // Tolerate a not-yet-available WorkflowTemplate (applied together via GitOps, or informer cache
    // lag): within a grace period measured from creation, keep the workflow Pending and requeue so it
    // starts once the template appears; after that, fail so a genuinely missing template still fails fast.
    gracePeriod := woc.controller.Config.GetWorkflowTemplateNotFoundGracePeriod()
    if apierr.IsNotFound(err) && time.Since(woc.wf.CreationTimestamp.Time) < gracePeriod {
        ctx = woc.markWorkflowPhase(ctx, wfv1.WorkflowPending,
            fmt.Sprintf("Waiting for referenced WorkflowTemplate to become available: %v", err))
        woc.requeueAfter(GetRequeueTime())
        return ctx, ErrWorkflowTemplateNotReady
    }
    ...
}

Key Point 1: Only check for "WorkflowTemplate not found" at setExecWorkflow, don't modify global rules

The controller has a global list of "which errors are transient and can be retried." The laziest fix would be to add NotFound to that global list, but this approach was not taken.

While it might be a one-line change, it's global - every place that calls IsTransientErr would now treat NotFound as retryable, including paths where "the thing was genuinely deleted and should permanently fail," causing infinite retries.

Instead, the fix uses apierr.IsNotFound specifically at err := woc.setStoredWfSpec(ctx) to determine if the error is a NotFound.

Key Point 2: Waiting must have a deadline, measured from Workflow creation time

In addition to using CreationTimestamp as the fixed anchor, a new configuration value workflowTemplateNotFoundGracePeriod is added. The default is 30 seconds. Setting it to 0 seconds completely restores the old immediate-failure behavior.

Key Point 3: Use a "special error" as a signal to tell Operate to wait

A new "signal" ErrWorkflowTemplateNotReady is defined. When a WorkflowTemplate is not found, it serves two purposes:

a. Non-nil error: causes the upper operate to finish early without continuing (since the template hasn't been parsed yet, running would produce incomplete data).

b. Upper layer recognizes this specific signal with errors.Is: when this expected error occurs, operate returns silently without printing an error log. Otherwise, every retry within the grace period would log a misleading ERROR, making it look like something is broken.

Key Point 4: markWorkflowPhase - changing the Workflow's "status phase"

The core content at operator.go:2627:

woc.wf.Status.Phase = phase // L2643

This marks the Workflow as Pending / Running / Error / Succeeded, etc. It only modifies the woc.wf object in memory and sets woc.updated = true (L2642). The actual write to etcd happens at the end of operate() via the deferred persistUpdates. So marking as Pending during the flow doesn't immediately hit the API server.

markWorkflowError is a thin wrapper around markWorkflowPhase (operator.go:2817):

func (woc *wfOperationCtx) markWorkflowError(ctx, err) context.Context {
    return woc.markWorkflowPhase(ctx, wfv1.WorkflowError, err.Error())
}

"Fallback to markWorkflowError after timeout" means: within the grace period, use markWorkflowPhase(Pending) (temporary wait); after the grace period expires, fall back to the original markWorkflowError, letting it reach the terminal Error state as before.

Key Point 5: requeueAfter - re-queue me after N seconds

The workflow is placed back into the pending wfQueue, but not immediately - it's re-queued after N seconds. Since the template might have just been applied and the controller's internal cache (informer) may not have synchronized yet, retrying immediately would be wasteful. Better to wait and check later.

The controller has three requeue mechanisms:

• wfQueue.Add(key): immediate enqueue
• wfQueue.AddAfter(key, d): fixed delay d
• wfQueue.AddRateLimited(key): interval determined by limiter

How long to wait? Use GetRequeueTime(), the system's unified parameter (default 10 seconds, adjustable via environment variable).

Why active requeue is necessary: the WFT informer has no event handler. When someone creates a WorkflowTemplate, it doesn't automatically wake up waiting Workflows. The workflow must rely on its own requeueAfter to periodically check back. This is also why marking as Pending (incomplete, so it can receive requeue) is necessary instead of Error (terminal, never re-examined).

Result

After review, the reviewer responded:

I don't think we should fix this. Even if we fix it for workflowTemplateRef we've still got a problem if your workflow references WFT A which needs B and B hasn't appeared yet. The only good fix here is to apply WorkflowTemplates before workflows, and I don't want the added complication and surprises at runtime which this fix would entail.

Clearly, the reviewer believes the correct approach is not to modify the current flow, but to ensure users guarantee deployment order - deploy all WorkflowTemplates first, then run Workflows that reference them - rather than asking the controller to "wait" at runtime.

Indeed, when solving this problem, the case where a WorkflowTemplate itself depends on another WorkflowTemplate (A needs B) was not considered. In that scenario, the same issue would recur, and the fix clearly doesn't solve the complete problem.