Fix issue with race condition during pod deletion

This PR fixes two issues
1. When desired_state_populator removes podvolume state, it should check
whether the actual state already has the volume before deleting it to
make sure actual state has a chance to add the volume into the state
2. When checking podVolume still exists, it not only checks the actual
state, but also the volume disk directory because actual state might not
reflect the real world when kubelet starts.
This commit is contained in:
Jing Xu 2018-03-12 16:11:43 -07:00
parent a1c693418c
commit 9bd006de40
5 changed files with 37 additions and 2 deletions

View File

@ -274,6 +274,24 @@ func (kl *Kubelet) getPodVolumePathListFromDisk(podUID types.UID) ([]string, err
return volumes, nil
}
func (kl *Kubelet) getMountedVolumePathListFromDisk(podUID types.UID) ([]string, error) {
mountedVolumes := []string{}
volumePaths, err := kl.getPodVolumePathListFromDisk(podUID)
if err != nil {
return mountedVolumes, err
}
for _, volumePath := range volumePaths {
isNotMount, err := kl.mounter.IsLikelyNotMountPoint(volumePath)
if err != nil {
return mountedVolumes, err
}
if !isNotMount {
mountedVolumes = append(mountedVolumes, volumePath)
}
}
return mountedVolumes, nil
}
// GetVersionInfo returns information about the version of cAdvisor in use.
func (kl *Kubelet) GetVersionInfo() (*cadvisorapiv1.VersionInfo, error) {
return kl.cadvisor.VersionInfo()

View File

@ -57,6 +57,18 @@ func (kl *Kubelet) podVolumesExist(podUID types.UID) bool {
volumetypes.UniquePodName(podUID)); len(mountedVolumes) > 0 {
return true
}
// TODO: This checks pod volume paths and whether they are mounted. If checking returns error, podVolumesExist will return true
// which means we consider volumes might exist and requires further checking.
// There are some volume plugins such as flexvolume might not have mounts. See issue #61229
volumePaths, err := kl.getMountedVolumePathListFromDisk(podUID)
if err != nil {
glog.Errorf("pod %q found, but error %v occurred during checking mounted volumes from disk", podUID, err)
return true
}
if len(volumePaths) > 0 {
glog.V(4).Infof("pod %q found, but volumes are still mounted on disk %v", podUID, volumePaths)
return true
}
return false
}

View File

@ -245,6 +245,10 @@ func (dswp *desiredStateOfWorldPopulator) findAndRemoveDeletedPods() {
continue
}
if !dswp.actualStateOfWorld.VolumeExists(volumeToMount.VolumeName) && podExists {
glog.V(4).Infof(volumeToMount.GenerateMsgDetailed("Actual state has not yet has this information skip removing volume from desired state", ""))
continue
}
glog.V(4).Infof(volumeToMount.GenerateMsgDetailed("Removing volume from desired state", ""))
dswp.desiredStateOfWorld.DeletePodFromVolume(

View File

@ -111,6 +111,7 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods(t *testing.T) {
}
podGet.Status.Phase = v1.PodFailed
fakePodManager.DeletePod(pod)
//pod is added to fakePodManager but fakeRuntime can not get the pod,so here findAndRemoveDeletedPods() will remove the pod and volumes it is mounted
dswp.findAndRemoveDeletedPods()
@ -220,7 +221,7 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods_Valid_Block_VolumeDevices(t
t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace)
}
podGet.Status.Phase = v1.PodFailed
fakePodManager.DeletePod(pod)
//pod is added to fakePodManager but fakeRuntime can not get the pod,so here findAndRemoveDeletedPods() will remove the pod and volumes it is mounted
dswp.findAndRemoveDeletedPods()

View File

@ -455,7 +455,7 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
// Check existence of mount point for filesystem volume or symbolic link for block volume
isExist, checkErr := rc.operationExecutor.CheckVolumeExistenceOperation(volumeSpec, volume.mountPath, volumeSpec.Name(), rc.mounter, uniqueVolumeName, volume.podName, pod.UID, attachablePlugin)
if checkErr != nil {
return nil, err
return nil, checkErr
}
// If mount or symlink doesn't exist, volume reconstruction should be failed
if !isExist {