release 1.4.0-alpha1 the 1.4.0 will have some breaking changes, this is a test release

Author: xphyr

ReleaseNotes.md

@@ -22,4 +22,13 @@ Initial test release of version 1.3.0 which includes the following changes:
 # [v1.3.2](https://github.com/xphyr/synology-csi/releases/tag/v1.3.2)
 - addressed issues with snapshotter not properly creating snapshots
   - **NOTE**: You will need to apply the new `deploy\kubernetes\v1.20\snapshotter\snapshotter.yaml` file to your cluster as this updates the external-csi snapshotter application as well as the ClusterRole required to make snapshotting work on clusters based on K8s V1.20 and higher
-  - All go pkg imports are now pointed to xphyr/synology-csi
+
+# v1.4.0
+This release introduces potentially **BREAKING CHANGES** If you are converting from the Upstream synology/synology-csi driver or a prior version of xphyr/synology-csi driver please read the following.
+  - All go pkg imports are now pointed to xphyr/synology-csi
+  - In order to start code cleanup, I have remvoed code that is duplicated by go built in packages such as `strconv.ParseBool()`. This means that use of the term "yes" in any configuration file is NO LONGER SUPPORTED. If your storageClass definition contains the word "yes" or "no" for settings, you need to update to "true" or "false". This also aligns with kubernetes defaults for boolean values
+  - By default the Synology RecycleBin is DISABLED for NFS and SMB shares. If you want to use the #RecycleBin in your NFS or SMB shares, you will need to add
+  - added new option for disabling the recycle bin on NFS/SMB shares
+    - **NOTE:** This is a "breaking change". Previous releases enabled the recyclebin by default. This causes issues with many apps in K8s, so going forward all NEW shares will have the recyclebin disabled by default. You can enable the recyclebin by setting `recycleBin: true` in the storageClass definition file.
+    - **NOTE:** This is also "breaking change". Previous releases with the recyclebin enabled only allowed "Administrator" access to the files in the recycleBin. This causes issues with many apps in K8s, so going forward all NEW shares that have the recyclebin enabled will allow anyone access to the recycleBin. You can change this recyclebin by setting `recycleBinAdminOnly: true` in the storageClass definition file.
+  - removed the final deprecated function calls from k8s.io/mount-utils from code base

TODO.md

@@ -39,3 +39,4 @@ The following is a list of tasks to bring the Synology CSI driver up to date.
 - [x] Fix Snapshotter functionality
   - CSI Snapshotter is based on version 4.2.1 (released in August of 2021) this needs to be updated to v6+
   - clusterRole needs to add additional privileges to work properly
+- [ ] Add support for the [csi-testing framework](https://github.com/kubernetes-csi/csi-test/tree/master)

pkg/driver/controllerserver.go

@@ -136,7 +136,10 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 
 	isThin := true
 	if params["thin_provisioning"] != "" {
-		isThin = utils.StringToBoolean(params["thin_provisioning"])
+		isThin, err = strconv.ParseBool(params["thin_provisioning"])
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "unsupported thin_provisioning setting: %s", params["thin_provisioning"])
+		}
 	}
 
 	protocol := strings.ToLower(params["protocol"])
@@ -159,6 +162,25 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 
 	devAttribs := params["devAttribs"]
 
