Wire the retry policy engine into the scheduler behind a feature flag

[dejanzele]

Problem

A failed run is retried today only when a failed pod check matches it. The checks answer yes or no, and a retry returns the lease silently: no failure category on the event, no per-queue control, no budget per failure kind, and no way to change the job before it runs again. Operators cannot express "retry OOM kills with more memory" or "give this queue three retries for infrastructure failures, away from the node that failed".

Earlier work this builds on

• Add category-based retry policy engine and RetryPolicy API resource #4998 added the retry engine and the RetryPolicy API resource.
• Delete failed pods matching a Delete-action category so retries can reuse the name #4999 made the executor delete a failed pod in a Delete-action category and report the failure only after the pod is gone, so a retry can reuse the pod name.
• Add RetryPolicy CRUD service and queue attachment validation #5049 and Add retry policy client package and armadactl commands #5048 added the RetryPolicy CRUD service, client, and armadactl commands.
• Classify delete-action failed pods with the pod-level failure reason #5080 and Add onPodEvents rule matcher to the error categorizer #5081 made pod-level failure messages and Kubernetes events matchable by category rules.
• Add avoidSameNode and memory bump mutations to retry policy rules #5012 added the mutate block on retry rules and the pipeline that delivers memory growth to the pod.

This PR

This PR connects those pieces into scheduler behaviour, behind the scheduling.retryPolicy feature flag. With the flag off, the scheduler produces the same event sequences as before, byte for byte, and identity tests pin this. With the flag on, a failed run consults the policy attached to its queue. The policy decides whether the categorized failure retries, and its verdict replaces the attempt counting for those runs. A retryable failure emits an intermediate event with retryable=true. The wiring also applies the matched rule's mutations at requeue: avoidSameNode steers the retry away from every node the job failed on, and a memory bump grows the job so placement, accounting, and the retried pod all see the new size. Gangs and fail-fast jobs keep their existing behaviour.

One operator requirement: avoidSameNode expresses the avoidance through the scheduler's nodeIdLabel, so that label must be in the executor's trackedNodeLabels.

Two small event-stream changes apply with the flag off as well, both additive. A terminal lease expiry now emits a JobFailedEvent with the reason "Lease expired" instead of an empty reason. Failure category fields now appear on all failure reason kinds, not only pod errors. Consumers that match on empty reason strings will see the new values.

Validating the full loop

The OOM-with-memory-bump loop is the best single check. Enable the flag and give a category for OOMKilled conditions action Delete, then:

# armadactl create retry-policy -f policy.yaml
apiVersion: armadaproject.io/v1beta1
kind: RetryPolicy
name: oom-grow
retryLimit: 2
defaultAction: Fail
rules:
  - action: Retry
    onCategory: oom
    mutate:
      resources:
        memory:
          factor: 2
# armadactl create queue oom-grow-queue --retry-policies oom-grow

Submit a job with a 64Mi memory limit that writes 100Mi into a memory-backed emptyDir. Expect: the run fails with category oom and a retryable=true event, the pod disappears before that event arrives, and the same pod name comes back with a 128Mi limit and succeeds. Lookout shows two runs: one failed with the category, one succeeded. This sequence was validated on a live cluster, together with avoidSameNode relocating a retry to a second node and failing the job cleanly once every node had been tried.

[datadog-armadaproject]

⚠️ Warnings

🚦 4 Pipeline jobs failed

CI | All jobs succeeded     

CI | test / Golang Integration Tests     

See error

Container exited with code 255 while executing 'docker compose -f _local/compose/full.yaml up -d --wait'.

Python Airflow Operator | airflow-integration-tests     

See error

Error running 'docker compose -f _local/compose/full.yaml up -d --wait'. Process exited with code 1.

View all 4 failed jobs.

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7d4f335 | Docs | Datadog PR Page | Give us feedback!}

[greptile-apps]

Greptile Summary

The PR integrates category-based retry-policy decisions into scheduler failure and lease-expiry handling behind a disabled-by-default feature flag.

• Adds policy lookup, retry budgets, retryable failure events, and terminal policy reasons.
• Applies optional memory growth and node avoidance before requeueing.
• Adds configuration validation, scheduler metrics, API fields, documentation, and end-to-end scheduler tests.

Confidence Score: 5/5

The PR appears safe to merge; the only remaining concern is a non-blocking redundant queue-policy lookup already reported in the existing thread.

No blocking failure remains; the prior lease-expiry conversion and retry-budget issues are addressed, and the remaining outstanding concern is limited to repeated queue-policy map construction.

Important Files Changed

