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

Refactor BSL controller with periodical enqueue source #4894

Merged
merged 1 commit into from
May 11, 2022

Conversation

blackpiglet
Copy link
Contributor

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #4463

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
Copy link
Contributor Author

Controller runtime cannot only use Watches function to setup controller. Controller runtime always require a For function.
https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/builder/controller.go#L185-L188

As a result, add BSL in the For function, but even all events of BSL are ignored, there is always some unexpected reconcile running. So need to verify whether need to verify the BSL in the reconcile, instead of only relying on periodical enqueue source.

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2022

Codecov Report

Merging #4894 (989a1e3) into main (8411c73) will increase coverage by 0.53%.
The diff coverage is 55.67%.

@@            Coverage Diff             @@
##             main    #4894      +/-   ##
==========================================
+ Coverage   41.09%   41.63%   +0.53%     
==========================================
  Files         204      204              
  Lines       18064    18193     +129     
==========================================
+ Hits         7424     7574     +150     
+ Misses      10093    10044      -49     
- Partials      547      575      +28     
Impacted Files Coverage Δ
...g/controller/backup_storage_location_controller.go 57.14% <49.41%> (-23.04%) ⬇️
pkg/util/kube/periodical_enqueue_source.go 72.72% <100.00%> (+5.15%) ⬆️
pkg/controller/gc_controller.go 76.31% <0.00%> (-1.87%) ⬇️
pkg/restore/restore.go 65.35% <0.00%> (-1.35%) ⬇️
pkg/util/kube/utils.go 74.28% <0.00%> (-1.09%) ⬇️
pkg/controller/pod_volume_backup_controller.go 44.14% <0.00%> (-0.78%) ⬇️
pkg/install/resources.go 52.55% <0.00%> (-0.25%) ⬇️
pkg/builder/restore_builder.go 0.00% <0.00%> (ø)
pkg/cmd/util/output/restore_describer.go 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8411c73...989a1e3. Read the comment docs.

@blackpiglet blackpiglet force-pushed the bsl-refactor branch 3 times, most recently from 3c0536a to c8563cc Compare May 7, 2022 15:27
@blackpiglet blackpiglet force-pushed the bsl-refactor branch 4 times, most recently from 284509e to af1f9ee Compare May 10, 2022 03:50
ywk253100
ywk253100 previously approved these changes May 10, 2022
Add filter functions for PeriodicalEnqueueSource.
Move BSL's valication frequency check test case to PeriodicalEnqueueSource's test.

Signed-off-by: Xun Jiang <jxun@vmware.com>
@ywk253100 ywk253100 merged commit 879d033 into vmware-tanzu:main May 11, 2022
@blackpiglet blackpiglet deleted the bsl-refactor branch October 15, 2022 04:04
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.

Backup Storage Location controller may reconcile one item multiple times in a single period
4 participants