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

Tiller causing Kubelet to leak memory #3121

Open
mikelorant opened this Issue Nov 9, 2017 · 39 comments

Comments

Projects
None yet
@mikelorant
Copy link

mikelorant commented Nov 9, 2017

The node that is running Tiller causes the Kubelet to leak memory. Depending on how frequently Tiller is polled by the Helm client, influences how much memory is leaked.

This has been verified on 3 different clusters running Kubernetes 1.6.8. Talking to other users, we have identified this problem also existing with 1.5.x, 1.7.x and 1.8.x clusters.

On a cluster where Helm commands were being issued 20 times per minute (over 100 releases), in a period of a day we caused the Kubelet to consume over 70GB of memory.

PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND
13268 root      20   0 78.295g 4.521g  48336 S  24.6 30.3  12:53.83 kubelet

Further investigation identified an extremely large number of socat processes running on the node (over 1000). Here is a sample of the process list.

root     27500  0.0  0.0  21740  2976 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134
root     27501  0.0  0.0  21740  3004 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134
root     27502  0.0  0.0  21740  3004 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134
root     27503  0.0  0.0  21740  2920 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134
root     27504  0.0  0.0  21740  2812 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134
root     27505  0.0  0.0  21740  2976 ?        S    07:52   0:00 /usr/bin/socat - TCP4:localhost:44134

I reached out to @justinsb who verified this on a 1.8.x cluster on a fresh installation of Helm.

Justin found the likely cause of the problem being the following block of code (and comment explaining the edge case).
https://github.com/kubernetes/kubernetes/blame/master/pkg/kubelet/dockershim/docker_streaming.go#L191-L192

// If we use Stdin, command.Run() won't return until the goroutine that's copying
// from stream finishes. Unfortunately, if you have a client like telnet connected
// via port forwarding, as long as the user's telnet client is connected to the user's
// local listener that port forwarding sets up, the telnet session never exits. This
// means that even if socat has finished running, command.Run() won't ever return
// (because the client still has the connection and stream open).
//
// The work around is to use StdinPipe(), as Wait() (called by Run()) closes the pipe
// when the command (socat) exits.

This code was added from the following PR:
kubernetes/kubernetes#12283

Which was designed to fix:
kubernetes/kubernetes#8766

Further discussion about this issue:
https://groups.google.com/forum/#!topic/grpc-io/69k_6HKVai0

It seems likely there are 2 issues.

  1. Tiller is keeping its connections open.
  2. Kubelet is not handling bad actors who don't close their connection.

kubectl logs and kubectl exec use this capability but are clearly closing the connection which is why we do not see this problem for every cluster. Only large scale Helm users would likely causing enough queries to make this a noticeable problem.

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 11, 2017

@mikelorant Thank you for the thorough troubleshooting and digging into this. We'll take a look at it

@thomastaylor312 thomastaylor312 added the bug label Nov 11, 2017

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 11, 2017

