velero icon indicating copy to clipboard operation
velero copied to clipboard

Refactor BackupItemAction to backupitemaction/v1

Open phuongatemc opened this issue 2 years ago • 1 comments

Signed-off-by: Hoang, Phuong [email protected]

Thank you for contributing to Velero!

Please add a summary of your change

Refactor BackupItemAction proto to backupitemaction/v1 and related code.

Does your change fix a particular issue?

This is part of the implementation for Plugin Versioning Design https://github.com/vmware-tanzu/velero/blob/main/design/plugin-versioning.md

Please indicate you've done the following:

  • [x] 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.

phuongatemc avatar May 27 '22 21:05 phuongatemc

Verified on my environment according to @sseago comments.

Need to --go_opt=module=github.com/vmware-tanzu/velero/pkg/plugin/generated in protoc command, and remove option go_package = "github.com/vmware-tanzu/velero/pkg/plugin/generated"; from proto files.

blackpiglet avatar Jul 19 '22 07:07 blackpiglet

Rebased to main to resolve conflicts, and fixed the hack/update-*.sh files to make sure protoc generation happens before crd generation.

sseago avatar Aug 25 '22 21:08 sseago

Codecov Report

Merging #4943 (5a5a4c1) into main (e849441) will not change coverage. The diff coverage is 44.00%.

@@           Coverage Diff           @@
##             main    #4943   +/-   ##
=======================================
  Coverage   41.03%   41.03%           
=======================================
  Files         231      231           
  Lines       19645    19645           
=======================================
  Hits         8062     8062           
  Misses      10993    10993           
  Partials      590      590           
Impacted Files Coverage Δ
pkg/plugin/framework/action_resolver.go 6.89% <0.00%> (ø)
pkg/plugin/framework/backup_item_action.go 0.00% <0.00%> (ø)
pkg/plugin/framework/backup_item_action_client.go 0.00% <0.00%> (ø)
pkg/plugin/framework/delete_item_action_server.go 0.00% <0.00%> (ø)
pkg/plugin/framework/item_snapshotter_client.go 0.00% <ø> (ø)
pkg/plugin/framework/item_snapshotter_server.go 0.00% <0.00%> (ø)
pkg/plugin/framework/restore_item_action_server.go 0.00% <0.00%> (ø)
pkg/plugin/clientmgmt/manager.go 81.32% <50.00%> (ø)
pkg/plugin/framework/backup_item_action_server.go 51.94% <57.14%> (ø)
pkg/backup/backup.go 79.49% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 25 '22 21:08 codecov-commenter

Updated to newer protobug/protoc-gen-go since the module option isn't supported in the ancient 1.0.0 protoc-gen-go we're currently using. This is breaking tests now, so I'll need to fix some things now. Looks like some changes to generated code may need to be reflected in the velero code that references it.

sseago avatar Aug 25 '22 22:08 sseago

I have tested this PR with our existing OADP/OpenShift backup item action plugins. The only thing I had to do for this to work was to rebuild the image with the velero deps updated to include the code changes in this PR.

Using the new code with an old plugin image (based on incompatible velero dependencies) will result in an error like the below when trying to run a backup:

time="2022-08-29T18:53:11Z" level=error msg="backup failed" controller=backup error="rpc error: code = Unimplemented desc = unknown service v1.BackupItemAction" key=openshift-adp/bia-test logSource="pkg/controller/backup_controller.go:301"

sseago avatar Aug 29 '22 20:08 sseago