Skip to content
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

Merged
merged 7 commits into from
Jan 19, 2022

Conversation

brito-rafa
Copy link
Contributor

@brito-rafa brito-rafa commented Aug 31, 2021

Signed-off-by: Rafael Brito [email protected]

Thank you for contributing to Velero!

Please add a summary of your change

Velero should provide a way to trigger actions before and after each backup and restore.
Important: These proposed plugin hooks are fundamentally different from the existing plugin hooks, BackupItemAction and RestoreItemAction, which are triggered per item during backup and restore, respectively.
The proposed plugin hooks are to be executed only once.

Does your change fix a particular issue?

Fixes #(issue)
#4067

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required.
  • [ NA] Updated the corresponding documentation in site/content/docs/main.

@github-actions github-actions bot added the Area/Design Design Documents label Aug 31, 2021
@dsu-igeek
Copy link
Contributor

Let's review individually and discuss in the next community meeting before merging.

design/new-prepost-backuprestore-plugin-hooks.md Outdated Show resolved Hide resolved

Other examples of plugins that can use the proposed plugin hooks are

- PostBackupAction: trigger a Velero Restore after a successful Velero backup (and complete the migration operation).
Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

design/new-prepost-backuprestore-plugin-hooks.md Outdated Show resolved Hide resolved
@brito-rafa brito-rafa changed the title #4067 WIP Initial design of the new plugins - pre-post backup and restore #4067 Initial design of the new plugins - pre-post backup and restore Sep 14, 2021
@zubron zubron self-requested a review November 9, 2021 00:14
@jaidevmane
Copy link
Contributor

Would love to participate in discussions on this. Our team has a use case for this workflow, where currently we are asking users to run through a two step restore process, and using a post-restore/backup hook would enable us to abstract away the step for them.
#1981

@brito-rafa
Copy link
Contributor Author

Please take a look here at this code: this is a functional implementation of this design, without the plugins logging - https:/cschaefer-vmw/velero

For the design, the only thing missing is defining how the logging of post-backup and post-restore plugins will act behind the scenes (the plan is expanding existent interfaces).

Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

LGTM

@dsu-igeek dsu-igeek added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jan 19, 2022
Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Today, Velero's Restic integration is the response for such use cases, but there are some limitations:

- Quiesce/unquiesce workloads: Pod hooks are useful for quiescing/unquiescing workloads, but platform engineers often do not have the luxury/visibility/time/knowledge to go through each pod in order to add specific commands to quiesce/unquiesce workloads.
- Orphan PVC/PV pairs: PVCs/PVs that do not have associated running pods are not backed up and consequently, are not migrated.
Copy link
Contributor

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

Copy link

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.

@dsu-igeek dsu-igeek merged commit 015e8e7 into vmware-tanzu:main Jan 19, 2022
danfengliu pushed a commit to danfengliu/velero that referenced this pull request Jan 25, 2022
… and restore (vmware-tanzu#4083)

* vmware-tanzu#4067 Initial design of the new plugins - pre-post backup and restore

Signed-off-by: Rafael Brito <[email protected]>

* Update new-prepost-backuprestore-plugin-hooks.md

* Updated design doc as per feedback

Signed-off-by: Rafael Brito <[email protected]>

* Adding design changes as per feedback

* Update design on prepost-backup-restore plugins

* More color on how to call plugins

Signed-off-by: Rafael Brito <[email protected]>

* Proposing annotations to skip plugin execution

Signed-off-by: Rafael Brito <[email protected]>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
… and restore (vmware-tanzu#4083)

* vmware-tanzu#4067 Initial design of the new plugins - pre-post backup and restore

Signed-off-by: Rafael Brito <[email protected]>

* Update new-prepost-backuprestore-plugin-hooks.md

* Updated design doc as per feedback

Signed-off-by: Rafael Brito <[email protected]>

* Adding design changes as per feedback

* Update design on prepost-backup-restore plugins

* More color on how to call plugins

Signed-off-by: Rafael Brito <[email protected]>

* Proposing annotations to skip plugin execution

Signed-off-by: Rafael Brito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Design Design Documents kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants