Skip to content

Periodic Reconciliation - Able to Set Max Interval Between Reconciliations #848

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

Closed
csviri opened this issue Jan 19, 2022 · 4 comments · Fixed by #871
Closed

Periodic Reconciliation - Able to Set Max Interval Between Reconciliations #848

csviri opened this issue Jan 19, 2022 · 4 comments · Fixed by #871
Assignees
Labels

Comments

@csviri
Copy link
Collaborator

csviri commented Jan 19, 2022

In some cases, like when managing external resources, or just having a simple controller that we want to periodically reschedule it would be handy to be able to define that "I want to have maximum delay [X time] between reconciliations".
This would be much better than fixed rate, since that can easily lead to very close executions after each other, what is not the intention in these use cases.

One way that this is already doable is to make sure that the UpdateControl returns with UpdateControl.<MyCustomResource>noUpdate().rescheduleAfter(8, TimeUnit.HOURS);. Where it would return just noUpdate() - (or alternaives like updateStatus() )

But this is not that nice, since basically puts kinda noise in the code.

This could be also a new annoation @PeriodicReconciliation(maxDelay=8, timeUnit=HOURS) on the reconciler
or a property of @ControllerConfuguration(periodicReconiliationMaxDelay=8, periodicReconiliationTimeUnit = HOURS )

please let me know what do you think.

Implementation notes

Implementation wise regarding the execution, it is very simple to do, we just always put a reschedule if there is missing from the UpdateControl.

@csviri csviri self-assigned this Jan 19, 2022
@csviri csviri added the feature label Jan 19, 2022
@andreaTP
Copy link
Collaborator

I think that @ControllerConfuguration(periodicReconiliationMaxDelay=8 ... with sensible defaults works perfectly for this use-case.

@metacosm
Copy link
Collaborator

I'd favor a configuration option as well. It makes more sense that way than having to remember to call a reschedule on UpdateControl

@csviri
Copy link
Collaborator Author

csviri commented Jan 19, 2022

see also: https://kubernetes.slack.com/archives/CAW0GV7A5/p1642594910160600

For the record, I think discussed before, if you have informers in place and correct code, this is not needed. It's really a fail safe.

@sclorng
Copy link

sclorng commented Jan 19, 2022

That would help for testing. Actually, and especialy true on quarkus side, tests are more integration tests than unit testing the Reconciler. Then, it is not easy to test if a reschedule has been set.

Informers can help detecting change but for time based behaviour, I guess we still need to use either TimedEventSource or reschedule.

@csviri csviri linked a pull request Jan 26, 2022 that will close this issue
@csviri csviri changed the title Periodic Reconciliation - Able to Set Max Delay Between Reconciliations Periodic Reconciliation - Able to Set Max Interval Between Reconciliations Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants