Remove GPU index for CDI spec generation / attribute advertisement

[klueska]

Fixes: #563

Tested with:

• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test1.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test2.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test3.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test4.yaml

[klueska]

/cc @varunrsekar

[jgehrcke]

I read the linked issue. To me, without historical context, the UUID seems to be the natural identifier for a problem like this. Maybe the only question I have then: why was this not considered in the first place, what are the downsides?

Edit: maybe I misunderstood. This only removes that index from the slice, yes? (At first I thought we now show the UUID instead, but that's said nowhere and I probably misread that from the patch).

[klueska]

The only real downside is if a new physical node gets swapped in for a given logical node in k8s (which can happen in GKE across a node reboot). When this happens the physical GPUs will have changed their UUIDs, whereas the indexing scheme would stay consistent.

Making the change in the PR would be an issue if we hadn't (previously) introduced the renaming of the device to its canonical name instead of the "ID" passed into cdi.nvcdiDevice.GetDeviceSpecsByID(). This ensures the UUID does not appear anywhere in the generated CDI spec, but rather the canonical name (based on the minor number).

Using the minor number is OK (whereas using the index wasn't) because it is generated once at boot time and is predictably computed based the PCIeBusID of the GPU. It would only be a problem if the rebooted node came back with a different number (or set) of GPUs than its predecessor, but (at least in the GKE case, which is the only one we are truly worried about) this never happens.

[klueska]

It's worth noting, I tested this change with the following, which all passed:

• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test1.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test2.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test3.yaml
• https://github.com/NVIDIA/k8s-dra-driver-gpu/blob/b3325005002081a4998531302cdf24b1a711b29c/demo/specs/quickstart/gpu-test4.yaml

[varunrsekar]

LGTM! Thanks for doing this!

[jgehrcke]

Thanks for the explanation and the test report.

[jgehrcke]

Using the minor number is OK

We probably need to review this; after #563 (comment):

It looks like even device minors are subject to the same issue as GPU index

[klueska]

Based on the discussion in #563 I have also removed the minor number as an advertised attribute. We will continue to use it internally to name the GPU devices in the resource slice as well as for some other internal bookkeeping, but users will not be able to select GPUs based on it.