Don't re-enqueue deletion if context is cancelled #548
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The for loop was only checking for context cancellation once, before going into an endless range on the event channel. Move the channel read into the select, so that the for loop actually runs more than once and checks for cancellation every iteration. We should also shut down the workqueue if the event channel is closed, as this is another indicator of some problem that should halt processing. On the processing side, don't reenqueue on context cancellation, as it will never succeed. Signed-off-by: Brad Davidson <[email protected]>
dereknola
approved these changes
Oct 28, 2025
DelayingInterface has been deprecated since k8s v1.31 Signed-off-by: Brad Davidson <[email protected]>
brandond
force-pushed
the
fix-ttl-context-cancel
branch
from
e96d35e to
f7636d9
Compare
dereknola
approved these changes
Oct 28, 2025
This was referenced Oct 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes error that has been reported on Slack a few times:
This happens when K3s or RKE2 use kine with TLS - kine is started up once without TLS to extract bootstrap data (the kine certs), and then stopped. Unfortunately the workqueue does not get the memo and keeps running with a cancelled context.
The for loop was only checking for context cancellation once, before going into an endless range on the event channel. Move the channel read into the select, so that the for loop actually runs more than once and checks for cancellation every iteration.
We should also shut down the workqueue if the event channel is closed, as this is another indicator of some problem that should halt processing.
On the processing side, don't reenqueue on context cancellation, as it will never succeed.
Also replaces
workqueue.DelayingInterfacewithworkqueue.TypedDelayingInterface[string]- the untyped version has been deprecated since 1.31, and the typed version saves us an assertion.