Merge pull request #50016 from yastij/photon-checkAttach

pkg/volume/photon_pd/attacher.go

@@ -67,14 +67,21 @@ func (attacher *photonPersistentDiskAttacher) Attach(spec *volume.Spec, nodeName
 		glog.Errorf("Photon Controller attacher: Attach failed to get volume source")
 		return "", err
 	}
+	attached, err := attacher.photonDisks.DiskIsAttached(volumeSource.PdID, nodeName)
 
-	glog.V(4).Infof("Photon Controller: Attach disk called for host %s", hostName)
-
-	// TODO: if disk is already attached?
-	err = attacher.photonDisks.AttachDisk(volumeSource.PdID, nodeName)
 	if err != nil {
-		glog.Errorf("Error attaching volume %q to node %q: %+v", volumeSource.PdID, nodeName, err)
-		return "", err
+		glog.Warningf("Photon Controller: couldn't check if disk is Attached for host %s, will try attach disk: %+v", hostName, err)
+		attached = false
+	}
+
+	if !attached {
+		glog.V(4).Infof("Photon Controller: Attach disk called for host %s", hostName)
+
+		err = attacher.photonDisks.AttachDisk(volumeSource.PdID, nodeName)
+		if err != nil {
+			glog.Errorf("Error attaching volume %q to node %q: %+v", volumeSource.PdID, nodeName, err)
+			return "", err
+		}
 	}
 
 	PdidWithNoHypens := strings.Replace(volumeSource.PdID, "-", "", -1)

pkg/volume/photon_pd/attacher_test.go

@@ -78,14 +78,14 @@ func TestAttachDetach(t *testing.T) {
 	nodeName := types.NodeName("instance")
 	readOnly := false
 	spec := createVolSpec(diskName, readOnly)
-	attachError := errors.New("Fake attach error")
 	detachError := errors.New("Fake detach error")
 	diskCheckError := errors.New("Fake DiskIsAttached error")
 	tests := []testcase{
 		// Successful Attach call
 		{
-			name:   "Attach_Positive",
-			attach: attachCall{diskName, nodeName, nil},
+			name:           "Attach_Positive",
+			diskIsAttached: diskIsAttachedCall{diskName, nodeName, false, diskCheckError},
+			attach:         attachCall{diskName, nodeName, nil},
 			test: func(testcase *testcase) (string, error) {
 				attacher := newAttacher(testcase)
 				return attacher.Attach(spec, nodeName)
@@ -93,15 +93,16 @@ func TestAttachDetach(t *testing.T) {
 			expectedDevice: "/dev/disk/by-id/wwn-0x000000000",
 		},
 
-		// Attach call fails
+		// Disk is already attached
 		{
-			name:   "Attach_Negative",
-			attach: attachCall{diskName, nodeName, attachError},
+			name:           "Attach_Positive_AlreadyAttached",
+			diskIsAttached: diskIsAttachedCall{diskName, nodeName, false, diskCheckError},
+			attach:         attachCall{diskName, nodeName, nil},
 			test: func(testcase *testcase) (string, error) {
 				attacher := newAttacher(testcase)
 				return attacher.Attach(spec, nodeName)
 			},
-			expectedError: attachError,
+			expectedDevice: "/dev/disk/by-id/wwn-0x000000000",
 		},
 
 		// Detach succeeds