diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 39e06ed4c57..26f991e79cf 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1283,14 +1283,20 @@ func (kl *Kubelet) SyncPods(allPods []*api.Pod, podSyncTypes map[types.UID]metri } // Remove any orphaned volumes. - err = kl.cleanupOrphanedVolumes(pods, runningPods) + // Note that we pass all pods (including terminated pods) to the function, + // so that we don't remove volumes associated with terminated but not yet + // deleted pods. + err = kl.cleanupOrphanedVolumes(allPods, runningPods) if err != nil { glog.Errorf("Failed cleaning up orphaned volumes: %v", err) return err } // Remove any orphaned pod directories. - err = kl.cleanupOrphanedPodDirs(pods) + // Note that we pass all pods (including terminated pods) to the function, + // so that we don't remove directories associated with terminated but not yet + // deleted pods. + err = kl.cleanupOrphanedPodDirs(allPods) if err != nil { glog.Errorf("Failed cleaning up orphaned pod directories: %v", err) return err diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 09d04249342..0a89d4053e9 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -4685,3 +4685,104 @@ func TestSyncPodsDoesNotDeletePodsThatRunTooLong(t *testing.T) { // Get pod status. "list", "inspect_container", "inspect_container", "list"}) } + +func TestDeletePodDirsForDeletedPods(t *testing.T) { + testKubelet := newTestKubelet(t) + testKubelet.fakeCadvisor.On("MachineInfo").Return(&cadvisorApi.MachineInfo{}, nil) + testKubelet.fakeCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiv2.FsInfo{}, nil) + testKubelet.fakeCadvisor.On("RootFsInfo").Return(cadvisorApiv2.FsInfo{}, nil) + kl := testKubelet.kubelet + pods := []*api.Pod{ + { + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "pod1", + Namespace: "ns", + }, + }, + { + ObjectMeta: api.ObjectMeta{ + UID: "12345679", + Name: "pod2", + Namespace: "ns", + }, + }, + } + + kl.podManager.SetPods(pods) + // Sync to create pod directories. + err := kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + for i := range pods { + if !dirExists(kl.getPodDir(pods[i].UID)) { + t.Errorf("expected directory to exist for pod %d", i) + } + } + + // Pod 1 has been deleted and no longer exists. + err = kl.SyncPods([]*api.Pod{pods[0]}, emptyPodUIDs, map[string]*api.Pod{}, time.Now()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !dirExists(kl.getPodDir(pods[0].UID)) { + t.Errorf("expected directory to exist for pod 0") + } + if dirExists(kl.getPodDir(pods[1].UID)) { + t.Errorf("expected directory to be deleted for pod 1") + } +} + +func TestDoesNotDeletePodDirsForTerminatedPods(t *testing.T) { + testKubelet := newTestKubelet(t) + testKubelet.fakeCadvisor.On("MachineInfo").Return(&cadvisorApi.MachineInfo{}, nil) + testKubelet.fakeCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiv2.FsInfo{}, nil) + testKubelet.fakeCadvisor.On("RootFsInfo").Return(cadvisorApiv2.FsInfo{}, nil) + kl := testKubelet.kubelet + pods := []*api.Pod{ + { + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "pod1", + Namespace: "ns", + }, + }, + { + ObjectMeta: api.ObjectMeta{ + UID: "12345679", + Name: "pod2", + Namespace: "ns", + }, + }, + { + ObjectMeta: api.ObjectMeta{ + UID: "12345680", + Name: "pod3", + Namespace: "ns", + }, + }, + } + + kl.podManager.SetPods(pods) + // Sync to create pod directories. + err := kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + for i := range pods { + if !dirExists(kl.getPodDir(pods[i].UID)) { + t.Errorf("expected directory to exist for pod %d", i) + } + } + // Pod 1 failed, and pod 2 succeeded. None of the pod directories should be + // deleted. + kl.statusManager.SetPodStatus(pods[1], api.PodStatus{Phase: api.PodFailed}) + kl.statusManager.SetPodStatus(pods[2], api.PodStatus{Phase: api.PodSucceeded}) + err = kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now()) + for i := range pods { + if !dirExists(kl.getPodDir(pods[i].UID)) { + t.Errorf("expected directory to exist for pod %d", i) + } + } +}