From 71e4449e1768f82e6c2187d94fc97361c6f49d89 Mon Sep 17 00:00:00 2001
From: Jan Safranek <jsafrane@redhat.com>
Date: Mon, 30 Apr 2018 17:19:12 +0200
Subject: [PATCH] Return attach error to A/D controller.

WaitForAttach runs on nodes, not in A/D controller.
---
 pkg/volume/csi/csi_attacher.go      | 10 ++-------
 pkg/volume/csi/csi_attacher_test.go | 34 +++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/pkg/volume/csi/csi_attacher.go b/pkg/volume/csi/csi_attacher.go
index ae6a267c00..48bf74f216 100644
--- a/pkg/volume/csi/csi_attacher.go
+++ b/pkg/volume/csi/csi_attacher.go
@@ -102,17 +102,11 @@ func (c *csiAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string
 		glog.V(4).Info(log("attachment [%v] for volume [%v] created successfully", attachID, csiSource.VolumeHandle))
 	}
 
-	// probe for attachment update here
-	// NOTE: any error from waiting for attachment is logged only.  This is because
-	// the primary intent of the enclosing method is to create VolumeAttachment.
-	// DONOT return that error here as it is mitigated in attacher.WaitForAttach.
-	volAttachmentOK := true
 	if _, err := c.waitForVolumeAttachment(csiSource.VolumeHandle, attachID, csiTimeout); err != nil {
-		volAttachmentOK = false
-		glog.Error(log("attacher.Attach attempted to wait for attachment to be ready, but failed with: %v", err))
+		return "", err
 	}
 
-	glog.V(4).Info(log("attacher.Attach finished OK with VolumeAttachment verified=%t: attachment object [%s]", volAttachmentOK, attachID))
+	glog.V(4).Info(log("attacher.Attach finished OK with VolumeAttachment object [%s]", attachID))
 
 	return attachID, nil
 }
diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go
index e8c227e306..f9224cbcc1 100644
--- a/pkg/volume/csi/csi_attacher_test.go
+++ b/pkg/volume/csi/csi_attacher_test.go
@@ -59,12 +59,13 @@ func makeTestAttachment(attachID, nodeName, pvName string) *storage.VolumeAttach
 func TestAttacherAttach(t *testing.T) {
 
 	testCases := []struct {
-		name       string
-		nodeName   string
-		driverName string
-		volumeName string
-		attachID   string
-		shouldFail bool
+		name                string
+		nodeName            string
+		driverName          string
+		volumeName          string
+		attachID            string
+		injectAttacherError bool
+		shouldFail          bool
 	}{
 		{
 			name:       "test ok 1",
@@ -104,6 +105,15 @@ func TestAttacherAttach(t *testing.T) {
 			attachID:   getAttachmentName("vol02", "driver02", "node02"),
 			shouldFail: true,
 		},
+		{
+			name:                "attacher error",
+			nodeName:            "node02",
+			driverName:          "driver02",
+			volumeName:          "vol02",
+			attachID:            getAttachmentName("vol02", "driver02", "node02"),
+			injectAttacherError: true,
+			shouldFail:          true,
+		},
 	}
 
 	// attacher loop
@@ -127,6 +137,9 @@ func TestAttacherAttach(t *testing.T) {
 			if !fail && err != nil {
 				t.Errorf("expecting no failure, but got err: %v", err)
 			}
+			if fail && err == nil {
+				t.Errorf("expecting failure, but got no err")
+			}
 			if attachID != id && !fail {
 				t.Errorf("expecting attachID %v, got %v", id, attachID)
 			}
@@ -154,7 +167,14 @@ func TestAttacherAttach(t *testing.T) {
 		if attach == nil {
 			t.Logf("attachment not found for id:%v", tc.attachID)
 		} else {
-			attach.Status.Attached = true
+			if tc.injectAttacherError {
+				attach.Status.Attached = false
+				attach.Status.AttachError = &storage.VolumeError{
+					Message: "attacher error",
+				}
+			} else {
+				attach.Status.Attached = true
+			}
 			_, err = csiAttacher.k8s.StorageV1beta1().VolumeAttachments().Update(attach)
 			if err != nil {
 				t.Error(err)