diff --git a/.github/workflows/linux.yaml b/.github/workflows/linux.yaml index 8836a641..c17c1242 100644 --- a/.github/workflows/linux.yaml +++ b/.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 diff --git a/charts/README.md b/charts/README.md index a06c2026..216b82f9 100644 --- a/charts/README.md +++ b/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 | diff --git a/charts/latest/csi-driver-nfs-v3.1.0.tgz b/charts/latest/csi-driver-nfs-v3.1.0.tgz index 37037ada..f2247d23 100644 Binary files a/charts/latest/csi-driver-nfs-v3.1.0.tgz and b/charts/latest/csi-driver-nfs-v3.1.0.tgz differ diff --git a/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml b/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml index b449d2c0..c2b9991b 100644 --- a/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml +++ b/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: diff --git a/charts/latest/csi-driver-nfs/values.yaml b/charts/latest/csi-driver-nfs/values.yaml index 17a862f7..aca7edd3 100755 --- a/charts/latest/csi-driver-nfs/values.yaml +++ b/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" diff --git a/cmd/nfsplugin/main.go b/cmd/nfsplugin/main.go index ddc48a66..7287d2b8 100644 --- a/cmd/nfsplugin/main.go +++ b/cmd/nfsplugin/main.go @@ -18,9 +18,7 @@ package main import ( "flag" - "fmt" "os" - "strconv" "github.com/kubernetes-csi/csi-driver-nfs/pkg/nfs" @@ -28,10 +26,11 @@ import ( ) 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) } diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index f596a34e..162074e4 100644 --- a/pkg/nfs/controllerserver.go +++ b/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 diff --git a/pkg/nfs/controllerserver_test.go b/pkg/nfs/controllerserver_test.go index 66c1c15d..6d1036a4 100644 --- a/pkg/nfs/controllerserver_test.go +++ b/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) } }) diff --git a/pkg/nfs/nfs.go b/pkg/nfs/nfs.go index 516fdea9..8c3d4e74 100644 --- a/pkg/nfs/nfs.go +++ b/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" ) +// DriverOptions defines driver parameters specified in driver deployment +type DriverOptions struct { + NodeID string + DriverName string + Endpoint string + MountPermissions uint64 + WorkingMountDir string +} + type Driver struct { - name string - nodeID string - version string - - endpoint string - - perm *uint32 + 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,19 +134,15 @@ 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 @@ -148,6 +150,5 @@ func (n *Driver) AddNodeServiceCapabilities(nl []csi.NodeServiceCapability_RPC_T 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) } diff --git a/pkg/nfs/nfs_test.go b/pkg/nfs/nfs_test.go index a7fe9d4e..61fd58d3 100644 --- a/pkg/nfs/nfs_test.go +++ b/pkg/nfs/nfs_test.go @@ -32,7 +32,6 @@ const ( func NewEmptyDriver(emptyField string) *Driver { var d *Driver - var perm *uint32 switch emptyField { case "version": d = &Driver{ @@ -40,7 +39,6 @@ func NewEmptyDriver(emptyField string) *Driver { version: "", nodeID: fakeNodeID, cap: map[csi.VolumeCapability_AccessMode_Mode]bool{}, - perm: perm, } case "name": d = &Driver{ @@ -48,7 +46,6 @@ func NewEmptyDriver(emptyField string) *Driver { version: driverVersion, nodeID: fakeNodeID, cap: map[csi.VolumeCapability_AccessMode_Mode]bool{}, - perm: perm, } default: d = &Driver{ @@ -56,7 +53,6 @@ func NewEmptyDriver(emptyField string) *Driver { version: driverVersion, nodeID: fakeNodeID, cap: map[csi.VolumeCapability_AccessMode_Mode]bool{}, - perm: perm, } } d.volumeLocks = NewVolumeLocks() diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index e3643c5c..1f43a981 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -50,10 +50,28 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, status.Error(codes.InvalidArgument, "Target path not provided") } + var server, baseDir string + for k, v := range req.GetVolumeContext() { + switch strings.ToLower(k) { + case paramServer: + server = v + case paramShare: + baseDir = v + } + } + + if server == "" { + return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("%v is a required parameter", paramServer)) + } + if baseDir == "" { + return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("%v is a required parameter", paramShare)) + } + source := fmt.Sprintf("%s:%s", server, baseDir) + notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) if err != nil { if os.IsNotExist(err) { - if err := os.MkdirAll(targetPath, 0750); err != nil { + if err := os.MkdirAll(targetPath, os.FileMode(ns.Driver.mountPermissions)); err != nil { return nil, status.Error(codes.Internal, err.Error()) } notMnt = true @@ -70,10 +88,6 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis mountOptions = append(mountOptions, "ro") } - s := req.GetVolumeContext()[paramServer] - ep := req.GetVolumeContext()[paramShare] - source := fmt.Sprintf("%s:%s", s, ep) - klog.V(2).Infof("NodePublishVolume: volumeID(%v) source(%s) targetPath(%s) mountflags(%v)", volumeID, source, targetPath, mountOptions) err = ns.mounter.Mount(source, targetPath, "nfs", mountOptions) if err != nil { @@ -86,13 +100,10 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, status.Error(codes.Internal, err.Error()) } - if ns.Driver.perm != nil { - klog.V(2).Infof("volumeID(%v): mount targetPath(%s) with permissions(0%o)", volumeID, targetPath, *ns.Driver.perm) - if err := os.Chmod(targetPath, os.FileMode(*ns.Driver.perm)); err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } + klog.V(2).Infof("volumeID(%v): mount targetPath(%s) with permissions(0%o)", volumeID, targetPath, ns.Driver.mountPermissions) + if err := os.Chmod(targetPath, os.FileMode(ns.Driver.mountPermissions)); err != nil { + return nil, status.Error(codes.Internal, err.Error()) } - return &csi.NodePublishVolumeResponse{}, nil } diff --git a/pkg/nfs/nodeserver_test.go b/pkg/nfs/nodeserver_test.go index 03fce0af..d417f098 100644 --- a/pkg/nfs/nodeserver_test.go +++ b/pkg/nfs/nodeserver_test.go @@ -40,6 +40,11 @@ func TestNodePublishVolume(t *testing.T) { t.Fatalf(err.Error()) } + params := map[string]string{ + "server": "server", + "share": "share", + } + volumeCap := csi.VolumeCapability_AccessMode{Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER} alreadyMountedTarget := testutil.GetWorkDirPath("false_is_likely_exist_target", t) targetTest := testutil.GetWorkDirPath("target_test", t) @@ -70,39 +75,48 @@ func TestNodePublishVolume(t *testing.T) { }, { desc: "[Success] Stage target path missing", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, - VolumeId: "vol_1", - TargetPath: targetTest}, + req: csi.NodePublishVolumeRequest{ + VolumeContext: params, + VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + VolumeId: "vol_1", + TargetPath: targetTest}, expectedErr: nil, }, { desc: "[Success] Valid request read only", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, - VolumeId: "vol_1", - TargetPath: targetTest, - Readonly: true}, + req: csi.NodePublishVolumeRequest{ + VolumeContext: params, + VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + VolumeId: "vol_1", + TargetPath: targetTest, + Readonly: true}, expectedErr: nil, }, { desc: "[Success] Valid request already mounted", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, - VolumeId: "vol_1", - TargetPath: alreadyMountedTarget, - Readonly: true}, + req: csi.NodePublishVolumeRequest{ + VolumeContext: params, + VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + VolumeId: "vol_1", + TargetPath: alreadyMountedTarget, + Readonly: true}, expectedErr: nil, }, { desc: "[Success] Valid request", - req: csi.NodePublishVolumeRequest{VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, - VolumeId: "vol_1", - TargetPath: targetTest, - Readonly: true}, + req: csi.NodePublishVolumeRequest{ + VolumeContext: params, + VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap}, + VolumeId: "vol_1", + TargetPath: targetTest, + Readonly: true}, expectedErr: nil, }, } // setup _ = makeDir(alreadyMountedTarget) + _ = makeDir(targetTest) for _, tc := range tests { if tc.setup != nil { diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 09680c67..be5e9c28 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -41,7 +41,6 @@ const ( var ( nodeID = os.Getenv("NODE_ID") - perm *uint32 nfsDriver *nfs.Driver isWindowsCluster = os.Getenv(testWindowsEnvVar) != "" defaultStorageClassParameters = map[string]string{ @@ -70,7 +69,12 @@ var _ = ginkgo.BeforeSuite(func() { handleFlags() framework.AfterReadingAllFlags(&framework.TestContext) - nfsDriver = nfs.NewDriver(nodeID, nfs.DefaultDriverName, fmt.Sprintf("unix:///tmp/csi-%s.sock", uuid.NewUUID().String()), perm) + options := nfs.DriverOptions{ + NodeID: nodeID, + DriverName: nfs.DefaultDriverName, + Endpoint: fmt.Sprintf("unix:///tmp/csi-%s.sock", uuid.NewUUID().String()), + } + nfsDriver = nfs.NewDriver(&options) controllerServer = nfs.NewControllerServer(nfsDriver) // install nfs server