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

Source controller does not reuse TCP connections when fetching helm charts #578

Closed
pjbgf opened this issue Feb 12, 2022 · 1 comment · Fixed by #590
Closed

Source controller does not reuse TCP connections when fetching helm charts #578

pjbgf opened this issue Feb 12, 2022 · 1 comment · Fixed by #590
Assignees
Labels
area/helm Helm related issues and pull requests bug Something isn't working

Comments

@pjbgf
Copy link
Member

pjbgf commented Feb 12, 2022

TL;DR

Source controller does not reuse tcp connections when fetching helm charts.

Initial investigation

  • In a cluster using the latest flux version, with all sources suspended apart from 4 HelmCharts, yield multiple long running TCP connections.
  • The issue is not new to v0.21 (refer to Comparison v0.20 vs v0.21).
  • The problem seems to be upstream as a new transport is being created per request. As per official golang documentation:

Transports should be reused instead of created as needed.
Transports are safe for concurrent use by multiple goroutines.

Comparison v0.20 vs v0.21

IMAGE: ghcr.io/fluxcd/source-controller:v0.21.2

~ $ netstat | grep github | sort
tcp        0      0 source-controller-6b58c59ccb-qhr5l:39252 cdn-185-199-111-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:43994 lb-140-82-121-3-fra.github.com:https TIME_WAIT   
tcp        0      0 source-controller-6b58c59ccb-qhr5l:43998 lb-140-82-121-3-fra.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:44010 lb-140-82-121-3-fra.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:44016 lb-140-82-121-3-fra.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:45354 cdn-185-199-111-133.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:45360 cdn-185-199-111-133.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:46304 cdn-185-199-108-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:46308 cdn-185-199-108-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:47060 cdn-185-199-109-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-6b58c59ccb-qhr5l:48736 cdn-185-199-110-133.github.com:https ESTABLISHED
~ $ netstat | grep github | wc -l
11

IMAGE: ghcr.io/fluxcd/source-controller:v0.20.1

root@source-controller-654f787978-64mn8:/# netstat | grep 443 | grep -v kubernetes
tcp        0      0 source-controller:38110 lb-140-82-121-4-fra:443 ESTABLISHED
tcp        0      0 source-controller:49064 cdn-185-199-108-153:443 ESTABLISHED
tcp        0      0 source-controller:54490 cdn-185-199-111-133:443 ESTABLISHED
tcp        0      0 source-controller:59364 cdn-185-199-109-133:443 ESTABLISHED
tcp        0      0 source-controller:41602 cdn-185-199-111-153:443 ESTABLISHED
tcp        0      0 source-controller:49074 cdn-185-199-108-153:443 ESTABLISHED
tcp        0      0 source-controller:59346 cdn-185-199-109-133:443 ESTABLISHED
tcp        0      0 source-controller:38082 lb-140-82-121-4-fra:443 ESTABLISHED
tcp        0      0 source-controller:38102 lb-140-82-121-4-fra:443 ESTABLISHED
tcp        0      0 source-controller:49066 cdn-185-199-108-153:443 ESTABLISHED
tcp        0      0 source-controller:38088 lb-140-82-121-4-fra:443 ESTABLISHED
root@source-controller-654f787978-64mn8:/# netstat | grep 443 | grep -v kubernetes | wc -l
11

Note that the netstat behaves slightly different between debian (v0.20) and alpine (v0.21), which reflects on the output of the command.

Initial Fix Attempt

By adding WithTransport upstream and using it in source-controller the connections created becomes one per HelmChart:

/ # netstat | grep github | sort
tcp        0      0 source-controller-84b7dc968-s9lwx:38106 cdn-185-199-108-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-84b7dc968-s9lwx:38110 cdn-185-199-108-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-84b7dc968-s9lwx:48376 lb-140-82-121-3-fra.github.com:https TIME_WAIT   
tcp        0      0 source-controller-84b7dc968-s9lwx:53616 cdn-185-199-110-153.github.com:https ESTABLISHED 
tcp        0      0 source-controller-84b7dc968-s9lwx:56430 cdn-185-199-108-133.github.com:https ESTABLISHED 
/ # netstat | grep github | wc -l
5
@makkes makkes added area/helm Helm related issues and pull requests bug Something isn't working labels Feb 16, 2022
@pjbgf
Copy link
Member Author

pjbgf commented Feb 16, 2022

Upstream PR fix: helm/helm#10568

hiddeco added a commit that referenced this issue Feb 23, 2022
This commit updates to a version of Helm 3.8.0, with patches applied to
deal with memory leak and HTTP transport issues. The latter being
described in #578.

Signed-off-by: Hidde Beydals <[email protected]>
pjbgf pushed a commit to pjbgf/source-controller that referenced this issue Feb 24, 2022
This commit updates to a version of Helm 3.8.0, with patches applied to
deal with memory leak and HTTP transport issues. The latter being
described in fluxcd#578.

Signed-off-by: Hidde Beydals <[email protected]>
pjbgf pushed a commit that referenced this issue Feb 25, 2022
This commit updates to a version of Helm 3.8.0, with patches applied to
deal with memory leak and HTTP transport issues. The latter being
described in #578.

Signed-off-by: Hidde Beydals <[email protected]>
pjbgf pushed a commit to pjbgf/source-controller that referenced this issue Feb 28, 2022
This commit updates to a version of Helm 3.8.0, with patches applied to
deal with memory leak and HTTP transport issues. The latter being
described in fluxcd#578.

Signed-off-by: Hidde Beydals <[email protected]>
pjbgf pushed a commit to pjbgf/source-controller that referenced this issue Mar 2, 2022
This commit updates to a version of Helm 3.8.0, with patches applied to
deal with memory leak and HTTP transport issues. The latter being
described in fluxcd#578.

Signed-off-by: Hidde Beydals <[email protected]>
@pjbgf pjbgf closed this as completed in #590 Mar 2, 2022
@pjbgf pjbgf self-assigned this Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants