Skip to content

Conversation

@haircommander
Copy link
Contributor

@haircommander haircommander commented Jan 9, 2026

As well as add a test to prove StopUnit is reentrant

Alternative to kubernetes/kubernetes#135826
should fix kubernetes/kubernetes#135825 (need to do some testing to double check)
credit to @rphillips for the idea and original piece of code

@rphillips
Copy link
Member

lgtm

@haircommander
Copy link
Contributor Author

@kolyshkin PTAL

haircommander added a commit to haircommander/kubernetes that referenced this pull request Jan 9, 2026
Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

Left a few comments.

Also, if you can briefly describe the whole issue in the commit message that'd be awesome (in the ideal world I want to know why we're changing the code just by looking at git log or git show).

@haircommander
Copy link
Contributor Author

updated as suggested!

Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99 PTAL

@kolyshkin kolyshkin requested a review from Luap99 January 20, 2026 04:31
Copy link
Collaborator

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

As well as add a test to prove StopUnit is reentrant

There are cases that multiple StopUnits may be called
for the same job. For instance, in kubernetes, the Kubelet does so in some cases,
when it races against itself to delete a pod. Instead of fixing in the caller, this
can be worked around in this library. Instead of losing track of an old job if a new
StopUnit is called for the same job, we can just track multiple jobs, and signal to
multiple channels when the job finishes.

see kubernetes/kubernetes#135826 and kubernetes/kubernetes#135825
for some more context

Signed-off-by: Peter Hunt <[email protected]>
@Luap99 Luap99 enabled auto-merge (rebase) January 20, 2026 12:00
@Luap99 Luap99 merged commit abb50b3 into coreos:main Jan 20, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kubelet SyncTerminatedPod times out after 30s due to race with cleanupOrphanedPodCgroups

4 participants