Merge pull request #263 from andyzhangx/workingMountDir

feat: add workingMountDir in chart config

.github/workflows/linux.yaml

@@ -13,7 +13,7 @@ jobs:
     - name: Set up Go 1.x
       uses: actions/setup-go@v2
       with:
-        go-version: ^1.16
+        go-version: ^1.17
       id: go
 
     - name: Check out code into the Go module directory

charts/README.md

@@ -58,6 +58,7 @@ The following table lists the configurable parameters of the latest NFS CSI Driv
 | `controller.replicas`                             | the replicas of csi-nfs-controller                         | `2`                                                                 |
 | `controller.runOnMaster`                          | run controller on master node                              | `false`                                                             |
 | `controller.logLevel`                             | controller driver log level                                                          |`5`                                                           |
+| `controller.workingMountDir`                      | working directory for provisioner to mount nfs shares temporarily                  | `/tmp`                                                             |
 | `controller.tolerations`                              | controller pod tolerations                            |                                                              |
 | `controller.resources.csiProvisioner.limits.memory`   | csi-provisioner memory limits                         | 100Mi                                                          |
 | `controller.resources.csiProvisioner.requests.cpu`    | csi-provisioner cpu requests limits                   | 10m                                                            |

charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml

@@ -73,6 +73,7 @@ spec:
             - "--endpoint=$(CSI_ENDPOINT)"
             - "--drivername={{ .Values.driver.name }}"
             - "--mount-permissions={{ .Values.driver.mountPermissions }}"
+            - "--working-mount-dir={{ .Values.controller.workingMountDir }}"
           env:
             - name: NODE_ID
               valueFrom:

charts/latest/csi-driver-nfs/values.yaml

@@ -26,7 +26,7 @@ rbac:
 
 driver:
   name: nfs.csi.k8s.io
-  mountPermissions: "0777"
+  mountPermissions: 0777
 
 feature:
   enableFSGroupPolicy: false
@@ -38,6 +38,7 @@ controller:
   livenessProbe:
     healthPort: 29652
   logLevel: 5
+  workingMountDir: "/tmp"
   tolerations:
     - key: "node-role.kubernetes.io/master"
       operator: "Exists"

cmd/nfsplugin/main.go

@@ -18,20 +18,19 @@ package main
 
 import (
 	"flag"
-	"fmt"
 	"os"
-	"strconv"
 
 	"github.com/kubernetes-csi/csi-driver-nfs/pkg/nfs"
 
 	"k8s.io/klog/v2"
 )
 
 var (
-	endpoint   = flag.String("endpoint", "unix://tmp/csi.sock", "CSI endpoint")
-	nodeID     = flag.String("nodeid", "", "node id")
-	perm       = flag.String("mount-permissions", "0777", "mounted folder permissions")
-	driverName = flag.String("drivername", nfs.DefaultDriverName, "name of the driver")
+	endpoint         = flag.String("endpoint", "unix://tmp/csi.sock", "CSI endpoint")
+	nodeID           = flag.String("nodeid", "", "node id")
+	mountPermissions = flag.Uint64("mount-permissions", 0777, "mounted folder permissions")
+	driverName       = flag.String("drivername", nfs.DefaultDriverName, "name of the driver")
+	workingMountDir  = flag.String("working-mount-dir", "/tmp", "working directory for provisioner to mount nfs shares temporarily")
 )
 
 func init() {
@@ -50,18 +49,13 @@ func main() {
 }
 
 func handle() {
-	// Converting string permission representation to *uint32
-	var parsedPerm *uint32
-	if perm != nil && *perm != "" {
-		permu64, err := strconv.ParseUint(*perm, 8, 32)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "incorrect mount-permissions value: %q", *perm)
-			os.Exit(1)
-		}
-		permu32 := uint32(permu64)
-		parsedPerm = &permu32
+	driverOptions := nfs.DriverOptions{
+		NodeID:           *nodeID,
+		DriverName:       *driverName,
+		Endpoint:         *endpoint,
+		MountPermissions: *mountPermissions,
+		WorkingMountDir:  *workingMountDir,
 	}
-
-	d := nfs.NewDriver(*nodeID, *driverName, *endpoint, parsedPerm)
+	d := nfs.NewDriver(&driverOptions)
 	d.Run(false)
 }

pkg/nfs/controllerserver.go

@@ -34,8 +34,6 @@ import (
 // ControllerServer controller server setting
 type ControllerServer struct {
 	Driver *Driver
-	// Working directory for the provisioner to temporarily mount nfs shares at
-	workingMountDir string
 }
 
 // nfsVolume is an internal representation of a volume
@@ -98,10 +96,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 	}()
 
