Skip to content

feat(api): implement DRA support for TrainJob (KEP-2782) - #4027

Open
Sridhar1030 wants to merge 2 commits into
kubeflow:masterfrom
Sridhar1030:feat/2782-dra-support-impl
Open

feat(api): implement DRA support for TrainJob (KEP-2782)#4027
Sridhar1030 wants to merge 2 commits into
kubeflow:masterfrom
Sridhar1030:feat/2782-dra-support-impl

Conversation

@Sridhar1030

Copy link
Copy Markdown
Member

Summary

Implements Phase 1 of KEP-2782, approved in #3540.

  • Add spec.trainer.resourceClaimsPerNode so users can request DRA devices at the same level as resourcesPerNode
  • Apply claims to the trainer node PodSpec and wire resources.claims on the node container
  • Extend runtimePatches with PodSpecPatch.resourceClaims and ContainerPatch.resources for sidecars/init containers
  • Resolve GPU count from ResourceClaimTemplate for numProcPerNode auto-detection in torch, torchtune, MPI, XGBoost, and Flux
  • Reject resourcesPerNode.claims via CRD CEL validation; validate dangling container claims in the JobSet plugin
  • Add DynamicResourceAllocation alpha feature gate (default false), RBAC for resourceclaimtemplates/resourceclaims, and operator docs

Out of scope for this PR: Kubeflow SDK changes (list_resource_claim_templates, runtime/step introspection) and DRA E2E in CI — both called out in the KEP as follow-up/deferred work.

Motivation

Kubernetes DRA is GA in 1.34+. Trainer users today must nest DRA claims under deep runtimePatches while GPU counts live in resourcesPerNode. This KEP unifies the UX with a top-level resourceClaimsPerNode field and keeps GPU auto-detection working without extended resources.

Test plan

  • make test — unit tests for runtime, plugins, webhooks
  • make test-integration — controller DRA integration tests + webhook validation
  • Manual Tier 2 smoke on Kind 1.37 + dra-example-driver:
    • ResourceClaimTemplate → TrainJob with resourceClaimsPerNode
    • JobSet pod gets resourceClaims + node container resources.claims
    • PET_NPROC_PER_NODE=auto when RCT resolves 2 GPUs
    • ResourceClaim reaches allocated,reserved; pod Running

Related

Implement Phase 1 of the approved KEP from kubeflow#3540.

- Add trainer.resourceClaimsPerNode and runtimePatches DRA fields
- Wire pod-level resourceClaims and node container resources.claims
- Resolve DRA GPU count for numProcPerNode in torch/MPI/XGBoost/Flux plugins
- Gate DRA behind DynamicResourceAllocation alpha feature flag
- Add RBAC, validation, admission warning, and operator docs

SDK changes from the KEP remain a follow-up in the Kubeflow SDK repo.
E2E tests with a DRA-capable cluster are deferred per the KEP.

Refs: kubeflow#3540
Closes: kubeflow#2782
Signed-off-by: Sridhar1030 <sridharpillai75@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 4, 2026 09:15
@github-project-automation github-project-automation Bot moved this to Needs Triage in Kubeflow Trainer Sep 4, 2026
@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign andreyvelich for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Sridhar1030 Sridhar1030 changed the title feat(runtime): implement DRA support for TrainJob (KEP-2782) feat(api): implement DRA support for TrainJob (KEP-2782) Sep 4, 2026
@Sridhar1030

Copy link
Copy Markdown
Member Author

One deviation from the KEP: this PR puts DRA behind a DynamicResourceAllocation
alpha gate (default false), which the KEP doesn't mention. @andreyvelich
@robert-bell does that sound ok ?

Signed-off-by: Sridhar1030 <sridharpillai75@gmail.com>
@Sridhar1030

Copy link
Copy Markdown
Member Author

/retest

@robert-bell robert-bell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Sridhar1030!

I've left some initial comments. I'll follow up with a more detailed review next week.

Comment on lines +287 to +288
// The controller adds these claims to the trainer node Pod's resourceClaims and wires
// container-level resources.claims on the node container. To attach a claim to other

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can document the outcome. Wdyt?

Suggested change
// The controller adds these claims to the trainer node Pod's resourceClaims and wires
// container-level resources.claims on the node container. To attach a claim to other
// These claims are added to the trainer node Pod's resourceClaims and automatically
// referenced in the node container's resources.claims. To attach a claim to another

// resourceClaimTemplateName is the name of a ResourceClaimTemplate in the TrainJob namespace.
// A separate ResourceClaim is created from it for every training node Pod.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these max lengths the same as the upstream DRA?

// +optional
Volumes []corev1.Volume `json:"volumes,omitempty"`

// resourceClaims patches the Pod's resourceClaims. Containers consume a claim by

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document that this is for targeting sidecar and init containers? And that users should prefer the resourceClaimsPerNode?

VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`

// resources patches the container's compute resources, including the resources.claims
// that reference the Pod's resourceClaims. For the node container, trainer.resourcesPerNode

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here- point users to the top level api.


func (r *TrainingRuntime) newRuntimeInfo(
trainJob *trainer.TrainJob, jobSetTemplateSpec trainer.JobSetTemplateSpec, mlPolicy *trainer.MLPolicy, podGroupPolicy *trainer.PodGroupPolicy,
ctx context.Context, trainJob *trainer.TrainJob, jobSetTemplateSpec trainer.JobSetTemplateSpec, mlPolicy *trainer.MLPolicy, podGroupPolicy *trainer.PodGroupPolicy,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need ctx here? I'd have expected we'd only need to look up ResourceClaimTemplates when resolving the gpu count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

3 participants