Filename	Overview
internal/scheduler/scheduler.go	Wires policy evaluation into failed-run and lease-expiry paths, applies retry mutations, and emits retryable or terminal events.
internal/scheduler/jobdb/job.go	Adds genuine-failure counting that excludes returned and historically preempted runs.
internal/scheduler/jobdb/job_run.go	Tracks historical preemption in memory while reconstructing it from the durable preempted run state.
internal/server/event/conversion/conversions.go	Converts non-terminal errors into retryable client events and now supplies a reason for lease expiry.
internal/scheduler/configuration/validation.go	Validates node-label indexing required by retry anti-affinity and warns when the global retry kill switch is active.
pkg/api/event.proto	Extends the client-facing failed-job event with an additive retryable field.

Sequence Diagram

sequenceDiagram
  participant E as Executor
  participant S as Scheduler
  participant P as Retry policy engine
  participant B as Event bus
  E->>S: Report categorized run failure
  S->>P: Evaluate policy and failure count
  alt Retry granted
    P-->>S: Retry with optional mutations
    S->>S: Check mutated job schedulability
    S->>B: "JobErrors(retryable=true)"
    S->>B: JobRequeued
  else Retry denied or budget exhausted
    P-->>S: Terminal decision and reason
    S->>B: Terminal JobErrors
  end

Loading

_{Reviews (48): Last reviewed commit: "Clarify that the retry probe checks stat..." | Re-trigger Greptile}

[dejanzele]

@greptileai

[dejanzele]

@greptileai

[dejanzele]

@greptileai

[masipauskas]

Code should validate this is the case, and fail with retry policy being created which would violate this.

[dejanzele]

Same as the other thread, this needs the executors to report their category configs first. Planning it as a follow-up.

[masipauskas]

Code should validate this is the case, and fail with retry policy being created which would violate this.

[dejanzele]

Agreed, but the server can't see the executors' categorizer configs today. Validating this at policy creation needs the executors to report their categories first, probably on the lease request. That's a protocol change I'd rather do as a follow-up. Until then the rollout guide carries it.

[masipauskas]

Can we make this invariant in validation? Also this should also be in indexed node labels in scheduler config, as we expect a lot of jobs using this label.

[dejanzele]

Good point on both. Config validation now fails startup when the retry engine is enabled and nodeIdLabel isn't in indexedNodeLabels. That part is inside the scheduler config, so we can enforce it. The trackedNodeLabels half is executor config the scheduler can't see, so it warns once per executor when reported nodes are missing the label. Proper validation there needs the executors to report their config first, which I'm planning as a follow-up.

[masipauskas]

should we not allow this? i.e. fail in this case - as config is invalid? Why would I want retries enabled, but disabled at the same time?

[dejanzele]

It's intentional. With the flag on and the cap at 0 the engine still runs and attributes failures, it just never grants anything. Turning the flag off instead changes what events new failures produce. So the 0 cap is how you freeze retries during an incident without touching event behaviour.

[masipauskas]

are we intentional on not exposing retries by queue+policy?

[dejanzele]

The scheduler's state metrics already track error classification by queue and pool, and the periodic reset keeps the cardinality bounded. I've moved this counter into the same module, so it now carries queue, pool, policy and decision.

[masipauskas]

does it keep submit checker caching? as it can get very slow, very quickly?

[dejanzele]

Yes, Check runs against the submit checker's cached snapshot, no live calls per job. The per-job calls under a mass failure are still a fair concern though. Check takes a batch, so the plan is a follow-up that collects a cycle's retry candidates and probes them in one call.

[dejanzele]

The follow-up is up: #5097. One engine evaluation per job, then at most two batched Check calls for the whole round, and the queue-policy map builds once per cycle. Validated with the e2e retry cases against a live stack.

[mergify]

Queued — the merge queue status continues in this comment ↓.

[mergify]

Merge Queue Status

• 🟠 Waiting for queue conditions
• ⏳ Enter queue
• ⏳ Run checks
• ⏳ Merge

Waiting for any of

• check-neutral = Rule: Require approval from Armada maintainers (post_check)
• check-skipped = Rule: Require approval from Armada maintainers (post_check)
• check-success = Rule: Require approval from Armada maintainers (post_check)

All conditions

• any of [🔀 queue conditions]:
  • all of [📌 queue conditions of queue rule default]:
    • any of [🛡 GitHub branch protection]:
      • check-neutral = Rule: Require approval from Armada maintainers (post_check)
      • check-skipped = Rule: Require approval from Armada maintainers (post_check)
      • check-success = Rule: Require approval from Armada maintainers (post_check)
    • github-require-last-push-approval [🛡 GitHub branch protection]
    • github-review-approved [🛡 GitHub branch protection]
    • any of [🛡 GitHub branch protection]:
      • check-success = All jobs succeeded
      • check-neutral = All jobs succeeded
      • check-skipped = All jobs succeeded
• -closed [📌 queue requirement]
• -conflict [📌 queue requirement]
• -draft [📌 queue requirement]
• any of [📌 queue -> configuration change requirements]:
  • -mergify-configuration-changed
  • check-success = Configuration changed

[mergify]

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.