-	fileMode := os.FileMode(0777)
-	if cs.Driver.perm != nil {
-		fileMode = os.FileMode(*cs.Driver.perm)
-	}
+	fileMode := os.FileMode(cs.Driver.mountPermissions)
 	// Create subdirectory under base-dir
 	internalVolumePath := cs.getInternalVolumePath(nfsVol)
 	if err = os.Mkdir(internalVolumePath, fileMode); err != nil && !os.IsExist(err) {
@@ -140,7 +135,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 		}
 	}
 
-	// Mount nfs base share so we can delete the subdirectory
+	// mount nfs base share so we can delete the subdirectory
 	if err = cs.internalMount(ctx, nfsVol, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error())
 	}
@@ -150,7 +145,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 		}
 	}()
 
-	// Delete subdirectory under base-dir
+	// delete subdirectory under base-dir
 	internalVolumePath := cs.getInternalVolumePath(nfsVol)
 
 	klog.V(2).Infof("Removing subdirectory at %v", internalVolumePath)
@@ -293,10 +288,7 @@ func (cs *ControllerServer) internalUnmount(ctx context.Context, vol *nfsVolume)
 
 // Convert VolumeCreate parameters to an nfsVolume
 func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[string]string) (*nfsVolume, error) {
-	var (
-		server  string
-		baseDir string
-	)
+	var server, baseDir string
 
 	// validate parameters (case-insensitive)
 	for k, v := range params {
@@ -310,7 +302,6 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 		}
 	}
 
-	// validate required parameters
 	if server == "" {
 		return nil, fmt.Errorf("%v is a required parameter", paramServer)
 	}
@@ -331,11 +322,7 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 
 // Get working directory for CreateVolume and DeleteVolume
 func (cs *ControllerServer) getInternalMountPath(vol *nfsVolume) string {
-	// use default if empty
-	if cs.workingMountDir == "" {
-		cs.workingMountDir = "/tmp"
-	}
-	return filepath.Join(cs.workingMountDir, vol.subDir)
+	return filepath.Join(cs.Driver.workingMountDir, vol.subDir)
 }
 
 // Get internal path where the volume is created

pkg/nfs/controllerserver_test.go

@@ -47,12 +47,13 @@ var (
 )
 
 func initTestController(t *testing.T) *ControllerServer {
-	var perm *uint32
 	mounter := &mount.FakeMounter{MountPoints: []mount.MountPoint{}}
-	driver := NewDriver("", "", "", perm)
+	driver := NewDriver(&DriverOptions{
+		WorkingMountDir:  "/tmp",
+		MountPermissions: 0777,
+	})
 	driver.ns = NewNodeServer(driver, mounter)
 	cs := NewControllerServer(driver)
-	cs.workingMountDir = "/tmp"
 	return cs
 }
 
