Raise a warning instead of an error if GPU mode labeler fails #1106
Conversation
|
LGTM. I am assuming that GFD just becomes a no-op when run on a node where pci devices aren't detected, yes? |
In WSL2, NVML is available so GFD behaves like normal, except for the gpu.mode label which requires information about the PCI device under /sys. |
| @@ -215,7 +215,8 @@ func isMPSCapable(manager resource.Manager) (bool, error) { | |||
| func newGPUModeLabeler(devices []resource.Device) (Labeler, error) { | |||
| classes, err := getDeviceClasses(devices) | |||
| if err != nil { | |||
| return nil, err | |||
| klog.Warningf("Failed to create GPU mode labeler: failed to get device classes: %v", err) | |||
There was a problem hiding this comment.
The one question is whether we should return an nvidia.com/gpu.mode="unknown" label here instead?
There was a problem hiding this comment.
I am not opposed. Do we have any precedent for using unknown for other labels where we are unable to gather the required information?
There was a problem hiding this comment.
We already return unknown when getting the mode below. We also use this for the machinetype label here
There was a problem hiding this comment.
Thanks, I clearly did not look at the code below carefully enough. I have updated this to return unknown instead.
For environments like WSL2 where GPUs do not show up as PCI devices, creating the GPU mode labeler will fail. This change allows GFD to proceed even if it is unable to get the GPU mode label. Signed-off-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
cdesiniotis
force-pushed
the
gpu-mode-labeler-optional
branch
from
a62dbde to
bd00e7d
Compare
|
@cdesiniotis do we need to backport this? |
No, as long as this change makes it into the next device-plugin release (0.18.0) and our upcoming GPU Operator release picks it up we should be fine. |
For environments like WSL2 where GPUs do not show up as PCI devices, creating the GPU mode labeler will fail. This change allows GFD to proceed even if it is unable to get the GPU mode label.