Skip to content

fix: cleanup hooks not receiving error signal#2475

Merged
yxxhero merged 6 commits intomainfrom
fix-cleanup-hooks-error-signal
Mar 21, 2026
Merged

fix: cleanup hooks not receiving error signal#2475
yxxhero merged 6 commits intomainfrom
fix-cleanup-hooks-error-signal

Conversation

@yxxhero
Copy link
Copy Markdown
Member

@yxxhero yxxhero commented Mar 9, 2026

Summary

  • Fixes .Event.Error is not populated for cleanup global hooks #1041 by propagating errors to cleanup global hooks
  • Changes withPreparedCharts callback signature from func() to func() []error
  • Modifies TriggerGlobalCleanupEvent to accept an error parameter
  • Updates all callback invocations to return errors

This fix ensures that .Event.Error is properly populated for cleanup global hooks when releases fail, matching the behavior already present for postsync release hooks.

Changes

  1. pkg/app/run.go: Modified withPreparedCharts to capture errors from the callback and pass the first error to TriggerGlobalCleanupEvent

  2. pkg/state/state.go: Updated TriggerGlobalCleanupEvent to accept an evtErr error parameter

  3. pkg/app/app.go: Updated all callback functions to return []error

Test Plan

  • Build compiles successfully
  • make check passes

References

@yxxhero yxxhero mentioned this pull request Mar 9, 2026
Closed
@yxxhero yxxhero requested a review from Copilot March 14, 2026 00:41
Copilot AI reviewed Mar 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@yxxhero yxxhero requested a review from Copilot March 19, 2026 09:31
Copilot AI reviewed Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread pkg/app/run.go
Comment thread pkg/state/state.go
@yxxhero yxxhero force-pushed the fix-cleanup-hooks-error-signal branch from c800910 to bfd2934 Compare March 19, 2026 09:42
yxxhero added 2 commits March 19, 2026 17:43
@yxxhero
fix: cleanup hooks not receiving error signal
da58318 
Closes #1041

Signed-off-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero
Add tests for cleanup hooks error propagation
ae9ac41 
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero yxxhero force-pushed the fix-cleanup-hooks-error-signal branch from bfd2934 to ae9ac41 Compare March 19, 2026 09:43
@yxxhero
fix tests
2edc0c6 
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero yxxhero requested a review from Copilot March 19, 2026 09:53
Copilot AI reviewed Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Comment thread pkg/app/app.go Outdated
Comment thread pkg/app/app.go
Comment thread pkg/state/state.go Outdated
Comment thread pkg/state/cleanup_hooks_test.go Outdated
Comment thread pkg/app/cleanup_hooks_error_test.go
Comment thread pkg/app/app.go Outdated
@yxxhero
fix tests
56b4332 
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero yxxhero requested a review from Copilot March 19, 2026 10:04
Copilot AI reviewed Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread pkg/state/state.go
Comment thread pkg/app/app.go
Comment thread pkg/app/run.go Outdated
@yxxhero yxxhero requested a review from Copilot March 19, 2026 12:03
Copilot AI reviewed Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment thread pkg/app/app.go
Comment thread pkg/app/app.go
Comment thread pkg/app/run.go Outdated
Comment thread pkg/state/cleanup_hooks_test.go Outdated
@yxxhero
fix tests
377a12b 
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero yxxhero requested a review from Copilot March 19, 2026 23:41
Copilot AI reviewed Mar 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread pkg/state/cleanup_hooks_test.go Outdated
@yxxhero
fix tests
7930eaf 
Signed-off-by: yxxhero <aiopsclub@163.com>
@yxxhero yxxhero requested a review from Copilot March 20, 2026 00:04
Copilot AI reviewed Mar 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@yxxhero yxxhero merged commit 43b4268 into main Mar 21, 2026
20 checks passed
@yxxhero yxxhero deleted the fix-cleanup-hooks-error-signal branch March 21, 2026 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@xiaomudk xiaomudk xiaomudk approved these changes

@mumoshu mumoshu Awaiting requested review from mumoshu mumoshu is a code owner

@itscaro itscaro Awaiting requested review from itscaro itscaro is a code owner

@zhaque44 zhaque44 Awaiting requested review from zhaque44 zhaque44 is a code owner

@aditmeno aditmeno Awaiting requested review from aditmeno aditmeno is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

.Event.Error is not populated for cleanup global hooks

3 participants

@yxxhero @xiaomudk