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

Revert "Migrate backup sync controller from code-generator to kubebui… #4457

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Dec 17, 2021

…lder (#4423)"

This reverts commit 5aaeb3e.

Thank you for contributing to Velero!

Please add a summary of your change

This commit is used to revert #4423.
After #4423 is merged, we found backup sync controller is not expected to work as a list-watch way. Before refactor, backup sync controller actually works as an time-routine job, which is implemented by velero generic controller as an goroutine sleeps by the resyncPeriod then runs the resyncFunc.
The refactor code uses Reconcile to watch Backup CR as event trigger to backup sync.
Actually, the backup sync task should work, but the sync process will be trigger every time a backup has some change. So there is behavior difference.

In the future, I think the time-base sync process can be replaced by Reconcile way. The controller can sync just the changed backup in the Reconcile function, instead of whole backups in cluster. This is a more Kubernetes or event-driven way.

The advantages are:

  • break down the batch sync task into small ones. If there is a lot of backups in cluster, this way has better performance.
  • More timely backup sync. After the backup change event occurs, the sync is triggered, rather than to wait for 30s sync period.

But due to the coming FC date, revert first, then doing the job in future release is more reasonable.

Does your change fix a particular issue?

Fixes #(issue)

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 Dec 17, 2021
@blackpiglet blackpiglet added kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes and removed has-changelog labels Dec 17, 2021
@blackpiglet blackpiglet force-pushed the revert-4423-backup-sync-controller-to-kubebuilder branch from 91c29c1 to 569aad2 Compare December 17, 2021 07:41
@blackpiglet blackpiglet force-pushed the revert-4423-backup-sync-controller-to-kubebuilder branch from 569aad2 to c32542f Compare December 17, 2021 08:04
Signed-off-by: Xun Jiang <jxun@vmware.com>
@blackpiglet blackpiglet force-pushed the revert-4423-backup-sync-controller-to-kubebuilder branch from c32542f to bdde758 Compare December 17, 2021 08:16
@ywk253100 ywk253100 merged commit d627362 into vmware-tanzu:main Dec 20, 2021
@blackpiglet blackpiglet deleted the revert-4423-backup-sync-controller-to-kubebuilder branch December 20, 2021 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-changelog has-unit-tests kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants