From 35cf8ab7d47138f33e55d3e71a2c8c1dafb02688 Mon Sep 17 00:00:00 2001 From: feisky Date: Sat, 3 Oct 2015 23:37:07 +0800 Subject: [PATCH 1/6] Use runtime instread of dockerclient in container gc --- pkg/kubelet/container/runtime.go | 3 + pkg/kubelet/container_gc.go | 213 +------------------------------ pkg/kubelet/kubelet.go | 13 +- 3 files changed, 17 insertions(+), 212 deletions(-) diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 2213a3a3da4..106a68955aa 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -22,6 +22,7 @@ import ( "io" "reflect" "strings" + "time" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -74,6 +75,8 @@ type Runtime interface { // specifies whether the runtime returns all containers including those already // exited and dead containers (used for garbage collection). GetPods(all bool) ([]*Pod, error) + // Garbage collection of dead containers + GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error // Syncs the running pod into the desired pod. SyncPod(pod *api.Pod, runningPod Pod, podStatus api.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error // KillPod kills all the containers of a pod. Pod may be nil, running pod must not be. diff --git a/pkg/kubelet/container_gc.go b/pkg/kubelet/container_gc.go index 95027a3574e..c116c019ff8 100644 --- a/pkg/kubelet/container_gc.go +++ b/pkg/kubelet/container_gc.go @@ -18,16 +18,9 @@ package kubelet import ( "fmt" - "os" - "path" - "path/filepath" - "sort" "time" - docker "github.com/fsouza/go-dockerclient" - "github.com/golang/glog" - "k8s.io/kubernetes/pkg/kubelet/dockertools" - "k8s.io/kubernetes/pkg/types" + "k8s.io/kubernetes/pkg/kubelet/container" ) // Specified a policy for garbage collecting containers. @@ -53,217 +46,25 @@ type containerGC interface { // TODO(vmarmol): Preferentially remove pod infra containers. type realContainerGC struct { - // Docker client to use. - dockerClient dockertools.DockerInterface + // Container runtime + runtime container.Runtime // Policy for garbage collection. policy ContainerGCPolicy - - // The path to the symlinked docker logs - containerLogsDir string } // New containerGC instance with the specified policy. -func newContainerGC(dockerClient dockertools.DockerInterface, policy ContainerGCPolicy) (containerGC, error) { +func newContainerGC(runtime container.Runtime, policy ContainerGCPolicy) (containerGC, error) { if policy.MinAge < 0 { return nil, fmt.Errorf("invalid minimum garbage collection age: %v", policy.MinAge) } return &realContainerGC{ - dockerClient: dockerClient, - policy: policy, - containerLogsDir: containerLogsDir, + runtime: runtime, + policy: policy, }, nil } -// Internal information kept for containers being considered for GC. -type containerGCInfo struct { - // Docker ID of the container. - id string - - // Docker name of the container. - name string - - // Creation time for the container. - createTime time.Time - - // Full pod name, including namespace in the format `namespace_podName`. - // This comes from dockertools.ParseDockerName(...) - podNameWithNamespace string - - // Container name in pod - containerName string -} - -// Containers are considered for eviction as units of (UID, container name) pair. -type evictUnit struct { - // UID of the pod. - uid types.UID - - // Name of the container in the pod. - name string -} - -type containersByEvictUnit map[evictUnit][]containerGCInfo - -// Returns the number of containers in this map. -func (cu containersByEvictUnit) NumContainers() int { - num := 0 - for key := range cu { - num += len(cu[key]) - } - - return num -} - -// Returns the number of pod in this map. -func (cu containersByEvictUnit) NumEvictUnits() int { - return len(cu) -} - -// Newest first. -type byCreated []containerGCInfo - -func (a byCreated) Len() int { return len(a) } -func (a byCreated) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a byCreated) Less(i, j int) bool { return a[i].createTime.After(a[j].createTime) } - func (cgc *realContainerGC) GarbageCollect() error { - // Separate containers by evict units. - evictUnits, unidentifiedContainers, err := cgc.evictableContainers() - if err != nil { - return err - } - - // Remove unidentified containers. - for _, container := range unidentifiedContainers { - glog.Infof("Removing unidentified dead container %q with ID %q", container.name, container.id) - err = cgc.dockerClient.RemoveContainer(docker.RemoveContainerOptions{ID: container.id, RemoveVolumes: true}) - if err != nil { - glog.Warningf("Failed to remove unidentified dead container %q: %v", container.name, err) - } - } - - // Enforce max containers per evict unit. - if cgc.policy.MaxPerPodContainer >= 0 { - cgc.enforceMaxContainersPerEvictUnit(evictUnits, cgc.policy.MaxPerPodContainer) - } - - // Enforce max total number of containers. - if cgc.policy.MaxContainers >= 0 && evictUnits.NumContainers() > cgc.policy.MaxContainers { - // Leave an equal number of containers per evict unit (min: 1). - numContainersPerEvictUnit := cgc.policy.MaxContainers / evictUnits.NumEvictUnits() - if numContainersPerEvictUnit < 1 { - numContainersPerEvictUnit = 1 - } - cgc.enforceMaxContainersPerEvictUnit(evictUnits, numContainersPerEvictUnit) - - // If we still need to evict, evict oldest first. - numContainers := evictUnits.NumContainers() - if numContainers > cgc.policy.MaxContainers { - flattened := make([]containerGCInfo, 0, numContainers) - for uid := range evictUnits { - flattened = append(flattened, evictUnits[uid]...) - } - sort.Sort(byCreated(flattened)) - - cgc.removeOldestN(flattened, numContainers-cgc.policy.MaxContainers) - } - } - - // Remove dead symlinks - should only happen on upgrade - // from a k8s version without proper log symlink cleanup - logSymlinks, _ := filepath.Glob(path.Join(cgc.containerLogsDir, fmt.Sprintf("*.%s", dockertools.LogSuffix))) - for _, logSymlink := range logSymlinks { - if _, err = os.Stat(logSymlink); os.IsNotExist(err) { - err = os.Remove(logSymlink) - if err != nil { - glog.Warningf("Failed to remove container log dead symlink %q: %v", logSymlink, err) - } - } - } - - return nil -} - -func (cgc *realContainerGC) enforceMaxContainersPerEvictUnit(evictUnits containersByEvictUnit, MaxContainers int) { - for uid := range evictUnits { - toRemove := len(evictUnits[uid]) - MaxContainers - - if toRemove > 0 { - evictUnits[uid] = cgc.removeOldestN(evictUnits[uid], toRemove) - } - } -} - -// Removes the oldest toRemove containers and returns the resulting slice. -func (cgc *realContainerGC) removeOldestN(containers []containerGCInfo, toRemove int) []containerGCInfo { - // Remove from oldest to newest (last to first). - numToKeep := len(containers) - toRemove - for i := numToKeep; i < len(containers); i++ { - err := cgc.dockerClient.RemoveContainer(docker.RemoveContainerOptions{ID: containers[i].id, RemoveVolumes: true}) - if err != nil { - glog.Warningf("Failed to remove dead container %q: %v", containers[i].name, err) - } - symlinkPath := dockertools.LogSymlink(cgc.containerLogsDir, containers[i].podNameWithNamespace, containers[i].containerName, containers[i].id) - err = os.Remove(symlinkPath) - if err != nil && !os.IsNotExist(err) { - glog.Warningf("Failed to remove container %q log symlink %q: %v", containers[i].name, symlinkPath, err) - } - } - - // Assume we removed the containers so that we're not too aggressive. - return containers[:numToKeep] -} - -// Get all containers that are evictable. Evictable containers are: not running -// and created more than MinAge ago. -func (cgc *realContainerGC) evictableContainers() (containersByEvictUnit, []containerGCInfo, error) { - containers, err := dockertools.GetKubeletDockerContainers(cgc.dockerClient, true) - if err != nil { - return containersByEvictUnit{}, []containerGCInfo{}, err - } - - unidentifiedContainers := make([]containerGCInfo, 0) - evictUnits := make(containersByEvictUnit) - newestGCTime := time.Now().Add(-cgc.policy.MinAge) - for _, container := range containers { - // Prune out running containers. - data, err := cgc.dockerClient.InspectContainer(container.ID) - if err != nil { - // Container may have been removed already, skip. - continue - } else if data.State.Running { - continue - } else if newestGCTime.Before(data.Created) { - continue - } - - containerInfo := containerGCInfo{ - id: container.ID, - name: container.Names[0], - createTime: data.Created, - } - - containerName, _, err := dockertools.ParseDockerName(container.Names[0]) - - if err != nil { - unidentifiedContainers = append(unidentifiedContainers, containerInfo) - } else { - key := evictUnit{ - uid: containerName.PodUID, - name: containerName.ContainerName, - } - containerInfo.podNameWithNamespace = containerName.PodFullName - containerInfo.containerName = containerName.ContainerName - evictUnits[key] = append(evictUnits[key], containerInfo) - } - } - - // Sort the containers by age. - for uid := range evictUnits { - sort.Sort(byCreated(evictUnits[uid])) - } - - return evictUnits, unidentifiedContainers, nil + return cgc.runtime.GarbageCollect(cgc.policy.MaxPerPodContainer, cgc.policy.MaxContainers, cgc.policy.MinAge) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 8c0ed742a00..c4f76804e53 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -236,11 +236,6 @@ func NewMainKubelet( Namespace: "", } - containerGC, err := newContainerGC(dockerClient, containerGCPolicy) - if err != nil { - return nil, err - } - diskSpaceManager, err := newDiskSpaceManager(cadvisorInterface, diskSpacePolicy) if err != nil { return nil, fmt.Errorf("failed to initialize disk manager: %v", err) @@ -275,7 +270,6 @@ func NewMainKubelet( streamingConnectionIdleTimeout: streamingConnectionIdleTimeout, recorder: recorder, cadvisor: cadvisorInterface, - containerGC: containerGC, diskSpaceManager: diskSpaceManager, statusManager: statusManager, volumeManager: volumeManager, @@ -361,6 +355,13 @@ func NewMainKubelet( return nil, fmt.Errorf("unsupported container runtime %q specified", containerRuntime) } + // setup containerGC + containerGC, err := newContainerGC(klet.containerRuntime, containerGCPolicy) + if err != nil { + return nil, err + } + klet.containerGC = containerGC + // setup imageManager imageManager, err := newImageManager(klet.containerRuntime, cadvisorInterface, recorder, nodeRef, imageGCPolicy) if err != nil { From 4c8a8362601a5f374f81b16fea48a5f95f3699b9 Mon Sep 17 00:00:00 2001 From: feisky Date: Sat, 3 Oct 2015 23:39:15 +0800 Subject: [PATCH 2/6] Move original container gc to docker runtime --- pkg/kubelet/container/fake_runtime.go | 8 + pkg/kubelet/dockertools/container_gc.go | 232 ++++++++++++++++++++++++ pkg/kubelet/dockertools/manager.go | 9 + pkg/kubelet/rkt/rkt.go | 2 +- 4 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 pkg/kubelet/dockertools/container_gc.go diff --git a/pkg/kubelet/container/fake_runtime.go b/pkg/kubelet/container/fake_runtime.go index 2a119ab20f0..ce20e334936 100644 --- a/pkg/kubelet/container/fake_runtime.go +++ b/pkg/kubelet/container/fake_runtime.go @@ -304,3 +304,11 @@ func (f *FakeRuntime) PortForward(pod *Pod, port uint16, stream io.ReadWriteClos f.CalledFunctions = append(f.CalledFunctions, "PortForward") return f.Err } + +func (f *FakeRuntime) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { + f.Lock() + defer f.Unlock() + + f.CalledFunctions = append(f.CalledFunctions, "GarbageCollect") + return f.Err +} diff --git a/pkg/kubelet/dockertools/container_gc.go b/pkg/kubelet/dockertools/container_gc.go new file mode 100644 index 00000000000..fa1da6327dd --- /dev/null +++ b/pkg/kubelet/dockertools/container_gc.go @@ -0,0 +1,232 @@ +/* +Copyright 2014 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dockertools + +import ( + "fmt" + "os" + "path" + "path/filepath" + "sort" + "time" + + docker "github.com/fsouza/go-dockerclient" + "github.com/golang/glog" + "k8s.io/kubernetes/pkg/types" +) + +type containerGC struct { + client DockerInterface + containerLogsDir string +} + +func NewContainerGC(client DockerInterface, containerLogsDir string) *containerGC { + return &containerGC{client: client, containerLogsDir: containerLogsDir} +} + +// Internal information kept for containers being considered for GC. +type containerGCInfo struct { + // Docker ID of the container. + id string + + // Docker name of the container. + name string + + // Creation time for the container. + createTime time.Time + + // Full pod name, including namespace in the format `namespace_podName`. + // This comes from dockertools.ParseDockerName(...) + podNameWithNamespace string + + // Container name in pod + containerName string +} + +// Containers are considered for eviction as units of (UID, container name) pair. +type evictUnit struct { + // UID of the pod. + uid types.UID + + // Name of the container in the pod. + name string +} + +type containersByEvictUnit map[evictUnit][]containerGCInfo + +// Returns the number of containers in this map. +func (cu containersByEvictUnit) NumContainers() int { + num := 0 + for key := range cu { + num += len(cu[key]) + } + + return num +} + +// Returns the number of pod in this map. +func (cu containersByEvictUnit) NumEvictUnits() int { + return len(cu) +} + +// Newest first. +type byCreated []containerGCInfo + +func (a byCreated) Len() int { return len(a) } +func (a byCreated) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byCreated) Less(i, j int) bool { return a[i].createTime.After(a[j].createTime) } + +func (cgc *containerGC) enforceMaxContainersPerEvictUnit(evictUnits containersByEvictUnit, MaxContainers int) { + for uid := range evictUnits { + toRemove := len(evictUnits[uid]) - MaxContainers + + if toRemove > 0 { + evictUnits[uid] = cgc.removeOldestN(evictUnits[uid], toRemove) + } + } +} + +// Removes the oldest toRemove containers and returns the resulting slice. +func (cgc *containerGC) removeOldestN(containers []containerGCInfo, toRemove int) []containerGCInfo { + // Remove from oldest to newest (last to first). + numToKeep := len(containers) - toRemove + for i := numToKeep; i < len(containers); i++ { + err := cgc.client.RemoveContainer(docker.RemoveContainerOptions{ID: containers[i].id, RemoveVolumes: true}) + if err != nil { + glog.Warningf("Failed to remove dead container %q: %v", containers[i].name, err) + } + symlinkPath := LogSymlink(cgc.containerLogsDir, containers[i].podNameWithNamespace, containers[i].containerName, containers[i].id) + err = os.Remove(symlinkPath) + if err != nil && !os.IsNotExist(err) { + glog.Warningf("Failed to remove container %q log symlink %q: %v", containers[i].name, symlinkPath, err) + } + } + + // Assume we removed the containers so that we're not too aggressive. + return containers[:numToKeep] +} + +// Get all containers that are evictable. Evictable containers are: not running +// and created more than MinAge ago. +func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByEvictUnit, []containerGCInfo, error) { + containers, err := GetKubeletDockerContainers(cgc.client, true) + if err != nil { + return containersByEvictUnit{}, []containerGCInfo{}, err + } + + unidentifiedContainers := make([]containerGCInfo, 0) + evictUnits := make(containersByEvictUnit) + newestGCTime := time.Now().Add(-minAge) + for _, container := range containers { + // Prune out running containers. + data, err := cgc.client.InspectContainer(container.ID) + if err != nil { + // Container may have been removed already, skip. + continue + } else if data.State.Running { + continue + } else if newestGCTime.Before(data.Created) { + continue + } + + containerInfo := containerGCInfo{ + id: container.ID, + name: container.Names[0], + createTime: data.Created, + } + + containerName, _, err := ParseDockerName(container.Names[0]) + + if err != nil { + unidentifiedContainers = append(unidentifiedContainers, containerInfo) + } else { + key := evictUnit{ + uid: containerName.PodUID, + name: containerName.ContainerName, + } + containerInfo.podNameWithNamespace = containerName.PodFullName + containerInfo.containerName = containerName.ContainerName + evictUnits[key] = append(evictUnits[key], containerInfo) + } + } + + // Sort the containers by age. + for uid := range evictUnits { + sort.Sort(byCreated(evictUnits[uid])) + } + + return evictUnits, unidentifiedContainers, nil +} + +// Garbage collection of dead containers +func (cgc *containerGC) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { + // Separate containers by evict units. + evictUnits, unidentifiedContainers, err := cgc.evictableContainers(minAge) + if err != nil { + return err + } + + // Remove unidentified containers. + for _, container := range unidentifiedContainers { + glog.Infof("Removing unidentified dead container %q with ID %q", container.name, container.id) + err = cgc.client.RemoveContainer(docker.RemoveContainerOptions{ID: container.id, RemoveVolumes: true}) + if err != nil { + glog.Warningf("Failed to remove unidentified dead container %q: %v", container.name, err) + } + } + + // Enforce max containers per evict unit. + if maxPerPodContainer >= 0 { + cgc.enforceMaxContainersPerEvictUnit(evictUnits, maxPerPodContainer) + } + + // Enforce max total number of containers. + if maxContainers >= 0 && evictUnits.NumContainers() > maxContainers { + // Leave an equal number of containers per evict unit (min: 1). + numContainersPerEvictUnit := maxContainers / evictUnits.NumEvictUnits() + if numContainersPerEvictUnit < 1 { + numContainersPerEvictUnit = 1 + } + cgc.enforceMaxContainersPerEvictUnit(evictUnits, numContainersPerEvictUnit) + + // If we still need to evict, evict oldest first. + numContainers := evictUnits.NumContainers() + if numContainers > maxContainers { + flattened := make([]containerGCInfo, 0, numContainers) + for uid := range evictUnits { + flattened = append(flattened, evictUnits[uid]...) + } + sort.Sort(byCreated(flattened)) + + cgc.removeOldestN(flattened, numContainers-maxContainers) + } + } + + // Remove dead symlinks - should only happen on upgrade + // from a k8s version without proper log symlink cleanup + logSymlinks, _ := filepath.Glob(path.Join(cgc.containerLogsDir, fmt.Sprintf("*.%s", LogSuffix))) + for _, logSymlink := range logSymlinks { + if _, err = os.Stat(logSymlink); os.IsNotExist(err) { + err = os.Remove(logSymlink) + if err != nil { + glog.Warningf("Failed to remove container log dead symlink %q: %v", logSymlink, err) + } + } + } + + return nil +} diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index bc8f4b11c83..bfbdf3ef581 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -139,6 +139,9 @@ type DockerManager struct { // If true, enforce container cpu limits with CFS quota support cpuCFSQuota bool + + // Container GC manager + containerGC *containerGC } func NewDockerManager( @@ -214,6 +217,7 @@ func NewDockerManager( } dm.runner = lifecycle.NewHandlerRunner(httpClient, dm, dm) dm.imagePuller = kubecontainer.NewImagePuller(recorder, dm, imageBackOff) + dm.containerGC = NewContainerGC(client, containerLogsDir) return dm } @@ -2019,3 +2023,8 @@ func (dm *DockerManager) GetNetNs(containerID kubecontainer.ContainerID) (string netnsPath := fmt.Sprintf(DockerNetnsFmt, inspectResult.State.Pid) return netnsPath, nil } + +// Garbage collection of dead containers +func (dm *DockerManager) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { + return dm.containerGC.GarbageCollect(maxPerPodContainer, maxContainers, minAge) +} diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 4fb510dd232..b1f825e23dd 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -1083,7 +1083,7 @@ func (r *Runtime) GetContainerLogs(pod *api.Pod, containerID kubecontainer.Conta // GarbageCollect collects the pods/containers. // TODO(yifan): Enforce the gc policy, also, it would be better if we can // just GC kubernetes pods. -func (r *Runtime) GarbageCollect() error { +func (r *runtime) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { if err := exec.Command("systemctl", "reset-failed").Run(); err != nil { glog.Errorf("rkt: Failed to reset failed systemd services: %v", err) } From 69867fb502a9ea670c319e1702416a89fada753a Mon Sep 17 00:00:00 2001 From: feisky Date: Sat, 3 Oct 2015 23:40:00 +0800 Subject: [PATCH 3/6] Refactor container gc tests --- .../{ => dockertools}/container_gc_test.go | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) rename pkg/kubelet/{ => dockertools}/container_gc_test.go (92%) diff --git a/pkg/kubelet/container_gc_test.go b/pkg/kubelet/dockertools/container_gc_test.go similarity index 92% rename from pkg/kubelet/container_gc_test.go rename to pkg/kubelet/dockertools/container_gc_test.go index c943a2fbc82..db68aee1baa 100644 --- a/pkg/kubelet/container_gc_test.go +++ b/pkg/kubelet/dockertools/container_gc_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package kubelet +package dockertools import ( "fmt" @@ -25,23 +25,17 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "k8s.io/kubernetes/pkg/kubelet/dockertools" ) -func newTestContainerGC(t *testing.T, MinAge time.Duration, MaxPerPodContainer, MaxContainers int) (containerGC, *dockertools.FakeDockerClient) { - fakeDocker := new(dockertools.FakeDockerClient) - gc, err := newContainerGC(fakeDocker, ContainerGCPolicy{ - MinAge: MinAge, - MaxPerPodContainer: MaxPerPodContainer, - MaxContainers: MaxContainers, - }) - require.Nil(t, err) +func newTestContainerGC(t *testing.T) (*containerGC, *FakeDockerClient) { + fakeDocker := new(FakeDockerClient) + gc := NewContainerGC(fakeDocker, "") return gc, fakeDocker } // Makes a stable time object, lower id is earlier time. func makeTime(id int) time.Time { + var zero time.Time return zero.Add(time.Duration(id) * time.Second) } @@ -90,7 +84,7 @@ func verifyStringArrayEqualsAnyOrder(t *testing.T, actual, expected []string) { } func TestGarbageCollectZeroMaxContainers(t *testing.T) { - gc, fakeDocker := newTestContainerGC(t, time.Minute, 1, 0) + gc, fakeDocker := newTestContainerGC(t) fakeDocker.ContainerList = []docker.APIContainers{ makeAPIContainer("foo", "POD", "1876"), } @@ -98,12 +92,12 @@ func TestGarbageCollectZeroMaxContainers(t *testing.T) { makeContainerDetail("1876", false, makeTime(0)), ) - assert.Nil(t, gc.GarbageCollect()) + assert.Nil(t, gc.GarbageCollect(1, 0, time.Minute)) assert.Len(t, fakeDocker.Removed, 1) } func TestGarbageCollectNoMaxPerPodContainerLimit(t *testing.T) { - gc, fakeDocker := newTestContainerGC(t, time.Minute, -1, 4) + gc, fakeDocker := newTestContainerGC(t) fakeDocker.ContainerList = []docker.APIContainers{ makeAPIContainer("foo", "POD", "1876"), makeAPIContainer("foo1", "POD", "2876"), @@ -119,12 +113,12 @@ func TestGarbageCollectNoMaxPerPodContainerLimit(t *testing.T) { makeContainerDetail("5876", false, makeTime(4)), ) - assert.Nil(t, gc.GarbageCollect()) + assert.Nil(t, gc.GarbageCollect(-1, 4, time.Minute)) assert.Len(t, fakeDocker.Removed, 1) } func TestGarbageCollectNoMaxLimit(t *testing.T) { - gc, fakeDocker := newTestContainerGC(t, time.Minute, 1, -1) + gc, fakeDocker := newTestContainerGC(t) fakeDocker.ContainerList = []docker.APIContainers{ makeAPIContainer("foo", "POD", "1876"), makeAPIContainer("foo1", "POD", "2876"), @@ -140,7 +134,7 @@ func TestGarbageCollectNoMaxLimit(t *testing.T) { makeContainerDetail("5876", false, makeTime(0)), ) - assert.Nil(t, gc.GarbageCollect()) + assert.Nil(t, gc.GarbageCollect(1, -1, time.Minute)) assert.Len(t, fakeDocker.Removed, 0) } @@ -309,10 +303,10 @@ func TestGarbageCollect(t *testing.T) { } for i, test := range tests { t.Logf("Running test case with index %d", i) - gc, fakeDocker := newTestContainerGC(t, time.Hour, 2, 6) + gc, fakeDocker := newTestContainerGC(t) fakeDocker.ContainerList = test.containers fakeDocker.ContainerMap = test.containerDetails - assert.Nil(t, gc.GarbageCollect()) + assert.Nil(t, gc.GarbageCollect(2, 6, time.Hour)) verifyStringArrayEqualsAnyOrder(t, fakeDocker.Removed, test.expectedRemoved) } } From d624c7de513ea8af9688c0df8364ea9ecb8d55b8 Mon Sep 17 00:00:00 2001 From: feisky Date: Tue, 6 Oct 2015 06:35:32 +0800 Subject: [PATCH 4/6] Pass the ContainerGCPolicy in Runtime.GarbageCollect --- cmd/kubelet/app/server.go | 2 +- pkg/kubelet/{ => container}/container_gc.go | 14 ++++++-------- pkg/kubelet/container/fake_runtime.go | 2 +- pkg/kubelet/container/runtime.go | 5 ++--- pkg/kubelet/dockertools/container_gc.go | 19 ++++++++++--------- pkg/kubelet/dockertools/container_gc_test.go | 9 +++++---- pkg/kubelet/dockertools/manager.go | 4 ++-- pkg/kubelet/kubelet.go | 6 +++--- pkg/kubelet/rkt/rkt.go | 2 +- 9 files changed, 31 insertions(+), 32 deletions(-) rename pkg/kubelet/{ => container}/container_gc.go (81%) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index b02d63e8d99..545f95b953f 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -860,7 +860,7 @@ func CreateAndInitKubelet(kc *KubeletConfig) (k KubeletBootstrap, pc *config.Pod kubeClient = kc.KubeClient } - gcPolicy := kubelet.ContainerGCPolicy{ + gcPolicy := kubecontainer.ContainerGCPolicy{ MinAge: kc.MinimumGCAge, MaxPerPodContainer: kc.MaxPerPodContainerCount, MaxContainers: kc.MaxContainerCount, diff --git a/pkg/kubelet/container_gc.go b/pkg/kubelet/container/container_gc.go similarity index 81% rename from pkg/kubelet/container_gc.go rename to pkg/kubelet/container/container_gc.go index c116c019ff8..cd69c1ab45e 100644 --- a/pkg/kubelet/container_gc.go +++ b/pkg/kubelet/container/container_gc.go @@ -14,13 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -package kubelet +package container import ( "fmt" "time" - - "k8s.io/kubernetes/pkg/kubelet/container" ) // Specified a policy for garbage collecting containers. @@ -39,7 +37,7 @@ type ContainerGCPolicy struct { // Manages garbage collection of dead containers. // // Implementation is thread-compatible. -type containerGC interface { +type ContainerGC interface { // Garbage collect containers. GarbageCollect() error } @@ -47,14 +45,14 @@ type containerGC interface { // TODO(vmarmol): Preferentially remove pod infra containers. type realContainerGC struct { // Container runtime - runtime container.Runtime + runtime Runtime // Policy for garbage collection. policy ContainerGCPolicy } -// New containerGC instance with the specified policy. -func newContainerGC(runtime container.Runtime, policy ContainerGCPolicy) (containerGC, error) { +// New ContainerGC instance with the specified policy. +func NewContainerGC(runtime Runtime, policy ContainerGCPolicy) (ContainerGC, error) { if policy.MinAge < 0 { return nil, fmt.Errorf("invalid minimum garbage collection age: %v", policy.MinAge) } @@ -66,5 +64,5 @@ func newContainerGC(runtime container.Runtime, policy ContainerGCPolicy) (contai } func (cgc *realContainerGC) GarbageCollect() error { - return cgc.runtime.GarbageCollect(cgc.policy.MaxPerPodContainer, cgc.policy.MaxContainers, cgc.policy.MinAge) + return cgc.runtime.GarbageCollect(cgc.policy) } diff --git a/pkg/kubelet/container/fake_runtime.go b/pkg/kubelet/container/fake_runtime.go index ce20e334936..99468376217 100644 --- a/pkg/kubelet/container/fake_runtime.go +++ b/pkg/kubelet/container/fake_runtime.go @@ -305,7 +305,7 @@ func (f *FakeRuntime) PortForward(pod *Pod, port uint16, stream io.ReadWriteClos return f.Err } -func (f *FakeRuntime) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { +func (f *FakeRuntime) GarbageCollect(gcPolicy ContainerGCPolicy) error { f.Lock() defer f.Unlock() diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 106a68955aa..570f46a6604 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -22,7 +22,6 @@ import ( "io" "reflect" "strings" - "time" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -75,8 +74,8 @@ type Runtime interface { // specifies whether the runtime returns all containers including those already // exited and dead containers (used for garbage collection). GetPods(all bool) ([]*Pod, error) - // Garbage collection of dead containers - GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error + // GarbageCollect removes dead containers using the specified container gc policy + GarbageCollect(gcPolicy ContainerGCPolicy) error // Syncs the running pod into the desired pod. SyncPod(pod *api.Pod, runningPod Pod, podStatus api.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error // KillPod kills all the containers of a pod. Pod may be nil, running pod must not be. diff --git a/pkg/kubelet/dockertools/container_gc.go b/pkg/kubelet/dockertools/container_gc.go index fa1da6327dd..48a0a4c19e9 100644 --- a/pkg/kubelet/dockertools/container_gc.go +++ b/pkg/kubelet/dockertools/container_gc.go @@ -26,6 +26,7 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" + kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/types" ) @@ -172,10 +173,10 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE return evictUnits, unidentifiedContainers, nil } -// Garbage collection of dead containers -func (cgc *containerGC) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { +// GarbageCollect removes dead containers using the specified container gc policy +func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy) error { // Separate containers by evict units. - evictUnits, unidentifiedContainers, err := cgc.evictableContainers(minAge) + evictUnits, unidentifiedContainers, err := cgc.evictableContainers(gcPolicy.MinAge) if err != nil { return err } @@ -190,14 +191,14 @@ func (cgc *containerGC) GarbageCollect(maxPerPodContainer, maxContainers int, mi } // Enforce max containers per evict unit. - if maxPerPodContainer >= 0 { - cgc.enforceMaxContainersPerEvictUnit(evictUnits, maxPerPodContainer) + if gcPolicy.MaxPerPodContainer >= 0 { + cgc.enforceMaxContainersPerEvictUnit(evictUnits, gcPolicy.MaxPerPodContainer) } // Enforce max total number of containers. - if maxContainers >= 0 && evictUnits.NumContainers() > maxContainers { + if gcPolicy.MaxContainers >= 0 && evictUnits.NumContainers() > gcPolicy.MaxContainers { // Leave an equal number of containers per evict unit (min: 1). - numContainersPerEvictUnit := maxContainers / evictUnits.NumEvictUnits() + numContainersPerEvictUnit := gcPolicy.MaxContainers / evictUnits.NumEvictUnits() if numContainersPerEvictUnit < 1 { numContainersPerEvictUnit = 1 } @@ -205,14 +206,14 @@ func (cgc *containerGC) GarbageCollect(maxPerPodContainer, maxContainers int, mi // If we still need to evict, evict oldest first. numContainers := evictUnits.NumContainers() - if numContainers > maxContainers { + if numContainers > gcPolicy.MaxContainers { flattened := make([]containerGCInfo, 0, numContainers) for uid := range evictUnits { flattened = append(flattened, evictUnits[uid]...) } sort.Sort(byCreated(flattened)) - cgc.removeOldestN(flattened, numContainers-maxContainers) + cgc.removeOldestN(flattened, numContainers-gcPolicy.MaxContainers) } } diff --git a/pkg/kubelet/dockertools/container_gc_test.go b/pkg/kubelet/dockertools/container_gc_test.go index db68aee1baa..e24e6ed3ab9 100644 --- a/pkg/kubelet/dockertools/container_gc_test.go +++ b/pkg/kubelet/dockertools/container_gc_test.go @@ -25,6 +25,7 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/stretchr/testify/assert" + kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) func newTestContainerGC(t *testing.T) (*containerGC, *FakeDockerClient) { @@ -92,7 +93,7 @@ func TestGarbageCollectZeroMaxContainers(t *testing.T) { makeContainerDetail("1876", false, makeTime(0)), ) - assert.Nil(t, gc.GarbageCollect(1, 0, time.Minute)) + assert.Nil(t, gc.GarbageCollect(kubecontainer.ContainerGCPolicy{time.Minute, 1, 0})) assert.Len(t, fakeDocker.Removed, 1) } @@ -113,7 +114,7 @@ func TestGarbageCollectNoMaxPerPodContainerLimit(t *testing.T) { makeContainerDetail("5876", false, makeTime(4)), ) - assert.Nil(t, gc.GarbageCollect(-1, 4, time.Minute)) + assert.Nil(t, gc.GarbageCollect(kubecontainer.ContainerGCPolicy{time.Minute, -1, 4})) assert.Len(t, fakeDocker.Removed, 1) } @@ -134,7 +135,7 @@ func TestGarbageCollectNoMaxLimit(t *testing.T) { makeContainerDetail("5876", false, makeTime(0)), ) - assert.Nil(t, gc.GarbageCollect(1, -1, time.Minute)) + assert.Nil(t, gc.GarbageCollect(kubecontainer.ContainerGCPolicy{time.Minute, 1, -1})) assert.Len(t, fakeDocker.Removed, 0) } @@ -306,7 +307,7 @@ func TestGarbageCollect(t *testing.T) { gc, fakeDocker := newTestContainerGC(t) fakeDocker.ContainerList = test.containers fakeDocker.ContainerMap = test.containerDetails - assert.Nil(t, gc.GarbageCollect(2, 6, time.Hour)) + assert.Nil(t, gc.GarbageCollect(kubecontainer.ContainerGCPolicy{time.Hour, 2, 6})) verifyStringArrayEqualsAnyOrder(t, fakeDocker.Removed, test.expectedRemoved) } } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index bfbdf3ef581..d1f67fc6680 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -2025,6 +2025,6 @@ func (dm *DockerManager) GetNetNs(containerID kubecontainer.ContainerID) (string } // Garbage collection of dead containers -func (dm *DockerManager) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { - return dm.containerGC.GarbageCollect(maxPerPodContainer, maxContainers, minAge) +func (dm *DockerManager) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy) error { + return dm.containerGC.GarbageCollect(gcPolicy) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index c4f76804e53..3efe41a5f38 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -147,7 +147,7 @@ func NewMainKubelet( pullBurst int, eventQPS float32, eventBurst int, - containerGCPolicy ContainerGCPolicy, + containerGCPolicy kubecontainer.ContainerGCPolicy, sourcesReady SourcesReadyFn, registerNode bool, standaloneMode bool, @@ -356,7 +356,7 @@ func NewMainKubelet( } // setup containerGC - containerGC, err := newContainerGC(klet.containerRuntime, containerGCPolicy) + containerGC, err := kubecontainer.NewContainerGC(klet.containerRuntime, containerGCPolicy) if err != nil { return nil, err } @@ -511,7 +511,7 @@ type Kubelet struct { recorder record.EventRecorder // Policy for handling garbage collection of dead containers. - containerGC containerGC + containerGC kubecontainer.ContainerGC // Manager for images. imageManager imageManager diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index b1f825e23dd..363dd8ff567 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -1083,7 +1083,7 @@ func (r *Runtime) GetContainerLogs(pod *api.Pod, containerID kubecontainer.Conta // GarbageCollect collects the pods/containers. // TODO(yifan): Enforce the gc policy, also, it would be better if we can // just GC kubernetes pods. -func (r *runtime) GarbageCollect(maxPerPodContainer, maxContainers int, minAge time.Duration) error { +func (r *runtime) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy) error { if err := exec.Command("systemctl", "reset-failed").Run(); err != nil { glog.Errorf("rkt: Failed to reset failed systemd services: %v", err) } From 43a654ed7bf197873bf24d84567a20dbfc1be785 Mon Sep 17 00:00:00 2001 From: feisky Date: Tue, 6 Oct 2015 11:33:20 +0800 Subject: [PATCH 5/6] Refacotor gcPolicy in contrib/mesos --- contrib/mesos/pkg/executor/service/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/mesos/pkg/executor/service/service.go b/contrib/mesos/pkg/executor/service/service.go index 07110616526..76d59487666 100644 --- a/contrib/mesos/pkg/executor/service/service.go +++ b/contrib/mesos/pkg/executor/service/service.go @@ -297,7 +297,7 @@ func (ks *KubeletExecutorServer) createAndInitKubelet( kubeClient = kc.KubeClient } - gcPolicy := kubelet.ContainerGCPolicy{ + gcPolicy := kubecontainer.ContainerGCPolicy{ MinAge: kc.MinimumGCAge, MaxPerPodContainer: kc.MaxPerPodContainerCount, MaxContainers: kc.MaxContainerCount, From fb04edea3a46bb32245c7730d053fd74d1ce2ae2 Mon Sep 17 00:00:00 2001 From: feisky Date: Wed, 7 Oct 2015 15:39:59 +0800 Subject: [PATCH 6/6] Replace rkt --grace-period and --expire-prepared with gcPolicy.MinAge --- pkg/kubelet/kubelet.go | 1 - pkg/kubelet/rkt/rkt.go | 12 ++---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 3efe41a5f38..f24d2c5adb5 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -346,7 +346,6 @@ func NewMainKubelet( return nil, err } klet.containerRuntime = rktRuntime - klet.containerGC = rktRuntime klet.imageManager = rkt.NewImageManager(rktRuntime) // No Docker daemon to put in a container. diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 363dd8ff567..6e86cf9cb6f 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -68,14 +68,6 @@ const ( authDir = "auth.d" dockerAuthTemplate = `{"rktKind":"dockerAuth","rktVersion":"v1","registries":[%q],"credentials":{"user":%q,"password":%q}}` - // TODO(yifan): Merge with ContainerGCPolicy, i.e., derive - // the grace period from MinAge in ContainerGCPolicy. - // - // Duration to wait before discarding inactive pods from garbage - defaultGracePeriod = "1m" - // Duration to wait before expiring prepared pods. - defaultExpirePrepared = "1m" - defaultImageTag = "latest" ) @@ -1083,11 +1075,11 @@ func (r *Runtime) GetContainerLogs(pod *api.Pod, containerID kubecontainer.Conta // GarbageCollect collects the pods/containers. // TODO(yifan): Enforce the gc policy, also, it would be better if we can // just GC kubernetes pods. -func (r *runtime) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy) error { +func (r *Runtime) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy) error { if err := exec.Command("systemctl", "reset-failed").Run(); err != nil { glog.Errorf("rkt: Failed to reset failed systemd services: %v", err) } - if _, err := r.runCommand("gc", "--grace-period="+defaultGracePeriod, "--expire-prepared="+defaultExpirePrepared); err != nil { + if _, err := r.runCommand("gc", "--grace-period="+gcPolicy.MinAge.String(), "--expire-prepared="+gcPolicy.MinAge.String()); err != nil { glog.Errorf("rkt: Failed to gc: %v", err) return err }