Modify Attach method to return device path

pkg/volume/aws_ebs/attacher.go

@@ -41,38 +41,28 @@ func (plugin *awsElasticBlockStorePlugin) NewAttacher() (volume.Attacher, error)
 	return &awsElasticBlockStoreAttacher{host: plugin.host}, nil
 }
 
-func (attacher *awsElasticBlockStoreAttacher) Attach(spec *volume.Spec, hostName string) error {
+func (attacher *awsElasticBlockStoreAttacher) Attach(spec *volume.Spec, hostName string) (string, error) {
 	volumeSource, readOnly, err := getVolumeSource(spec)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	volumeID := volumeSource.VolumeID
 
 	awsCloud, err := getCloudProvider(attacher.host.GetCloudProvider())
 	if err != nil {
-		return err
+		return "", err
 	}
 
-	attached, err := awsCloud.DiskIsAttached(volumeID, hostName)
+	// awsCloud.AttachDisk checks if disk is already attached to node and
+	// succeeds in that case, so no need to do that separately.
+	devicePath, err := awsCloud.AttachDisk(volumeID, hostName, readOnly)
 	if err != nil {
-		// Log error and continue with attach
-		glog.Errorf(
-			"Error checking if volume (%q) is already attached to current node (%q). Will continue and try attach anyway. err=%v",
-			volumeID, hostName, err)
-	}
-
-	if err == nil && attached {
-		// Volume is already attached to node.
-		glog.Infof("Attach operation is successful. volume %q is already attached to node %q.", volumeID, hostName)
-		return nil
-	}
-
-	if _, err = awsCloud.AttachDisk(volumeID, hostName, readOnly); err != nil {
 		glog.Errorf("Error attaching volume %q: %+v", volumeID, err)
-		return err
+		return "", err
 	}
-	return nil
+
+	return devicePath, nil
 }
 
 func (attacher *awsElasticBlockStoreAttacher) WaitForAttach(spec *volume.Spec, timeout time.Duration) (string, error) {

pkg/volume/cinder/attacher.go

@@ -45,25 +45,25 @@ func (plugin *cinderPlugin) NewAttacher() (volume.Attacher, error) {
 	return &cinderDiskAttacher{host: plugin.host}, nil
 }
 
-func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) error {
+func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) (string, error) {
 	volumeSource, _, err := getVolumeSource(spec)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	volumeID := volumeSource.VolumeID
 
 	cloud, err := getCloudProvider(attacher.host.GetCloudProvider())
 	if err != nil {
-		return err
+		return "", err
 	}
 	instances, res := cloud.Instances()
 	if !res {
-		return fmt.Errorf("failed to list openstack instances")
+		return "", fmt.Errorf("failed to list openstack instances")
 	}
 	instanceid, err := instances.InstanceID(hostName)
 	if err != nil {
-		return err
+		return "", err
 	}
 	if ind := strings.LastIndex(instanceid, "/"); ind >= 0 {
 		instanceid = instanceid[(ind + 1):]
@@ -79,15 +79,20 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) e
 	if err == nil && attached {
 		// Volume is already attached to node.
 		glog.Infof("Attach operation is successful. volume %q is already attached to node %q.", volumeID, instanceid)
-		return nil
+	} else {
+		_, err = cloud.AttachDisk(instanceid, volumeID)
+		if err != nil {
+			glog.Infof("attach volume %q to instance %q gets %v", volumeID, instanceid, err)
+		}
 	}
 
-	_, err = cloud.AttachDisk(instanceid, volumeID)
-	if err != nil {
-		glog.Infof("attach volume %q to instance %q gets %v", volumeID, instanceid, err)
-	}
 	glog.Infof("attached volume %q to instance %q", volumeID, instanceid)
-	return err
+	devicePath, err := cloud.GetAttachmentDiskPath(instanceid, volumeID)
+	if err != nil {
+		return "", err
+	}
+
+	return devicePath, err
 }
 
 func (attacher *cinderDiskAttacher) WaitForAttach(spec *volume.Spec, timeout time.Duration) (string, error) {

pkg/volume/gce_pd/attacher.go

@@ -60,10 +60,10 @@ func (plugin *gcePersistentDiskPlugin) NewAttacher() (volume.Attacher, error) {
 // Callers are responsible for retryinging on failure.
 // Callers are responsible for thread safety between concurrent attach and
 // detach operations.
-func (attacher *gcePersistentDiskAttacher) Attach(spec *volume.Spec, hostName string) error {
+func (attacher *gcePersistentDiskAttacher) Attach(spec *volume.Spec, hostName string) (string, error) {
 	volumeSource, readOnly, err := getVolumeSource(spec)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	pdName := volumeSource.PDName
@@ -79,15 +79,14 @@ func (attacher *gcePersistentDiskAttacher) Attach(spec *volume.Spec, hostName st
 	if err == nil && attached {
 		// Volume is already attached to node.
 		glog.Infof("Attach operation is successful. PD %q is already attached to node %q.", pdName, hostName)
-		return nil
+	} else {
+		if err := attacher.gceDisks.AttachDisk(pdName, hostName, readOnly); err != nil {
+			glog.Errorf("Error attaching PD %q to node %q: %+v", pdName, hostName, err)
+			return "", err
+		}
 	}
 
-	if err = attacher.gceDisks.AttachDisk(pdName, hostName, readOnly); err != nil {
+	return path.Join(diskByIdPath, diskGooglePrefix+pdName), nil
-		glog.Errorf("Error attaching PD %q to node %q: %+v", pdName, hostName, err)
-		return err
-	}
-
-	return nil
 }
 
 func (attacher *gcePersistentDiskAttacher) WaitForAttach(spec *volume.Spec, timeout time.Duration) (string, error) {

pkg/volume/gce_pd/attacher_test.go

@@ -18,6 +18,7 @@ package gce_pd
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -86,7 +87,11 @@ func TestAttachDetach(t *testing.T) {
 			attach:         attachCall{diskName, instanceID, readOnly, nil},
 			test: func(testcase *testcase) error {
 				attacher := newAttacher(testcase)
-				return attacher.Attach(spec, instanceID)
+				devicePath, err := attacher.Attach(spec, instanceID)
+				if devicePath != "/dev/disk/by-id/google-disk" {
+					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
+				}
+				return err
 			},
 		},
 
@@ -96,7 +101,11 @@ func TestAttachDetach(t *testing.T) {
 			diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil},
 			test: func(testcase *testcase) error {
 				attacher := newAttacher(testcase)
-				return attacher.Attach(spec, instanceID)
+				devicePath, err := attacher.Attach(spec, instanceID)
+				if devicePath != "/dev/disk/by-id/google-disk" {
+					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
+				}
+				return err
 			},
 		},
 
@@ -107,7 +116,11 @@ func TestAttachDetach(t *testing.T) {
 			attach:         attachCall{diskName, instanceID, readOnly, nil},
 			test: func(testcase *testcase) error {
 				attacher := newAttacher(testcase)
-				return attacher.Attach(spec, instanceID)
+				devicePath, err := attacher.Attach(spec, instanceID)
+				if devicePath != "/dev/disk/by-id/google-disk" {
+					return fmt.Errorf("devicePath incorrect. Expected<\"/dev/disk/by-id/google-disk\"> Actual: <%q>", devicePath)
+				}
+				return err
 			},
 		},
 
@@ -118,7 +131,11 @@ func TestAttachDetach(t *testing.T) {
 			attach:         attachCall{diskName, instanceID, readOnly, attachError},
 			test: func(testcase *testcase) error {
 				attacher := newAttacher(testcase)
-				return attacher.Attach(spec, instanceID)
+				devicePath, err := attacher.Attach(spec, instanceID)
+				if devicePath != "" {
+					return fmt.Errorf("devicePath incorrect. Expected<\"\"> Actual: <%q>", devicePath)
+				}
+				return err
 			},
 			expectedReturn: attachError,
 		},

pkg/volume/testing/testing.go

@@ -358,11 +358,11 @@ func (fv *FakeVolume) TearDownAt(dir string) error {
 	return os.RemoveAll(dir)
 }
 
-func (fv *FakeVolume) Attach(spec *Spec, hostName string) error {
+func (fv *FakeVolume) Attach(spec *Spec, hostName string) (string, error) {
 	fv.Lock()
 	defer fv.Unlock()
 	fv.AttachCallCount++
-	return nil
+	return "", nil
 }
 
 func (fv *FakeVolume) GetAttachCallCount() int {

pkg/volume/util/operationexecutor/operation_executor.go

@@ -385,7 +385,7 @@ func (oe *operationExecutor) generateAttachVolumeFunc(
 
 	return func() error {
 		// Execute attach
-		attachErr := volumeAttacher.Attach(
+		_, attachErr := volumeAttacher.Attach(
 			volumeToAttach.VolumeSpec, volumeToAttach.NodeName)
 
 		if attachErr != nil {

pkg/volume/volume.go

@@ -134,8 +134,10 @@ type Deleter interface {
 
 // Attacher can attach a volume to a node.
 type Attacher interface {
-	// Attach the volume specified by the given spec to the given host
+	// Attaches the volume specified by the given spec to the given host.
-	Attach(spec *Spec, hostName string) error
+	// On success, returns the device path where the device was attache don the
+	// node.
+	Attach(spec *Spec, hostName string) (string, error)
 
 	// WaitForAttach blocks until the device is attached to this
 	// node. If it successfully attaches, the path to the device