From d0ea11dac3716a2ef0182eccd6bc117a8dfc6d79 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 2 Jan 2022 03:57:51 +0000 Subject: [PATCH 1/2] feat: support mountOptions in DeleteVolume chore: provider secrets get permission add logging --- charts/latest/csi-driver-nfs-v3.1.0.tgz | Bin 3542 -> 3551 bytes .../templates/rbac-csi-nfs-controller.yaml | 3 ++ deploy/rbac-csi-nfs-controller.yaml | 3 ++ docs/driver-parameters.md | 2 +- pkg/nfs/controllerserver.go | 20 ++++++++-- pkg/nfs/nfs.go | 3 +- pkg/nfs/utils.go | 11 ++++++ pkg/nfs/utils_test.go | 36 ++++++++++++++++++ 8 files changed, 72 insertions(+), 6 deletions(-) diff --git a/charts/latest/csi-driver-nfs-v3.1.0.tgz b/charts/latest/csi-driver-nfs-v3.1.0.tgz index 9716a4e2f881364e69b62e4fa269e36e1d4999e7..37037adab64c06ad5e66a60ad2c80b24979c0f9e 100644 GIT binary patch delta 3423 zcmV-l4WRPY8{Zp{L4WTvKgC|Tb0^IWsek^{%NakUw$tNj<9MtzXF8pZ2O?JzjzNG2 z0A;JL?`OXQfRspzvSd54PbR{HEfEV~vDjTKehVs6)Q=^dqV)MlwT~u5>R?6^_TA?z zold8-zq@PycRHQ=|4w(m_g#0lyT9A-^!IuP-*vjXoqp##=zn}h9#)o2X(GStJbA48 z1MhM&)sm zU_xu*Uv(`<-sx{(Jj-Tl;^GLK8B^&;tPA zBjrArA~6+;YJVYTAtaFu5*kUN#7GB`NZMCFs&=Gkn~#(qDLQYZ$$0KJif3uY*tuXd znuTyWIurU_qC)NnB1O-om{Q{*P&N;L^`i>LkqoG4D?0X-7Ltt7jPIw5wpAXcy`UHL zHsgWODRNZmT#6w+h=rnMYFaKauorYTV}OeookP9AF@IHBl7~ye^ideHEP7dDX?GKr zJcV*fBYYc0BIDYy5=kT)L(noq*nxjeklW91Ag2Z5h!tE@#$bqflh|$%Gm!-_Kn+^J zh%kjfN9G}16K)@73e9&3;+h9$Ji)aTjG=VfiPf|DGhY$dfisdIh!G5O4sv!)cctNl zG@ZwuCx3?|s!zoOa9ftyr#vUUu>3HH6b+2jsqg?2)Bicjges$RDuh<&;NYO;c_Sn` zlU8|3Me18I6bv!RWBe_y_);C~d^yx@y$g2@<9! z=R)cbdjGq>XWB9`{uQUlLfG>Fv|uQSw#?fmK7U-{EQFR-A1N3HSM%-Ms99o1ZRJWM zQqx+<5LzcURB2UI;An)A4xx1>2GIoLjA3h|{_|O5Dq;mn6P>9ykTN6+O#~Bq9)NKek!Ilg#-LSy zN+LYGy@lWcEnRR{JcYYE!-MP0_Z2dXw2+4_=_r?4!QEX8eE*v0X!6oB{hG>3`@ftZ zOHryor%#0f*4TfybFf#l|NY(G!Pfqtquk!MU&E9pAt=-^q73yc#lysiC7NIeuiJNb zo@dEUZc@Ty&=a)#u{nI1&ymH9E#2*Ztq(DdC`V|Sd2ByB))*NgQ}(bqL6$6HM`+~Bk-N^^ zXA?$|G_4&^C|8>B$b=qE4yUnALO2;?>eVeRl_{SxxIzL3d=$aR<@ge1FUv5 zC)>Ak95-)fq!Oq}+b&jxG@p#Wcv7_d+nz&>H*Wn;3tE>g+gC~WFRi@W^5nA8|1aYb zk4A;p`2Pdr>?{7i*V*g!xBmY*itqd073FaV$2QbUkZZ4;6e4M=+S9I=pe8ZVWPuMD zhmm{ll9F)ePI4+6%sJAg7|*+Zy#>l6mc=#49Khc|d5l~`&(Nt9Eqb>A#+pYj>Kz9~ z*yzs4iHQw=oqKyzNSdynM`06rFLOC;!27=I;Q@mI6jL?D_*y!ex2PYF7rieXoDFwc z{7w?Ky7ipR9~T`J-hKXHB9uNueJ$iwj{m|=37-YRlmLzxTlBy}q`*gi_KSmBA*KZo z9dlJ6qDSr=@s3KR|4H@aA8weOu!=yR)zz(9T@vJiZ7yOQRF>bA>c%Nl2#oUVW?o;D zcUiF(7;I@ttNlsAFa!sgR=PwU&l{zZ3Q6_sh!LgS%+yS2O#J+4B^0nJ&64X}9!iu|ET|%|!6JhJcX#1BYE8y@ z$TGJZIWF<#cx(C7!(MCYp>Mb)aV$}(!|z|cJwCoT84P}CAY@2ll)lvvDtMU!`LsTq z6Gh%#r4GQROhd5wut?EBiRh{>rSnp*Wt{*Pd6}~F0GJAvCHUTdW?Aa|xy%I76URr5 zL&v@03$?$nMu0NLY8AESF)EHVC#>l;00KMz9r%Jq; z@>JDi)xtSS%eAj*f+Ew0eYHg35N4;L`8%VceSQA%KEi8vs$&{%|6qg zu2RuFk5iLS-pcHyG(PnOV8-3%*WnndMEE%UaVY|J`O&Q0twAAwr-DTdM=9h{M#8LcJt4pQOA`&cwk4NXF zx)rx7tbhoAWtqf}6NzdfSnRTgi!#wV{TX##wyE<%E7-6TZK|W3YD(BKGBO)rBzUYs z=yxhKxqqKmcdOMYN-E;!#_N4+Dq@vu@BRLDN@SbSp-rnX)NRwSkz9(DjB~fq`iaI= zx+sRQy4^fyU|^#DW}oh4)DT>*6pcA`Shl?e4OlCGCp}9sgkvifA!kd&HqIE6RL_p7 z4B__f+sP`f#eZBS@Sje!pDe%d)Sh;8Wawyc>T-Mc zgFw|Cd=A+Vtw}Vwz<*~{nrN!Hz~r35uXKdbEMoXx#5lhXa&A~s+Fg*V9m?~&Rzbr> zE9-E7Myhi4>K$e1m5<>sE%E=Xvey1h^zy{}!1vpKx7+FWYx%!}-d=ZW|Ibl&;NyfU z`2FqsUv2iU2ptvFFO$N5L!@Z_*y3v`%f^($+w)Tp!+&9L02ZW?!$_ca3bR;-;0W(~SjKwfS7)o)4 z+`_FQ)8#%uxw|kA#jqcSKrdmvR?5$Y@_wTzofYyoiO<=YKN^|4_hp0ou)5`IwQI2o zyM`vq3Um+JfH#x83LSs={j1}P(@!TCmlr2LUk*MT{c?H!_TxW(XzV2sF&;K|RkO!O z;u(8ujjgKL4>iE2h+!ov*VJ%D@?{AUurbCwigjOCvF}Fv<+q(YUg__jgeyeXLh%nE3;<244Zj|527fPcXyue`(EwGVjOZis5EJSYK;<=_DQ2y zMa)-yssA&Bw}#`|n0j1Kmhxox;?tE0c4gWh=0k0qb;?{e%^7*4mtBNy5Am*>`B&qO@^kL8OMJY4gZV1nBj}35e(AmO zTPU^oFPI=p=$H$MpKpwH_kZ?!`%Az7wY$y#KTFwxbE1vO+BbIG%DdnG7-p0iw*@JQ zuE-cw;N_PERF+x@3e*G{e}gd-LzkYWeEbF^GNS1e?IE@FobcG&0get(fLE!+5xv3K zr9J=ahX6isHnTrVk~)?kMG1^@47}iYa5>OIqPGJ_B1r^?Pe%iYsZ?Gtrn+taI`+L_ z_?K+kzr};exNZIvU)7Yi7aa{rbd{y{_Ji_X2kJWYUI#;R<-HDcJ(7B_|IgcjPef9Y zDL6eoQC^TrVH~s<&=^VE(Mjd%In(K!@j&EC!Z8T2 z04Q5^eLwph0Hj1xlqLU(eKHjuY>8L^i^c9@@mo-lqJAvt6s6Bcs(m;iQU^1Vuvbr4 zI-O2uzu&k2JDpDbf2X_Od)4iC_j|qFUcc9W)#>&-z1>&Pd4ED4R+da@B42eLJyv~k z-$)@T{en_a!9$pKJ(8yLuZ(C^+KW*|lBU`|I~<(A>4yOvS(Sm2knnRhM9ER3@;FH_ zgsMt;(|Py1K{x1lFFoUJDVLi4PYKIVJ+T8=WB>i#PPcCVz5UMC{-2}Jgp4ut008(% zxeumDOogIa$bVS~NhE`WMp7s-(m^DW_T>+%9Vy!8Bjrbm&Rc0Rp8JjBS(-6+CK!!o zA)Jg(g+7z0kUN4%(K9Kg)OZM#&BI^*pn`EE11j2zj(w$tBx5w=`{|->m4|6B=mova zcwlsj9F;ngVu<%*p=g!XIe3a$sd3Mnmjfe`t=&;QAC;pKrZN%ZaYSh{Z5~e3- zLh2BD|GU3u+A=Zz1*gbD*z*9iU?_>U%-beDTz}#$gqBqwDHsNq^X=TISz-+-(FEg+VQZuQ^I4~iaD3<}5|zkgg!z=3T?(TK)g}Ly zU}8Sa|4t|EQ@Q7Ie@=_@))^>o}p7ITJ&xKj5Uv5)H@D} zu+g276B8T$GWYhTkThLCkHRMMUgmPxfcJgZ!#xHAD5h$P@wId`Z&5!UFM3}(I2-P? z_>ClNb?Z5sKP);by!-sYL@0fV`bx;l9RG!#5=y^MLQD%F zI_9cCL=W9L;scdR|C8#;Kin`mVHJTst*cwLx+KU2+g!vrs4TxJ)s0iA5E$jz^}N0& z@3LYoFxb+PR{N8JVF(T~t#pYzo;OM*6_V=NAtOq;nW>r5nE3h8%8w+~G$PFIv{zJ5 zn)oS!Xp2c#khPKm)K-6bX^rOqO$kl%ODJGdnkCn{Jd`M_SWrb^gGB}dZg0bN)S8U* zkY#Q+a$Mre@z(OE2ffzPL*H;q;#i_m2j9JZe{^(yJQ#f6K**59D1ECTRPZtd@@aiI zCyKngN*#brnTBBVVUePN647N{N@t~9%Q^up@-k)T0WcLTOYncA&9c<_bD0UECytLA zhmL#07ixcDjR0kg)hcSuV^kb#PFQ0X{sNxsR<~DXxDZ+a+xpcc*&XUr0eaVIPnCEx z<*BO4s)ciumTOW3S7$}%$0{O>xT`g_QP!`$nboQlZ}yq~ zbd`$cd7PSr@>XUqrSZXcuTMW69bcRrHThHIaVjX+2j73aJ{+7}9G@PYeL6Y)+(hb* z8)l8m+_lxZwPb)opKCvdx{y~bQzQzC}iYz|I(Q-{*=S;@L za)A+_+2|WbE(@!9W63zZ4%_WjZpt{GrKO&4=q~g2IKNbEAdceCT3srI7Li~fd_FuY z)vdTyVFiCgD9a>%o=8*^!D5#^T$G8{=})NZvQ3>ATET{$Xj2{KR8zu^kdfH{Bf(=8 z!fvNRll%91b+=lbqNE~jZoJ;NrXp6k_TKGZr$n|H9on=SL)|tF8_A_e$vAf#tsiMj zrHf(+tJ}?U1_mbTZ}#a6Fcb^7nRTm0uaN*U_O?~~;hp4!uHjtm_RPF!y9 zZV;%tgU=xwqBV&o=lDlPrHQ7B3rx-_{6a?<%_4>$MU3R8pku|rA+)o`S@2`Rm3Uv zOLBEKtMx^Md{a)%E=uWN$%TDWy>m~Q~F-T@)$m~z4_G#v@fUBC=iB4aTO5r$G+ zBDZj>$aJ|6Q0^|weKG9&A<#=$ua)w%p}c=nl+FtIZ;8*@nLikryYpp(yRf?DYqbn( zvAc#Q%L;S{+JM)Sy9ynD@ZIa9^OG;f=NIS4KV1wy9sYcA_WtufzHjU$5iuS#cU7~; z2jUrfYmKd{+4nWT$B1DiD%aF-Me=0{60kAGJc@N+SF!I#`}x-&k1s3`c>4b1@v}jJ z8{oVR7VxZ|WYM^?ymxm|CVi+^YMA*wvj*P)#{Z-_4P)D|4)}LDyWSS^|C>Pm_X={h zVEC>rDoq-oTBAgzebOja z5%X1F>VIPJ)^J>VQ=~Fi%SdU4d9uradgp>+Wxkf0WO^bb{nI+hQl9Kie7Z7Wyw)ZV zX5~AT_Qm*(rzJDyc1a&dN^P!7nSawreh4Y;#=p1Hc^)m_U2~!-%Aq9~qi(&07Ng4k zx3ABc4^AxRQ*3q?wCx@+V8jSbHXhZ%>`h};Q4TX|v{z!RD`S@#CXn*a(vp>v- z+Bj#Fxm=ht(MBJ+2)gd$8#nWg#+T%$)MbbEaPI_jEx1F_6?gm6JKMHwQfl#EFhQ2k zF&7e_Zj5#JfA)I&OTYiMyUqVUOWA=lqK(npH+J00yWjp8W|SGn2Pugz$rx4O<(CCi zmRbl3)C3uWF%v_Vo~C^K7Jno%qUjXvA+_|J@Yve{jt)?O*Qvx2y~fz3J^$Er)=B5#e>PX zZT=Kr)s(jv9SuoznWgsjgYw=4>MHf#1VeJ^y$N)ZdT;*E+kr1cQZtb$I5|32UXV&* sT)Y?17)jgFN%3zln5sy`*xvfSZP}J>`3B1W2LJ&7|LQy-&;VEf00VrpO8@`> diff --git a/charts/latest/csi-driver-nfs/templates/rbac-csi-nfs-controller.yaml b/charts/latest/csi-driver-nfs/templates/rbac-csi-nfs-controller.yaml index 5fd44e2f0..73e4ae07d 100644 --- a/charts/latest/csi-driver-nfs/templates/rbac-csi-nfs-controller.yaml +++ b/charts/latest/csi-driver-nfs/templates/rbac-csi-nfs-controller.yaml @@ -37,6 +37,9 @@ rules: - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get"] --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/deploy/rbac-csi-nfs-controller.yaml b/deploy/rbac-csi-nfs-controller.yaml index 20860e594..2ac4a680a 100644 --- a/deploy/rbac-csi-nfs-controller.yaml +++ b/deploy/rbac-csi-nfs-controller.yaml @@ -33,6 +33,9 @@ rules: - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get"] --- kind: ClusterRoleBinding diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md index 351f53471..d2cdc542d 100644 --- a/docs/driver-parameters.md +++ b/docs/driver-parameters.md @@ -1,5 +1,5 @@ ## Driver Parameters -> This plugin driver itself only provides a communication layer between resources in the cluser and the NFS server, you need to bring your own NFS server before using this driver. +> This driver requires existing and already configured NFSv3 or NFSv4 server, it supports dynamic provisioning of Persistent Volumes via Persistent Volume Claims by creating a new sub directory under NFS server. ### Storage Class Usage (Dynamic Provisioning) > [`StorageClass` example](../deploy/example/storageclass-nfs.yaml) diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index 3478f1b84..f596a34ee 100644 --- a/pkg/nfs/controllerserver.go +++ b/pkg/nfs/controllerserver.go @@ -127,8 +127,21 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol return &csi.DeleteVolumeResponse{}, nil } + var volCap *csi.VolumeCapability + mountOptions := getMountOptions(req.GetSecrets()) + if mountOptions != "" { + klog.V(2).Infof("DeleteVolume: found mountOptions(%s) for volume(%s)", mountOptions, volumeID) + volCap = &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{ + MountFlags: []string{mountOptions}, + }, + }, + } + } + // Mount nfs base share so we can delete the subdirectory - if err = cs.internalMount(ctx, nfsVol, nil); err != nil { + if err = cs.internalMount(ctx, nfsVol, volCap); err != nil { return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error()) } defer func() { @@ -285,8 +298,7 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str baseDir string ) - // Validate parameters (case-insensitive). - // TODO do more strict validation. + // validate parameters (case-insensitive) for k, v := range params { switch strings.ToLower(k) { case paramServer: @@ -298,7 +310,7 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str } } - // Validate required parameters + // validate required parameters if server == "" { return nil, fmt.Errorf("%v is a required parameter", paramServer) } diff --git a/pkg/nfs/nfs.go b/pkg/nfs/nfs.go index b728eb41e..516fdea90 100644 --- a/pkg/nfs/nfs.go +++ b/pkg/nfs/nfs.go @@ -49,7 +49,8 @@ const ( // The base directory must be a direct child of the root directory. // The root directory is omitted from the string, for example: // "base" instead of "/base" - paramShare = "share" + paramShare = "share" + mountOptionsField = "mountoptions" ) func NewDriver(nodeID, driverName, endpoint string, perm *uint32) *Driver { diff --git a/pkg/nfs/utils.go b/pkg/nfs/utils.go index 1c20569f0..d0810bb5f 100644 --- a/pkg/nfs/utils.go +++ b/pkg/nfs/utils.go @@ -121,3 +121,14 @@ func (vl *VolumeLocks) Release(volumeID string) { defer vl.mux.Unlock() vl.locks.Delete(volumeID) } + +// getMountOptions get mountOptions value from a map +func getMountOptions(context map[string]string) string { + for k, v := range context { + switch strings.ToLower(k) { + case mountOptionsField: + return v + } + } + return "" +} diff --git a/pkg/nfs/utils_test.go b/pkg/nfs/utils_test.go index ca761893d..9a8f0c9d0 100644 --- a/pkg/nfs/utils_test.go +++ b/pkg/nfs/utils_test.go @@ -118,3 +118,39 @@ func TestGetLogLevel(t *testing.T) { } } } + +func TestGetMountOptions(t *testing.T) { + tests := []struct { + desc string + context map[string]string + result string + }{ + { + desc: "nil context", + context: nil, + result: "", + }, + { + desc: "empty context", + context: map[string]string{}, + result: "", + }, + { + desc: "valid mountOptions", + context: map[string]string{"mountOptions": "nfsvers=3"}, + result: "nfsvers=3", + }, + { + desc: "valid mountOptions(lowercase)", + context: map[string]string{"mountoptions": "nfsvers=4"}, + result: "nfsvers=4", + }, + } + + for _, test := range tests { + result := getMountOptions(test.context) + if result != test.result { + t.Errorf("Unexpected result: %s, expected: %s", result, test.result) + } + } +} From 498e8373705415f73df7ebfbdd28ff1ca615812d Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 2 Jan 2022 08:24:15 +0000 Subject: [PATCH 2/2] test: add e2e test for mountOptions support in DeleteVolume --- Makefile | 2 ++ test/e2e/e2e_suite_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Makefile b/Makefile index c011f026a..9593ccc4a 100644 --- a/Makefile +++ b/Makefile @@ -151,6 +151,8 @@ endif .PHONY: install-nfs-server install-nfs-server: kubectl apply -f ./deploy/example/nfs-provisioner/nfs-server.yaml + kubectl delete secret mount-options --ignore-not-found + kubectl create secret generic mount-options --from-literal mountOptions="nfsvers=4.1" .PHONY: install-helm install-helm: diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index a3b69328d..09680c676 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -47,6 +47,8 @@ var ( defaultStorageClassParameters = map[string]string{ "server": "nfs-server.default.svc.cluster.local", "share": "/", + "csi.storage.k8s.io/provisioner-secret-name": "mount-options", + "csi.storage.k8s.io/provisioner-secret-namespace": "default", } controllerServer *nfs.ControllerServer )