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

Add the PV backup information design document. #6962

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Oct 16, 2023

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)
This design is used to add the design document to address these issues:
pv-backup-info

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 as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #6962 (23b9484) into main (b4fb2d9) will increase coverage by 0.33%.
Report is 59 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #6962      +/-   ##
==========================================
+ Coverage   60.80%   61.14%   +0.33%     
==========================================
  Files         250      252       +2     
  Lines       26652    26916     +264     
==========================================
+ Hits        16205    16457     +252     
+ Misses       9297     9295       -2     
- Partials     1150     1164      +14     

see 21 files with indirect coverage changes

@blackpiglet blackpiglet force-pushed the 6595_design branch 2 times, most recently from f5919b5 to 2b632b7 Compare October 17, 2023 02:41
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
Lyndon-Li
Lyndon-Li previously approved these changes Oct 26, 2023
Lyndon-Li
Lyndon-Li previously approved these changes Oct 26, 2023
ywk253100
ywk253100 previously approved these changes Oct 27, 2023
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
@blackpiglet blackpiglet dismissed stale reviews from ywk253100 and Lyndon-Li via 31f963c October 30, 2023 15:12
@blackpiglet blackpiglet force-pushed the 6595_design branch 3 times, most recently from 3f913dd to 35b42a6 Compare November 2, 2023 02:09
@Lyndon-Li Lyndon-Li merged commit 166a58b into vmware-tanzu:main Nov 2, 2023
24 checks passed
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved

CSISnapshotInfo CSISnapshotInfo
SnapshotDataMoveInfo SnapshotDataMoveInfo
NativeSnapshotInfo VeleroNativeSnapshotInfo
Copy link
Collaborator

Choose a reason for hiding this comment

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

@blackpiglet Should this be CloudProviderSnapshotInfo / CloudStorageSnapshotInfo / ClusterStorageSnapshotInfo instead ?

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 does have a point. The Velero native snapshot is actually a cloud storage snapshot, but the Velero Native Snapshot is a widely used termination in the Velero code and document.
I think we should keep it consistent here.
But I will add more comments on the structure definition to explain what it actually does.

CSISnapshotInfo CSISnapshotInfo
SnapshotDataMoveInfo SnapshotDataMoveInfo
NativeSnapshotInfo VeleroNativeSnapshotInfo
PVBInfo PodVolumeBackupInfo
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we are adding PodVolumeBackupInfo , Shouldn't the DataUploadInfo be added too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SnapshotDataMovementInfo covers the snapshot data movement information. That structure covers the DataUpload information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants