Bugfix: DeleteVolume does not work

Author: boddumanohar

pkg/smb/controllerserver.go

@@ -30,6 +30,11 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const (
+	volumeIDRegex            = "^(.*)#(.*)$"
+	sourceAndSubDirSeperator = "#"
+)
+
 // smbVolume is an internal representation of a volume
 // created by the provisioner.
 type smbVolume struct {
@@ -68,13 +73,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	}
 
 	reqCapacity := req.GetCapacityRange().GetRequiredBytes()
-	smbVol, err := d.newSMBVolume(name, reqCapacity, req.GetParameters())
+	parameters := req.GetParameters()
+	smbVol, err := d.newSMBVolume(name, reqCapacity, parameters)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	// check if create SubDir is enable in storage class parameters
-	parameters := req.GetParameters()
 	createSubDir := true
 	for k, v := range parameters {
 		switch strings.ToLower(k) {
@@ -117,7 +122,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
 	if volumeID == "" {
 		return nil, status.Error(codes.InvalidArgument, "volume id is empty")
 	}
-	smbVol, err := d.getSmbVolFromID(volumeID)
+	smbVol, err := getSmbVolFromID(volumeID)
 	if err != nil {
 		// An invalid ID should be treated as doesn't exist
 		klog.Warningf("failed to get smb volume for volume id %v deletion: %v", volumeID, err)
@@ -213,7 +218,7 @@ func (d *Driver) getVolumeIDFromSmbVol(vol *smbVolume) string {
 	idElements := make([]string, totalIDElements)
 	idElements[idsourceField] = strings.Trim(vol.sourceField, "/")
 	idElements[idSubDir] = strings.Trim(vol.subDir, "/")
-	return strings.Join(idElements, "/")
+	return strings.Join(idElements, sourceAndSubDirSeperator)
 }
 
 // Get working directory for CreateVolume and DeleteVolume
@@ -311,15 +316,17 @@ func (d *Driver) smbVolToCSI(vol *smbVolume, parameters map[string]string) *csi.
 }
 
 // Given a CSI volume id, return a smbVolume
-func (d *Driver) getSmbVolFromID(id string) (*smbVolume, error) {
-	volRegex := regexp.MustCompile("^([^/]+)/([^/]+)$")
+// a sample volume Id: smb-server.default.svc.cluster.local/share/pvc-4729891a-f57e-4982-9c60-e9884af1be2f
+func getSmbVolFromID(id string) (*smbVolume, error) {
+	volRegex := regexp.MustCompile(volumeIDRegex)
 	tokens := volRegex.FindStringSubmatch(id)
-	if tokens == nil {
-		return nil, fmt.Errorf("Could not split %q into server, baseDir and subDir", id)
+	if tokens == nil || len(tokens) != 3 {
+		return nil, fmt.Errorf("Could not split %q into server and subDir", id)
 	}
+	source := "//" + tokens[1]
 	return &smbVolume{
 		id:          id,
-		sourceField: tokens[1],
+		sourceField: source,
 		subDir:      tokens[2],
 	}, nil
 }

pkg/smb/controllerserver_test.go

@@ -33,9 +33,9 @@ import (
 )
 
 const (
-	testServer    = "test-server"
+	testServer    = "test-server/baseDir"
 	testCSIVolume = "test-csi"
-	testVolumeID  = "test-server/test-csi"
+	testVolumeID  = "test-server/baseDir#test-csi"
 )
 
 func TestControllerGetCapabilities(t *testing.T) {
@@ -403,3 +403,50 @@ func TestListSnapshots(t *testing.T) {
 		t.Errorf("Unexpected error: %v", err)
 	}
 }
+
+func TestGetSmbVolFromID(t *testing.T) {
+
+	cases := []struct {
+		desc      string
+		volumeID  string
+		source    string
+		subDir    string
+		expectErr bool
+	}{
+		{
+			desc:      "correct volume id",
+			volumeID:  "smb-server.default.svc.cluster.local/share#pvc-4729891a-f57e-4982-9c60-e9884af1be2f",
+			source:    "//smb-server.default.svc.cluster.local/share",
+			subDir:    "pvc-4729891a-f57e-4982-9c60-e9884af1be2f",
+			expectErr: false,
+		},
+		{
+			desc:      "correct volume id with multiple base directories",
+			volumeID:  "smb-server.default.svc.cluster.local/share/dir1/dir2#pvc-4729891a-f57e-4982-9c60-e9884af1be2f",
+			source:    "//smb-server.default.svc.cluster.local/share/dir1/dir2",
+			subDir:    "pvc-4729891a-f57e-4982-9c60-e9884af1be2f",
+			expectErr: false,
+		},
+		{
+			desc:      "incorrect volume id",
+			volumeID:  "smb-server.default.svc.cluster.local/share",
+			source:    "//smb-server.default.svc.cluster.local/share",
+			subDir:    "pvc-4729891a-f57e-4982-9c60-e9884af1be2f",
+			expectErr: true,
+		},
+	}
+	for _, test := range cases {
+		test := test //pin
+		t.Run(test.desc, func(t *testing.T) {
+			smbVolume, err := getSmbVolFromID(test.volumeID)
+
+			if !test.expectErr {
+				assert.Equal(t, smbVolume.sourceField, test.source)
+				assert.Equal(t, smbVolume.subDir, test.subDir)
+				assert.Nil(t, err)
+			} else {
+				assert.NotNil(t, err)
+			}
+		})
+	}
+}