Fix CSI volume detach when the volume is already detached.

"NotFound" error should be treated as successful detach.
pull/8/head
Jan Safranek 2018-05-04 12:22:15 +02:00
parent 456b56a2fb
commit a884a00d30
3 changed files with 33 additions and 9 deletions

View File

@ -48,6 +48,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",

View File

@ -384,6 +384,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
}

View File

@ -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()
@ -216,7 +217,7 @@ func TestAttacherWaitForVolumeAttachment(t *testing.T) {
}
for i, tc := range testCases {
plug, fakeWatcher, tmpDir := newTestWatchPlugin(t)
plug, fakeWatcher, tmpDir, _ := newTestWatchPlugin(t)
defer os.RemoveAll(tmpDir)
attacher, err := plug.NewAttacher()
@ -331,16 +332,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 {
@ -385,7 +403,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 {
@ -498,7 +516,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 {
@ -620,7 +638,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 {
@ -678,7 +696,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.RaceFreeFakeWatcher, string) {
func newTestWatchPlugin(t *testing.T) (*csiPlugin, *watch.RaceFreeFakeWatcher, string, *fakeclient.Clientset) {
tmpDir, err := utiltesting.MkTmpdir("csi-test")
if err != nil {
t.Fatalf("can't create temp dir: %v", err)
@ -706,5 +724,5 @@ func newTestWatchPlugin(t *testing.T) (*csiPlugin, *watch.RaceFreeFakeWatcher, s
t.Fatalf("cannot assert plugin to be type csiPlugin")
}
return csiPlug, fakeWatcher, tmpDir
return csiPlug, fakeWatcher, tmpDir, fakeClient
}