RFC: KEP-4381: DRA: inline parameters

[pohly]

• One-line PR description: inline parameters

• Issue link: DRA: structured parameters #4381

• Other comments:

This simplifies using the API for those cases where no vendor CRD is needed. The downside is that there are now multiple ways to specify in-tree parameters (inline and in separate object).

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pohly
Once this PR has been reviewed and has the lgtm label, please assign derekwaynecarr for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• keps/sig-node/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[pohly]

Inlined, so the YAML is the same as before.

[toVersus]

So, if users want to embed ResourceClaimParameters into ResourceClaim or generate ResourceClaimParameters manually, they may need to write some CEL expressions to filter out resources, right?

[pohly]

Yes.

There's an ongoing debate between @johnbelamaric and @klueska whether asking users to write CEL expressions is going to be a good API. I suspect it is both: it's okay in some cases and too complex in others, so we'll need support for CRDs as an optional feature.

[toVersus]

This may be unrelated to this PR, but what happens to existing Pods if users later change the fields in the ResourceClaimParameter's embedded spec like selector? Just ignore it and keep running as is? I understand that ResourceClaim is immutable, so inline parameters cannot be changed.

[pohly]

That's a good point: for the sake of feature parity, changing the ResourceClaim.Parameters content should be allowed.

In the current design, parameters matter right until allocation happens. After that, the parameters are ignored.

With classic DRA, a DRA driver could have watched parameters after allocation and somehow adapt the allocation result on-the-fly internally. This is not part of "structured parameters" at the moment. It's also not clear what that would mean for real-world hardware, so it's low priority.

[pohly]

Actually, the ResourceClaimSpec is already mutable. That's because even if it wasn't (i.e. ResourceClassName and ParametersRef don't change after creation), that still wouldn't prevent someone from changing the content of those objects or, if they were immutable themselves, delete and recreate them.

What is protected against undesirable changes is the status. In particular, the allocation result is immutable.

[toVersus]

That's because even if it wasn't (i.e. ResourceClassName and ParametersRef don't change after creation), that still wouldn't prevent someone from changing the content of those objects or, if they were immutable themselves, delete and recreate them.

I see, indeed, that perspective makes sense. It might be possible to protect deletions by setting the finalizer (dra.k8s.io/delete-protection) not only on the ResourceClaim but also on the ResourceClaimParameters referenced by ResourceClaim.

Anyway, for now, it seems better to make the ResourceClaimParametersSpec immutable if the allocation result of ResourceClaim cannot be changed and changes to already allocated resources are ignored. I think we should avoid behavior that confuses users. If a use case arises where users want to modify allocated resources by changing parameters, we can simply change the field from immutable to mutable, as shown in https://github.com/kubernetes/kubernetes/blob/v1.30.0/pkg/apis/storage/types.go#L287.

[pohly]

It might be possible to protect deletions by setting the finalizer (dra.k8s.io/delete-protection) not only on the ResourceClaim but also on the ResourceClaimParameters referenced by ResourceClaim.

I'm not sure who should be doing this and when it's safe to remove the finalizer again. A finalizer also doesn't make the content immutable.

it seems better to make the ResourceClaimParametersSpec immutable if the allocation result of ResourceClaim cannot be changed and changes to already allocated resources are ignored

This seems like a valid usage:

• create claim with parameters
• create a pod using the claim -> allocated
• change parameters
• kill pod to get it started again -> deallocated (because allocations get removed as soon as the claim is unused)
• recreate pod -> allocated with new parameters

From a conceptual perspective it seems odd to me to treat inline parameters differently from referenced parameters.

[toVersus]

From a conceptual perspective it seems odd to me to treat inline parameters differently from referenced parameters.

Yes, I thought that referenced ResourceClaimParameters should be made immutable. However, I was convinced by the use cases mentioned above. It's certainly cumbersome to have to recreate ResourceClaim or ResourceClaimParameters 😄
Currently, ResourceClaimSpec is immutable, but only inline parameters will be mutable, right?
https://github.com/kubernetes/kubernetes/blob/v1.30.0/pkg/apis/resource/validation/validation.go#L112

[pohly]

Hmm, how did I miss that ("Actually, the ResourceClaimSpec is already mutable.")? You are right, it is not.

As I argued above, I think we have to accept that claim parameters are mutable and should allow that consistently everywhere. That includes making the ResourceClaimSpec mutable. The KEP and API description don't describe this anywhere - need to fix that!

[pohly]

/close

Will become part of a future KEP updated after prototyping it in https://github.com/kubernetes-sigs/wg-device-management.

[k8s-ci-robot]

@pohly: Closed this PR.

In response to this:

/close

Will become part of a future KEP updated after prototyping it in https://github.com/kubernetes-sigs/wg-device-management.

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.