Introduce DockerCache in Kubelet. #4592
Conversation
|
cc @vmarmol |
wojtek-t
force-pushed
the
docker_cache
branch
from
39ab575 to
4fb8cab
Compare
| @@ -432,6 +433,10 @@ func (kl *Kubelet) Run(updates <-chan PodUpdate) { | |||
| if kl.dockerPuller == nil { | |||
| kl.dockerPuller = dockertools.NewDockerPuller(kl.dockerClient, kl.pullQPS, kl.pullBurst) | |||
| } | |||
| if kl.dockerCache == nil { | |||
There was a problem hiding this comment.
We should do this in NewMainKubelet. It means some of the non Main initializations may need it. Those are all in tests.
|
The Shipable build is timing out too for some reason. Any ideas? |
|
Re shippable - I don't know what's going on there - what is strange all tests are passing with 1.4 go. I will look deeper into it tomorrow (but just curious - did you have have such situation that tests are passing with 1.4 go and timing out/failing with 1.3)? |
|
Not sure about the build. Shippable is rather new to us so I haven't seen many cases yet. |
wojtek-t
force-pushed
the
docker_cache
branch
from
4fb8cab to
53c2519
Compare
|
I've updated the PR addressing the comments. PTAL |
wojtek-t
force-pushed
the
docker_cache
branch
from
53c2519 to
b243b4b
Compare
| @@ -691,3 +693,63 @@ type ContainerCommandRunner interface { | |||
| RunInContainer(containerID string, cmd []string) ([]byte, error) | |||
| GetDockerServerVersion() ([]uint, error) | |||
| } | |||
|
|
|||
| type DockerCache interface { | |||
There was a problem hiding this comment.
Can we move this to a docker_cache.go file?
wojtek-t
force-pushed
the
docker_cache
branch
from
3b6849e to
6d17346
Compare
|
I've applied your comments - PTAL. |
| ) | ||
|
|
||
| type DockerCache interface { | ||
| KubeletRunningDockerContainers() (DockerContainers, error) |
There was a problem hiding this comment.
nit: Can we rename to RunningConainers()?
|
A couple of nits, but this looks good. |
wojtek-t
force-pushed
the
docker_cache
branch
2 times, most recently
from
19e147c to
f5be903
Compare
|
I updated the PR and squashed the previous commits. PTAL |
|
That was my last nit I promise :) We already have plans to start using this cache for the metrics so thanks again for getting this in! |
|
Thanks a lot for this review - I'm trying to get up to speed with both Go and Kubernetes. |
wojtek-t
force-pushed
the
docker_cache
branch
from
057aeeb to
dcef5e2
Compare
|
I just squashed all the commits - hopefully you will merge the PR in the morning then :) |
|
LGTM @wojtek-t thanks again! |
Introduce DockerCache in Kubelet.
Create and use DockerCache in Kubelet to get running containers from Docker in SyncPods method. This was the most walltime-consuming part of SyncPods method and was significantly slowing down starting containers (especially in cases when I was starting 50 of them on the same node at the same time).
This PR is partially addressing #4119.