Add RetryPolicy CRUD service, client, and armadactl support

[dejanzele]

Stacked on #4998. This PR targets master for review, so its diff currently includes #4998's commit as well. The changes that belong to this PR are the most recent commit; the isolated diff is at dejanzele/armada@retry-policy-engine...retry-policy-api. Please merge #4998 first.

Implements the RetryPolicyService whose proto was defined in #4998: create, get, update, and delete for retry policies, backed by a Postgres repository and a new lookout schema migration for the retry_policy table. Deletion is rejected while any queue still references the policy.

Adds the client package and armadactl support: armadactl create/get/update/delete retry-policy, and a --retry-policy flag on armadactl create queue and armadactl update queue to attach a policy to a queue. Adds create_retry_policy, update_retry_policy, and delete_retry_policy permissions.

The one piece of churn worth calling out: requireGrpcCode, a test helper that three existing server test files each defined privately, is moved into the shared servertest package so the new retry policy service test can reuse it. The three existing tests (executor, node, queue) now import it from there instead of defining their own copies.

[datadog-armadaproject]

⚠️ Warnings

🚦 1 Pipeline job failed

CI | test / Proto Up To Date     

Useful? React with 👍 / 👎

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

[greptile-apps]

Greptile Summary

This PR implements the RetryPolicyService (create/get/update/delete) with a Postgres-backed repository and Lookout migration, a scheduler-side ApiPolicyCache, the full armadactl CLI surface, and the client package. It also consolidates a previously-duplicated requireGrpcCode test helper into the shared servertest package.

• Service layer (internal/server/retrypolicy/): authorization gates all mutating calls, ValidatePolicy is enforced before any DB write, and delete is blocked while queues reference the policy (with an acknowledged TOCTOU caveat documented in comments and already raised in previous review threads).
• Scheduler retry engine (internal/scheduler/retry/): the engine cleanly separates preemption retries from genuine-failure retries, charges them against independent tallies, and excludes preemptions from the global cap to avoid the scheduler's own actions consuming a job's failure budget.
• CLI / client (cmd/armadactl/, internal/armadactl/, pkg/client/retrypolicy/): all five CRUD operations are wired; CreateResource now parses the file even on --dry-run for RetryPolicy, matching the Queue case behavior (previously flagged, now fixed).

Confidence Score: 5/5

Safe to merge; all mutating paths are authorized and validated, the acknowledged TOCTOU gap in delete degrades gracefully (scheduler falls back to no-policy rather than corrupting state), and the migration is non-destructive.

The service, repository, engine, and CLI are each individually well-tested. The only known correctness gap (TOCTOU between the queue-reference check and the delete) is explicitly documented in the code and degrades safely. No new issues were found beyond what was already discussed in previous review threads.

No files require special attention; previous review findings on internal/server/retrypolicy/service.go (empty-name guard ordering, TOCTOU) are documented and either addressed or intentionally deferred.

Important Files Changed

