Enable DRA Partitionable Devices support in kueue-operator CI#80886
Conversation
WalkthroughTwo CI job configs ( ChangesDRA Partitionable Devices GPU partition support
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse pull-ci-openshift-kueue-operator-main-test-e2e-ci-build-upstream-4-21 |
|
@PannagaRao: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-origin-main-e2e-aws-ovn-dra-example |
|
/pj-rehearse periodic-ci-openshift-kueue-operator-main-test-e2e-dra-gpu-4-21 |
|
@PannagaRao: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 similar comment
|
@PannagaRao: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
Signed-off-by: Pannaga Rao Bhoja Ramamanohara
PannagaRao
force-pushed
the
kueue-dra-pd-support
branch
from
1b27998 to
1339d4c
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse pull-ci-openshift-kueue-operator-main-test-e2e-ci-build-upstream-4-21 |
|
@PannagaRao: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-commands.sh (1)
73-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using array syntax for helm arguments.
The unquoted expansion of
${GPU_PARTITION_ARGS}on line 84 relies on word splitting to pass--setandgpu.partitions=Nas separate arguments. While this works, array syntax is more robust and idiomatic:♻️ Refactor to use array syntax
-GPU_PARTITION_ARGS="" +GPU_PARTITION_ARGS=() if [ -n "${DRA_GPU_PARTITIONS:-}" ]; then echo "GPU partitions: ${DRA_GPU_PARTITIONS}" - GPU_PARTITION_ARGS="--set gpu.partitions=${DRA_GPU_PARTITIONS}" + GPU_PARTITION_ARGS=(--set "gpu.partitions=${DRA_GPU_PARTITIONS}") fi helm upgrade --install \ --namespace "${DRA_EXAMPLE_DRIVER_NAMESPACE}" \ dra-example-driver \ "${DRA_CHART_DIR}" \ --set kubeletPlugin.containers.plugin.securityContext.privileged=true \ - ${GPU_PARTITION_ARGS} \ + "${GPU_PARTITION_ARGS[@]}" \ --wait \ --timeout 10mThis approach:
- Eliminates reliance on word splitting
- Protects against unexpected characters in
DRA_GPU_PARTITIONS- Makes the intent clearer to maintainers
- Resolves the Shellcheck SC2086 warning
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-commands.sh` around lines 73 - 84, The GPU_PARTITION_ARGS variable currently relies on unquoted word splitting when passed to the helm upgrade command, which is fragile and generates Shellcheck warnings. Refactor this to use bash array syntax: declare GPU_PARTITION_ARGS as an array (initialize empty), conditionally append the --set flag and its value to the array when the condition check for DRA_GPU_PARTITIONS passes, and then expand the array in the helm upgrade command using proper array syntax (with double quotes and `@-expansion`) to safely pass the arguments without relying on word splitting.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@ci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-commands.sh`:
- Around line 73-84: The GPU_PARTITION_ARGS variable currently relies on
unquoted word splitting when passed to the helm upgrade command, which is
fragile and generates Shellcheck warnings. Refactor this to use bash array
syntax: declare GPU_PARTITION_ARGS as an array (initialize empty), conditionally
append the --set flag and its value to the array when the condition check for
DRA_GPU_PARTITIONS passes, and then expand the array in the helm upgrade command
using proper array syntax (with double quotes and `@-expansion`) to safely pass
the arguments without relying on word splitting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 82a746d6-112b-43aa-81bd-d18afab1d6f6
📒 Files selected for processing (5)
ci-operator/config/openshift/kueue-operator/openshift-kueue-operator-main.yamlci-operator/config/openshift/kueue-operator/openshift-kueue-operator-release-1.4.yamlci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-commands.shci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-ref.yamlci-operator/step-registry/kueue-operator/test/e2e/dra/enable-feature-gate/kueue-operator-test-e2e-dra-enable-feature-gate-commands.sh
✅ Files skipped from review due to trivial changes (1)
- ci-operator/step-registry/kueue-operator/test/e2e/dra/enable-feature-gate/kueue-operator-test-e2e-dra-enable-feature-gate-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/dra-example-driver/install/dra-example-driver-install-ref.yaml
|
This PR enables the DRAPartitionableDevices feature gate and adds dra/whole-device and dra/counter to the upstream e2e target folders. Rehearsals for dra/counter will fail because the test code doesn't exist
/assign @kannon92 @sohankunkerkar |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kannon92, PannagaRao The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
@kannon92: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
/hold |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@PannagaRao: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-bot
Bot
merged commit 33dbcce
into
openshift:main
…ift#80886) Signed-off-by: Pannaga Rao Bhoja Ramamanohara Signed-off-by: Pannaga Rao Bhoja Ramamanohara Co-authored-by: Pannaga Rao Bhoja Ramamanohara <pbhojara@pbhojara-thinkpadp16vgen1.rht.csb>
…ift#80886) Signed-off-by: Pannaga Rao Bhoja Ramamanohara Signed-off-by: Pannaga Rao Bhoja Ramamanohara Co-authored-by: Pannaga Rao Bhoja Ramamanohara <pbhojara@pbhojara-thinkpadp16vgen1.rht.csb>
…ift#80886) Signed-off-by: Pannaga Rao Bhoja Ramamanohara Signed-off-by: Pannaga Rao Bhoja Ramamanohara Co-authored-by: Pannaga Rao Bhoja Ramamanohara <pbhojara@pbhojara-thinkpadp16vgen1.rht.csb>
…ift#80886) Signed-off-by: Pannaga Rao Bhoja Ramamanohara Signed-off-by: Pannaga Rao Bhoja Ramamanohara Co-authored-by: Pannaga Rao Bhoja Ramamanohara <pbhojara@pbhojara-thinkpadp16vgen1.rht.csb>
2 participants
Signed-off-by: Pannaga Rao Bhoja Ramamanohara
Summary by CodeRabbit
This PR enables Dynamic Resource Allocation (DRA) Partitionable Devices support in the
kueue-operatorContinuous Integration pipeline (OpenShift CI configuration), expanding the end-to-end test coverage for DRA GPU partitioning scenarios.Key changes:
CI job configuration updates (main + release-1.4)
Updated the
test-e2e-ci-build-upstream-4-21job in bothopenshift-kueue-operator-main.yamlandopenshift-kueue-operator-release-1.4.yamlto:DRA_GPU_PARTITIONS: "4"in the jobenvE2E_TARGET_FOLDERSto run withsequentialand to target bothdra/whole-deviceanddra/countertest suites (replacing the prior singledratarget selection)DRA example driver upgrade + conditional GPU partition configuration
In the
ci-operator/step-registry/dra-example-driverinstall step:DRA_EXAMPLE_DRIVER_VERSIONis bumped tov0.3.0DRA_GPU_PARTITIONSenv var is introduced; when set, the Helm install/upgrade command conditionally applies--set gpu.partitions=<value>Extended DRA feature gate coverage
Updated the
kueue-operator-test-e2e-dra-enable-feature-gatescript to enable bothDRAExtendedResourceandDRAPartitionableDevices(previously onlyDRAExtendedResource), improving feature coverage for DRA Partitionable Devices tests.Overall, these updates allow OpenShift CI to reliably run DRA end-to-end tests that exercise both whole-device and counter-based partitionable device behaviors, including GPU partitioning into the configured number of logical devices.