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

Require a finalizer for timers — same as for daemons #548

Merged
merged 1 commit into from
Sep 12, 2020

Conversation

nolar
Copy link
Owner

@nolar nolar commented Sep 12, 2020

What do these changes do?

Fix a bug when the timer never stops after the resource is deleted.

Description

Thorough unit-testing only covered the daemons and daemon exiting routine/ The assumption was, since timers are built-in daemons with a short handler, they will behave the same. The assumption is still valid, but the timers were defined not exactly as daemons: they didn't require a finalizer, while the daemons did.

The finalizer is supposed to slow down the deletion until the framework confirms that all the cleanups are done by removing the finalizer.

The absence of the finalizer caused an issue, where the deletion of an object was instantly fast and went unnoticed for the timer, so the timer was not stopped and continued to run.

The proper procedure is: the metadata.deletionTimestamp is set, the framework notices it, initiates the daemons/timers stopping, removes the finalizer, and the object actually disappears, and the "DELETED" event is sent. The actual procedure was: the object just disappeared, and the "DELETED" event was sent — but it was too late to react.

Predictably, in the presence of other daemons (not timers), or the deletion handlers, the timers were stopping properly — as the finalizer was put on the object as expected.

Issues/PRs

Issues: fixes #390

Related: introduced in #330, #342

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

@nolar nolar added the bug Something isn't working label Sep 12, 2020
@nolar nolar merged commit 32265c3 into master Sep 12, 2020
@nolar nolar deleted the timers-never-end branch September 12, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@kopf.timer continues to run after CR is deleted
1 participant