Prefer NVML MIG profile names - #680
Conversation
|
Linked issue in dcgm-exporter repo: #713 |
rahulait
left a comment
rahulait
left a comment
There was a problem hiding this comment.
Overall, LGTM. I would wait for getting an approval from someone managing dcgm-exporter codebase as well as this might require a release note or migration note for users who were using the older incorrect values in their alerts.
| return "", fmt.Errorf("failed to get parent device handle for UUID %s: %s", parentGPUUUID, nvml.ErrorString(ret)) | ||
| } | ||
|
|
||
| infoV2, ret := device.GetGpuInstanceProfileInfoByIdV(int(profileID)).V2() |
There was a problem hiding this comment.
GetGpuInstanceProfileInfoByIdV was added in driver R580, but the reported setup uses R535. On R535, the exporter can crash before the fallback runs. Please use or guard the older API, add an R535 compatibility test that reaches the NVML call, and fix the G115 lint error here.
| parentGPUUUID = gpu.DeviceInfo.UUID | ||
| } | ||
|
|
||
| nvmlProfileName, err := nvmlprovider.Client().GetGPUInstanceProfileName(parentGPUUUID, instance.Info.NvmlMigProfileId) |
There was a problem hiding this comment.
Should this also work outside Kubernetes?
There was a problem hiding this comment.
I think that it should.
Let's implement this as a follow up so this MR can merge as-is.
| assert.Contains(t, err.Error(), "failed to get MIG device process memory") | ||
| } | ||
|
|
||
| func TestGetGPUInstanceProfileName_When_NVML_Not_Initialized(t *testing.T) { |
There was a problem hiding this comment.
These tests do not exercise the production NVML lookup. Please add a provider-level test that covers GPU handle lookup and the R535-compatible profile query, and proves it returns 7g.80gb safely. A live GPU is not required; a mocked NVML binding or library shim is enough.
|
cc: @nccurry for review |
Signed-off-by: Shivkumar Ople <sople@nvidia.com>
a3d1b46 to
ede2553
Compare
|
Fix has been merged in GitLab repo. Closing this. |
4 participants
Summary
Related issue: NVIDIA/gpu-operator#687
Tests