fix: release ascend device resources when scheduling fails#5378
fix: release ascend device resources when scheduling fails#5378DSFans2014 wants to merge 4 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request refactors several methods in AscendDevices to be private and implements the Release method to clean up resources and remove pod annotations. It also introduces a utility function RemovePodAnnotations to perform strategic merge patches for annotation removal. Feedback from the review highlights potential nil pointer dereferences in both Release and RemovePodAnnotations, suggests using tabs instead of spaces for standard Go formatting, and recommends a more idiomatic slice initialization in removeAnnotations to avoid multiple allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Signed-off-by: james <open4pd@4paradigm.com>
b6a53cc to
0814e8b
Compare
|
Adding label DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…resource-in-release
Signed-off-by: james <open4pd@4paradigm.com>
646d247 to
43b6404
Compare
…resource-in-release Signed-off-by: james <open4pd@4paradigm.com>
|
@DSFans2014 Hi, why it's been closed? |
#5416 cleans all device resources, authored by @archlitchi |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
If scheduling fails (e.g., a dry-run failure as mentioned in #5335), the sequence
unAllocate -> DeAllocateFunc -> Releasewill be triggered. However, theReleaseinterface of the ascend device currently does not release any resources.Special notes for your reviewer:
Does this PR introduce a user-facing change?