Skip to content

Conversation

@alaypatel07
Copy link
Contributor

@alaypatel07 alaypatel07 commented Sep 29, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds support for extended resources feature in DRA dependency as well as add testing manifests for extended resource feature.

Which issue(s) this PR fixes:

Fixes kubernetes/kubernetes#133757

Special notes for your reviewer:

This PR is blocked by the fixes here: kubernetes/kubernetes#134312

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 29, 2025
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 29, 2025
@alaypatel07 alaypatel07 force-pushed the dra-extended-resources branch 2 times, most recently from 2f250e1 to 45252c2 Compare September 30, 2025 00:13
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 30, 2025
@alaypatel07 alaypatel07 force-pushed the dra-extended-resources branch from 45252c2 to 7cef5a6 Compare September 30, 2025 00:17
@alaypatel07 alaypatel07 changed the title WIP: add support for DRAExtendedResources add support for DRAExtendedResources Sep 30, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 30, 2025
@mengqiy
Copy link
Member

mengqiy commented Oct 23, 2025

Discussed offline. And we want to reconcile this with existing DRA test folder to reduce duplication.

- kube-scheduler
- kubelet

2. **DRA Driver**: A DRA driver must be running (installed automatically by the test)
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's installed, then it's not a prerequesite? Same for Prometheus.

Timeout: 5m

steps:
- name: Start measurements
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move monitoring to submodule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1, I like this idea, there are a lot of changes in pipeline waiting to move forward, can I take this as follow-up?

@mborsz
Copy link
Member

mborsz commented Oct 24, 2025

The change looks safe from CL2 perspective, please address Marek's comments before submitting.

/approve
/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 24, 2025
@alaypatel07 alaypatel07 force-pushed the dra-extended-resources branch from 7cef5a6 to db99097 Compare October 24, 2025 18:29
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 24, 2025
@alaypatel07
Copy link
Contributor Author

@mengqiy @serathius I have consolidated both the dra and dra-extended-resources test into one. PTAL

@serathius can I take the using measurements as submodule as a followup? It will help me land this within the deadline

@alaypatel07 alaypatel07 force-pushed the dra-extended-resources branch 4 times, most recently from 47bce54 to 3d11d4e Compare October 28, 2025 18:12
@alaypatel07 alaypatel07 force-pushed the dra-extended-resources branch from 3d11d4e to f737d3c Compare October 28, 2025 18:13
@alaypatel07
Copy link
Contributor Author

@serathius Addressed your comments, PTAL

Comment on lines +28 to +30
{{$extendedResourceName := ""}}
{{if $ENABLE_EXTENDED_RESOURCES}}
{{$extendedResourceName = DefaultParam .CL2_EXTENDED_RESOURCE_NAME "example.com/gpu"}}
Copy link
Contributor

@serathius serathius Oct 30, 2025

Choose a reason for hiding this comment

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

Suggested change
{{$extendedResourceName := ""}}
{{if $ENABLE_EXTENDED_RESOURCES}}
{{$extendedResourceName = DefaultParam .CL2_EXTENDED_RESOURCE_NAME "example.com/gpu"}}
{{$extendedResourceName = DefaultParam .CL2_EXTENDED_RESOURCE_NAME "example.com/gpu"}}
{{if $ENABLE_EXTENDED_RESOURCES}}

Nit: Always defaulting $extendedResourceName simplifies the code, while not having any bad side effects as it's always used under ENABLE_EXTENDED_RESOURCES.

@serathius
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 30, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alaypatel07, mborsz, serathius

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

The pull request process is described here

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

@serathius
Copy link
Contributor

/unhold

Nit can be addressed in followup.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2025
@k8s-ci-robot k8s-ci-robot merged commit 344698d into kubernetes:master Oct 30, 2025
7 checks passed
@yliaog
Copy link

yliaog commented Oct 30, 2025

thanks @serathius @mborsz for the review and approval.

@yliaog
Copy link

yliaog commented Oct 31, 2025

/cc @johnbelamaric

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DRA extended resources performance and scalability testing

6 participants