Skip to content

Mark devices unhealthy when health monitoring cannot initialize - #2037

Open
Shivkumar13 wants to merge 1 commit into
NVIDIA:mainfrom
Shivkumar13:fix/health-check-init-failure
Open

Shivkumar13 wants to merge 1 commit into
NVIDIA:mainfrom
Shivkumar13:fix/health-check-init-failure

Conversation

@Shivkumar13

Copy link
Copy Markdown

Description

When NVML initialization or event-set creation fails after device discovery, health monitoring exits without reporting the devices unhealthy. The plugin can remain connected to kubelet with its last Healthy device list, even though health monitoring is unavailable.

Report every device managed by the affected resource manager through the existing unhealthy notification channel before returning from either setup failure. Stop-aware sends allow shutdown when ListAndWatch is not receiving notifications. Preserve the existing FAIL_ON_INIT_ERROR return behavior and explicit health-check bypasses, and update the plugin error log to describe the failure accurately.

Related to #1640. This addresses the health-monitor setup failure path described there; it does not claim to resolve every allocation failure or the complete reported incident. Initial Healthy publication before health-check setup, per-device discovery/allocation isolation, and CDI behavior are unchanged. Devices follow the existing Unhealthy lifecycle; monitor retries and automatic recovery are outside this change.

Checklist

  • No secrets, sensitive information, or unrelated changes
  • Lint checks passing (make lint)
  • Unit tests passing (make test)
  • Go mod artifacts in-sync (make check-modules)
  • Third-party notices in-sync (make check-third-party-notices)
  • Test cases are added for new code paths

Testing

Deterministic mocked-NVML tests cover initialization failure with both FAIL_ON_INIT_ERROR values, event-set creation failure, cleanup, explicit bypasses, and shutdown during unbuffered unhealthy notification. A protocol test uses the real resource manager and ListAndWatch implementation to verify that setup failure produces an all-Unhealthy response and that a subsequent stream retains that state.

Validated on Linux/amd64 with Go 1.26.0:

  • The new regression tests fail against unmodified main at 6256903: no unhealthy notifications are sent, and the ListAndWatch handler retains only its initial Healthy snapshot.
  • go test ./internal/rm ./internal/plugin -count=1 passes with the change.
  • make test passes, including its build targets and all cmd/internal/api unit tests.
  • go test -race ./internal/rm ./internal/plugin -count=1 passes.
  • make lint passes with golangci-lint v2.13.2 (0 issues).
  • make check-modules and make -C deployments/devel check-modules pass with no module/vendor changes.
  • make check-third-party-notices passes with no notice changes.

The ListAndWatch tests invoke the real handler with a test stream; they do not run a kubelet or Kubernetes cluster. No physical GPU failure or customer environment reproduction is claimed. The initial Healthy response remains covered as existing behavior; preventing it is separate follow-up work.

Report managed devices unhealthy when NVML initialization or event-set
creation fails during health-check setup. Make the new notifications
cancellation-aware and preserve initialization-error and bypass policies.

Capture health-check lifecycle channels before starting the goroutine and
remove the misleading continuing-with-checks-disabled log message.

Add mocked-NVML regression tests and ListAndWatch coverage, including
reconnect state and shutdown during notification.

Related to NVIDIA#1640.

Signed-off-by: Shivkumar Ople <sople@nvidia.com>
@Shivkumar13
Shivkumar13 marked this pull request as ready for review September 16, 2026 14:15
@Shivkumar13

Copy link
Copy Markdown
Author

Please take a look, Thank you @tariq1890

This branch has not been deployed

No deployments
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.

2 participants