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

Merge CSI plugin code. #7609

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Merge CSI plugin code. #7609

merged 2 commits into from
Apr 11, 2024

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Apr 2, 2024

Thank you for contributing to Velero!

Please add a summary of your change

This PR contains many file changes.
To reduce the reviewers' burden, highlight the main change here:

  • Replace the external-snapshotter client and the k8s native client with the controller-runtime client.
  • Modify how the plugins are registered.
  • Refactor some functions.

P.S. Reviewers can ignore the change under the directory github.com/vmware-tanzu/velero/pkg/restore/actions because correcting the files' package name is the only change.

Does your change fix a particular issue?

Fixes #7408

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.

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 53.43811% with 711 lines in your changes are missing coverage. Please review.

Project coverage is 61.72%. Comparing base (b91b907) to head (8df4e6a).

Files Patch % Lines
pkg/backup/actions/csi/volumesnapshot_action.go 0.00% 175 Missing ⚠️
pkg/backup/actions/csi/pvc_action.go 64.45% 90 Missing and 28 partials ⚠️
pkg/restore/actions/csi/volumesnapshot_action.go 3.29% 88 Missing ⚠️
pkg/util/csi/volume_snapshot.go 74.39% 74 Missing and 11 partials ⚠️
pkg/restore/actions/csi/pvc_action.go 80.16% 50 Missing and 20 partials ⚠️
...backup/actions/csi/volumesnapshotcontent_action.go 0.00% 48 Missing ⚠️
...estore/actions/csi/volumesnapshotcontent_action.go 0.00% 40 Missing ⚠️
...g/backup/actions/csi/volumesnapshotclass_action.go 0.00% 38 Missing ⚠️
.../restore/actions/csi/volumesnapshotclass_action.go 0.00% 38 Missing ⚠️
pkg/util/podvolume/pod_volume.go 81.81% 3 Missing and 3 partials ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7609      +/-   ##
==========================================
- Coverage   61.90%   61.72%   -0.18%     
==========================================
  Files         266      276      +10     
  Lines       29467    24709    -4758     
==========================================
- Hits        18241    15252    -2989     
+ Misses       9934     8101    -1833     
- Partials     1292     1356      +64     

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

@blackpiglet blackpiglet force-pushed the merge_csi branch 12 times, most recently from 2001b16 to c7432f6 Compare April 3, 2024 10:21
@blackpiglet blackpiglet marked this pull request as ready for review April 3, 2024 10:22
@blackpiglet blackpiglet force-pushed the merge_csi branch 3 times, most recently from 6512567 to d9e3e87 Compare April 5, 2024 05:59
@blackpiglet blackpiglet marked this pull request as draft April 5, 2024 06:32
@reasonerjt
Copy link
Contributor

There are many RIA moved into restore/actions but only CSI related BIA into bakcup/actions.

For the first step, I wish to suggest put all CSI plugins into backup/csi or just into backup package and name the file explicitly like csi_xxxx.go

@blackpiglet blackpiglet force-pushed the merge_csi branch 2 times, most recently from db53ace to 1dc97b1 Compare April 7, 2024 16:24
@blackpiglet
Copy link
Contributor Author

There are many RIA moved into restore/actions but only CSI related BIA into bakcup/actions.

For the first step, I wish to suggest put all CSI plugins into backup/csi or just into backup package and name the file explicitly like csi_xxxx.go

Backup-related actions are already put into the github.com/vmware-tanzu/velero/pkg/backup/actions/ in the previous PR.
This PR modifies the restore-related actions because the package names were not set correctly.

@blackpiglet blackpiglet marked this pull request as ready for review April 8, 2024 01:52
@blackpiglet blackpiglet force-pushed the merge_csi branch 2 times, most recently from c77ec8a to ec26129 Compare April 8, 2024 08:21
pkg/util/csi/volume_snapshot.go Outdated Show resolved Hide resolved
pkg/util/csi/volume_snapshot.go Outdated Show resolved Hide resolved
pkg/util/csi/volume_snapshot.go Outdated Show resolved Hide resolved
pkg/util/csi/volume_snapshot.go Outdated Show resolved Hide resolved
pkg/util/csi/volume_snapshot.go Outdated Show resolved Hide resolved
@blackpiglet blackpiglet force-pushed the merge_csi branch 3 times, most recently from 2782df0 to 613379b Compare April 9, 2024 08:58
@github-actions github-actions bot added Website non-docs changes for the website Area/Design Design Documents Dependencies Pull requests that update a dependency file Documentation labels Apr 9, 2024
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
Lyndon-Li
Lyndon-Li previously approved these changes Apr 10, 2024
reasonerjt
reasonerjt previously approved these changes Apr 10, 2024
Signed-off-by: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com>
@blackpiglet blackpiglet dismissed stale reviews from reasonerjt and Lyndon-Li via 8df4e6a April 10, 2024 10:54
@blackpiglet blackpiglet merged commit 6ef3836 into vmware-tanzu:main Apr 11, 2024
64 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Design Design Documents Dependencies Pull requests that update a dependency file Documentation has-changelog has-e2e-2tests has-unit-tests Website non-docs changes for the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge csi plugin into velero repo
4 participants