WIP: add kep for DRA Attributes Downward API #5606
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
sig/node
|
/wg-device-management |
alaypatel07
force-pushed
the
kep-5304
branch
3 times, most recently
from
3c92417 to
3557abc
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alaypatel07 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/XL
Signed-off-by: Alay Patel <[email protected]>
alaypatel07
force-pushed
the
kep-5304
branch
2 times, most recently
from
92f13db to
96e3424
Compare
k8s-ci-robot
added
size/XXL
Signed-off-by: Alay Patel <[email protected]>
k8s-ci-robot
added
size/XL
|
Please combine PRR into this PR. We usually review as one PR. |
|
@kannon92 This PR already has the PRR. If a single PR is to be reviewed, all I need to do is close the other PR |
|
|
||
| - Provide a stable Downward API path for device attributes associated with `pod.spec.resourceClaims[*]` requests | ||
| - Support any attributes defined in ResourceSlice objects, controlled by the allowlist mechanism | ||
| - Allow resource owners to explicitly allowlist which attributes are exposed (maximum 8 per request) for security and control |
There was a problem hiding this comment.
Who do we consider as the "resource owners" here and how do specify the allowlist? From reading this KEP, it seems like there is a system allowlist that for alpha will contain resource.kubernetes.io/pcieRoot and dra.kubervirt.io/mdevUUID and an allowlist on the request. I would think of the "resource owners" as the cluster admin (or whoever configures the DRA driver) and there doesn't seem to be an allowlist available for them.
| - Environment variables are set at container start time: Once a container starts, its environment variables are immutable. If device attributes change after container start, env vars will not reflect the change. | ||
| - Projected volumes support updates: Files in projected volumes will be updated atomically when the kubelet's cache detects changes to the underlying ResourceClaim or ResourceSlice. | ||
| - Allowlist enforcement: The kubelet MUST enforce the per-request allowlist. If an attribute is not in the allowlist, attempts to reference it via `draDeviceFieldRef` will fail. | ||
| - Attribute names: Any attribute name that exists in the ResourceSlice can be referenced, as long as it is allowlisted. The API server does not validate attribute names against a hardcoded list, allowing vendors to define their own attributes (e.g., `dra.kubervirt.io/mdevUUID`). |
There was a problem hiding this comment.
The sentence Enforce membership in the supported attribute set (alpha: resource.kubernetes.io/pcieRoot, dra.kubervirt.io/mdevUUID) in https://github.com/kubernetes/enhancements/blob/656815421ad608bb4b0ed0f3f9a067d819e97cda/keps/sig-node/5304-dra-attributes-downward-api/README.md#resource-api-per-request-allowlist seems to contradict this?
| // DRADeviceFieldRef selects a DRA-resolved device attribute for a given claim+request. | ||
| // +featureGate=DRADownwardDeviceAttributes | ||
| // +structType=atomic | ||
| type DRADeviceFieldRef struct { |
There was a problem hiding this comment.
This can map to more than one device, since each request might ask for multiple devices. How is the data surfaced in the env variables or the volume? The DRA device names doesn't necessarily map to any identifier that is known to the consumer of this information.
| } | ||
| ``` | ||
|
|
||
| Validation: |
There was a problem hiding this comment.
So this is validation on admission, but presumably it is an error if any of the references can't be resolved when the kubelet tries to resolve the references? Or is it just ignored?
| 3. Watches ResourceSlices: Resolves standardized attributes from `spec.devices[*].attributes` for the matching device name | ||
| 4. Maintains Cache: Keeps a per-Pod map of `(claimName, requestName) -> {attribute: value}` with a readiness flag | ||
|
|
||
| Resolution Semantics: |
There was a problem hiding this comment.
What about error handling here? I'm wondering what happens if:
- A claim or request can't be found?
- The requested attribute is not available for one or more of the allocated devices?
| // DRADeviceFieldRef selects a DRA-resolved device attribute for a given claim+request. | ||
| // +featureGate=DRADownwardDeviceAttributes | ||
| // +structType=atomic | ||
| type DRADeviceFieldRef struct { |
There was a problem hiding this comment.
The name here seems inconsistent with the current naming convention: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/core/v1/types.go#L2473
| - For container environment variables, resolution happens at container start using the latest ready values | ||
| - For projected volumes, kubelet updates files via AtomicWriter when cache changes | ||
| - Kubelet MUST honor the per-request allowlist; if `draDeviceFieldRef.attribute` is not in the allowlist, the value is not exposed | ||
| - If data is not ready when a container starts, resolution fails for required env vars or leaves optional ones unset |
There was a problem hiding this comment.
How can users specify a reference as optional? Looking at the current code, it looks like this should be a field on DRADeviceFieldRef.
KEP-5304: Adding downward API for DRA Device Attributes to Pod