Merge pull request #30737 from saad-ali/fix29358Round2

pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go

@@ -67,7 +67,7 @@ func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error {
 				"Could not update node status. Failed to find node %q in NodeInformer cache. %v",
 				nodeName,
 				err)
-			return nil
+			continue
 		}
 
 		node, ok := nodeObj.(*api.Node)

pkg/volume/util/operationexecutor/operation_executor.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/types"
 	"k8s.io/kubernetes/pkg/util/mount"
@@ -544,9 +545,52 @@ func (oe *operationExecutor) generateDetachVolumeFunc(
 
 	return func() error {
 		if verifySafeToDetach {
+			safeToDetachErr := oe.verifyVolumeIsSafeToDetach(volumeToDetach)
+			if safeToDetachErr != nil {
+				// On failure, return error. Caller will log and retry.
+				return err
+			}
+		}
+
+		// Execute detach
+		detachErr := volumeDetacher.Detach(volumeName, volumeToDetach.NodeName)
+		if detachErr != nil {
+			// On failure, return error. Caller will log and retry.
+			return fmt.Errorf(
+				"DetachVolume.Detach failed for volume %q (spec.Name: %q) from node %q with: %v",
+				volumeToDetach.VolumeName,
+				volumeToDetach.VolumeSpec.Name(),
+				volumeToDetach.NodeName,
+				detachErr)
+		}
+
+		glog.Infof(
+			"DetachVolume.Detach succeeded for volume %q (spec.Name: %q) from node %q.",
+			volumeToDetach.VolumeName,
+			volumeToDetach.VolumeSpec.Name(),
+			volumeToDetach.NodeName)
+
+		// Update actual state of world
+		actualStateOfWorld.MarkVolumeAsDetached(
+			volumeToDetach.VolumeName, volumeToDetach.NodeName)
+
+		return nil
+	}, nil
+}
+
+func (oe *operationExecutor) verifyVolumeIsSafeToDetach(
+	volumeToDetach AttachedVolume) error {
 	// Fetch current node object
 	node, fetchErr := oe.kubeClient.Core().Nodes().Get(volumeToDetach.NodeName)
 	if fetchErr != nil {
+		if errors.IsNotFound(fetchErr) {
+			glog.Warningf("Node %q not found on API server. DetachVolume will skip safe to detach check.",
+				volumeToDetach.NodeName,
+				volumeToDetach.VolumeName,
+				volumeToDetach.VolumeSpec.Name())
+			return nil
+		}
+
 		// On failure, return error. Caller will log and retry.
 		return fmt.Errorf(
 			"DetachVolume failed fetching node from API server for volume %q (spec.Name: %q) from node %q with: %v",
@@ -574,37 +618,12 @@ func (oe *operationExecutor) generateDetachVolumeFunc(
 		}
 	}
 
-			// Volume not attached, return error. Caller will log and retry.
+	// Volume is not marked as in use by node
 	glog.Infof("Verified volume is safe to detach for volume %q (spec.Name: %q) from node %q.",
 		volumeToDetach.VolumeName,
 		volumeToDetach.VolumeSpec.Name(),
 		volumeToDetach.NodeName)
-		}
-
-		// Execute detach
-		detachErr := volumeDetacher.Detach(volumeName, volumeToDetach.NodeName)
-		if detachErr != nil {
-			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
-				"DetachVolume.Detach failed for volume %q (spec.Name: %q) from node %q with: %v",
-				volumeToDetach.VolumeName,
-				volumeToDetach.VolumeSpec.Name(),
-				volumeToDetach.NodeName,
-				detachErr)
-		}
-
-		glog.Infof(
-			"DetachVolume.Detach succeeded for volume %q (spec.Name: %q) from node %q.",
-			volumeToDetach.VolumeName,
-			volumeToDetach.VolumeSpec.Name(),
-			volumeToDetach.NodeName)
-
-		// Update actual state of world
-		actualStateOfWorld.MarkVolumeAsDetached(
-			volumeToDetach.VolumeName, volumeToDetach.NodeName)
-
 	return nil
-	}, nil
 }
 
 func (oe *operationExecutor) generateMountVolumeFunc(