chore: improve error handling in scheduler routes and device plugin

[shinigami-777]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
While investigating the codebase, I ran errcheck manually and noticed some unhandled errors that could lead to silent failures. These slipped through CI because errcheck is disabled in the project's .golangci.yaml configuration. Furthermore, the entire pkg/device-plugin directory is excluded from linter checks.

This PR adds proper error checking and logging to prevent silent failures in these components.

Changes Made

1. pkg/scheduler/routes/route.go:
   Added error handling for HTTP w.Write() calls. If writing the JSON response to the client fails (e.g., due to a dropped connection), it will now properly log a klog.ErrorS instead of failing silently.

2. pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go:

• Added proper error handling when parsing the MIG configuration via yaml.Unmarshal. If unmarshaling fails, the plugin now logs an error, explicitly aborts the rest of the MIG setup block, and falls back to a non-MIG configuration. This prevents the plugin from silently continuing with an invalid, zero-value migCurrent configuration.
• Added error handling for os.MkdirAll, os.RemoveAll, and os.Chmod .

Special notes for your reviewer:

Does this PR introduce a user-facing change?:
Yes. By adding explicit error handling, the behavior of the device plugin has shifted from "silently ignoring" these errors to "explicitly aborting" the pod's resource allocation. This makes the plugin much safer and more predictable (since it no longer allows pods to start in an incomplete or incorrectly permissioned state).

AI assistance disclosure:
I manually ran errcheck to catch the unhandled cases. The changes was made using assistance from Gemini 3.1 Pro. I understand and take responsibility for the implementation and have manually verified and tested it using make verify and make test before creating this PR.

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of invalid MIG configuration data by reporting parsing errors and safely falling back to non-MIG configuration.
  • GPU allocation now detects and clearly reports failures while preparing required cache and lock directories.
  • Improved scheduler response reliability by detecting failures when sending responses.
  • Enhanced error handling helps prevent incomplete GPU allocations and improves visibility into configuration and response-delivery issues.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shinigami-777
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hami-robot]

Welcome @shinigami-777! It looks like this is your first PR to Project-HAMi/HAMi 🎉

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a5ecc53b-1048-4cd2-b40c-7234b5cf1a4e

📥 Commits

Reviewing files that changed from the base of the PR and between ed3ffb5 and 2df4ad3.

📒 Files selected for processing (1)

• pkg/device-plugin/nvidiadevice/nvinternal/plugin/server_test.go

---

📝 Walkthrough

Walkthrough

The change adds error handling for MIG parsing, vGPU filesystem setup, and scheduler HTTP response writes. Allocation tests now isolate the host hook path.

Changes

Device plugin error handling

Layer / File(s)	Summary
Plugin parsing and allocation failures pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go, pkg/device-plugin/nvidiadevice/nvinternal/plugin/server_test.go	MIG YAML parsing errors are logged, and invalid configurations skip further processing. vGPU cache cleanup, directory creation, and permission errors mark allocation as failed and return errors. Allocation tests use temporary host hook paths.

Scheduler response error handling

Layer / File(s)	Summary
Predicate and bind response writes pkg/scheduler/routes/route.go	Predicate and bind handlers log write errors for successful responses and marshal-failure fallback responses.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• Project-HAMi/HAMi#2132: Both changes modify NVIDIA device-plugin MIG configuration or allocation behavior.
• Project-HAMi/HAMi#2245: Both changes add safeguards for MIG allocation and lock handling.
• Project-HAMi/HAMi#2293: Both changes improve NVIDIA device-plugin allocation failure handling.

Suggested labels: enhancement

Suggested reviewers: ouyangluwei163, peachest

Poem

A rabbit checks the YAML file,
Guards cache paths mile by mile.
Writes report errors when they fail,
Tests use temp paths without derail.
Safe allocation leaves a trail.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes to error handling in scheduler routes and the NVIDIA device plugin.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with 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.

Inline comments:
In `@pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go`:
- Around line 315-317: Update the MIG handling around yaml.Unmarshal to parse
into a zero-valued local nvidia.MigPartedSpec, and only assign it to
plugin.migCurrent after unmarshalling succeeds. On failure, reject the
configuration so writeMigConfig and any later plugin.ApplyMigTemplate call
cannot use stale or invalid MIG state.
- Around line 680-688: Update the directory creation and permission handling in
the Allocate flow around cacheFileHostDirectory and /tmp/vgpulock to remove the
world-writable 0777 defaults and the unconditional os.Chmod reset. Use
least-privilege directory modes with explicit ownership/group access, and
preserve isolation for pod-specific cache data and lock files while retaining
the existing failure handling.
- Around line 680-688: Check the return value of each os.Chmod call for
cacheFileHostDirectory and /tmp/vgpulock in the Allocate flow. On failure, call
PodAllocationFailed with the same allocation context and return an Allocate
error before mounts are appended, matching the existing MkdirAll failure
handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 004e9d20-18e8-4aaf-8729-809d4d139e8f

📥 Commits

Reviewing files that changed from the base of the PR and between 57bda65 and 4ac24e1.

📒 Files selected for processing (2)

• pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go
• pkg/scheduler/routes/route.go

[mesutoezdil]

the mkdir part changes behavior (silent -> abort) so pls put that in the release note field instead of "no".

[mesutoezdil]

logging alone is not enough, migCurrent stays zero and the next lines still run w/ it, pls fall back to non mig like the sibling branch above

[shinigami-777]

Yes, made it similar to the sibling branch above it.

[shinigami-777]

The test for pkg/device-plugin/nvidiadevice/nvinternal/plugin was failing earlier as expected due to the strict error handling for filesystem operations during pod allocation. When the plugin fails to clean up or set permissions on the cache directory or the lock directory, it aborts the allocation and returns an error.
I was earlier testing locally using sudo so missed it :(
To fix the permission denied issue, we can override the hostHookPath variable with a safe temporary directory (using t.TempDir()) at the start of Allocate tests.

[archlitchi]

/assign @mesutoezdil

[FouoF]

Thanks for auditing these error paths. The PR bundles three unrelated changes—HTTP write logging, MIG YAML fallback behavior, and allocation-directory failure handling—without a shared reproducible defect. The added test setup does not exercise the newly changed failure paths, and the normal exact-head CI suite has not produced reviewable results. A broad mechanical error-handling sweep is difficult to validate and maintain, so we are closing this PR. Please submit any real failure as a separate, minimal change with a reproduction and a regression test for that exact path.