fix: Close response to fix file descriptor leak #416
Merged
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 aws/karpenter-provider-aws#4296
Description
#4296 caught an issue where Karpenter was leaking file descriptors in its webhook checks. This can be observed from the following graph that shows the Karpenter
container_file_descriptors
metric that tracks the number of open file descriptors on a container.The root cause of this issue is that Golang's default HTTP client opens a connection and holds that connection open when receiving a valid response from the server, which causes a file descriptor to remain open until the connection is closed (some details described here).
This issue with not closing the response body was technically introduced in the following PR: #334 (introduced in Karpenter
v0.28.0
) but did not become an issue untilv0.29.1
since the response returned by the server used inv0.28.0-v0.29.0
did not send a response body. Adding a response body to the response sent from the server called out to in these version also results in a file descriptor leak.This change closes the response body and and ensures that the response body is read so that the http client can re-use the same connection from the connection pool instead of having to create another one. The file descriptor leak can seen to be resolved through the
container_file_descriptors
metric now:How was this change tested?
make presubmit
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.