Merge pull request #735 from andyzhangx/fix-archive-dir-error2

fix: archive dir error#2

pkg/nfs/controllerserver.go

@@ -236,8 +236,6 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error())
 		}
 		defer func() {
-			// make sure archiving is completed before unmounting
-			time.Sleep(time.Second * 2)
 			if err = cs.internalUnmount(ctx, nfsVol); err != nil {
 				klog.Warningf("failed to unmount nfs server: %v", err.Error())
 			}
@@ -263,6 +261,11 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			if err = os.Rename(internalVolumePath, archivedInternalVolumePath); err != nil {
 				return nil, status.Errorf(codes.Internal, "archive subdirectory(%s, %s) failed with %v", internalVolumePath, archivedInternalVolumePath, err.Error())
 			}
+			// make sure internalVolumePath does not exist with 1 minute timeout
+			if err = waitForPathNotExistWithTimeout(internalVolumePath, time.Minute); err != nil {
+				return nil, status.Errorf(codes.Internal, "DeleteVolume: internalVolumePath(%s) still exists after 1 minute", internalVolumePath)
+			}
+			klog.V(2).Infof("archived subdirectory %s --> %s", internalVolumePath, archivedInternalVolumePath)
 		} else {
 			// delete subdirectory under base-dir
 			klog.V(2).Infof("removing subdirectory at %v", internalVolumePath)
@@ -365,12 +368,12 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 
 	srcPath := getInternalVolumePath(cs.Driver.workingMountDir, srcVol)
 	dstPath := filepath.Join(snapInternalVolPath, snapshot.archiveName())
-	klog.V(2).Infof("archiving %v -> %v", srcPath, dstPath)
+	klog.V(2).Infof("tar %v -> %v", srcPath, dstPath)
 	out, err := exec.Command("tar", "-C", srcPath, "-czvf", dstPath, ".").CombinedOutput()
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to create archive for snapshot: %v: %v", err, string(out))
 	}
-	klog.V(2).Infof("archived %s -> %s", srcPath, dstPath)
+	klog.V(2).Infof("tar %s -> %s complete", srcPath, dstPath)
 
 	var snapshotSize int64
 	fi, err := os.Stat(dstPath)

pkg/nfs/utils.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/kubernetes-csi/csi-lib-utils/protosanitizer"
@@ -196,3 +197,21 @@ func setKeyValueInMap(m map[string]string, key, value string) {
 	}
 	m[key] = value
 }
+
+func waitForPathNotExistWithTimeout(path string, timeout time.Duration) error {
+	// Loop until the path no longer exists or the timeout is reached
+	timeoutTime := time.Now().Add(time.Duration(timeout) * time.Second)
+	for {
+		if _, err := os.Lstat(path); err != nil {
+			if os.IsNotExist(err) {
+				return nil
+			}
+			return err
+		}
+
+		if time.Now().After(timeoutTime) {
+			return fmt.Errorf("time out waiting for path %s not exist", path)
+		}
+		time.Sleep(500 * time.Microsecond)
+	}
+}

pkg/nfs/utils_test.go

@@ -22,6 +22,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 )
 
 var (
@@ -357,3 +358,32 @@ func TestValidateOnDeleteValue(t *testing.T) {
 		}
 	}
 }
+
+func TestWaitForPathNotExistWithTimeout(t *testing.T) {
+	tests := []struct {
+		desc     string
+		path     string
+		timeout  int
+		expected error
+	}{
+		{
+			desc:     "path does not exist",
+			path:     "non-existent-path",
+			timeout:  1,
+			expected: nil,
+		},
+		{
+			desc:     "path exists",
+			path:     "/",
+			timeout:  2,
+			expected: fmt.Errorf("time out waiting for path / not exist"),
+		},
+	}
+
+	for _, test := range tests {
+		err := waitForPathNotExistWithTimeout(test.path, time.Duration(test.timeout))
+		if !reflect.DeepEqual(err, test.expected) {
+			t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, err, test.expected)
+		}
+	}
+}