Use predicate in finalizer controllers to only process update events. #7941
Conversation
|
/kind changelog-not-required |
kaovilai
force-pushed
the
finalizerControllerPredicates
branch
from
9261b6f
to
9307517
Compare
github-actions
bot
added
the
kind/changelog-not-required
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7941 +/- ##
==========================================
- Coverage 58.79% 58.79% -0.01%
==========================================
Files 345 346 +1
Lines 28766 28917 +151
==========================================
+ Hits 16914 17002 +88
- Misses 10423 10483 +60
- Partials 1429 1432 +3 ☔ View full report in Codecov by Sentry. |
| if !ok { | ||
| return false | ||
| } | ||
| return backup.Status.Phase == velerov1api.BackupPhaseFinalizing || backup.Status.Phase == velerov1api.BackupPhaseFinalizingPartiallyFailed |
There was a problem hiding this comment.
We should return the opposite result here.
Restore reconciler should also be changed.
There was a problem hiding this comment.
// Update returns true if the Update event should be processed
UpdateFunc func(event.UpdateEvent) boolDon't we want events to be processed if CR was updated to finalizing?
There was a problem hiding this comment.
@kaovilai so there are 2 cases where we want to process backups in this controller:
- backups updated to Finalizing or FinalizingPartiallyFailed phase
- backups already in finalizing phase that returned err from prior reconcile (i.e. requeueing)
Does this predicate change affect requeue behavior?
There was a problem hiding this comment.
But yes, for the predicate, it looks like we want to return true for backups in these two phases.
| if !ok { | ||
| return false | ||
| } | ||
| return backup.Status.Phase == velerov1api.BackupPhaseFinalizing || backup.Status.Phase == velerov1api.BackupPhaseFinalizingPartiallyFailed |
There was a problem hiding this comment.
@kaovilai so there are 2 cases where we want to process backups in this controller:
- backups updated to Finalizing or FinalizingPartiallyFailed phase
- backups already in finalizing phase that returned err from prior reconcile (i.e. requeueing)
Does this predicate change affect requeue behavior?
| if !ok { | ||
| return false | ||
| } | ||
| return backup.Status.Phase == velerov1api.BackupPhaseFinalizing || backup.Status.Phase == velerov1api.BackupPhaseFinalizingPartiallyFailed |
There was a problem hiding this comment.
But yes, for the predicate, it looks like we want to return true for backups in these two phases.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
kaovilai
force-pushed
the
finalizerControllerPredicates
branch
from
9307517
to
e62d4a8
Compare
Signed-off-by: Tiger Kaovilai tkaovila@redhat.com
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #7868
Please indicate you've done the following:
/kind changelog-not-requiredas a comment on this pull request.site/content/docs/main./kind changelog-not-required