Fix CSI volume detach when the volume is already detached.

pkg/volume/csi/BUILD

@@ -50,6 +50,7 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",

pkg/volume/csi/csi_attacher.go

@@ -385,6 +385,11 @@ func (c *csiAttacher) Detach(volumeName string, nodeName types.NodeName) error {
 	volID := parts[1]
 	attachID := getAttachmentName(volID, driverName, string(nodeName))
 	if err := c.k8s.StorageV1beta1().VolumeAttachments().Delete(attachID, nil); err != nil {
+		if apierrs.IsNotFound(err) {
+			// object deleted or never existed, done
+			glog.V(4).Info(log("VolumeAttachment object [%v] for volume [%v] not found, object deleted", attachID, volID))
+			return nil
+		}
 		glog.Error(log("detacher.Detach failed to delete VolumeAttachment [%s]: %v", attachID, err))
 		return err
 	}

pkg/volume/csi/csi_attacher_test.go

@@ -26,6 +26,7 @@ import (
 	storage "k8s.io/api/storage/v1beta1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	fakeclient "k8s.io/client-go/kubernetes/fake"
@@ -110,7 +111,7 @@ func TestAttacherAttach(t *testing.T) {
 	for i, tc := range testCases {
 		t.Logf("test case: %s", tc.name)
 
-		plug, fakeWatcher, tmpDir := newTestWatchPlugin(t)
+		plug, fakeWatcher, tmpDir, _ := newTestWatchPlugin(t)
 		defer os.RemoveAll(tmpDir)
 
 		attacher, err := plug.NewAttacher()
@@ -166,7 +167,7 @@ func TestAttacherAttach(t *testing.T) {
 
 func TestAttacherWaitForVolumeAttachment(t *testing.T) {
 
-	plug, fakeWatcher, tmpDir := newTestWatchPlugin(t)
+	plug, fakeWatcher, tmpDir, _ := newTestWatchPlugin(t)
 	defer os.RemoveAll(tmpDir)
 
 	attacher, err := plug.NewAttacher()
@@ -337,16 +338,33 @@ func TestAttacherDetach(t *testing.T) {
 		volID      string
 		attachID   string
 		shouldFail bool
+		reactor    func(action core.Action) (handled bool, ret runtime.Object, err error)
 	}{
 		{name: "normal test", volID: "vol-001", attachID: getAttachmentName("vol-001", testDriver, nodeName)},
 		{name: "normal test 2", volID: "vol-002", attachID: getAttachmentName("vol-002", testDriver, nodeName)},
-		{name: "object not found", volID: "vol-001", attachID: getAttachmentName("vol-002", testDriver, nodeName), shouldFail: true},
+		{name: "object not found", volID: "vol-non-existing", attachID: getAttachmentName("vol-003", testDriver, nodeName)},
+		{
+			name:       "API error",
+			volID:      "vol-004",
+			attachID:   getAttachmentName("vol-004", testDriver, nodeName),
+			shouldFail: true, // All other API errors should be propagated to caller
+			reactor: func(action core.Action) (handled bool, ret runtime.Object, err error) {
+				// return Forbidden to all DELETE requests
+				if action.Matches("delete", "volumeattachments") {
+					return true, nil, apierrs.NewForbidden(action.GetResource().GroupResource(), action.GetNamespace(), fmt.Errorf("mock error"))
+				}
+				return false, nil, nil
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Logf("running test: %v", tc.name)
-		plug, fakeWatcher, tmpDir := newTestWatchPlugin(t)
+		plug, fakeWatcher, tmpDir, client := newTestWatchPlugin(t)
 		defer os.RemoveAll(tmpDir)
+		if tc.reactor != nil {
+			client.PrependReactor("*", "*", tc.reactor)
+		}
 
 		attacher, err0 := plug.NewAttacher()
 		if err0 != nil {
@@ -391,7 +409,7 @@ func TestAttacherDetach(t *testing.T) {
 func TestAttacherGetDeviceMountPath(t *testing.T) {
 	// Setup
 	// Create a new attacher
-	plug, _, tmpDir := newTestWatchPlugin(t)
+	plug, _, tmpDir, _ := newTestWatchPlugin(t)
 	defer os.RemoveAll(tmpDir)
 	attacher, err0 := plug.NewAttacher()
 	if err0 != nil {
@@ -504,7 +522,7 @@ func TestAttacherMountDevice(t *testing.T) {
 
 		// Setup
 		// Create a new attacher
-		plug, fakeWatcher, tmpDir := newTestWatchPlugin(t)
+		plug, fakeWatcher, tmpDir, _ := newTestWatchPlugin(t)
 		defer os.RemoveAll(tmpDir)
 		attacher, err0 := plug.NewAttacher()
 		if err0 != nil {
@@ -619,7 +637,7 @@ func TestAttacherUnmountDevice(t *testing.T) {
 		t.Logf("Running test case: %s", tc.testName)
 		// Setup
 		// Create a new attacher
-		plug, _, tmpDir := newTestWatchPlugin(t)
+		plug, _, tmpDir, _ := newTestWatchPlugin(t)
 		defer os.RemoveAll(tmpDir)
 		attacher, err0 := plug.NewAttacher()
 		if err0 != nil {
@@ -677,7 +695,7 @@ func TestAttacherUnmountDevice(t *testing.T) {
 }
 
 // create a plugin mgr to load plugins and setup a fake client
-func newTestWatchPlugin(t *testing.T) (*csiPlugin, *watch.FakeWatcher, string) {
+func newTestWatchPlugin(t *testing.T) (*csiPlugin, *watch.FakeWatcher, string, *fakeclient.Clientset) {
 	tmpDir, err := utiltesting.MkTmpdir("csi-test")
 	if err != nil {
 		t.Fatalf("can't create temp dir: %v", err)
@@ -705,5 +723,5 @@ func newTestWatchPlugin(t *testing.T) (*csiPlugin, *watch.FakeWatcher, string) {
 		t.Fatalf("cannot assert plugin to be type csiPlugin")
 	}
 
-	return csiPlug, fakeWatcher, tmpDir
+	return csiPlug, fakeWatcher, tmpDir, fakeClient
 }