fix(plugin): release node lock on getAllocateResponse failure

[manmathbh]

What type of PR is this?

/kind bug

What this PR does / why we need it:

getAllocateResponse failure path in the non-MIG Allocate loop was missing
a PodAllocationFailed call. Every other error exit in the same function
calls it, this was the only one that skipped it.

Node lock stays set on the node after the failure. Next GPU request on
that node either hangs or fails.

Which issue(s) this PR fixes:
Fixes #2214

Special notes for your reviewer:

Only affects the non-MIG path. getAllocateResponse isn't called in the
MIG branch.

Does this PR introduce a user-facing change?:

NONE

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of failed pod resource allocations by recording the failure state when an allocation response cannot be generated.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: manmathbh
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

[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: b63e883c-c9e5-428f-a403-f8d2596f0447

📥 Commits

Reviewing files that changed from the base of the PR and between c7891de and 9e29734.

📒 Files selected for processing (1)

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

---

📝 Walkthrough

Walkthrough

Changes

NVIDIA allocation error handling

Layer / File(s)	Summary
Record failed allocation state pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go	Allocate now calls PodAllocationFailed before returning an allocation-response error.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: copilot

Poem

A rabbit saw the lock stay tight,
Then marked the failed allocation right.
The node can breathe, the GPUs flow,
And waiting pods can safely go.

🚥 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 identifies the node lock release fix for getAllocateResponse failure.
Linked Issues check	✅ Passed	The change calls PodAllocationFailed on getAllocateResponse failure, satisfying issue #2214 and releasing the node lock.
Out of Scope Changes check	✅ Passed	The single-line change directly addresses the linked issue and introduces no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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.}

[Copilot]

Pull request overview

Fixes a node-lock leak in the NVIDIA device plugin Allocate() non-MIG path by ensuring the failure path for getAllocateResponse() performs the same cleanup as other error exits (releasing the node lock via PodAllocationFailed(...)).

Changes:

• Add missing PodAllocationFailed(nodename, current, NodeLockNvidia) call when getAllocateResponse(...) fails in the non-MIG allocation branch.
• Align the non-MIG getAllocateResponse error handling with the existing MIG branch and other non-MIG error exits to prevent stale hami.io/mutex.lock on the node.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mesutoezdil]

pls answer Shouren's question on #2214 first, if kubelet retries Allocate after an error then releasing the lock here may be wrong and the existing 7 call sites have the same problem, this needs a decision before adding an 8th. also cotishq had offered to fix that issue, pls sync w/ them.

[manmathbh]

hey @cotishq go ahead and take this one, i hadn't realized you were planning to raise the pr for it. @Shouren that's a great point about the kubelet retry behavior breaking the single-pod assumption on #2214. i'll close this out so you guys can align on the right approach for all 8 call sites.