Mark detached only if no pending operations

To safely mark a volume detached when the volume controller manager is used.

An example of one such problem:

1. pod is created, volume is added to desired state of the world
2. reconciler process starts
3. reconciler starts MountVolume, which is kicked off asynchronously via
   operation_executor.go
4. MountVolume mounts the volume, but hasn't yet marked it as mounted
5. pod is deleted, volume is removed from desired state of the world
6. reconciler detects volume is no longer in desired state of world,
   removes it from volumes in use
7. MountVolume tries to mark volume in use, throws an error because
   volume is no longer in actual state of world list.
8. controller-manager tries to detach the volume, this fails because it
   is still mounted to the OS.
9. EBS gets stuck indefinitely in busy state trying to detach.
This commit is contained in:
James Ravn 2017-01-30 15:08:43 +00:00
parent d46dc10b49
commit 9992bd23c2
3 changed files with 18 additions and 14 deletions

View File

@ -314,7 +314,9 @@ func (rc *reconciler) reconcile() {
// Ensure devices that should be detached/unmounted are detached/unmounted. // Ensure devices that should be detached/unmounted are detached/unmounted.
for _, attachedVolume := range rc.actualStateOfWorld.GetUnmountedVolumes() { for _, attachedVolume := range rc.actualStateOfWorld.GetUnmountedVolumes() {
if !rc.desiredStateOfWorld.VolumeExists(attachedVolume.VolumeName) { // Check IsOperationPending to avoid marking a volume as detached if it's in the process of mounting.
if !rc.desiredStateOfWorld.VolumeExists(attachedVolume.VolumeName) &&
!rc.operationExecutor.IsOperationPending(attachedVolume.VolumeName, nestedpendingoperations.EmptyUniquePodName) {
if attachedVolume.GloballyMounted { if attachedVolume.GloballyMounted {
// Volume is globally mounted to device, unmount it // Volume is globally mounted to device, unmount it
glog.V(12).Infof("Attempting to start UnmountDevice for volume %q (spec.Name: %q)", glog.V(12).Infof("Attempting to start UnmountDevice for volume %q (spec.Name: %q)",
@ -341,11 +343,13 @@ func (rc *reconciler) reconcile() {
} }
} else { } else {
// Volume is attached to node, detach it // Volume is attached to node, detach it
// Kubelet not responsible for detaching or this volume has a non-attachable volume plugin.
if rc.controllerAttachDetachEnabled || !attachedVolume.PluginIsAttachable { if rc.controllerAttachDetachEnabled || !attachedVolume.PluginIsAttachable {
// Kubelet not responsible for detaching or this volume has a non-attachable volume plugin, rc.actualStateOfWorld.MarkVolumeAsDetached(attachedVolume.VolumeName, attachedVolume.NodeName)
// so just remove it to actualStateOfWorld without attach. glog.Infof("Detached volume %q (spec.Name: %q) devicePath: %q",
rc.actualStateOfWorld.MarkVolumeAsDetached( attachedVolume.VolumeName,
attachedVolume.VolumeName, rc.nodeName) attachedVolume.VolumeSpec.Name(),
attachedVolume.DevicePath)
} else { } else {
// Only detach if kubelet detach is enabled // Only detach if kubelet detach is enabled
glog.V(12).Infof("Attempting to start DetachVolume for volume %q (spec.Name: %q)", glog.V(12).Infof("Attempting to start DetachVolume for volume %q (spec.Name: %q)",

View File

@ -36,11 +36,11 @@ import (
) )
const ( const (
// emptyUniquePodName is a UniquePodName for empty string. // EmptyUniquePodName is a UniquePodName for empty string.
emptyUniquePodName types.UniquePodName = types.UniquePodName("") EmptyUniquePodName types.UniquePodName = types.UniquePodName("")
// emptyUniqueVolumeName is a UniqueVolumeName for empty string // EmptyUniqueVolumeName is a UniqueVolumeName for empty string
emptyUniqueVolumeName v1.UniqueVolumeName = v1.UniqueVolumeName("") EmptyUniqueVolumeName v1.UniqueVolumeName = v1.UniqueVolumeName("")
) )
// NestedPendingOperations defines the supported set of operations. // NestedPendingOperations defines the supported set of operations.
@ -160,7 +160,7 @@ func (grm *nestedPendingOperations) isOperationExists(
podName types.UniquePodName) (bool, int) { podName types.UniquePodName) (bool, int) {
// If volumeName is empty, operation can be executed concurrently // If volumeName is empty, operation can be executed concurrently
if volumeName == emptyUniqueVolumeName { if volumeName == EmptyUniqueVolumeName {
return false, -1 return false, -1
} }
@ -170,8 +170,8 @@ func (grm *nestedPendingOperations) isOperationExists(
continue continue
} }
if previousOp.podName != emptyUniquePodName && if previousOp.podName != EmptyUniquePodName &&
podName != emptyUniquePodName && podName != EmptyUniquePodName &&
previousOp.podName != podName { previousOp.podName != podName {
// No match, keep searching // No match, keep searching
continue continue
@ -274,7 +274,7 @@ func (grm *nestedPendingOperations) Wait() {
func getOperationName( func getOperationName(
volumeName v1.UniqueVolumeName, podName types.UniquePodName) string { volumeName v1.UniqueVolumeName, podName types.UniquePodName) string {
podNameStr := "" podNameStr := ""
if podName != emptyUniquePodName { if podName != EmptyUniquePodName {
podNameStr = fmt.Sprintf(" (%q)", podName) podNameStr = fmt.Sprintf(" (%q)", podName)
} }

View File

@ -411,7 +411,7 @@ func (oe *operationExecutor) MountVolume(
return err return err
} }
podName := volumetypes.UniquePodName("") podName := nestedpendingoperations.EmptyUniquePodName
// TODO: remove this -- not necessary // TODO: remove this -- not necessary
if !volumeToMount.PluginIsAttachable { if !volumeToMount.PluginIsAttachable {
// Non-attachable volume plugins can execute mount for multiple pods // Non-attachable volume plugins can execute mount for multiple pods