Skip to content

Commit

Permalink
fix #52462. Do not GC exited containers in running pods
Browse files Browse the repository at this point in the history
  • Loading branch information
dashpole committed Sep 28, 2017
1 parent 208ae55 commit 4300c75
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 63 deletions.
10 changes: 10 additions & 0 deletions pkg/kubelet/kubelet_pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,16 @@ func (kl *Kubelet) podIsTerminated(pod *v1.Pod) bool {
return status.Phase == v1.PodFailed || status.Phase == v1.PodSucceeded || (pod.DeletionTimestamp != nil && notRunning(status.ContainerStatuses))
}

// IsPodTerminated returns trus if the pod with the provided UID is in a terminated state ("Failed" or "Succeeded")
// or if the pod has been deleted or removed
func (kl *Kubelet) IsPodTerminated(uid types.UID) bool {
pod, podFound := kl.podManager.GetPodByUID(uid)
if !podFound {
return true
}
return kl.podIsTerminated(pod)
}

// IsPodDeleted returns true if the pod is deleted. For the pod to be deleted, either:
// 1. The pod object is deleted
// 2. The pod's status is evicted
Expand Down
23 changes: 15 additions & 8 deletions pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,25 @@ func (f *fakeHTTP) Get(url string) (*http.Response, error) {
return nil, f.err
}

type fakePodDeletionProvider struct {
pods map[types.UID]struct{}
type fakePodStateProvider struct {
existingPods map[types.UID]struct{}
runningPods map[types.UID]struct{}
}

func newFakePodDeletionProvider() *fakePodDeletionProvider {
return &fakePodDeletionProvider{
pods: make(map[types.UID]struct{}),
func newFakePodStateProvider() *fakePodStateProvider {
return &fakePodStateProvider{
existingPods: make(map[types.UID]struct{}),
runningPods: make(map[types.UID]struct{}),
}
}

func (f *fakePodDeletionProvider) IsPodDeleted(uid types.UID) bool {
_, found := f.pods[uid]
func (f *fakePodStateProvider) IsPodDeleted(uid types.UID) bool {
_, found := f.existingPods[uid]
return !found
}

func (f *fakePodStateProvider) IsPodTerminated(uid types.UID) bool {
_, found := f.runningPods[uid]
return !found
}

Expand All @@ -79,7 +86,7 @@ func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageS
return nil, err
}

kubeRuntimeManager.containerGC = NewContainerGC(runtimeService, newFakePodDeletionProvider(), kubeRuntimeManager)
kubeRuntimeManager.containerGC = NewContainerGC(runtimeService, newFakePodStateProvider(), kubeRuntimeManager)
kubeRuntimeManager.runtimeName = typedVersion.RuntimeName
kubeRuntimeManager.imagePuller = images.NewImageManager(
kubecontainer.FilterEventRecorder(recorder),
Expand Down
30 changes: 15 additions & 15 deletions pkg/kubelet/kuberuntime/kuberuntime_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ import (

// containerGC is the manager of garbage collection.
type containerGC struct {
client internalapi.RuntimeService
manager *kubeGenericRuntimeManager
podDeletionProvider podDeletionProvider
client internalapi.RuntimeService
manager *kubeGenericRuntimeManager
podStateProvider podStateProvider
}

// NewContainerGC creates a new containerGC.
func NewContainerGC(client internalapi.RuntimeService, podDeletionProvider podDeletionProvider, manager *kubeGenericRuntimeManager) *containerGC {
func NewContainerGC(client internalapi.RuntimeService, podStateProvider podStateProvider, manager *kubeGenericRuntimeManager) *containerGC {
return &containerGC{
client: client,
manager: manager,
podDeletionProvider: podDeletionProvider,
client: client,
manager: manager,
podStateProvider: podStateProvider,
}
}

Expand Down Expand Up @@ -201,7 +201,7 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE
}

// evict all containers that are evictable
func (cgc *containerGC) evictContainers(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error {
func (cgc *containerGC) evictContainers(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool, evictTerminatedPods bool) error {
// Separate containers by evict units.
evictUnits, err := cgc.evictableContainers(gcPolicy.MinAge)
if err != nil {
Expand All @@ -211,7 +211,7 @@ func (cgc *containerGC) evictContainers(gcPolicy kubecontainer.ContainerGCPolicy
// Remove deleted pod containers if all sources are ready.
if allSourcesReady {
for key, unit := range evictUnits {
if cgc.podDeletionProvider.IsPodDeleted(key.uid) || evictNonDeletedPods {
if cgc.podStateProvider.IsPodDeleted(key.uid) || (cgc.podStateProvider.IsPodTerminated(key.uid) && evictTerminatedPods) {
cgc.removeOldestN(unit, len(unit)) // Remove all.
delete(evictUnits, key)
}
Expand Down Expand Up @@ -253,7 +253,7 @@ func (cgc *containerGC) evictContainers(gcPolicy kubecontainer.ContainerGCPolicy
// 2. contains no containers.
// 3. belong to a non-existent (i.e., already removed) pod, or is not the
// most recently created sandbox for the pod.
func (cgc *containerGC) evictSandboxes(evictNonDeletedPods bool) error {
func (cgc *containerGC) evictSandboxes(evictTerminatedPods bool) error {
containers, err := cgc.manager.getKubeletContainers(true)
if err != nil {
return err
Expand Down Expand Up @@ -297,7 +297,7 @@ func (cgc *containerGC) evictSandboxes(evictNonDeletedPods bool) error {
}

for podUID, sandboxes := range sandboxesByPod {
if cgc.podDeletionProvider.IsPodDeleted(podUID) || evictNonDeletedPods {
if cgc.podStateProvider.IsPodDeleted(podUID) || (cgc.podStateProvider.IsPodTerminated(podUID) && evictTerminatedPods) {
// Remove all evictable sandboxes if the pod has been removed.
// Note that the latest dead sandbox is also removed if there is
// already an active one.
Expand All @@ -323,7 +323,7 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error {
for _, dir := range dirs {
name := dir.Name()
podUID := types.UID(name)
if !cgc.podDeletionProvider.IsPodDeleted(podUID) {
if !<