Repository: kubernetes-csi/csi-driver-nfs
Pull request #461 (2023-06-05 14:39:34)

bugfix: correct snapshot path when using share parameter
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
When using the `share` parameter on either `StorageClass` or `SnapshotVolumeClass`, the path to snapshots would contain the `share` parameter twice, e.g.
```
/mnt/nfs/mnt/nfs/snapshot-id/src-pvc-id.tar.gz   # duplicate
/mnt/nfs/snapshot-id/src-pvc-id.tar.gz           # desired
```
This PR ensures it's the desired path.

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes https://github.com/kubernetes-csi/csi-driver-nfs/issues/459

**Special notes for your reviewer**:
Should I also introduce some special treatment regarding the old broken snapshot paths? For those who already have created some snapshots and they are stored in the broken path.

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
bugfix: correct snapshot path when using share parameter
```



Patch:
diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go
index a1911ac0..90f5706a 100644
--- a/pkg/nfs/controllerserver.go
+++ b/pkg/nfs/controllerserver.go
@@ -78,18 +78,10 @@ type nfsSnapshot struct {
 	src string
 }
 
-func (snap nfsSnapshot) archiveSubPath() string {
-	return snap.uuid
-}
-
 func (snap nfsSnapshot) archiveName() string {
 	return fmt.Sprintf("%v.tar.gz", snap.src)
 }
 
-func (snap nfsSnapshot) archivePath() string {
-	return filepath.Join(snap.archiveSubPath(), snap.archiveName())
-}
-
 // Ordering of elements in the CSI volume id.
 // ID is of the form {server}/{baseDir}/{subDir}.
 // TODO: This volume id format limits baseDir and
@@ -331,7 +323,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 			klog.Warningf("failed to unmount snapshot nfs server: %v", err)
 		}
 	}()
-	snapInternalVolPath := filepath.Join(getInternalVolumePath(cs.Driver.workingMountDir, snapVol), snapshot.archiveSubPath())
+	snapInternalVolPath := getInternalVolumePath(cs.Driver.workingMountDir, snapVol)
 	if err = os.MkdirAll(snapInternalVolPath, 0777); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to make subdirectory: %v", err)
 	}
@@ -409,7 +401,7 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS
 	}()
 
 	// delete snapshot archive
-	internalVolumePath := filepath.Join(getInternalVolumePath(cs.Driver.workingMountDir, vol), snap.archiveSubPath())
+	internalVolumePath := getInternalVolumePath(cs.Driver.workingMountDir, vol)
 	klog.V(2).Infof("Removing snapshot archive at %v", internalVolumePath)
 	if err = os.RemoveAll(internalVolumePath); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to delete subdirectory: %v", err.Error())
@@ -503,7 +495,7 @@ func (cs *ControllerServer) copyFromSnapshot(ctx context.Context, req *csi.Creat
 	}()
 
 	// untar snapshot archive to dst path
-	snapPath := filepath.Join(getInternalVolumePath(cs.Driver.workingMountDir, snapVol), snap.archivePath())
+	snapPath := filepath.Join(getInternalVolumePath(cs.Driver.workingMountDir, snapVol), snap.archiveName())
 	dstPath := getInternalVolumePath(cs.Driver.workingMountDir, dstVol)
 	klog.V(2).Infof("copy volume from snapshot %v -> %v", snapPath, dstPath)
 	out, err := exec.Command("tar", "-xzvf", snapPath, "-C", dstPath).CombinedOutput()
@@ -803,7 +795,7 @@ func volumeFromSnapshot(snap *nfsSnapshot) *nfsVolume {
 		id:      snap.id,
 		server:  snap.server,
 		baseDir: snap.baseDir,
-		subDir:  snap.baseDir,
+		subDir:  snap.uuid,
 		uuid:    snap.uuid,
 	}
 }


Test patch:
diff --git a/pkg/nfs/controllerserver_test.go b/pkg/nfs/controllerserver_test.go
index 8e5d4333..6c2c789d 100644
--- a/pkg/nfs/controllerserver_test.go
+++ b/pkg/nfs/controllerserver_test.go
@@ -742,10 +742,10 @@ func TestCopyVolume(t *testing.T) {
 				uuid:    "dst-pv-name",
 			},
 			prepare: func() error {
-				if err := os.MkdirAll("/tmp/snapshot-name/share/snapshot-name/", 0777); err != nil {
+				if err := os.MkdirAll("/tmp/snapshot-name/snapshot-name", 0777); err != nil {
 					return err
 				}
-				file, err := os.Create("/tmp/snapshot-name/share/snapshot-name/src-pv-name.tar.gz")
+				file, err := os.Create("/tmp/snapshot-name/snapshot-name/src-pv-name.tar.gz")
 				if err != nil {
 					return err
 				}