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

Create restore-hooks_product-requirements.md #2699

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Conversation

stephbman
Copy link
Contributor

This file contains the MVP use cases and product requirements tied the Restore Hooks Design Proposal #2465.

This requirements document ties to Epic: #2116
And is related to issue: #2678

Signed-off-by: Stephanie Bauman, bstephanie@vmware.com

@stephbman stephbman added this to the v1.5 milestone Jul 8, 2020
@stephbman stephbman self-assigned this Jul 8, 2020
Copy link
Contributor

@nrb nrb left a comment

Choose a reason for hiding this comment

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

Thanks for getting this in @stephbman, and sorry I didn't look at it sooner! I have some questions about the wording on a few of the use cases, and just want to be sure I'm clear on them before we move forward on it.

restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved

For questions, please contact michaelmi@vmware.com, [bstephanie@vmware.com](mailto:bstephanie@vmware.com)

Signed-off-by: Stephanie Bauman bstephanie@vmware.com
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be removed - it only needs to be on the git commit message :)

@nrb nrb changed the base branch from master to main July 17, 2020 22:41
@stephbman stephbman marked this pull request as ready for review July 20, 2020 20:19
@nrb nrb added Breaking change Impacts backwards compatibility kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes and removed Breaking change Impacts backwards compatibility labels Jul 20, 2020
Copy link
Member

@ashish-amarnath ashish-amarnath left a comment

Choose a reason for hiding this comment

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

Added some comments and questions.

restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
**Title: **Check for latest snapshot


**Description: **As a user, I would like Velero to run a check for the latest snapshot in object storage prior to starting restore operations on a pod.
Copy link
Member

Choose a reason for hiding this comment

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

There is a 1:1 mapping between restores and backups and backups have volumesnapshots in them.

On restoring from a backup, the volumesnapshots from that backup are what are used to restore data into the volumes.

IMO, changing that behavior, to choose a volumesnapshot, different from the one in the backup, is out of scope for this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This use case is not changing that behavior. Rather, the use case specifies that as part of restore requirements for the product, we want the restore to use the latest volumesnapshot available (latest meaning the volume snapshot that has the most recent timestamp).

On restoring from backup, we want to make sure that our restore requirements leverage the actions and the 1:1 mapping between restores and backups and that the behavior is not changed.

Copy link
Member

Choose a reason for hiding this comment

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

Quick clarification. Every volume will have exactly one snapshot in a backup which is the one that will be picked up when a user restores from that backup. Current;ly, we do not choose a snapshot with the latest timestamp, instead we will choose a snapshot that is in the backup- this may or may not be the latest snapshot.

That is the reason why this requirement seems like a change in behavior which is out of scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Velero will use the snapshot that is in the backup being restored.

restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
Copy link
Contributor Author

@stephbman stephbman left a comment

Choose a reason for hiding this comment

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

Completed review of all comments in document from contributors and maintainers.

restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Outdated Show resolved Hide resolved
**Title: **Check for latest snapshot


**Description: **As a user, I would like Velero to run a check for the latest snapshot in object storage prior to starting restore operations on a pod.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This use case is not changing that behavior. Rather, the use case specifies that as part of restore requirements for the product, we want the restore to use the latest volumesnapshot available (latest meaning the volume snapshot that has the most recent timestamp).

On restoring from backup, we want to make sure that our restore requirements leverage the actions and the 1:1 mapping between restores and backups and that the behavior is not changed.

restore-hooks_product-requirements.md Show resolved Hide resolved
restore-hooks_product-requirements.md Show resolved Hide resolved
@stephbman stephbman removed the request for review from carlisia July 23, 2020 15:53
Copy link
Member

@ashish-amarnath ashish-amarnath left a comment

Choose a reason for hiding this comment

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

Added some comments/ responses.

restore-hooks_product-requirements.md Show resolved Hide resolved
**Title: **Check for latest snapshot


**Description: **As a user, I would like Velero to run a check for the latest snapshot in object storage prior to starting restore operations on a pod.
Copy link
Member

Choose a reason for hiding this comment

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

Quick clarification. Every volume will have exactly one snapshot in a backup which is the one that will be picked up when a user restores from that backup. Current;ly, we do not choose a snapshot with the latest timestamp, instead we will choose a snapshot that is in the backup- this may or may not be the latest snapshot.

That is the reason why this requirement seems like a change in behavior which is out of scope.

restore-hooks_product-requirements.md Show resolved Hide resolved
Copy link
Contributor Author

@stephbman stephbman left a comment

Choose a reason for hiding this comment

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

Reviewed changes w/ Ashish and the changes are acceptable.

This file contains the MVP use cases and product requirements tied the Restore Hooks Design Proposal #2465.

This requirements document ties to Epic: #2116
And is related to issue:  #2678

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
```
pre.hook.restore.velero.io/container

kubectl patch backupstoragelocation <STORAGE LOCATION NAME> \
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of setting the backup storage location to read only here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the same question.

@ashish-amarnath
Copy link
Member

@carlisia leaving the merge for you bc you had some open questions.

Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

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

@stephbman when you have a chance please let us know: #2699 (comment)

@carlisia carlisia merged commit dd2d040 into main Aug 12, 2020
@carlisia carlisia deleted the stephbman-patch-1 branch August 12, 2020 23:14
georgettica pushed a commit to georgettica/velero that referenced this pull request Dec 23, 2020
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
georgettica pushed a commit to georgettica/velero that referenced this pull request Jan 26, 2021
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
vadasambar pushed a commit to vadasambar/velero that referenced this pull request Feb 3, 2021
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
dharmab pushed a commit to dharmab/velero that referenced this pull request May 25, 2021
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
ywk253100 pushed a commit to ywk253100/velero that referenced this pull request Jun 29, 2021
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
Restore Hooks Design Proposal

Signed-off-by: Stephanie Bauman <bstephanie@vmware.com>
Co-authored-by: Ashish Amarnath <ashisham@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backup/Restore Hooks 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.

5 participants