So I did a little bit of digging and based on the linked discussion list, it looks like our gRPC version (1.2.1) might not have the server-side connection management (see grpc/grpc-go#1119), so Tiller might be holding on to the PortForward connection from helm. Haven't validated it for sure, but that may help

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 11, 2017

As a note, we are closing the PortForward from the helm side based on what I was seeing in the code

thomastaylor312 added a commit to thomastaylor312/helm that referenced this issue Nov 11, 2017

fix(tiller): Forces close of idle gRPC connections
Possibly fixes helm#3121. This forces idle connections to drop after 10 minutes
@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 11, 2017

@mikelorant Any way you could give #3129 a try and see if it works? I have no way to test my hypothesis I talked about above because I don't have a cluster readily available (not sure if minikube will do it). Just to note, that PR is just an educated guess as to what may fix it

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 12, 2017

I'll try and get this implemented in our testing cluster over the next few days. Is it just a case of using the docker image for the master branch of Tiller?

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 13, 2017

I can build one for you @mikelorant if you haven't built it before and don't want to mess with it

@SlickNik

This comment has been minimized.

Copy link
Collaborator

SlickNik commented Nov 14, 2017

@mikelorant @thomastaylor312 I was able to repro the issue on one of our Kubernetes clusters that is running Helm. Hopefully I should have some time to build and test out @thomastaylor312's fix either this afternoon or tomorrow. Will update the issue once I've had a chance to do that. Thanks!

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 15, 2017

If someone has an image I an pull, post the link and I'll update our dev cluster to use it today.

@SlickNik

This comment has been minimized.

Copy link
Collaborator

SlickNik commented Nov 15, 2017

I've built a tiller image from @thomastaylor312's fix (PR #3129) and pushed it up to slicknik/tiller:git-a26100a on dockerhub. I haven't had a chance to verify it as yet. @mikelorant feel free to pull that down and verify it on your dev cluster when you get a chance. Thanks so much for your help with this!

@bodepd

This comment has been minimized.

Copy link
Contributor

bodepd commented Nov 16, 2017

@thomastaylor312 - we've been experiencing a lot of instability lately as we've given tiller more load and we're open to test this out to see if it's an improvement.

@bacongobbler

This comment has been minimized.

Copy link
Member

bacongobbler commented Nov 16, 2017

Should be able to test with helm init --tiller-image slicknik/tiller:git-a26100a. :)

@SlickNik

This comment has been minimized.

Copy link
Collaborator

SlickNik commented Nov 16, 2017

@thomastaylor312 @mikelorant I was able to verify that #3129 fixes the issue. After deploying a tiller built with that fix (slicknik/tiller:git-a26100a) I ran a continuous load that installed and deleted a bunch of charts, and didn't see any lingering socat processes after. (Running this on a tiller without the fix did leave behind about 40-50 socat processes, when I ran it yesterday). Based on this, the fix gets my 👍. Thanks!

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 16, 2017

@SlickNik Thanks for checking into this and building an image! I'll update the PR and wait for at least one more person to verify (ping @bodepd and @mikelorant)

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 16, 2017

Before swapping out tiller with slicknik/tiller:git-a26100a

 1693 root      20   0 15.853g 432920  16852 S  17.6  1.4   1646:24 kubelet

I'll report back in about 6-7 hours with the memory use.

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 17, 2017

Results after 24 hours.

 1694 root      20   0 15.869g 369140  19892 S   9.6  1.2 159:28.68 kubelet

@michelleN michelleN added this to the Upcoming - Minor milestone Nov 17, 2017

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 17, 2017

I've just restarted the kubelet and am running fresh checks again. I'll see where I am at after 24 hours again.

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 19, 2017

After 24 hours...

16044 root      20   0 1224916 138592  50444 S   6.4  0.4  99:38.70 kubelet

Since its the weekend there is the possibility with no developers doing releases Helm hasn't had much access. Tomorrow, once they are back at work we will have a conclusive answer.

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 20, 2017

16044 root      20   0 4399860 188748  50604 S   4.3  0.6 236:27.45 kubelet

Grown to 4.3GB in 2 days. I don't really have a conclusive answer just yet. Do we think this is reasonable?

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Nov 21, 2017

@mikelorant It seems to be better (if I am reading things correctly), but not all the way. Is it enough of a difference to justify merging in the fix?

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 21, 2017

Has now jumped up to 8GB of ram. I don't feel the issue is fixed.

16044 root      20   0 8704916 292888  47884 S   6.4  0.9 382:09.35 kubelet
@SlickNik

This comment has been minimized.

Copy link
Collaborator

SlickNik commented Nov 21, 2017

@mikelorant Can you please also verify if you're still seeing the large number of socat processes running as child processes of kubelet even after the fix? Thanks!

@mikelorant

This comment has been minimized.

Copy link

mikelorant commented Nov 21, 2017

Here is some comparison info of other nodes.

1693  root      20   0 15.855g 367412  21332 S   6.6  1.2   2914:20 kubelet
1657  root      20   0  497344 104288  54624 S   6.6  0.7 390:10.89 kubelet
1678  root      20   0  556260 103728  54116 S   0.0  0.7 358:15.96 kubelet
1681  root      20   0  564128 103500  53928 S   0.0  0.7 375:13.98 kubelet
1694  root      20   0  676584 103468  17572 S   0.0  0.3 372:56.74 kubelet
1699  root      20   0  702748 106812  17872 S   6.7  0.3 370:51.03 kubelet
1733  root      20   0  743696 108224  18560 S   6.6  0.3 385:10.96 kubelet
1649  root      20   0  802828 106096  18788 S  13.3  0.3 358:19.33 kubelet
1684  root      20   0  817372 106044  19880 S   6.7  0.3 382:16.65 kubelet
1624  root      20   0  828220 107368  19052 S   0.0  0.3 372:00.08 kubelet
1651  root      20   0  828452  99828  16952 S   0.0  0.3 272:51.84 kubelet
1653  root      20   0  845568 103700  19320 S   6.7  0.3 382:58.71 kubelet
1697  root      20   0  870676 103412  18728 S   0.0  0.3 392:50.11 kubelet
1639  root      20   0  920972 107268  19708 S   0.0  0.3 394:12.33 kubelet
1713  root      20   0 1390652 170440  17116 S   0.0  0.5   2229:09 kubelet
1682  root      20   0 1438164 135140  18792 S   6.6  0.4   1512:20 kubelet
1686  root      20   0 1447508 141700  16668 S  13.3  0.5   1711:27 kubelet
3478  root      20   0 1746376 150836  18504 S   0.0  0.5 491:20.18 kubelet
16409 root      20   0 2588248 169276  19496 S   0.0  0.5   1043:24 kubelet
1686  root      20   0 2630072 152084  24736 S   0.0  0.5   1502:34 kubelet
16044 root      20   0 8705172 526084  47680 S   0.0  1.7 395:01.25 kubelet

1693 and 16044 are where Tiller has run previously.

@egeland

This comment has been minimized.

Copy link

egeland commented Nov 21, 2017

Maybe the fix that's in is partially successful - it sounds like it's dealt with the socat processes part of the problem, but there's still some leaks from other, similar things?

@SlickNik

This comment has been minimized.

Copy link
Collaborator

SlickNik commented Nov 21, 2017

@egeland Yes, that was exactly my line of reasoning -- it's possible that the fix addresses only part of the issue causing the memory leak.

thomastaylor312 added a commit to thomastaylor312/helm that referenced this issue Nov 29, 2017

fix(tiller): Forces close of idle gRPC connections
Possibly fixes helm#3121. This forces idle connections to drop after 10 minutes
@bacongobbler

This comment has been minimized.

Copy link
Member

bacongobbler commented Nov 29, 2017

Re-opening; this was automatically closed through GitHub interpreting “partially fixes” as “fixes” :P

@MattiasGees

This comment has been minimized.

Copy link

MattiasGees commented Dec 19, 2017

Hello We are having the same problem on one of our clusters. I now deployed the tiller with slicknik/tiller:git-a26100a and I see that connections are killed in a timely matter. The kubelet still has a big memory buildup. In our case Kubelet uses 2.5GB on a node with 3.9GB. I am happy to test any other fixes, provide any information you need or help in any other way I can.
My knowledge about the internals of Helm is quite limited atm but I am happy to learn more about it.

@technosophos

This comment has been minimized.

Copy link
Member

technosophos commented Dec 20, 2017

This is a summary of some of our recent investigation on this, and some conversations at KubeCon:

To the best we can determine, this is caused by the kubelet itself leaking memory on port forwarding requests. It should be reproducible by merely creating and destroying a few thousand port-forwards with kubectl. But it does appear to be beyond our ability to fix in Tiller code, since it's a bug in the kubelet.

If anyone can reproduce with just kubectl, we should open an upstream bug.

MattiasGees added a commit to skyscrapers/terraform-kubernetes that referenced this issue Dec 21, 2017

Fix for HELM so it can't impact other nodes/pods
HELM has a bug that makes kubelet use a lot of memory helm/helm#3121
To mitigate this until it is solved, we will run HELM on a seperate node so it can't affect other nodes/pods

MattiasGees added a commit to skyscrapers/terraform-kubernetes that referenced this issue Dec 21, 2017

Fix for HELM so it can't impact other nodes/pods (#33)
HELM has a bug that makes kubelet use a lot of memory helm/helm#3121
To mitigate this until it is solved, we will run HELM on a seperate node so it can't affect other nodes/pods
@SamClinckspoor

This comment has been minimized.

Copy link

SamClinckspoor commented Jan 8, 2018

using the following script, I was able to observe a memory increase of the kubelet:

for i in {8000..8300}; do
kubectl port-forward anypod $i:8080 &; sleep 0.2;
done

...
killall kubectl

Can someone verify this as well?

@mattfarina

This comment has been minimized.

Copy link
Collaborator

mattfarina commented Jan 8, 2018

I did a little looking in the kubernetes issues and PRs. I couldn't find an issue on a memory leak for port forwarding (either open or closed). If this is the case can someone with details file an issue over there.

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

thomastaylor312 commented Jan 9, 2018

@SamClinckspoor I think multiple people have seen the same behavior with a similar command, so I think it is a port forwarding issue. Do you think you can file an issue against kubernetes with the details from your last comment? (cc @mikelorant as well since he has been deep in this before and might be able to help)

@SamClinckspoor

This comment has been minimized.

Copy link

SamClinckspoor commented Jan 9, 2018

created kubernetes/kubernetes#57992 for the port-forwarding memory leak.

@thenano

This comment has been minimized.

Copy link

thenano commented Mar 26, 2018

Hi, I know this is a kubernetes issue, wanted to ask here as this mostly makes tiller unusable in small instances. Is there a know work around or any news if kubernets has/will fix this, or if it's no longer an issue with 1.9 or newer versions of tiller?

@IBMRob

This comment has been minimized.

Copy link

IBMRob commented Mar 26, 2018

We are also hitting this problem in 1.9. On a 16GB node hypercube is up to ~9.7-10GB

@SamClinckspoor

This comment has been minimized.

Copy link

SamClinckspoor commented Mar 26, 2018

To mitigate this we schedule tiller on small dedicated node using taints and tolerations. That node also restarts kubelet every 10 minutes.

Not very pretty but it has worked so far. Interested if anyone has a more graceful workaround.

@thenano

This comment has been minimized.

Copy link

thenano commented Mar 26, 2018

@SamClinckspoor do you deliberately restart kubelet or does it just keep restarting it?
We've also isolated Tiller into it's own node, we tried putting it into an aws t2.nano but it died every 5 minutes and took a long time to come back, any special configs (memory requests/limits) you are using?

@SamClinckspoor

This comment has been minimized.

Copy link

SamClinckspoor commented Mar 27, 2018

we use t2.small I believe. No other config except for the toleration to run on the tiller node.
We restart it deliberately:
restart-kubelet.service

[Unit]
Description=restart-kubelet service

[Service]
Type=oneshot

ExecStart=-/bin/systemctl restart kubelet.service

[Install]
WantedBy=multi-user.target

restart-kubelet.timer

[Unit]
Description=restart-kubelet service timer

[Timer]
OnBootSec=2min
OnUnitActiveSec=10min

[Install]
WantedBy=timers.target

@k8s-ci-robot k8s-ci-robot added bug and removed bug labels Jun 5, 2018

@frankh

This comment has been minimized.

Copy link
Contributor

frankh commented Jun 8, 2018

Just wanted to confirm this is happening to me on k8s 1.10.3 with tiller 2.9.1

Very frustrating, I only hit the issue after I created a service which runs "helm status" a bunch of times - which causes kubelet to use 4+gb of memory in about 15mins

@bacongobbler

This comment has been minimized.

Copy link
Member

bacongobbler commented Jun 8, 2018

kubernetes/kubernetes#57992 is the ticket if anyone feels like tackling this issue upstream.

@bacongobbler bacongobbler removed this from the Upcoming - Minor milestone Jun 8, 2018

@moshe0076

This comment has been minimized.

Copy link

moshe0076 commented Jun 17, 2018

I noticed that if you delete the tiller pod the memory is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment