UPSTREAM: 53167: Do not GC exited containers in running pods

:100644 100644 28a8d1f652... 895fd233a9... M	pkg/kubelet/kubelet_pods.go
:100644 100644 ed9301afe0... f2809348bf... M	pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go
:100644 100644 063a89d6b3... 28a831d848... M	pkg/kubelet/kuberuntime/kuberuntime_gc.go
:100644 100644 389b1e52cc... 4a419524dd... M	pkg/kubelet/kuberuntime/kuberuntime_gc_test.go
:100644 100644 7e5d1d0d29... 7286bf44c0... M	pkg/kubelet/kuberuntime/kuberuntime_manager.go

Author: sjenning

pkg/kubelet/kubelet_pods.go

@@ -744,6 +744,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

pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go

@@ -42,18 +42,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
 }
 
@@ -77,7 +84,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),

pkg/kubelet/kuberuntime/kuberuntime_gc.go

@@ -32,17 +32,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,
 	}
 }
 
@@ -200,7 +200,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 {
@@ -210,7 +210,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)
 			}
@@ -252,7 +252,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
@@ -298,7 +298,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.
@@ -324,7 +324,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 !cgc.podStateProvider.IsPodDeleted(podUID) {
 				continue
 			}
 			err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, name))
@@ -358,14 +358,14 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error {
 // * removes oldest dead containers by enforcing gcPolicy.MaxContainers.
 // * gets evictable sandboxes which are not ready and contains no containers.
 // * removes evictable sandboxes.
-func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error {
+func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool, evictTerminatedPods bool) error {
 	// Remove evictable containers
-	if err := cgc.evictContainers(gcPolicy, allSourcesReady, evictNonDeletedPods); err != nil {
+	if err := cgc.evictContainers(gcPolicy, allSourcesReady, evictTerminatedPods); err != nil {
 		return err
 	}
 
 	// Remove sandboxes with zero containers
-	if err := cgc.evictSandboxes(evictNonDeletedPods); err != nil {
+	if err := cgc.evictSandboxes(evictTerminatedPods); err != nil {
 		return err
 	}