updates to device failures for alpha2

[SergeyKanzhelev]

/sig node
/cc @johnbelamaric

KEP: #4680

[johnbelamaric]

I think that's better, it's simpler too.

[johnbelamaric]

@pohly @klueska if you have a chance, would appreciate your review especially on the kubelet/driver interface.

/lgtm

[johnbelamaric]

/cc @mrunalp

since he has been handling DRA related changes, and this is adding DRA support to this feature

[pohly]

How are "pod level DRA resources" defined?

Is the goal to list health of all allocated devices here and then also for each container that references them?

Not a blocker for the KEP, but this needs to be clarified before implementing this API.

[SergeyKanzhelev]

I would think container-associated devices will be in container statuses. And this is for network DRA for now

[johnbelamaric]

Yes. I would expect devices associated with requests that are attached to containers get sorted into those. Resource claims not tied to any container, but tied to this pod, would show up in here. In the normal case, that's just network devices. The one question I have is if someone does tie a request to a container, but the actual content of the request is pod level, which slice should it show up in? I think probably pod level?

[pohly]

That's why I was asking for a definition. The code needs to decide where to put things and users must know where to expect what.

The only definition that I can think of is "each container reports everything it has access to" and then this here is "whatever is not accessed by any container" - so basically everything that is left.

We don't have an explicit "pod level" flag.

[johnbelamaric]

But a container doesn't have "access" to a network device. The network namespace is pod level. So, that driver would know to put it at the pod level.

Also, who is doing the sorting? Based on the grpc call, it's actually kubelet that sorts the devices into these buckets, since the response only includes the deviceid and health. kubelet wouldn't know how to sort these except by how the requests are defined.

Looking a little deeper, if kubelet is going to sort individual device ids coming back from the grpc call into the right container buckets, it needs the mapping from device id -> container. That would come from here. So kubelet will need to load the resource claim status, and create a map of allocated device ID from that array into claim and request, then look up in the podspec and containerspecs which container and/or pod that goes to.

So, if a user lists a pod-level request in a container, then it will show up in that container. Likewise, if two containers share a device via a claim, it will show up in BOTH containers statuses!

I guess that's ok? or should we just have one list of all the claims and their device statuses, and make the user map those back to containers? Seems unfriendly.

[johnbelamaric]

Yeah, I think it's ok.

[pohly]

The grRPC API isn't important enough to block the KEP, but this here looks like something that hasn't been thought through yet.

However, it's also "only the API", so perhaps we can waive it through and do the real API review during the implementation review? I'll defer to @johnbelamaric here.

[johnbelamaric]

I'd like to see the discussion above captured in the KEP - basically grab the comment above and add it to design details. Key points being:

• driver only needs to report deviceID/health pairs
• kubelet needs to use the resource claims status and pod/container specs to map deviceID -> claim/request names -> correct status field here
• for DRA, resource name for container resources is something like the string "claim" along with the Name and Request from the container ResourceRequirements ResourceClaim
• for DRA, any device not mapped to a container will show up in the pod resource list, and will use "claim" along with the name from PodResourceClaim Name. I don't think we can get a request name at that point, I think without associating the claim to a specfic container, we just actuate all the requests in the claim?

[pohly]

I think without associating the claim to a specfic container, we just actuate all the requests in the claim?

We always prepare all claims referenced by a pod , whether they are referenced by a container or not. Based on the information returned by the driver, the kubelet then knows how to associated CDI device IDs with containers.

There is no such thing as "actuate a request".

[johnbelamaric]

There is no such thing as "actuate a request".

Got it, thanks. So the above should work.

[pohly]

See question about "pod level DRA resources" above.

[SergeyKanzhelev]

Add a small clarification here

[pohly]

Can you put this into a new gRPC interface? Then it can be versioned as part of this KEP and drivers are not forced to implement this method - they would be if it became part of the NodeServer interface.

At a practical level it'll help avoid code conflicts. I need to promote that dra/v1alpha4 API to dra/v1beta1 in 1.32.

[SergeyKanzhelev]

Is there any implications of having two interfaces running? Would it mean we need an extra port for the new interface?

[pohly]

You can serve different gRPC interfaces over the same connection. They are like different endpoints in an HTTP server.

That we version them is a bit odd. There's no difference between v1alpha4 and v1beta1, but because we put the version into the name, they are completely different as far as gRPC is concerned. But that's just an aside.

[pohly]

I'm okay with treating this as an implementation detail that'll get addressed later - no need to update the KEP for it now.

[SergeyKanzhelev]

added a note about it.

[johnbelamaric]

/lgtm

Thanks @SergeyKanzhelev, will be great to have this.

@mrunalp or @dchen1107 can you PTAL and approve?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrunalp, SergeyKanzhelev

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

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/sig-node/OWNERS [mrunalp]

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