Ensure that FindMountedVolume returns ("", nil) when not found

pkg/volumes/aws/volumes.go

@@ -250,6 +250,7 @@ func (a *AWSVolumes) FindMountedVolume(volume *volumes.Volume) (string, error) {
 		glog.V(2).Infof("volume %s not mounted at %s", volume.ProviderID, expected)
 	}
 
+	// When not found, the interface says we return ("", nil)
 	return "", nil
 }
 

pkg/volumes/do/volumes.go

@@ -349,7 +349,8 @@ func (a *DOVolumes) FindMountedVolume(volume *volumes.Volume) (string, error) {
 		return "", fmt.Errorf("error checking for device %q: %v", device, err)
 	}
 
-	return "", fmt.Errorf("error checking for device %q: %v", device, err)
+	// When not found, the interface says we return ("", nil)
+	return "", nil
 }
 
 // AttachVolume attaches the specified volume to this instance, returning the mountpoint & nil if successful

pkg/volumes/gce/volumes.go

@@ -328,6 +328,7 @@ func (g *GCEVolumes) FindMountedVolume(volume *volumes.Volume) (string, error) {
 		return device, nil
 	}
 	if os.IsNotExist(err) {
+		// When not found, the interface says we return ("", nil)
 		return "", nil
 	}
 	return "", fmt.Errorf("error checking for device %q: %v", device, err)

pkg/volumes/openstack/volumes.go

@@ -327,7 +327,7 @@ func (stack *OpenstackVolumes) FindVolumes() ([]*volumes.Volume, error) {
 	return volumes, nil
 }
 
-func GetDevicePath(volumeID string) (string, error) {
+func findDevicePath(volumeID string) (string, error) {
 	// Build a list of candidate device paths
 	candidateDeviceNodes := []string{
 		// KVM
@@ -351,7 +351,7 @@ func GetDevicePath(volumeID string) (string, error) {
 		}
 	}
 
-	return "", fmt.Errorf("Failed to find device for the volumeID: %q", volumeID)
+	return "", nil
 }
 
 // probeVolume probes volume in compute
@@ -379,44 +379,50 @@ func probeVolume() error {
 
 // FindMountedVolume implements Volumes::FindMountedVolume
 func (_ *OpenstackVolumes) FindMountedVolume(volume *volumes.Volume) (string, error) {
-	// wait for 2.5min is the volume attached and path found
+	// wait for 2.5min max for the volume to be attached and path found
 	var backoff = volumes.Backoff{
 		Duration: 6 * time.Second,
-		Steps:    25,
+		Attempts: 25,
 	}
 
 	device := ""
-	err := volumes.SleepUntil(backoff, func() (bool, error) {
-		devpath, err := GetDevicePath(volume.ProviderID)
+	done, err := volumes.SleepUntil(backoff, func() (bool, error) {
+		devpath, err := findDevicePath(volume.ProviderID)
 		if err != nil {
-			glog.V(2).Infof("%v... retrying", err)
-			return false, nil
+			return false, err
 		}
-		if devpath == "" {
-			perr := probeVolume()
-			if perr != nil {
-				glog.V(2).Infof("Could not find device path and error probing for volume %v... retrying", perr)
-			} else {
-				glog.V(2).Infof("Could not find device path for volume... retrying")
-			}
-			return false, nil
+		if devpath != "" {
+			device = devpath
+			return true, nil
+		}
+
+		glog.V(2).Infof("Could not find device path for volume; scanning buses")
+		if err := probeVolume(); err != nil {
+			glog.V(2).Infof("Error scanning buses: %v", err)
 		}
-		device = devpath
-		return true, nil
+
+		return false, nil
 	})
-	if err != nil || device == "" {
+	if err != nil {
 		// TODO: in this case we must make ensure that the volume is not attached to machine?
-		return "", fmt.Errorf("failed to find device path for volume")
+		return "", fmt.Errorf("failed to find device path for volume %q: %v", volume.ProviderID, err)
 	}
 
-	_, err = os.Stat(volumes.PathFor(device))
-	if err == nil {
-		return device, nil
-	}
-	if os.IsNotExist(err) {
+	// If we didn't find the volume, the contract says we should return "", nil
+	if !done || device == "" {
 		return "", nil
 	}
-	return "", fmt.Errorf("error checking for device %q: %v", device, err)
+
+	if _, err := os.Stat(volumes.PathFor(device)); err != nil {
+		if os.IsNotExist(err) {
+			// Unexpected, but treat as not-found
+			glog.Warningf("did not find device %q at expected path %q", device, volumes.PathFor(device))
+			return "", nil
+		}
+		return "", fmt.Errorf("error checking for device %q: %v", device, err)
+	}
+
+	return device, nil
 }
 
 // AttachVolume attaches the specified volume to this instance, returning the mountpoint & nil if successful

pkg/volumes/retry.go

@@ -17,30 +17,36 @@ limitations under the License.
 package volumes
 
 import (
-	"fmt"
 	"time"
+
+	"github.com/golang/glog"
 )
 
 // Backoff ...
 type Backoff struct {
 	Duration time.Duration
-	Steps    int
+	Attempts int
 }
 
 type conditionFunc func() (done bool, err error)
 
 // SleepUntil is for retrying functions
-func SleepUntil(backoff Backoff, condition conditionFunc) error {
-	for backoff.Steps > 0 {
-		if ok, err := condition(); err != nil || ok {
-			return err
+// It calls condition repeatedly, with backoff.
+// When condition returns done==true or a err != nil; this function returns those values.
+// If first we exhaust the backoff Attempts; we return false, nil
+func SleepUntil(backoff Backoff, condition conditionFunc) (done bool, err error) {
+	for backoff.Attempts > 0 {
+		if done, err := condition(); err != nil || done {
+			return done, err
 		}
-		if backoff.Steps == 1 {
+		backoff.Attempts--
+		if backoff.Attempts == 0 {
 			break
 		}
-		backoff.Steps--
+		if err != nil {
+			glog.Infof("retrying after error: %v", err)
+		}
 		time.Sleep(backoff.Duration)
-
 	}
-	return fmt.Errorf("Timed out waiting for the condition")
+	return false, nil
 }