@@ -189,7 +190,7 @@ func TestCreateVolume(t *testing.T) {
 				t.Errorf("test %q failed: got resp %+v, expected %+v", test.name, resp, test.resp)
 			}
 			if !test.expectErr {
-				info, err := os.Stat(filepath.Join(cs.workingMountDir, test.req.Name, test.req.Name))
+				info, err := os.Stat(filepath.Join(cs.Driver.workingMountDir, test.req.Name, test.req.Name))
 				if err != nil {
 					t.Errorf("test %q failed: couldn't find volume subdirectory: %v", test.name, err)
 				}
@@ -227,8 +228,8 @@ func TestDeleteVolume(t *testing.T) {
 		t.Run(test.desc, func(t *testing.T) {
 			// Setup
 			cs := initTestController(t)
-			_ = os.MkdirAll(filepath.Join(cs.workingMountDir, testCSIVolume), os.ModePerm)
-			_, _ = os.Create(filepath.Join(cs.workingMountDir, testCSIVolume, testCSIVolume))
+			_ = os.MkdirAll(filepath.Join(cs.Driver.workingMountDir, testCSIVolume), os.ModePerm)
+			_, _ = os.Create(filepath.Join(cs.Driver.workingMountDir, testCSIVolume, testCSIVolume))
 
 			// Run
 			resp, err := cs.DeleteVolume(context.TODO(), test.req)
@@ -243,7 +244,7 @@ func TestDeleteVolume(t *testing.T) {
 			if !reflect.DeepEqual(resp, test.resp) {
 				t.Errorf("test %q failed: got resp %+v, expected %+v", test.desc, resp, test.resp)
 			}
-			if _, err := os.Stat(filepath.Join(cs.workingMountDir, testCSIVolume, testCSIVolume)); test.expectedErr == nil && !os.IsNotExist(err) {
+			if _, err := os.Stat(filepath.Join(cs.Driver.workingMountDir, testCSIVolume, testCSIVolume)); test.expectedErr == nil && !os.IsNotExist(err) {
 				t.Errorf("test %q failed: expected volume subdirectory deleted, it still exists", test.desc)
 			}
 		})

pkg/nfs/nfs.go

@@ -17,21 +17,27 @@ limitations under the License.
 package nfs
 
 import (
-	"fmt"
-
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"k8s.io/klog/v2"
 	mount "k8s.io/mount-utils"
 )
 
-type Driver struct {
-	name    string
-	nodeID  string
-	version string
-
-	endpoint string
+// DriverOptions defines driver parameters specified in driver deployment
+type DriverOptions struct {
+	NodeID           string
+	DriverName       string
+	Endpoint         string
+	MountPermissions uint64
+	WorkingMountDir  string
+}
 
-	perm *uint32
+type Driver struct {
+	name             string
+	nodeID           string
+	version          string
+	endpoint         string
+	mountPermissions uint64
+	workingMountDir  string
 
 	//ids *identityServer
 	ns          *NodeServer
@@ -53,16 +59,17 @@ const (
 	mountOptionsField = "mountoptions"
 )
 
-func NewDriver(nodeID, driverName, endpoint string, perm *uint32) *Driver {
-	klog.V(2).Infof("Driver: %v version: %v", driverName, driverVersion)
+func NewDriver(options *DriverOptions) *Driver {
+	klog.V(2).Infof("Driver: %v version: %v", options.DriverName, driverVersion)
 
 	n := &Driver{
-		name:     driverName,
-		version:  driverVersion,
-		nodeID:   nodeID,
-		endpoint: endpoint,
-		cap:      map[csi.VolumeCapability_AccessMode_Mode]bool{},
-		perm:     perm,
+		name:             options.DriverName,
+		version:          driverVersion,
+		nodeID:           options.NodeID,
+		endpoint:         options.Endpoint,
+		mountPermissions: options.MountPermissions,
+		workingMountDir:  options.WorkingMountDir,
+		cap:              map[csi.VolumeCapability_AccessMode_Mode]bool{},
 	}
 
 	vcam := []csi.VolumeCapability_AccessMode_Mode{
@@ -102,7 +109,7 @@ func (n *Driver) Run(testMode bool) {
 	if err != nil {
 		klog.Fatalf("%v", err)
 	}
-	klog.Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
+	klog.V(2).Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
 
 	n.ns = NewNodeServer(n, mount.New(""))
 	s := NewNonBlockingGRPCServer()
@@ -119,7 +126,6 @@ func (n *Driver) Run(testMode bool) {
 func (n *Driver) AddVolumeCapabilityAccessModes(vc []csi.VolumeCapability_AccessMode_Mode) []*csi.VolumeCapability_AccessMode {
 	var vca []*csi.VolumeCapability_AccessMode
 	for _, c := range vc {
-		klog.Infof("Enabling volume access mode: %v", c.String())
 		vca = append(vca, &csi.VolumeCapability_AccessMode{Mode: c})
 		n.cap[c] = true
 	}
@@ -128,26 +134,21 @@ func (n *Driver) AddVolumeCapabilityAccessModes(vc []csi.VolumeCapability_Access
 
 func (n *Driver) AddControllerServiceCapabilities(cl []csi.ControllerServiceCapability_RPC_Type) {
 	var csc []*csi.ControllerServiceCapability
-
 	for _, c := range cl {
-		klog.Infof("Enabling controller service capability: %v", c.String())
 		csc = append(csc, NewControllerServiceCapability(c))
 	}
-
 	n.cscap = csc
 }
 
 func (n *Driver) AddNodeServiceCapabilities(nl []csi.NodeServiceCapability_RPC_Type) {
 	var nsc []*csi.NodeServiceCapability
 	for _, n := range nl {
-		klog.Infof("Enabling node service capability: %v", n.String())
 		nsc = append(nsc, NewNodeServiceCapability(n))
 	}
 	n.nscap = nsc
 }
 
 func IsCorruptedDir(dir string) bool {
 	_, pathErr := mount.PathExists(dir)
-	fmt.Printf("IsCorruptedDir(%s) returned with error: %v", dir, pathErr)
 	return pathErr != nil && mount.IsCorruptedMnt(pathErr)
 }

pkg/nfs/nfs_test.go

@@ -32,31 +32,27 @@ const (
 
 func NewEmptyDriver(emptyField string) *Driver {
 	var d *Driver
-	var perm *uint32
 	switch emptyField {
 	case "version":
 		d = &Driver{
 			name:    DefaultDriverName,
 			version: "",
 			nodeID:  fakeNodeID,
 			cap:     map[csi.VolumeCapability_AccessMode_Mode]bool{},
-			perm:    perm,
 		}
 	case "name":
 		d = &Driver{
 			name:    "",
 			version: driverVersion,
 			nodeID:  fakeNodeID,
 			cap:     map[csi.VolumeCapability_AccessMode_Mode]bool{},
-			perm:    perm,
 		}
 	default:
 		d = &Driver{
 			name:    DefaultDriverName,
 			version: driverVersion,
 			nodeID:  fakeNodeID,
 			cap:     map[csi.VolumeCapability_AccessMode_Mode]bool{},
-			perm:    perm,
 		}
 	}
 	d.volumeLocks = NewVolumeLocks()