+	// capture recyclebin support for nfs/smb shares
+	// by default we are DISABLING the recyclebin
+	// recyclebin does not fit in the general storage model of Kubernetes
+	enableRecycleBin := false
+	if params["enableRecycleBin"] != "" {
+		enableRecycleBin, err = strconv.ParseBool(params["enableRecycleBin"])
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "unsupported recyclebin setting: %s", params["enableRecycleBin"])
+		}
+	}
+	recycleBinAdminOnly := false
+	if params["recycleBinAdminOnly"] != "" && enableRecycleBin {
+		// the recyclebin must be enabled for this option to be valid
+		recycleBinAdminOnly, err = strconv.ParseBool(params["recycleBinAdminOnly"])
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "unsupported recyclebinadminonly setting: %s", params["recycleBinAdminOnly"])
+		}
+	}
+
 	lunDescription := ""
 	if _, ok := params["csi.storage.k8s.io/pvc/name"]; ok {
 		// if the /pvc/name is present, the namespace is present too
@@ -174,22 +196,24 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 
 	spec := &models.CreateK8sVolumeSpec{
-		DsmIp:            params["dsm"],
-		K8sVolumeName:    volName,
-		LunName:          models.GenLunName(volName),
-		LunDescription:   lunDescription,
-		ShareName:        models.GenShareName(volName),
-		Location:         params["location"],
-		Size:             sizeInByte,
-		Type:             params["type"],
-		ThinProvisioning: isThin,
-		TargetName:       fmt.Sprintf("%s-%s", models.TargetPrefix, volName),
-		MultipleSession:  multiSession,
-		SourceSnapshotId: srcSnapshotId,
-		SourceVolumeId:   srcVolumeId,
-		Protocol:         protocol,
-		NfsVersion:       nfsVer,
-		DevAttribs:       devAttribs,
+		DsmIp:               params["dsm"],
+		K8sVolumeName:       volName,
+		LunName:             models.GenLunName(volName),
+		LunDescription:      lunDescription,
+		ShareName:           models.GenShareName(volName),
+		Location:            params["location"],
+		Size:                sizeInByte,
+		Type:                params["type"],
+		ThinProvisioning:    isThin,
+		TargetName:          fmt.Sprintf("%s-%s", models.TargetPrefix, volName),
+		MultipleSession:     multiSession,
+		SourceSnapshotId:    srcSnapshotId,
+		SourceVolumeId:      srcVolumeId,
+		Protocol:            protocol,
+		NfsVersion:          nfsVer,
+		DevAttribs:          devAttribs,
+		EnableRecycleBin:    enableRecycleBin,
+		RecycleBinAdminOnly: recycleBinAdminOnly,
 	}
 
 	// idempotency
@@ -401,18 +425,24 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		}, nil
 	}
 
+	isLocked, err := strconv.ParseBool(params["is_locked"])
+	if err != nil {
+		log.Errorf("failed to parse the is_locked parameter. %s, is not a valid option", params["is_locked"])
+		return nil, err
+	}
+
 	// not exist, going to create a new snapshot
 	spec := &models.CreateK8sVolumeSnapshotSpec{
 		K8sVolumeId:  srcVolId,
 		SnapshotName: snapshotName,
 		Description:  params["description"],
 		TakenBy:      models.K8sCsiName,
-		IsLocked:     utils.StringToBoolean(params["is_locked"]),
+		IsLocked:     isLocked,
 	}
 
 	snapshot, err := cs.dsmService.CreateSnapshot(spec)
 	if err != nil {
-		log.Errorf("Failed to CreateSnapshot, snapshotName: %s, srcVolId: %s, err: %v", snapshotName, srcVolId, err)
+		log.Errorf("failed to CreateSnapshot, snapshotName: %s, srcVolId: %s, err: %v", snapshotName, srcVolId, err)
 		return nil, err
 	}
 

pkg/driver/driver.go

