Skip to content

fix: improve error handling for discarded hooks in WaitExecHookHandler#9750

Open
priyansh17 wants to merge 1 commit into
velero-io:mainfrom
priyansh17:priyansh/issue-9749
Open

fix: improve error handling for discarded hooks in WaitExecHookHandler#9750
priyansh17 wants to merge 1 commit into
velero-io:mainfrom
priyansh17:priyansh/issue-9749

Conversation

@priyansh17

@priyansh17 priyansh17 commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Signed-off-by: Priyansh Choudhary im1706@gmail.com

Thank you for contributing to Velero!

Please add a summary of your change

The early-return in HandleHooks (when byContainer empty or pod==nil) should Record(... hookFailed=true ...) in the multiHookTracker, or else never Add such hooks in the first place. Otherwise, tracker cannot complete, and the restore gets stuck forever.

Does your change fix a particular issue?

Yes
Fixes #(issue)
#9749

Please indicate you've done the following:

@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 19.04762% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/hook/wait_exec_hook_handler.go 10.52% 17 Missing ⚠️

📢 Thoughts on this report? Let us know!

@priyansh17

Copy link
Copy Markdown
Collaborator Author

The suggested fix adds an error along with the warning logger so it moves the Restore to PartiallyFailed Instead of Success with warnings logged.
Let me know if you think we should ignore and just add logger warnings.

@priyansh17 priyansh17 requested a review from anshulahuja98 May 6, 2026 09:33
Signed-off-by: Priyansh Choudhary <im1706@gmail.com>
@blackpiglet blackpiglet force-pushed the priyansh/issue-9749 branch from 08bc63c to 9fb9e89 Compare May 9, 2026 07:17
@blackpiglet

Copy link
Copy Markdown
Contributor

The suggested fix adds an error along with the warning logger so it moves the Restore to PartiallyFailed Instead of Success with warnings logged. Let me know if you think we should ignore and just add logger warnings.

I think the behavior of this PR is correct.
However, it seems don't add the hook at first should be better.
Is it possible to not track the unexpected exec wait hook, and also mark the restore as PartiallyFailed?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants