From 405a69cf24fb277be9394e1589c5a7148c33ea03 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Mon, 3 Jan 2022 11:35:04 +0000 Subject: [PATCH] feat: add workingMountDir in chart config fix tests sudo fix test: fix test failure fix test install go 1.17 fix ut fix test fix test fix test fix tests fix workingMountDir --- .github/workflows/linux.yaml | 2 +- charts/README.md | 1 + charts/latest/csi-driver-nfs-v3.1.0.tgz | Bin 3551 -> 3578 bytes .../templates/csi-nfs-controller.yaml | 1 + charts/latest/csi-driver-nfs/values.yaml | 3 +- cmd/nfsplugin/main.go | 30 +++++------ pkg/nfs/controllerserver.go | 23 ++------ pkg/nfs/controllerserver_test.go | 15 +++--- pkg/nfs/nfs.go | 49 +++++++++--------- pkg/nfs/nfs_test.go | 4 -- pkg/nfs/nodeserver.go | 33 ++++++++---- pkg/nfs/nodeserver_test.go | 44 ++++++++++------ test/e2e/e2e_suite_test.go | 8 ++- 13 files changed, 112 insertions(+), 101 deletions(-) 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 37037adab64c06ad5e66a60ad2c80b24979c0f9e..f2247d2321a50b2452ad573b430bfa2daba3384b 100644 GIT binary patch delta 3454 zcmV-^4T19C8~PiNK>^Q^LNUbay-by;sorf-O6U@ z`sBWmMpF72rJ{m|FztFIP3M0yqETrtMioh#YWwVHa0+Md2XJgn21aN?!q3?dB}a|Q z<0QclswU-4=j{7IH|TgTJ>zXFx0?S?3CmD@;Q+A4|9AWSZr%TT2V4JtmPQjY#?S)* z;3MTem?kk5ifSQeAtaFu5*kUN#7GB`NZMCFsCJ}in~#(qDLU_^$#@=uTgYjFIAR6Ylrb1$-X*qwTf|Ic0Sr)s7BC`AA<&U|2-k$$hnYh2w*+y`12dlB zS_;Ncy6wc;+5DNW2<*TaNf5*c2Dt>exQ1P6cp(k**z@F&MD?k70B*}N`;_Oj7nUCe zk)naoIu#y3V)&nVD(5{_-vgeg1H6LSn za-LmsMkAs^FcvH`{*iwmN*iT3R*g10LBjOpTu2>4?|=9AOkXC(zu**E2zwsDwUAeo zk3X1c98(!WtF4o?(lSoZ(VK|=L zP^DGz0!Jf^bO^08F^DFA7-tMy8}ZNekut*Zv7<;-B9js3Q)&)1j3!i<;$DJ@b+z42 z=R*p=?Ri=v4EQD?+S?bm+dNl@>RfBb(I9MriuNtEFgdse$O3OBDo*J(pH@|W+ zC6pP{523qccN-XMzu5>Im%O&F&#URxq51_ExBryI-PwT)9HB&i9-#@If}@F=iI4~i zBZ*q&G1UZ8xHhrh>>ZSk!DbK2u#h+kVWM@a!gjmr%dv=5TN8Dqj9ZWBIFsn7BDO(~ zP>!-~`1zKZpGrC<8jDSA*7kOGJp1ZAV{G6A3gP!ZydCg!i)`gx4HqYJYkSPttkv>= zjY(?rxAxh`PbY(aK;P)6kFoCj-|y|#^8dZP{=qi?e~t#9CRjh!Qw3wx+BkTODHAh{ zO$4wBTEsH5^9P>yNkBBg=&Ig36|n-PiO$qpNEs4^CWi?<55NS5NHg$#^nza~0=n!L15zooX)|1T%VQj{vt z=~JPAHU8i29PHKne}A{XyY>I)Xt%fRH!!712nscfC__C<@h~xJi6$7roA%wE=UK9o zo0RYv^aSm`YYt!Lb7V1NOLu$gLyRNJ5nAT>WXM|JFW!)hX$Tz3Y~42a><}kI`EiO; z(iq_{b2{UH5$yNvN16<>(TLta%U>`<9^21OG)9KVls#-tkR^*85gO%kX_#;x3^GuVOfd6%6@Zz)sE(L`)-co z=G}}|0ySyd#j23zlkpc%ik^Sjlc#}8kd6ks^(#E@OPi`yY|1vW1XjFJj z{6E;+t>u5c&R)N>jsMTleBbx3D33!pwy9o%TzloB5J^+jo_4(iHHnEP3mm`%j68am zjD)*zl2h4a&XG3dc;4+TP#&=?t_9`*{sPKlJX?kt>` z-0+t;d9*i;r0M#36h4u2nd@N#+4miX2Mh*KOw}|KYw2j-qkarta9=tg8|k$8jU;S! z@41*iEEpBhlN<&Zf0V#Nq`*h^4+phEObZ}7=BmI%kK8%pJ(WuTlj_Mo+%P#|6@fmh zn_KmDNs$Y&xrlL4S$@;18>dhqFv_!=`TLr_%Z9Z;VM|L|}c)gswF=2HQ5*I7@Mcr)Xvn#rn#bCgzUU(*CdrVsmSiNGP*ek!Cs zY!2>K5f{jJb$K1P>0$3dwo~Q(7}L4OI80k@S&uNCe~PqgEr*ZPCgx(c-Ze&INR~}Q zVU*cOEV#xST~$mf>69`YV>~YvE?PuVGNg=ZiZy)#kT?$E_txOI!R5Q-52t6XKPm)- zF>!rfFh5ZdVZ>dnsg1IJ?ai!Kt$1_H^rx##G_TjxG?e!;dnt_%zk7Z5@%ZHO^tdUW zB9Bu+f4M&V?)A~&^z!8F`26GP*{3E_cib>*T$aW?Ei$2oZq^e0bWwh7=v}YSHgn4B zpgG5w$`x9v)k|y4R&EoymYTMS_fjNvg8>*(Wbp-#mfz&>od21`FfeOjvWCWy%bIWA zr7})$!ghOA%rjAJ$!_M^fh*TNEyxwSk)!ytfA%hwLW@YS5I!B9m)@% znn+X=!D3f(T(pVS=})NZx=me3TOo#>Xwy5&siuS-BO|i`MuNvGgnp+&lW!LD*WK#t z6eSgLbLaJeIu)^6816m6dP-!Q(V?xyG1P6-v5{Pgl#KHj)cTReRJxRiu)6C!XO>}( ze-O+E&$M2z!$e;?;YB&FR2x!R#TzikyXT(z<>Vx%gUuijIJ zUildQ(i;EIYHR)9Bpgq?4}8D>ce|Z_zgGV{=owGD18V6bujpLb%QYK}geEh3@Riqa6%W!outM7}X#-$h+ zWrmMIuWO3m~Q~F-T@)$m~z4_G#v@fU5*RaB4aTO5r$G+A-8a=$aI+pD0dg; zp&0hV5a=bW*J}CMP~N{ON@tb)f49Wv?93mH%-#ECgZr?$rwa!+x?1wtwQ`E2$NNei2qWH3Oinlq&Jd1UIf36bWjrQ}e zKb~A#?C$K{hm&VR?>4}B8$94yBgvw3WqI#sQ6_zCRBD*{1FHt#0LK4CYZ}J4VIA=A zcD=s`IMl zzgg*SaQb(5p6~l!?VrUs~f&qg8jz@Pb*T?WKdspLQi5C2_a+3 z%O21)hV|R8qqN4=e|H6frt$vp7T<9wIrop0$CQu1hL6{-O#8!psEw;mnX9HbCvS|h zi?r<_(RH)kf6Ffms4TS*6sQR@24g0M zt~^co_$^3eMAIqSLu%O_JRB{0e{ z@PgyPbCvq#P@>XpR#R#77r%lw)s)~siwTWU^FDr zRhHV@56XKJNT}=7dlL-FmG>slN$S1%KW_&<6G=s;;Pm)Jc|j_L3D90ZVRV|9GSZZva>T0IRjy-2eap delta 3446 zcmV-+4T_cPLNT`YbJDpJ&sC7q)5`AD^oCPeCBMiTbj z=PR8~r?bDiYyWpTo%;Vycfa>tcelI0+wb)EdI#Tiy1Siz=R4?pMjlp{Olcy&>pXd^ z`sBWnLQ?t(rJ{m|FztFIP3K=3(WtZ+qlzR=wS9IpIEAx!130!S10!i6;g@WPlA}iD zagtyNRh9Cl^X~gWH|TgTJ>zXDmzw=g3CmD@W(Tmw{&)MGZr%QS`+Hmae~v;EGRDvY z0N^9#KA0jg6^d#hXCWk!3=$eip~OfBkx1HCKdN@5Xq%6eA1OL-rO9~ilV1TFf8!xg zHV=RGqYB2645(-;I`)+ol8n)e@288lRUW3jpcnKu8-6P7%Ma!Mn78$}}H+OQHyBpO4|GDFyb ze@>9w&u<{71>%SmTvNtih*4c)wB6CUlG`WGm;>P5e#w;a&}F3rQwA%oyVRhha{>`#RG6#mf5E~C%v%z zFo+ZljMS;{020&xIm(18qjM^RR_Ea0pyhcZBs!B;d61JK!*_$9rO47kf4+9oo>!XI ze3a$sd3Mnmjfe`t=&;QA2mYQYZN%ZaYSh{Z5~e5TLh2BD|GU3u+A=Zz6{pBT*z*9i zU?_>U%-beDT;VK)mQ^1q7zS7K?cAtYVn=P|N+VL!TF4MuCpT1SRaD?;gpm%RbtVSU z1mlchYoq@2S*MI}eC#L^f0f8&g!z=3T?(TK)g}LyU}8;bx6^rF_%Dh7&QO)9qBIMk z+ewz1^@>)F&STYJ-6sxK^3bb>=sjTw&#cn2W}2r)`^n93oJf-+r59EC8^I#pr2UA5&{M5?Wcx>ClhM|7M?^ivUA*G4Eu**5+8wlhDKbV@W9 zo7JrC?d*8A>O5m?;1~+wk3YQ~@NWhNpBJny4` zXoAsIy>%*L1xgd0sW*@^BnnLg6M7ziaTt+i;QPj)RZ1c}yuF3s0xex|Ry>8fJHvzP z%=Z;CjI@x4E$JwiTEX323w-~Y=xFlNGX0v$O8dW@AWKoIe?X^Cg#y;tf46h6SF`{9 z-QL00{-2}V-nL)ElqMl4)G(q9^(@81#E2!DU-D$( z|2c~9``#7haR|pY)Ju?SubdPjX{y@Ou9u)DG0|j!4;Y7$d+(BxaOX~PDjUo>(xw>C zyS)Xv%HO2T^I-0krACDKkFCCl>cUt^T61KYaoXsB>9Tnbv{$L`M zK0|#i0?z(SAfiX^9Py4yrTOpVif^T3r(4f^9Bh98{Lyl|u@rIHFs_3VfdrQFQaOleH~{AlGzl4=?eW_Q|aswYkSlt8q_q$|i;Ndamry|l)2 zfTn~d`6U#vDb14WTpmi4RV=6?u)!jO0e5%dI%-YEdB`%i8#ylV<#=oP)5BhC>7j49 ze$ImzyqWS;)nwJeIZDg5uW5oJ(}#Vv zMBor^KNV6RHaqt!iwi`&y1cHp>0$3dx>N1@F{*Qqa+tDOvmT*36>isR4j-va)WvMQ zYm~&0EUSjXD6^heaE&**DwtH#DP=guc%CX;l!&BcNEy`>Yw`pjaU8-Qt-k5{QlM1 zhvSpW)8i(8iabsQ<@)gZS4V@>%agO?^AD$IADc+sal@=}nH%@Ch=dxNSxxlQMf$a& zbv;8{>uIHD&nMR8+22W#)Ylkbe?*bR7dTq3$?KfSm{=|_;xik4`CFLXWrSP9l zzkdI>*V#ST-{L>dQOZzHe}11Vzwp$ac5`ItXmILsd-sDt)g62e*$}NsG`YZkXH=SK zs<^=9oWiekgwZTw_+G>~zYlV5SW?!^ ztg_brP4x1_`@r|xf4AG|_iOpTgWg_uYyZztcHrZLD){~F`(JJLf3FA~71S@2!hb`g zX#UvZYbnddl*8NeQxL;{VWgF}1A0O<5Q)GsLZyhDffgV$Ztn?{nvI0CN4y+;CYT65 zz(;`3Rztrm?3|@B(KyhOXdKU+moo7W<>TLMRS~DuFUi%}tkxG1l8aX^${juiy{-x3 zOX1eFVZH*ydIyB0e`CrCv(R)TIClXvT#JmwFhm$iafRH%ts>LqK0vvl?A@i8Pgp!=o|nZt zRU3@~tM>MnhNiaV8OlohR}M+!f1XRgYw|z+UcdJKSFgL<+y4H`vy>WMSHD^5ZgBc{ zcb@P2UhT(X9CACTG--fpjS`jiNuyXr%vXJ>|1*QPe}?1Qn0j1Kmhxox;?tE0Lr7^i z{-c%7^Jw|*niEY?4lThLb?XhZ7*+PaeSOY+aAGl^VzaZLZTE-)BSvVl@u&`FZyKwb znrs?5f2W8TQEJ0^Za62LBG-?QTt1sO&`MVgw|-iYq9%g+tP^@thmjC6ro3zcO=DQQ z{W3ypTz!`yXd3TNZ}E;p$+&-_Jf?j7C2YKQW!fL+Lv5UO%3LAmq=D7E-6f0!Uk=$H$MpKpwH_kZ?!`%Az7wY$y# zKTFwxbE1vO+BbIG%DdnG7-p0iw*@JQuE-cw;N_PERF+x@3e*G{gE13Bm!774{01a4 zqUjXvA+_|J@Yve{jt)?OSE`3lPa2LJ&7|3!;AE&x~n01NZeEC2ui 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