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 DataUpload Result and CSI VolumeSnapshot check for restore PV. #7061

Merged

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Nov 6, 2023

Thank you for contributing to Velero!

Please add a summary of your change

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 blackpiglet force-pushed the 6595_backward_compatability branch 2 times, most recently from 392b5ba to a4b822e Compare November 6, 2023 14:10
Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #7061 (1fb0529) into main (a221a88) will increase coverage by 0.03%.
Report is 34 commits behind head on main.
The diff coverage is 65.20%.

@@            Coverage Diff             @@
##             main    #7061      +/-   ##
==========================================
+ Coverage   61.01%   61.04%   +0.03%     
==========================================
  Files         255      255              
  Lines       27050    27119      +69     
==========================================
+ Hits        16504    16556      +52     
- Misses       9364     9382      +18     
+ Partials     1182     1181       -1     
Files Coverage Δ
.../resourcemodifiers/resource_modifiers_validator.go 100.00% <100.00%> (ø)
pkg/exposer/csi_snapshot.go 90.88% <100.00%> (ø)
pkg/restore/request.go 100.00% <ø> (ø)
pkg/repository/config/aws.go 50.00% <50.00%> (ø)
pkg/controller/restore_controller.go 65.84% <40.00%> (-0.25%) ⬇️
pkg/repository/provider/unified_repo.go 86.05% <25.00%> (ø)
internal/resourcemodifiers/json_merge_patch.go 55.00% <55.00%> (ø)
internal/resourcemodifiers/json_patch.go 81.48% <81.48%> (ø)
internal/resourcemodifiers/resource_modifiers.go 71.92% <75.47%> (ø)
pkg/restore/restore.go 66.08% <71.08%> (+0.32%) ⬆️
... and 1 more

... and 5 files with indirect coverage changes

Signed-off-by: Xun Jiang <jxun@vmware.com>
@anshulahuja98
Copy link
Collaborator

@blackpiglet is this PR for resolving the issue discussed in #6595
If yes, do you want to take it as different PR/commit which can be cherry picked to previous releases?

@blackpiglet
Copy link
Contributor Author

@blackpiglet is this PR for resolving the issue discussed in #6595 If yes, do you want to take it as different PR/commit which can be cherry picked to previous releases?

@anshulahuja98
Yes, this PR is used to add the backward-compatible fix for the previous releases.
Sure, after this PR is merged, I will create cherry-pick PRs for older releases.

@Lyndon-Li
Copy link
Contributor

@blackpiglet Shouldn't the implementation use the backup volume info in design #6962?

@blackpiglet
Copy link
Contributor Author

@blackpiglet Shouldn't the implementation use the backup volume info in design #6962?

The reason this change is also needed for the main release is that the main Velero release also needs to handle the backup generated by the old Velero.
The old version backup doesn't have the VolumeInfo metadata.
The main Velero needs to have this PR as a fall-back.

@shubham-pampattiwar
Copy link
Collaborator

@blackpiglet So for the implementation PR of the design #6962, you will be updating the switch case function here to check for VolumeInfo and use that struct/file/data if present or else fall back on the current PR's code, Is this the plan ?
Also, the CP would be clean and easy to backup-port to earlier releases. ++ @blackpiglet

@blackpiglet
Copy link
Contributor Author

@blackpiglet So for the implementation PR of the design #6962, you will be updating the switch case function here to check for VolumeInfo and use that struct/file/data if present or else fall back on the current PR's code, Is this the plan ? Also, the CP would be clean and easy to backup-port to earlier releases. ++ @blackpiglet

@shubham-pampattiwar
Exactly. The cherry-pick will only contain this PR's content.

@blackpiglet blackpiglet merged commit cb5ffe2 into vmware-tanzu:main Nov 10, 2023
28 checks passed
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.

None yet

4 participants