Repository navigation
fix[device-plugin] add DEVICE_LIST_STRATEGY env var guardrail - #3081
Conversation
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
35602f4 to
a4ef034
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
coverage: 45.025% (+0.04%) from 44.983% — cdi-validate-device-list-strategy into main |
68f0eaf to
3270b24
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
3270b24 to
44ce354
Compare
This commit adds additional validation to the devicePlugin operand transformation logic. When CDI is enabled, it is required that the DEVICE_LIST_STRATEGY env var value includes one of "cdi-annotations" and "cdi-cri". While the current transformation code adds these values to DEVICE_LIST_STRATEGY when .spec.cdi.enabled is true, the same resolved value gets overridden when the gpu-operator user sets the DEVICE_LIST_STRATEGY env var explicitly in the devicePlugin section of the ClusterPolicy custom resource. This changes addresses a major limitation in the gpu-operator where feedback from an invalid devicePlugin config only happens at gpu pod workload scheduling which is a significant delay. This provides a poor UX as users waste hours debugging the containerCreate failures instead of identifying the invalid devicePlugin config. This fix changes that by failing at the ClusterPolicy reconcile stage. Signed-off-by: Tariq Ibrahim <tibrahim@nvidia.com>
44ce354 to
54cc02c
Compare
tariq1890
changed the title
|
Backport to |
Yes, I am open to this |
aditighag
left a comment
aditighag
left a comment
There was a problem hiding this comment.
Thanks for picking this up.
Failing early instead of at pod creation time is a good improvement, but it doesn't fix the linked issue - #3077.
Please see inline review comments.
Separately, can we have similar guardrails for ACCEPT_NVIDIA_VISIBLE_DEVICES_AS_VOLUME_MOUNTS? Setting it alongside CDI is the main cause of #3077.
|
/cherry-pick release-26.7 |
|
🤖 Backport PR created for |
This commit adds additional validation to the devicePlugin operand transformation logic. When CDI is enabled, it is required that the DEVICE_LIST_STRATEGY env var value includes one of "cdi-annotations" and "cdi-cri". While the current transformation code adds these values to DEVICE_LIST_STRATEGY when .spec.cdi.enabled is true, the same resolved value gets overridden when the gpu-operator user sets the DEVICE_LIST_STRATEGY env var explicitly in the devicePlugin section of the ClusterPolicy custom resource.
This changes addresses a major limitation in the gpu-operator where feedback from an invalid devicePlugin config only happens at gpu pod workload scheduling which is a significant delay. This provides a poor UX as users waste hours debugging the containerCreate failures instead of identifying the invalid devicePlugin config. This fix changes that by failing at the ClusterPolicy reconcile stage.