Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
#4067 Initial design of the new plugins - pre-post backup and restore #4083
#4067 Initial design of the new plugins - pre-post backup and restore #4083
Changes from 2 commits
eb576cb
96b2285
206bd8c
c674211
f47dbc2
82ce62e
cd5fa3b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This seems more like a bug in Velero rather than something to work around
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.
Agreed. I thought there was already an issue about this topic, but if so I cannot find it now due to community/community#6403.
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.
This may not be early enough to trigger a restore. We would want to hold off on triggering a restore until the backup was written to the backup store, since in a migration use case the restore would be run in a different cluster, and it can only reference the backup after the backup is written to the backup store and the destination cluster's velero instance syncs the backup. If PostBackupActiions are run after recordBackupMetrics but before persistBackup, this is probably the wrong place for triggering a migration restore.
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.
Agree.
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.
Just a note from what we discussed during the 2021-09-14 community meeting: it is better for the postback plugins to run after the upload of the backup on the BSL. Why? We want to be positive the backup is really completed before doing any further operation. We want to have separate backup/restore phases for these plugins. Let's have their logs persisted separately as well. On the pre restore actions, run proactive sync on the BSL to make sure the latest backup is there.
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.
Added the changes on the design as per feedback from the last call. Will go for another round of feedback.