Skip to content

fix: skip faulty devices instead of panicking during registration #2231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shellyco-code wants to merge 2 commits into from
Closed

fix: skip faulty devices instead of panicking during registration #2231

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87e9b60
fix: skip faulty devices instead of panicking during registration
shellyco-code Jul 31, 2026
4efc068
fix: address maintainer feedback on skip path
shellyco-code Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ func (plugin *NvidiaDevicePlugin) getAPIDevices() *[]*device.DeviceInfo {
ndev, ret := nvml.DeviceGetHandleByUUID(UUID)
if ret != nvml.SUCCESS {
klog.Errorln("nvml new device by index error uuid=", UUID, "err=", ret)
panic(0)
continue
Comment thread
shellyco-code marked this conversation as resolved.
}
idx, ret := ndev.GetIndex()
if ret != nvml.SUCCESS {
klog.Errorln("nvml get index error ret=", ret)
panic(0)
continue
Comment on lines +109 to +114

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 6 'nvml\.Init\(\)|panic\(0\)|getAPIDevices|RegisterInAnnotation' \
  pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go \
  pkg/device-plugin/nvidiadevice/nvinternal/plugin/register_test.go

Repository: Project-HAMi/HAMi

Length of output: 4931

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== register.go relevant sections ==="
sed -n '89,200p' pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go | cat -n | sed 's/^/register.go /'
sed -n '250,285p' pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go | cat -n | sed 's/^/register.go /'

echo "=== occurrences of nvidia.Init() / nvml.Init() / getAPIDevices / WatchAndRegister / RegisterInAnnotation ==="
rg -n -C 3 'nvml\.Init\(|\.Init\(\)|getAPIDevices|WatchAndRegister|RegisterInAnnotation|initInterface|Nvml' pkg/device-plugin/nvidiadevice/nvinternal/pkg pkg/device-plugin/nvidiadevice/nvinternal/plugin -g '*.go'

Repository: Project-HAMi/HAMi

Length of output: 27427

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== RegisterInAnnotation body ==="
sed -n '197,245p' pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go | cat -n | sed 's/^/register.go /'

echo "=== factory init handling ==="
sed -n '90,130p' pkg/device-plugin/nvidiadevice/nvinternal/plugin/factory.go | cat -n | sed 's/^/factory.go /'

echo "=== Read-only structural check: getAPIDevices returns pointer and RegisterInAnnotation propagates no error from getAPIDevices ==="
python3 - <<'PY'
from pathlib import Path
p = Path('pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go')
s = p.read_text()
print('signature:', 'func (plugin *NvidiaDevicePlugin) getAPIDevices() *[]*device.DeviceInfo {' in s)
print('getAPIDevices returns &res:', '&res' in s[s.index('func (plugin *NvidiaDevicePlugin) getAPIDevices'):s.index('// RegisterInAnnotation')])
check = 'devices := plugin.getAPIDevices()'
idx = s.index(check)
block = s[idx:idx+400]
print('RegisterInAnnotation calls getAPIDevices:', True)
print('RegisterInAnnotation immediately accesses result:', '*devices' in block or 'len(*devices)' in block)
print('RegisterInAnnotation has explicit error from getAPIDevices:', any(tok in block for tok in [':= getAPIDevices', '= getAPIDevices', 'return nil']))
print('current RegisterInAnnotation return:', 'return true, err' in block or 'return false, err' in block)
PY

Repository: Project-HAMi/HAMi

Length of output: 5559

Handle nvml.Init() failures from getAPIDevices() without panicking.

getAPIDevices() still calls panic(0) when nvml.Init() fails, which crashes WatchAndRegister and can skip the node annotation path. Return this error through RegisterInAnnotation() and call it from WatchAndRegister() so WatchAndRegister() retries without replacing the last valid annotation. Add tests for NVML initialization and per-device failure paths.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/device-plugin/nvidiadevice/nvinternal/plugin/register.go` around lines
109 - 114, Replace the panic in getAPIDevices() when nvml.Init() fails with
error propagation, then have RegisterInAnnotation() return that error and update
WatchAndRegister() to handle it by retrying while preserving the last valid
annotation. Add coverage for NVML initialization failure and individual device
failure paths.

}
memoryTotal := 0
memory, ret := ndev.GetMemoryInfo()
Expand All @@ -133,12 +133,13 @@ func (plugin *NvidiaDevicePlugin) getAPIDevices() *[]*device.DeviceInfo {
}
default:
klog.Error("nvml get memory error ret=", ret)
panic(0)
// continue skips to the next device in the outer loop, not just the switch
continue
Comment thread
shellyco-code marked this conversation as resolved.
}
Model, ret := ndev.GetName()
if ret != nvml.SUCCESS {
klog.Error("nvml get name error ret=", ret)
panic(0)
continue
}

registeredmem := int32(memoryTotal / 1024 / 1024)
Expand Down Expand Up @@ -185,6 +186,11 @@ func (plugin *NvidiaDevicePlugin) getAPIDevices() *[]*device.DeviceInfo {
})
klog.V(3).Infof("Registered device id=%v, memory=%vMB, type=%v, numa=%v, health=%v", idx, registeredmem, Model, numa, health)
}

if len(res) == 0 && len(devs) > 0 {
klog.Warningf("All %d GPU devices failed NVML queries and were skipped. Returning an empty device list.", len(devs))
}

return &res
}

Expand Down
Loading