-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix backup post hook issue #8490
base: main
Are you sure you want to change the base?
Conversation
2554f9d
to
b318704
Compare
b318704
to
671353d
Compare
changelog check Edit: nvm.. ci failed |
cd72052
to
714a8e7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8490 +/- ##
==========================================
+ Coverage 59.02% 59.08% +0.06%
==========================================
Files 369 374 +5
Lines 39056 39214 +158
==========================================
+ Hits 23052 23169 +117
- Misses 14542 14575 +33
- Partials 1462 1470 +8 ☔ View full report in Codecov by Sentry. |
fbf41f6
to
4952618
Compare
Fix backup post hook issue: hook executes before the backup is finished Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
4952618
to
3a45a10
Compare
// So when Pods are backed up (when hooks execute), the snapshot is taken already | ||
func (p *podBackupPostHookHandler) WaitUntilReadyToExec(ctx context.Context, log logrus.FieldLogger, res *unstructured.Unstructured) error { | ||
// wait all PVBs processed | ||
_, err := p.podVolumeBackupper.WaitAllPodVolumesProcessed(log) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a limitation:
The logic here waits all PVBs of the whole backup to be completed before executing the hooks. This delays the hook execution of one pod although it isn't necessary.
The ideal behavior should be only wait the PVBs belongs to one ItemBlock to be completed before executing the hooks, but this involves the refactor for podvolume.Backupper to make is capable to check whether the PVBs of one ItemBlock completed. The effort isn't tiny, so created another issue #8506 to track.
Fixes #8159 on
main
branch.This PR introduces an overall
Handler
to handle all kinds of hooks.The basic workflow is as follows:
Parser
parses the hooks from different specs into a commonResourceHook
structure.Handler
handles theResourceHook
by delegating to a specificResourceHandler
according to the hook type:podBackupPreHookHandler
: handles the backup pre hook for podspodBackupPostHookHandler
: handles the backup post hook for podsThe changes in this PR will be cherry-picked to branch 1.15 to fix #8159.
And will create another PR to cover restore init/exec hooks for pods and the PR will be only available on
main
(tracked by #8507 )