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

Thin plugin: don't wait too long for an answer from API Server #1372

Merged

Conversation

pmtk
Copy link
Contributor

@pmtk pmtk commented Dec 19, 2024

If Multus plugin gets a DEL request, but the API Server is down (e.g. via 'crictl rmp'), the call takes so long, it actually never finishes. This prevents CRI-O from deleting the Pods.

Proposed fix sets a timeout for Pods().Get() using context.

If Multus plugin gets a DEL request, but the API Server is down (e.g.
via 'crictl rmp'), the call takes so long, it actually never finishes.
This prevents CRI-O from deleting the Pods.
@pmtk pmtk force-pushed the dont-wait-too-long-for-apiserver branch from 1f49553 to 4ff141c Compare December 19, 2024 15:13
@coveralls
Copy link

Coverage Status

coverage: 56.585% (+0.1%) from 56.454%
when pulling 4ff141c on pmtk:dont-wait-too-long-for-apiserver
into 4fc16b3 on k8snetworkplumbingwg:master.

@pmtk pmtk changed the title Thick plugin: don't wait too long for an answer from API Server Thin plugin: don't wait too long for an answer from API Server Dec 19, 2024
Copy link
Member

@dougbtv dougbtv left a comment

Choose a reason for hiding this comment

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

nice approach with the context timeout. Potentially this can be tuned further on DEL and skip API requests where they're not necessary, but this is really clean and still efficient. Thanks

@dougbtv dougbtv merged commit 5338017 into k8snetworkplumbingwg:master Dec 19, 2024
24 checks passed
@pmtk pmtk deleted the dont-wait-too-long-for-apiserver branch December 19, 2024 16:29
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.

3 participants