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

Make restore controller adopting the controller-runtime framework. #5864

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Feb 16, 2023

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #5027

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 self-assigned this Feb 16, 2023
@blackpiglet blackpiglet force-pushed the restore_controller_use_controller_runtime branch from 4edb17e to 7ab7bf8 Compare February 16, 2023 07:23
@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2023

Codecov Report

Merging #5864 (c2ac761) into main (a467488) will decrease coverage by 0.04%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #5864      +/-   ##
==========================================
- Coverage   39.97%   39.94%   -0.04%     
==========================================
  Files         253      253              
  Lines       22254    22259       +5     
==========================================
- Hits         8897     8892       -5     
- Misses      12707    12716       +9     
- Partials      650      651       +1     
Impacted Files Coverage Δ
pkg/cmd/server/server.go 6.62% <0.00%> (+0.01%) ⬆️
pkg/util/kube/predicate.go 39.13% <0.00%> (-9.97%) ⬇️
pkg/controller/restore_controller.go 69.01% <74.76%> (+0.12%) ⬆️

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

@blackpiglet blackpiglet force-pushed the restore_controller_use_controller_runtime branch 2 times, most recently from ca1ae0a to 1156ea5 Compare February 16, 2023 08:03
@blackpiglet blackpiglet force-pushed the restore_controller_use_controller_runtime branch 4 times, most recently from 185ef99 to 616e005 Compare February 16, 2023 13:01
@blackpiglet
Copy link
Contributor Author

@blackpiglet blackpiglet marked this pull request as ready for review February 16, 2023 13:21
qiuming-best
qiuming-best previously approved these changes Feb 23, 2023
Copy link
Contributor

@qiuming-best qiuming-best left a comment

Choose a reason for hiding this comment

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

LGTM

@sseago
Copy link
Collaborator

sseago commented Feb 23, 2023

@blackpiglet This has conflicts with restore_controller now (I think due to the refactoring to add backup error/warnings, since restore.Result{} was pulled into a shared package). It would be great to get this merged this week, since I may need to deal with minor merge conflicts from some of these files in my current async PR.

@blackpiglet blackpiglet force-pushed the restore_controller_use_controller_runtime branch from 38457d0 to c2ac761 Compare February 25, 2023 03:55
@blackpiglet
Copy link
Contributor Author

@sseago
Resolved the conflict. PTAL.

@shubham-pampattiwar shubham-pampattiwar merged commit c6fba55 into vmware-tanzu:main Feb 25, 2023
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.

Refactor Restore controller with kubebuilder
5 participants