Skip to content

fix: log dGPU discoverer initialization errors - #2119

Open
truongbo17 wants to merge 1 commit into
NVIDIA:mainfrom
truongbo17:fix/log-dgpu-discovery-errors
Open

truongbo17 wants to merge 1 commit into
NVIDIA:mainfrom
truongbo17:fix/log-dgpu-discovery-errors

Conversation

@truongbo17

Copy link
Copy Markdown

Summary

Log warnings when nvsandboxutils or NVML dGPU discoverer initialization fails, for both full GPU and MIG devices. The warning names the failing backend and includes the original error, while the existing fallback and joined-error behavior stays intact.

Fixes #2114.

Tests

  • Added regression tests for all four warning paths (confirmed failing before the fix).
  • make build, make fmt, and make test passed in a Linux Go container.

Signed-off-by: Tony Nguyen <91400591+truongbo17@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@myeolenv

Copy link
Copy Markdown

Thanks for the contribution. We will review this PR soon.

@henry118 henry118 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@truongbo17 Thanks for picking up the TODO. A couple of thoughts:

  • All-failed case: the error is already returned and logged at the top level.
  • One discoverer fails: this is the case nothing logs today.

imo we just log when at least one discoverer was created and another failed, at debug level, like:

if len(discoverers) == 0 {
    return nil, errs
}
if errs != nil {
    o.logger.Debugf("Ignoring discoverer errors: %v", errs)
}

The same change is needed in NewForMigDevice.

And I’d also drop the new tests that are just check log messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Address TODO comments in internal/platform-support/dgpu/dgpu.go

3 participants