@@ -24,8 +24,8 @@ import (
 )
 
 const (
-	DriverName    = "csi.san.synology.com" // CSI dirver name
-	DriverVersion = "1.2.0"
+	DriverName    = "csi.san.synology.com" // CSI driver name
+	DriverVersion = "1.4.0-xphyr"
 )
 
 var (

pkg/driver/nodeserver.go

@@ -132,31 +132,31 @@ func createTargetMountPathNFS(mounter mount.Interface, mountPath string, mountPe
 }
 
 func createTargetMountPath(mounter mount.Interface, mountPath string, isBlock bool) (bool, error) {
-	notMount, err := mount.IsNotMountPoint(mounter, mountPath)
+	isMount, err := mounter.IsMountPoint(mountPath)
 	if err != nil {
 		if os.IsNotExist(err) {
 			if isBlock {
 				pathFile, err := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750)
 				if err != nil {
 					log.Errorf("Failed to create mountPath:%s with error: %v", mountPath, err)
-					return notMount, err
+					return isMount, err
 				}
 				if err = pathFile.Close(); err != nil {
 					log.Errorf("Failed to close mountPath:%s with error: %v", mountPath, err)
-					return notMount, err
+					return isMount, err
 				}
 			} else {
 				err = os.MkdirAll(mountPath, 0750)
 				if err != nil {
-					return notMount, err
+					return isMount, err
 				}
 			}
-			notMount = true
+			isMount = true
 		} else {
 			return false, err
 		}
 	}
-	return notMount, nil
+	return isMount, nil
 }
 
 func (ns *nodeServer) getPortals(dsmIp string) []string {

pkg/dsm/service/dsm.go

@@ -180,7 +180,7 @@ func (service *DsmService) createMappingTarget(dsm *webapi.DSM, spec *models.Cre
 	targetId, err := dsm.TargetCreate(targetSpec)
 
 	if err != nil && !errors.Is(err, utils.AlreadyExistError("")) {
-		return webapi.TargetInfo{}, status.Errorf(codes.Internal, "%s", fmt.Sprintf("Failed to create target with spec: %v, err: %v", targetSpec, err))
+		return webapi.TargetInfo{}, status.Errorf(codes.Internal, "%s", fmt.Sprintf("Failed to create target with spec: %v, err: %v, targetID: %v", targetSpec, err, targetId))
 	}
 
 	targetInfo, err := dsm.TargetGet(targetSpec.Name)

pkg/dsm/service/share_volume.go

@@ -159,8 +159,8 @@ func (service *DsmService) createSMBorNFSVolumeByDsm(dsm *webapi.DSM, spec *mode
 			VolPath:             spec.Location,
 			Desc:                "Created by Synology K8s CSI",
 			EnableShareCow:      false,
-			EnableRecycleBin:    true,
-			RecycleBinAdminOnly: true,
+			EnableRecycleBin:    spec.EnableRecycleBin,
+			RecycleBinAdminOnly: spec.RecycleBinAdminOnly,
 			Encryption:          0,
 			QuotaForCreate:      &sizeInMB,
 		},

pkg/models/dsm_req_spec.go

@@ -10,22 +10,24 @@ import (
 )
 
 type CreateK8sVolumeSpec struct {
-	DsmIp            string
-	K8sVolumeName    string
-	LunName          string
-	LunDescription   string
-	ShareName        string
-	Location         string
-	Size             int64
-	Type             string
-	ThinProvisioning bool
-	TargetName       string
-	MultipleSession  bool
-	SourceSnapshotId string
-	SourceVolumeId   string
-	Protocol         string
-	NfsVersion       string
-	DevAttribs       string
+	DsmIp               string
+	K8sVolumeName       string
+	LunName             string
+	LunDescription      string
+	ShareName           string
+	Location            string
+	Size                int64
+	Type                string
+	ThinProvisioning    bool
+	TargetName          string
+	MultipleSession     bool
+	SourceSnapshotId    string
+	SourceVolumeId      string
+	Protocol            string
+	NfsVersion          string
+	DevAttribs          string
+	EnableRecycleBin    bool
+	RecycleBinAdminOnly bool
 }
 
 type K8sVolumeRespSpec struct {

pkg/utils/error.go

@@ -25,38 +25,38 @@ type ShareDefaultError struct {
 	ErrCode int
 }
 
-func (_ OutOfFreeSpaceError) Error() string {
+func (OutOfFreeSpaceError) Error() string {
 	return "Out of free space"
 }
-func (_ AlreadyExistError) Error() string {
+func (AlreadyExistError) Error() string {
 	return "Already Existed"
 }
-func (_ BadParametersError) Error() string {
+func (BadParametersError) Error() string {
 	return "Invalid input value"
 }
 
 // ISCSI errors
-func (_ NoSuchLunError) Error() string {
+func (NoSuchLunError) Error() string {
 	return "No such LUN"
 }
 
-func (_ LunReachMaxCountError) Error() string {
+func (LunReachMaxCountError) Error() string {
 	return "Number of LUN reach limit"
 }
 
-func (_ TargetReachMaxCountError) Error() string {
+func (TargetReachMaxCountError) Error() string {
 	return "Number of target reach limit"
 }
 
-func (_ NoSuchSnapshotError) Error() string {
+func (NoSuchSnapshotError) Error() string {
 	return "No such snapshot uuid"
 }
 
-func (_ BadLunTypeError) Error() string {
+func (BadLunTypeError) Error() string {
 	return "Bad LUN type"
 }
 
-func (_ SnapshotReachMaxCountError) Error() string {
+func (SnapshotReachMaxCountError) Error() string {
 	return "Number of snapshot reach limit"
 }
 
@@ -65,15 +65,15 @@ func (e IscsiDefaultError) Error() string {
 }
 
 // Share errors
-func (_ NoSuchShareError) Error() string {
+func (NoSuchShareError) Error() string {
 	return "No such share"
 }
 
-func (_ ShareReachMaxCountError) Error() string {
+func (ShareReachMaxCountError) Error() string {
 	return "Number of share reach limit"
 }
 
-func (_ ShareSystemBusyError) Error() string {
+func (ShareSystemBusyError) Error() string {
 	return "Share system is temporary busy"
 }
 

pkg/utils/utils.go

@@ -46,11 +46,6 @@ func BytesToMBCeil(size int64) int64 {
 	return (size + UNIT_MB - 1) / UNIT_MB
 }
 
-func StringToBoolean(value string) bool {
-	value = strings.ToLower(value)
-	return value == "yes" || value == "true" || value == "1"
-}
-
 func StringToSlice(value string) []string {
 	return strings.Fields(value)
 }