From b80bea4a62629437b938f080db732eea1b962cc8 Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 14 Nov 2016 16:44:28 -0600 Subject: [PATCH] fix leaking memory backed volumes of terminated pods --- .../desired_state_of_world_populator.go | 23 +++- test/e2e_node/BUILD | 1 + test/e2e_node/volume_manager_test.go | 127 ++++++++++++++++++ 3 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 test/e2e_node/volume_manager_test.go diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index 6aac183a7e..888cf9c343 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -129,10 +129,18 @@ func (dswp *desiredStateOfWorldPopulator) populatorLoopFunc() func() { } } +func isPodTerminated(pod *api.Pod) bool { + return pod.Status.Phase == api.PodFailed || pod.Status.Phase == api.PodSucceeded +} + // Iterate through all pods and add to desired state of world if they don't // exist but should func (dswp *desiredStateOfWorldPopulator) findAndAddNewPods() { for _, pod := range dswp.podManager.GetPods() { + if isPodTerminated(pod) { + // Do not (re)add volumes for terminated pods + continue + } dswp.processPodVolumes(pod) } } @@ -144,9 +152,18 @@ func (dswp *desiredStateOfWorldPopulator) findAndRemoveDeletedPods() { runningPodsFetched := false for _, volumeToMount := range dswp.desiredStateOfWorld.GetVolumesToMount() { - if _, podExists := - dswp.podManager.GetPodByUID(volumeToMount.Pod.UID); podExists { - continue + pod, podExists := dswp.podManager.GetPodByUID(volumeToMount.Pod.UID) + if podExists { + // Skip running pods + if !isPodTerminated(pod) { + continue + } + // Skip non-memory backed volumes belonging to terminated pods + volume := volumeToMount.VolumeSpec.Volume + if (volume.EmptyDir == nil || volume.EmptyDir.Medium != api.StorageMediumMemory) && + volume.ConfigMap == nil && volume.Secret == nil { + continue + } } // Once a pod has been deleted from kubelet pod manager, do not delete diff --git a/test/e2e_node/BUILD b/test/e2e_node/BUILD index 8ed218398b..320ea78301 100644 --- a/test/e2e_node/BUILD +++ b/test/e2e_node/BUILD @@ -67,6 +67,7 @@ go_test( "restart_test.go", "runtime_conformance_test.go", "summary_test.go", + "volume_manager_test.go", ], library = "go_default_library", tags = ["automanaged"], diff --git a/test/e2e_node/volume_manager_test.go b/test/e2e_node/volume_manager_test.go new file mode 100644 index 0000000000..8e41a9230d --- /dev/null +++ b/test/e2e_node/volume_manager_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2016 The Kubernetes Authors. + +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 e2e_node + +import ( + "time" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/util/uuid" + "k8s.io/kubernetes/test/e2e/framework" + + "fmt" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = framework.KubeDescribe("Kubelet Volume Manager", func() { + f := framework.NewDefaultFramework("kubelet-volume-manager") + Describe("Volume Manager", func() { + Context("On terminatation of pod with memory backed volume", func() { + It("should remove the volume from the node", func() { + var ( + memoryBackedPod *api.Pod + volumeName string + ) + By("Creating a pod with a memory backed volume that exits success without restart", func() { + volumeName = "memory-volume" + memoryBackedPod = f.PodClient().Create(&api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "pod" + string(uuid.NewUUID()), + Namespace: f.Namespace.Name, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{ + { + Image: "gcr.io/google_containers/busybox:1.24", + Name: "container" + string(uuid.NewUUID()), + Command: []string{"sh", "-c", "echo"}, + VolumeMounts: []api.VolumeMount{ + { + Name: volumeName, + MountPath: "/tmp", + }, + }, + }, + }, + Volumes: []api.Volume{ + { + Name: volumeName, + VolumeSource: api.VolumeSource{ + EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory}, + }, + }, + }, + }, + }) + err := framework.WaitForPodSuccessInNamespace(f.ClientSet, memoryBackedPod.Name, f.Namespace.Name) + Expect(err).NotTo(HaveOccurred()) + }) + By("Verifying the memory backed volume was removed from node", func() { + volumePath := fmt.Sprintf("/tmp/%s/volumes/kubernetes.io~empty-dir/%s", string(memoryBackedPod.UID), volumeName) + var err error + for i := 0; i < 10; i++ { + // need to create a new verification pod on each pass since updates + //to the HostPath volume aren't propogated to the pod + pod := f.PodClient().Create(&api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "pod" + string(uuid.NewUUID()), + Namespace: f.Namespace.Name, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{ + { + Image: "gcr.io/google_containers/busybox:1.24", + Name: "container" + string(uuid.NewUUID()), + Command: []string{"sh", "-c", "if [ -d " + volumePath + " ]; then exit 1; fi;"}, + VolumeMounts: []api.VolumeMount{ + { + Name: "kubelet-pods", + MountPath: "/tmp", + }, + }, + }, + }, + Volumes: []api.Volume{ + { + Name: "kubelet-pods", + VolumeSource: api.VolumeSource{ + // TODO: remove hardcoded kubelet volume directory path + // framework.TestContext.KubeVolumeDir is currently not populated for node e2e + HostPath: &api.HostPathVolumeSource{Path: "/var/lib/kubelet/pods"}, + }, + }, + }, + }, + }) + err = framework.WaitForPodSuccessInNamespace(f.ClientSet, pod.Name, f.Namespace.Name) + gp := int64(1) + f.PodClient().Delete(pod.Name, &api.DeleteOptions{GracePeriodSeconds: &gp}) + if err == nil { + break + } + <-time.After(10 * time.Second) + } + Expect(err).NotTo(HaveOccurred()) + }) + }) + }) + }) +})