Filename	Overview
internal/server/retrypolicy/service.go	New RetryPolicyService implementing CRUD operations; authorizes all mutating calls, validates policies before DB writes, and checks queue references before deletion (TOCTOU race acknowledged in comments).
internal/server/retrypolicy/repository.go	Postgres-backed retry policy repository using parameterized queries; CreateRetryPolicy uses ON CONFLICT DO NOTHING to detect duplicates without a race, UpdateRetryPolicy detects missing rows via RowsAffected().
internal/scheduler/retry/engine.go	Retry evaluation engine that correctly separates preemption retries from failure retries, subtracts preemptions from the global cap, and handles the zero-globalMaxRetries kill-switch.
internal/scheduler/retry/cache.go	ApiPolicyCache that periodically polls GetRetryPolicies and stores compiled policies via an atomic pointer; fails open on refresh errors, increments the failure counter, and emits a last-success gauge.
internal/server/retrypolicy/validate.go	Server-side policy validation enforcing RFC 1123 name format, non-empty DefaultAction (Fail or Retry), and non-empty on_category per rule; consistent with the scheduler-side ValidatePolicy.
pkg/api/deserialize.go	Adds UnmarshalJSON for RetryAction accepting canonical names, friendly aliases (Fail/Retry), and numeric values via a shared unmarshalProtoEnum helper; alias matching uses normalizeAlias (lowercase + strip underscores).
internal/armadactl/queue.go	Adds ResourceKindRetryPolicy to CreateResource, parsing the file even on dry-run to match the Queue case behavior (fixing the previous review finding).
internal/lookout/schema/migrations/035_create_retry_policy.sql	Creates the retry_policy table with name (PK) and definition (bytea). No FK constraint from queue to retry_policy, meaning a deleted policy leaves referencing queues with a dangling name (acknowledged in service comments).
internal/armadactl/retrypolicy.go	armadactl App methods for CRUD of retry policies; uses sigs.k8s.io/yaml for marshalling output; DeleteRetryPolicy message correctly conveys idempotency ("or it did not exist").
internal/server/servertest/grpc.go	Extracts RequireGrpcCode helper from three duplicate private definitions into the shared servertest package, reducing test boilerplate.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CLI as armadactl
    participant Server as RetryPolicyService
    participant Auth as Authorizer
    participant RPRepo as RetryPolicyRepo (Postgres)
    participant QRepo as QueueRepo (Postgres)
    participant Scheduler as ApiPolicyCache

    CLI->>Server: CreateRetryPolicy(policy)
    Server->>Auth: AuthorizeAction(create_retry_policy)
    Auth-->>Server: ok
    Server->>Server: ValidatePolicy(policy)
    Server->>RPRepo: CreateRetryPolicy(policy)
    RPRepo-->>Server: ok / AlreadyExists
    Server-->>CLI: Empty / gRPC status

    CLI->>Server: DeleteRetryPolicy(name)
    Server->>Auth: AuthorizeAction(delete_retry_policy)
    Auth-->>Server: ok
    Server->>QRepo: GetAllQueues()
    QRepo-->>Server: []Queue
    Server->>Server: filter queues referencing name
    alt queues reference policy
        Server-->>CLI: FailedPrecondition
    else no references
        Server->>RPRepo: DeleteRetryPolicy(name)
        RPRepo-->>Server: ok
        Server-->>CLI: Empty
    end

    loop every updateFrequency
        Scheduler->>Server: GetRetryPolicies()
        Server->>RPRepo: GetAllRetryPolicies()
        RPRepo-->>Server: []RetryPolicy
        Server-->>Scheduler: RetryPolicyList
        Scheduler->>Scheduler: ConvertPolicy each to compiled map
        Scheduler->>Scheduler: "atomic.Store(&compiled)"
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant CLI as armadactl
    participant Server as RetryPolicyService
    participant Auth as Authorizer
    participant RPRepo as RetryPolicyRepo (Postgres)
    participant QRepo as QueueRepo (Postgres)
    participant Scheduler as ApiPolicyCache

    CLI->>Server: CreateRetryPolicy(policy)
    Server->>Auth: AuthorizeAction(create_retry_policy)
    Auth-->>Server: ok
    Server->>Server: ValidatePolicy(policy)
    Server->>RPRepo: CreateRetryPolicy(policy)
    RPRepo-->>Server: ok / AlreadyExists
    Server-->>CLI: Empty / gRPC status

    CLI->>Server: DeleteRetryPolicy(name)
    Server->>Auth: AuthorizeAction(delete_retry_policy)
    Auth-->>Server: ok
    Server->>QRepo: GetAllQueues()
    QRepo-->>Server: []Queue
    Server->>Server: filter queues referencing name
    alt queues reference policy
        Server-->>CLI: FailedPrecondition
    else no references
        Server->>RPRepo: DeleteRetryPolicy(name)
        RPRepo-->>Server: ok
        Server-->>CLI: Empty
    end

    loop every updateFrequency
        Scheduler->>Server: GetRetryPolicies()
        Server->>RPRepo: GetAllRetryPolicies()
        RPRepo-->>Server: []RetryPolicy
        Server-->>Scheduler: RetryPolicyList
        Scheduler->>Scheduler: ConvertPolicy each to compiled map
        Scheduler->>Scheduler: "atomic.Store(&compiled)"
    end

Loading

_{Reviews (2): Last reviewed commit: "Add RetryPolicy CRUD service, client, an..." | Re-trigger Greptile}

[greptile-apps]

TOCTOU race: queue reference check and delete are not atomic

queuesReferencingPolicy reads the current queue list and then, in a separate operation, DeleteRetryPolicy removes the row. Between these two calls another process could create or update a queue that references the policy, and the delete would proceed, leaving that queue pointing at a non-existent policy. The scheduler cache would then return (nil, false) for that queue's policy name, silently disabling its retry rules. Making the guard transactional (or at minimum adding a database-level foreign key) would close this window.