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 VolumeInfo metadata structures. #7070

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Nov 7, 2023

Modify design according to comments.
Add PVInfo structure.

Thank you for contributing to Velero!

Please add a summary of your change

Add the VolumeInfo metadata structures for issues pv-backup-info

Does your change fix a particular issue?

Fixes #(issue)

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.

@blackpiglet
Copy link
Contributor Author

@draghuram
Please take a look at the PR. I modified the design document according to comments in #6962.

@blackpiglet
Copy link
Contributor Author

@allenxu404
Please also take a look. I added the PVInfo into the VolumeInfo structure.

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (cb5ffe2) 61.16% compared to head (b440a4f) 61.19%.
Report is 27 commits behind head on main.

Files Patch % Lines
pkg/persistence/object_store.go 64.70% 4 Missing and 2 partials ⚠️
pkg/controller/backup_controller.go 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7070      +/-   ##
==========================================
+ Coverage   61.16%   61.19%   +0.02%     
==========================================
  Files         256      258       +2     
  Lines       27222    27353     +131     
==========================================
+ Hits        16650    16738      +88     
- Misses       9389     9430      +41     
- Partials     1183     1185       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@draghuram draghuram left a comment

Choose a reason for hiding this comment

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

Design changes look good to me though I have few minor comments.

design/pv_backup_info.md Outdated Show resolved Hide resolved
design/pv_backup_info.md Outdated Show resolved Hide resolved
pkg/volume/volume_info_common.go Show resolved Hide resolved
pkg/volume/volume_info_v1.go Outdated Show resolved Hide resolved
Lyndon-Li
Lyndon-Li previously approved these changes Nov 10, 2023
pkg/volume/volume_info_common.go Show resolved Hide resolved
pkg/volume/volume_info_v1.go Outdated Show resolved Hide resolved
pkg/volume/volume_info_common.go Show resolved Hide resolved
design/pv_backup_info.md Show resolved Hide resolved
pkg/volume/volume_info_v1.go Outdated Show resolved Hide resolved
@blackpiglet blackpiglet force-pushed the 6595_interface branch 4 times, most recently from 2a0454b to 11047b1 Compare November 17, 2023 03:49
pkg/volume/volume_info_common.go Outdated Show resolved Hide resolved
pkg/volume/volume_info_common.go Show resolved Hide resolved
@blackpiglet blackpiglet force-pushed the 6595_interface branch 2 times, most recently from 746a675 to 2797d50 Compare November 17, 2023 08:30
anshulahuja98
anshulahuja98 previously approved these changes Nov 17, 2023
Modify design according to comments.
Add PVInfo structure.
Add backup VolumeInfo's object storage's put and get methods.

Signed-off-by: Xun Jiang <[email protected]>
@ywk253100 ywk253100 merged commit 939dd71 into vmware-tanzu:main Nov 17, 2023
24 checks passed
@Lyndon-Li Lyndon-Li mentioned this pull request Nov 21, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants