-
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
Create restore-hooks_product-requirements.md #2699
Conversation
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.
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.
|
||
For questions, please contact [email protected], [[email protected]](mailto:[email protected]) | ||
|
||
Signed-off-by: Stephanie Bauman [email protected] |
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 line can be removed - it only needs to be on the git commit message :)
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 some comments and questions.
**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. |
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.
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.
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 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.
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.
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.
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.
Velero will use the snapshot that is in the backup being restored.
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.
Completed review of all comments in document from contributors and maintainers.
**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. |
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 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.
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 some comments/ responses.
**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. |
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.
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.
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.
Reviewed changes w/ Ashish and the changes are acceptable.
57339a5
to
d009c9e
Compare
d009c9e
to
28136d3
Compare
28136d3
to
a5028b2
Compare
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 <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
a5028b2
to
20b9f53
Compare
``` | ||
pre.hook.restore.velero.io/container | ||
|
||
kubectl patch backupstoragelocation <STORAGE LOCATION NAME> \ |
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.
What's the purpose of setting the backup storage location to read only here?
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.
I have the same question.
@carlisia leaving the merge for you bc you had some open questions. |
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.
@stephbman when you have a chance please let us know: #2699 (comment)
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
Restore Hooks Design Proposal Signed-off-by: Stephanie Bauman <[email protected]> Co-authored-by: Ashish Amarnath <[email protected]>
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, [email protected]