diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 03f2ca5eabe..ba64265ac32 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2409,6 +2409,13 @@ func (kl *Kubelet) HandlePodReconcile(pods []*api.Pod) { // Update the pod in pod manager, status manager will do periodically reconcile according // to the pod manager. kl.podManager.UpdatePod(pod) + + // After an evicted pod is synced, all dead containers in the pod can be removed. + if eviction.PodIsEvicted(pod.Status) { + if podStatus, err := kl.podCache.Get(pod.UID); err == nil { + kl.containerDeletor.deleteContainersInPod("", podStatus, true) + } + } } } @@ -2989,10 +2996,12 @@ func (kl *Kubelet) ListenAndServeReadOnly(address net.IP, port uint) { // Delete the eligible dead container instances in a pod. Depending on the configuration, the latest dead containers may be kept around. func (kl *Kubelet) cleanUpContainersInPod(podId types.UID, exitedContainerID string) { if podStatus, err := kl.podCache.Get(podId); err == nil { - if status, ok := kl.statusManager.GetPodStatus(podId); ok { - // If a pod is evicted, we can delete all the dead containers. - kl.containerDeletor.deleteContainersInPod(exitedContainerID, podStatus, eviction.PodIsEvicted(status)) + removeAll := false + if syncedPod, ok := kl.podManager.GetPodByUID(podId); ok { + // When an evicted pod has already synced, all containers can be removed. + removeAll = eviction.PodIsEvicted(syncedPod.Status) } + kl.containerDeletor.deleteContainersInPod(exitedContainerID, podStatus, removeAll) } } diff --git a/pkg/kubelet/pod_container_deletor.go b/pkg/kubelet/pod_container_deletor.go index 122170409a3..4daf1a5021b 100644 --- a/pkg/kubelet/pod_container_deletor.go +++ b/pkg/kubelet/pod_container_deletor.go @@ -58,29 +58,33 @@ func newPodContainerDeletor(runtime kubecontainer.Runtime, containersToKeep int) } } -// getContainersToDeleteInPod returns the exited containers in a pod whose name matches the name inferred from exitedContainerID, ordered by the creation time from the latest to the earliest. -func getContainersToDeleteInPod(exitedContainerID string, podStatus *kubecontainer.PodStatus, containersToKeep int) containerStatusbyCreatedList { - var matchedContainer *kubecontainer.ContainerStatus - var exitedContainers []*kubecontainer.ContainerStatus - // Find all exited containers in the pod +// getContainersToDeleteInPod returns the exited containers in a pod whose name matches the name inferred from filterContainerId (if not empty), ordered by the creation time from the latest to the earliest. +// If filterContainerId is empty, all dead containers in the pod are returned. +func getContainersToDeleteInPod(filterContainerId string, podStatus *kubecontainer.PodStatus, containersToKeep int) containerStatusbyCreatedList { + matchedContainer := func(filterContainerId string, podStatus *kubecontainer.PodStatus) *kubecontainer.ContainerStatus { + if filterContainerId == "" { + return nil + } + for _, containerStatus := range podStatus.ContainerStatuses { + if containerStatus.ID.ID == filterContainerId { + return containerStatus + } + } + return nil + }(filterContainerId, podStatus) + + if filterContainerId != "" && matchedContainer == nil { + glog.Warningf("Container %q not found in pod's containers", filterContainerId) + return containerStatusbyCreatedList{} + } + + // Find the exited containers whose name matches the name of the container with id being filterContainerId + var candidates containerStatusbyCreatedList for _, containerStatus := range podStatus.ContainerStatuses { if containerStatus.State != kubecontainer.ContainerStateExited { continue } - if containerStatus.ID.ID == exitedContainerID { - matchedContainer = containerStatus - } - exitedContainers = append(exitedContainers, containerStatus) - } - if matchedContainer == nil { - glog.Warningf("Container %q not found in pod's exited containers", exitedContainerID) - return containerStatusbyCreatedList{} - } - - // Find the exited containers whose name matches the name of the container with id being exitedContainerID - var candidates containerStatusbyCreatedList - for _, containerStatus := range exitedContainers { - if matchedContainer.Name == containerStatus.Name { + if matchedContainer == nil || matchedContainer.Name == containerStatus.Name { candidates = append(candidates, containerStatus) } } @@ -93,12 +97,13 @@ func getContainersToDeleteInPod(exitedContainerID string, podStatus *kubecontain } // deleteContainersInPod issues container deletion requests for containers selected by getContainersToDeleteInPod. -func (p *podContainerDeletor) deleteContainersInPod(exitedContainerID string, podStatus *kubecontainer.PodStatus, removeAll bool) { +func (p *podContainerDeletor) deleteContainersInPod(filterContainerId string, podStatus *kubecontainer.PodStatus, removeAll bool) { containersToKeep := p.containersToKeep if removeAll { containersToKeep = 0 } - for _, candidate := range getContainersToDeleteInPod(exitedContainerID, podStatus, containersToKeep) { + + for _, candidate := range getContainersToDeleteInPod(filterContainerId, podStatus, containersToKeep) { select { case p.worker <- candidate.ID: default: diff --git a/pkg/kubelet/pod_container_deletor_test.go b/pkg/kubelet/pod_container_deletor_test.go index 51797d14c3e..29c66f45156 100644 --- a/pkg/kubelet/pod_container_deletor_test.go +++ b/pkg/kubelet/pod_container_deletor_test.go @@ -24,7 +24,7 @@ import ( kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) -func testGetContainersToDeleteInPod(t *testing.T) { +func TestGetContainersToDeleteInPodWithFilter(t *testing.T) { pod := kubecontainer.PodStatus{ ContainerStatuses: []*kubecontainer.ContainerStatus{ { @@ -62,7 +62,7 @@ func testGetContainersToDeleteInPod(t *testing.T) { testCases := []struct { containersToKeep int - expectedContainersToDelete []*kubecontainer.ContainerStatus + expectedContainersToDelete containerStatusbyCreatedList }{ { 0, @@ -80,7 +80,123 @@ func testGetContainersToDeleteInPod(t *testing.T) { for _, test := range testCases { candidates := getContainersToDeleteInPod("4", &pod, test.containersToKeep) - if !reflect.DeepEqual(getContainersToDeleteInPod("4", &pod, test.containersToKeep), test.expectedContainersToDelete) { + if !reflect.DeepEqual(candidates, test.expectedContainersToDelete) { + t.Errorf("expected %v got %v", test.expectedContainersToDelete, candidates) + } + } +} + +func TestGetContainersToDeleteInPod(t *testing.T) { + pod := kubecontainer.PodStatus{ + ContainerStatuses: []*kubecontainer.ContainerStatus{ + { + ID: kubecontainer.ContainerID{Type: "test", ID: "1"}, + Name: "foo", + CreatedAt: time.Now(), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "2"}, + Name: "bar", + CreatedAt: time.Now().Add(time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "3"}, + Name: "bar", + CreatedAt: time.Now().Add(2 * time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "4"}, + Name: "bar", + CreatedAt: time.Now().Add(3 * time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "5"}, + Name: "bar", + CreatedAt: time.Now().Add(4 * time.Second), + State: kubecontainer.ContainerStateRunning, + }, + }, + } + + testCases := []struct { + containersToKeep int + expectedContainersToDelete containerStatusbyCreatedList + }{ + { + 0, + []*kubecontainer.ContainerStatus{pod.ContainerStatuses[3], pod.ContainerStatuses[2], pod.ContainerStatuses[1], pod.ContainerStatuses[0]}, + }, + { + 1, + []*kubecontainer.ContainerStatus{pod.ContainerStatuses[2], pod.ContainerStatuses[1], pod.ContainerStatuses[0]}, + }, + { + 2, + []*kubecontainer.ContainerStatus{pod.ContainerStatuses[1], pod.ContainerStatuses[0]}, + }, + } + + for _, test := range testCases { + candidates := getContainersToDeleteInPod("", &pod, test.containersToKeep) + if !reflect.DeepEqual(candidates, test.expectedContainersToDelete) { + t.Errorf("expected %v got %v", test.expectedContainersToDelete, candidates) + } + } +} + +func TestGetContainersToDeleteInPodWithNoMatch(t *testing.T) { + pod := kubecontainer.PodStatus{ + ContainerStatuses: []*kubecontainer.ContainerStatus{ + { + ID: kubecontainer.ContainerID{Type: "test", ID: "1"}, + Name: "foo", + CreatedAt: time.Now(), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "2"}, + Name: "bar", + CreatedAt: time.Now().Add(time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "3"}, + Name: "bar", + CreatedAt: time.Now().Add(2 * time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "4"}, + Name: "bar", + CreatedAt: time.Now().Add(3 * time.Second), + State: kubecontainer.ContainerStateExited, + }, + { + ID: kubecontainer.ContainerID{Type: "test", ID: "5"}, + Name: "bar", + CreatedAt: time.Now().Add(4 * time.Second), + State: kubecontainer.ContainerStateRunning, + }, + }, + } + + testCases := []struct { + filterId string + expectedContainersToDelete containerStatusbyCreatedList + }{ + { + "abc", + []*kubecontainer.ContainerStatus{}, + }, + } + + for _, test := range testCases { + candidates := getContainersToDeleteInPod(test.filterId, &pod, len(pod.ContainerStatuses)) + if !reflect.DeepEqual(candidates, test.expectedContainersToDelete) { t.Errorf("expected %v got %v", test.expectedContainersToDelete, candidates) } }