Document KEP-5963: DRADeviceCompatibilityGroups for K8s 1.37 - #56288
Conversation
👷 Deploy Preview for kubernetes-io-vnext-staging processing.
|
|
Welcome @omeryahud! |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
When you know what feature you want to document, try this Prow command: As it's marked as draft, you don't need to say that the PR is a placeholder. |
|
/retitle Document KEP-5963: DRADeviceCompatibilityGroups for K8s 1.37 |
|
@omeryahud: Re-titling can only be requested by trusted users, like repository collaborators. DetailsIn response to this:
Instructions 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. |
Done, thank you! |
|
I guess I can't do that yet, so I manually updated the title |
|
Hi @omeryahud 👋! v1.37 Docs team here We noticed that this Pull Request is currently in the Draft state. Important Upcoming Docs Deadlines:
Thanks! |
634d09f to
63240fe
Compare
Add a new section under DRA alpha features describing the DRADeviceCompatibilityGroups feature gate, which lets DRA drivers declare opaque compatibility groups on each consumesCounters[] entry of a ResourceSlice to prevent the scheduler from co-allocating incompatible partitioned devices (e.g. MIG vs vGPU on the same GPU). Includes: motivation, how-it-works, a MIG/vGPU YAML example, API constraints, and version-skew safety notes.
b5221cf to
7320b68
Compare
|
/wg device-management |
| If a Pod or PodGroup requests two devices from this pool, the scheduler checks | ||
| whether the two chosen devices share a common compatibility group on the | ||
| `gpu-0-memory` counter set. Since `{"mig"} ∩ {"vgpu"} = ∅`, the pair is |
There was a problem hiding this comment.
In this example, are the two different partitions already not co-allocateable given that the counter set defines 8Gi memory and each partition consumes 6Gi of that? The example would be a bit more compelling if the partitions consumed <=4Gi of memory where the compatibility groups are the only limiting factor.
|
|
||
| A driver adds a `compatibilityGroups` list to each | ||
| `device.consumesCounters[]` entry in a `ResourceSlice`. The list contains | ||
| one or two opaque string names that represent the operating mode or partition |
There was a problem hiding this comment.
"One or two" implies "not zero" which actually is valid as explained in a later paragraph. Could we align this with the "at most 2" language used in the "Constraints" section below?
| When the `DRADeviceCompatibilityGroups` feature gate is disabled (the default | ||
| for alpha), the kube-apiserver strips the `compatibilityGroups` field from any | ||
| new or updated `ResourceSlice` — unless the old object already had the field | ||
| set. The scheduler then treats devices in any pool that previously had grouped | ||
| devices as belonging to an incomplete pool and skips them entirely. |
There was a problem hiding this comment.
Since the "How it works" section mentions "an unset, nil, or empty list", I think it's worth mentioning which of those constitute the field being "set". e.g. If a ResourceSlice is created with compatibilityGroups: nil and the DRADeviceCompatibilityGroups feature enabled, then the feature is disabled, can that device still be allocated, or does the scheduler consider the pool incomplete?
There was a problem hiding this comment.
Hi @nojnhuh !
In the case you describe, the device can be allocated (only with other devices that leave the field unset OR declare ANY of nil, []) and the pool is considered complete.
The only case where a pool is considered incomplete is when both of the following are true:
DRADeviceCompatibilityGroupsis disabled- At least one device in the pool specifies
compatibilityGroups(len(compatibilityGroups) > 0)
So the field is being "set" only when len(compatibilityGroups) > 0
There was a problem hiding this comment.
I'll make sure to add this somewhere in the doc
Co-authored-by: Jon Huhn <nojnhuh@users.noreply.github.com>
…the limiting factor
|
I've addressd everything @nojnhuh , thanks! |
Thanks for the approval @johnbelamaric ! I've had a look, and it is not obvious to me how copmat groups would affect it. Have you had something else in mind? |
nojnhuh
left a comment
There was a problem hiding this comment.
Looks good, thanks! Just a couple other tiny comments.
Co-authored-by: Jon Huhn <nojnhuh@users.noreply.github.com>
Co-authored-by: Jon Huhn <nojnhuh@users.noreply.github.com>
Done! Thanks! |
nojnhuh
left a comment
There was a problem hiding this comment.
/lgtm
Thanks! Could you please squash to one commit?
|
LGTM label has been added. DetailsGit tree hash: f333c1ae9f014fbcc636a9ed07f92af27cd7449c |
I keep forgetting we can do this the easy way: /label tide/merge-method-squash Manually squashing is preferred, but we don't need to lose sleep over it if you run into issues. |
|
/assign @lmktfy |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnbelamaric, katcosgrove, nojnhuh 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 |
Docs PR for: