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

minor fixes for backup_operations_controller: #5996

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

sseago
Copy link
Collaborator

@sseago sseago commented Mar 18, 2023

  1. default frequency 10s
  2. per-reconcile log is now Debug not info
  3. added predicate to reduce reconcile events

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • [x ] Accepted the DCO. Commits without the DCO will delay acceptance.
  • [x ] 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.

@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2023

Codecov Report

Merging #5996 (5169981) into main (5505110) will decrease coverage by 0.04%.
The diff coverage is 14.28%.

❗ Current head 5169981 differs from pull request most recent head 1b3be3b. Consider uploading reports for the commit 1b3be3b to get more accurate results

@@            Coverage Diff             @@
##             main    #5996      +/-   ##
==========================================
- Coverage   40.35%   40.32%   -0.04%     
==========================================
  Files         252      252              
  Lines       23213    23217       +4     
==========================================
- Hits         9368     9362       -6     
- Misses      13133    13143      +10     
  Partials      712      712              
Impacted Files Coverage Δ
pkg/controller/backup_operations_controller.go 50.80% <14.28%> (-1.24%) ⬇️

... and 1 file with indirect coverage changes

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

@@ -82,9 +82,14 @@ func NewBackupOperationsReconciler(

func (c *backupOperationsReconciler) SetupWithManager(mgr ctrl.Manager) error {
s := kube.NewPeriodicalEnqueueSource(c.logger, mgr.GetClient(), &velerov1api.BackupList{}, c.frequency, kube.PeriodicalEnqueueSourceOption{})
gp := kube.NewGenericEventPredicate(func(object client.Object) bool {
restore := object.(*velerov1api.Restore)
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be backup := object.(*velerov1api.Backup) because we want to filter backup in backupOperationsReconciler

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Lyndon-Li oops. Sorry, copy-and-paste error from the similar fix on the restore side. I just noticed the e2e test failures which are probably related to this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@sseago sseago force-pushed the biav2-second-followon branch 2 times, most recently from b3b537e to c55de52 Compare March 20, 2023 13:03
1) default frequency 10s
2) per-reconcile log is now Debug not info
3) added predicate to reduce reconcile events

Signed-off-by: Scott Seago <[email protected]>
@sseago sseago force-pushed the biav2-second-followon branch from c55de52 to e500e2d Compare March 20, 2023 13:44
@shubham-pampattiwar shubham-pampattiwar merged commit 2c26c1d into vmware-tanzu:main Mar 20, 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.

4 participants