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

Refoctor backup controller based on kubebuilder #5835

Closed
wants to merge 2 commits into from

Conversation

qiuming-best
Copy link
Contributor

@qiuming-best qiuming-best commented Feb 6, 2023

Signed-off-by: Ming mqiu@vmware.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)
#5026, Refoctor backup controller based on kubebuilder

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.

@qiuming-best qiuming-best force-pushed the remove-client-go branch 2 times, most recently from bc71190 to 8a65c40 Compare February 6, 2023 14:40
@qiuming-best qiuming-best marked this pull request as draft February 7, 2023 07:07
@qiuming-best qiuming-best changed the title Remove redundant client from backup controller Refoctor backup controller based on kubebuilder Feb 15, 2023
@qiuming-best qiuming-best force-pushed the remove-client-go branch 2 times, most recently from b09aa22 to 727b8b3 Compare February 16, 2023 07:17
@qiuming-best qiuming-best marked this pull request as ready for review February 16, 2023 07:31
ywk253100
ywk253100 previously approved these changes Feb 17, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2023

Codecov Report

Merging #5835 (02fa8c3) into main (c6fba55) will increase coverage by 0.17%.
The diff coverage is 39.44%.

@@            Coverage Diff             @@
##             main    #5835      +/-   ##
==========================================
+ Coverage   39.94%   40.12%   +0.17%     
==========================================
  Files         253      253              
  Lines       22259    22239      -20     
==========================================
+ Hits         8892     8923      +31     
+ Misses      12716    12665      -51     
  Partials      651      651              
Impacted Files Coverage Δ
pkg/podvolume/backupper_factory.go 0.00% <0.00%> (ø)
pkg/podvolume/restorer_factory.go 0.00% <0.00%> (ø)
pkg/cmd/server/server.go 6.11% <9.83%> (-0.51%) ⬇️
pkg/controller/backup_controller.go 57.12% <57.03%> (+1.24%) ⬆️
pkg/backup/backup.go 79.12% <58.33%> (+0.24%) ⬆️
pkg/controller/restore_controller.go 67.56% <0.00%> (-1.45%) ⬇️
pkg/persistence/object_store.go 53.19% <0.00%> (-0.04%) ⬇️
pkg/util/results/result.go 100.00% <0.00%> (ø)
pkg/cmd/util/output/restore_describer.go 0.00% <0.00%> (ø)
pkg/restore/request.go 100.00% <0.00%> (ø)
... and 5 more

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

// By far, PodVolumeBackup, PodVolumeRestore, BackupStorageLocation controllers
// are not included in --disable-controllers list.
// This is because of PVB and PVR are used by node agent DaemonSet,
// and BSL controller is mandatory for Velero to work.
enabledControllers := map[string]func() controllerRunInfo{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

enabledControllers now are been removed

for _, newController := range enabledControllers {
controllers = append(controllers, newController())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not need anymore

// Adding the controllers to the manager will register them as a (runtime-controller) runnable,
// so the manager will ensure the cache is started and ready before all controller are started
s.mgr.Add(managercontroller.Runnable(controllerRunInfo.controller, controllerRunInfo.numWorkers))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not need anymore

pkg/cmd/server/server.go Show resolved Hide resolved
pkg/controller/generic_controller.go Show resolved Hide resolved
@reasonerjt reasonerjt removed their request for review February 28, 2023 09:02
@blackpiglet blackpiglet force-pushed the remove-client-go branch 4 times, most recently from 7112c06 to 02fa8c3 Compare March 1, 2023 12:28
@blackpiglet
Copy link
Contributor

blackpiglet commented Mar 2, 2023

This is the basic E2E test cases result. Only node-port case failed. Checked with Danfeng. Some cloud providers have the same error. This is not the Velero function defect.

https://gist.github.com/blackpiglet/2a2176ec5913b1bdf1937cdda421ab75

Signed-off-by: Ming <mqiu@vmware.com>
Signed-off-by: Ming <mqiu@vmware.com>
Signed-off-by: Xun Jiang <blackpiglet@gmail.com>
@blackpiglet blackpiglet closed this Mar 8, 2023
@qiuming-best qiuming-best deleted the remove-client-go branch March 10, 2023 01:25
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.

4 participants