From e86dc68c2ae3d1c5178536528b8b415500ecb662 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Tue, 11 Jan 2022 13:08:58 +0000 Subject: [PATCH] feat: add mountOptions parameter for inline volume fix test failure --- Makefile | 2 +- charts/README.md | 2 +- charts/latest/csi-driver-nfs-v3.1.0.tgz | Bin 3602 -> 3603 bytes charts/latest/csi-driver-nfs/values.yaml | 2 +- deploy/example/nginx-pod-inline-volume.yaml | 1 + pkg/nfs/nodeserver.go | 16 ++++++++++------ test/e2e/dynamic_provisioning_test.go | 11 ++++++----- .../dynamically_provisioned_inline_volume.go | 13 +++++++------ test/e2e/testsuites/specs.go | 4 ++-- test/e2e/testsuites/testsuites.go | 7 ++++--- 10 files changed, 33 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 9593ccc4..b84a5555 100644 --- a/Makefile +++ b/Makefile @@ -42,7 +42,7 @@ REGISTRY_NAME ?= $(shell echo $(REGISTRY) | sed "s/.azurecr.io//g") IMAGE_TAG = $(REGISTRY)/$(IMAGENAME):$(IMAGE_VERSION) IMAGE_TAG_LATEST = $(REGISTRY)/$(IMAGENAME):latest -E2E_HELM_OPTIONS ?= --set image.nfs.repository=$(REGISTRY)/$(IMAGENAME) --set image.nfs.tag=$(IMAGE_VERSION) --set image.nfs.pullPolicy=Always +E2E_HELM_OPTIONS ?= --set image.nfs.repository=$(REGISTRY)/$(IMAGENAME) --set image.nfs.tag=$(IMAGE_VERSION) --set image.nfs.pullPolicy=Always --set feature.enableInlineVolume=true E2E_HELM_OPTIONS += ${EXTRA_HELM_OPTIONS} # Output type of docker buildx build diff --git a/charts/README.md b/charts/README.md index 3edef385..b1fb613b 100644 --- a/charts/README.md +++ b/charts/README.md @@ -40,7 +40,7 @@ The following table lists the configurable parameters of the latest NFS CSI Driv | `driver.name` | alternative driver name | `nfs.csi.k8s.io` | | `driver.mountPermissions` | mounted folder permissions name | `0777` | `feature.enableFSGroupPolicy` | enable `fsGroupPolicy` on a k8s 1.20+ cluster | `false` | -| `feature.enableInlineVolume` | enable inline volume | `true` | +| `feature.enableInlineVolume` | enable inline volume | `false` | | `image.nfs.repository` | csi-driver-nfs image | `mcr.microsoft.com/k8s/csi/nfs-csi` | | `image.nfs.tag` | csi-driver-nfs image tag | `latest` | | `image.nfs.pullPolicy` | csi-driver-nfs image pull policy | `IfNotPresent` | diff --git a/charts/latest/csi-driver-nfs-v3.1.0.tgz b/charts/latest/csi-driver-nfs-v3.1.0.tgz index 30095df0908287e47e153a009e9ac2bca949413a..5c0d8c74d439308ab2c7cd207f98858265b584ef 100644 GIT binary patch delta 3577 zcmVX(9FrW7Jb&G9NjXZ;m(o#JA2x}sy`3HVztic||9853y*J&R?%w`(r?b1=ebedgbbor=Z=myxJS;7l(nP-L zJbA483SqhXI~l7sI(WOiX=_7eRkMCfzywDII=1OLm}bkY=DxZ zM&)smUH z3HH6b+2jsqg?2L;nnALY2`u6+&l!f8XL4crh9CPsNs*<6bnTR9l_#80j$Z`J%pk7LD@}Md{^GJWPWySHolGBNrECx6I7*!2Leg}kDC^vO)|h{_OJZJneo z4?qisl4v_&OU#EWoQBY{DklZQ;A$2E7}ZQHKdoHRL~42qL-F{ADy@nN91bzkA+%0K zKN@44F>I|xKbJ?!2*<~cB2kGs8?5`p!Af3z*$}-a4B?qoTGUzd)L0C;`IVCiq0DH02;Bv}TSHm5 zo0YI}$!qKSvXWlz#$R!9x1X}OTU&6BLzKuPG>%npG#)h(5ZH^%GQj7mJCaI0z+NYl{j{AYW z(N7;^)%kzBw^NJ%cXxZcoB01F3S5k_eyXPmMyRzh;uupVrWhM5w*gwjGPAn}o_}{C zAR1$IRqvgOSb@?+XX-tq42eP$r-YsdV2nS~41C`hr%FkL2e-EnoTF_QoEA^v?#}Sw zi21%khLINXpe0?)gv9Lj9XHu&NYC)4ut1f`@g!e8ce#v|C2uC*5OOR`?oPQJ|X{y?ju9u)DG0|jx);A6#_ueHX;m)1pt!yyonl{CF z-t8?=9>Uk74 zkyDw;VGZ8*9EArA22f1Z6ys~@aMq)K)SXjbI0PH+wD^rAYkV4{ca9PyD#rT$fV;bJRclQ~dB`%e8#yZR<#=oPlY?Gs;h}H1 zC2=fKse|v{emFWhKkoOxZ)nJn#3+5MAyn{k3*?jfa849CxqnI>fK8c(VDn*-qJa|8 zRb5JFrCiG@0nGCPh0qEJ)~_bX?opo#XuC>#s*TrEo~oKGTR7L!a_wuHpvd&WwpwW5 z&}u&wQXe!s_kSvjb9B49ypG!Bp!Xo%snULo>fECorYzU2N2pGP+m)KbhiV;lF<6)4&nD!|F{0-hoes? zr>#FK4G3f6`m7*+tRljQyINBlW%b&dSuI=fW}oR#mVc>eo|CCbDDP$VQW_t8_xAMj z(edTUQIkJK9;bqGeem7e!~V(T@#)do=abWmrl#(=Vb-|JjeA-|LJi%lCi>|-{o2sG zo}q1~l;=8giZPW-q*BX?)`%_LCUPk?Z4)1*Na_j$Fr>)ha~v+#ck885b=H?!!#rR1LG+H5M#(X~lUNYn}dtx-Q$)xwIv0*oijPQBE}_>B6v$GsYylKL}La!Do;S z(SMpm<8%CXMx}|Sifc;Fj(?#;jHVI8Pa?+I-HtQElG5&iT()Gw35e}6-yX#UvZYbnb{l*5O!6A*)cVWgF}1$s<0 z5Q)GsLZyhDf)*e%Ztn?{x($W2N4y+;CYT7`$47wARztro?3|@B(b(6LXdF$Qmogy> z<)dG1RS{a$FTB;+tkxGnjSDf(%LE^TUe^TiwQ%d&Fy8=Ty#+$j5#@wgXgU&{yMGuL zu0_UT7$6L!xI%8>R*~s44N&eb%tJBkhau1lSg)1xi=n)KQ^v%)o;Jdd+ z=OG;LayESm$1`BvmPcrXZS>F3el$$=* zD>cmgfmwrZ0ONnwoQAP&SOxsMUGMJ!`EOAAhXpwseEvRs{%J|Muk4&(D}OccFqi&t zT)$B$55x4EqJjHB{?GEz-d%eA9K&Pgd0D(4wbmH0Y;S*UXlhelpe)6I<$y%~^IQU6 zk^kB5ZP)WZ-JR~{_g`M5)bP6c%}RHJ)4#j(eBbwKKNjPV+kT}>15|62sI*TS#VTUH z>P!944Bi@!Yj28F=4u%!&3`aYb~#Y*TrjN6*S02^9t%nTv`(^=C%YG)u1pwjvn z&zTQSEap>eb{4e#9x-6V2u;==)xqpdV^vd=O(W+mB1V*2bDnF?34bTZ^&=#g&*nX} z(q+S~o>ru&iJ+c!LQf(X2_a+3%NEcyhPB(TBecfVcL{=~@&54U?>Ll<`$x)S%12+r z#%oum{Xss|+F7T}Wz(FIH+tE5*!B?bx}JYE-Y7rkF1p0W`!|@a;yr?{IPBNn8~+Za z7XJlfWCQaQJfAhjN%oVNLOfz~t1u(G8}6Zxj~ z13gwiB_#q5M%C!+Clt?u8LCX|j2mU!hZa=?=j24I^R&Y%jgCS;JBD+P*L>j;V zHE01N!W04>nTK#qxP6!^G=EDF*E}%e39hAJ45iyntewrD`HH{}oRSzpj9`#Skcn&9 z6^0kmFn^CcPYy{~or(wGwk)$xc}{!%;=>?RG%#AH!UKp6|1*>^RmSF2A6onS`<^#K zqEl&22RRus{5beYiZsc!Yo|PadBPdx_(iZZ&c*Nw^PLaBJa5lh1EV2PeK5u?Q~sHM zB1#)Q<)hS?^8^XglQSW8A3FcLw`=+`G5!Uo$bb5<>j7K~c}4m7lbPWWm3?S5b(}Oj z04*3wqV0SwGas&S)`y0*I4KwgSMvk7tX_ddg(FjA`hsLQG zgcFQXhK-H*XZlDO;rQ55Br1{05VI-OM!4bgeR5T043Wt%lmjl+*;i*NV*@8pAAbMC+W|kb$VS%HaB(6xwkME6tK|P0ZyV;YHhqY!i0$#M#jx;f)=vWZ0~{RU4IA& zCm3E;dnY1Npfu5`dJhRhqR=ENea{0h-XCfPzHhu!B_zay+gk|E(b5H{`BS*NGcq`4 zzORsBsD(UeNJqKU3-0b3;QQA^hm+UV>9^EY`v2twS%OjpI(aG-u*Uz}t^M7K|L^s> zy{-ShM7zChzJn=^`=C(6h%(f(1b+`=qn2=jeR$WryYoCtc6^f%9)X^q-CWJ#%X|(k zW^CzhZ+(bxL^(pk9G?tX1N_Ask}?f}Lz%7H245WFWGFvRQA!#i{AG?sJcPZj{Yc|M zIvUX%X!r|e$RqpNiN?qfnX-q?39@*R2tut~j@&VGpG{6glB9Chpj>IfLw}P;)CHW{ zJ~82BjFCT_ReI$I)m0s{gyr@YaxW|@F<9Ae4zSwMoNhnNaont%(Mq5uO}khn(rhyR z;z{1~FMHfH(YWzH4QO08EU$v{U)y-M?a6Ir{9lG29*qjGiU0e%y-NPqY3+7f+xY(y z&G&upit?xrM>f@qk!!D56n`Q~qMFmT7o#Q-(PVMhHvuDy-UTD!E}UdkHkoszbvd4Q zdkd6@ER8CGIe@=_@(8(xj-gWtTJ&y#f7OqkzjqSkX`?&v#wIuXWghKKBT2G;9=T6s zT;_V%K=wVy;Q@mIwFk zSVo{vtL9dIT~OpgY%U@kl$PJL^2RAt9~kB7&HQ~u-$lb(K&^!(E%TFtVILg+Sz?Ji zns-Vh6_V=NAtOq;nSZI7(ir>M(aH}c)ifl`?zGobPwM0;AYzM2mteAj0aR9cX^rP- zO92}43y@x2nI*HiER`slSWt#jgGB`c?(X{Qs5Kd9Da+h%;G&;F)c;J4JzWrR5l`UoMi}V?HI+cAfQ2)^hkrZDKBF>s@0c zhGf|^6o#pd#DZ(Q(PhOXl1?eZF-G%J;i82kAw$ZjrdZJ@0EwbL{N5P+Hn{w7^y%cZ z@kfb(Fea|ga^}Y>B#gMLHI-4;uf3Vosuiz~nf`Q@iGOBAnTm$8US=<)(ZP3bPd^_W zU!EM*#Z%-_A}H4f-@QE?oLnBC9-VzYIlZVOb;k{}#zkq|(>xQZ>1Hj_PZs6Zn%>n4 zZ9S)~*qL#Rs9d3yN>Q}JY~?nQX{m0T_$WnOH5h;qMHZdoX!%X{&iS873> z{<>X$ouH&5s_(o$P$wc%3&Xu9SWk&;GCH&cH-Cn@X*$-DOOcRq7K0i;(uhiz5)oE) zotMlq%n_n~gK#>k$y8UW#vEa+*j|kVtksjACD?}}s}><=OT*UA7~@3G>_v*(yZ`8V z<68X3y`+33z83z|>g_M%Kdo*3??qY>>dEetWfz{B({_do9S%-hZSQ^%sJw&EAseDK z34bT&`0tcTlTGFKl$;;`LPr?RLWZA2g!9WCXGSEY-37VIp**{76)fMT%0h$>$H@d^ zl!STZ^Y^`+N|&}Q`AC(naebr=OAKFo?Vh8p^?#FaJn=s8{r=x>x4PX*{cpdsySMfK zmuNe1F`){6`|#-(Tm8$kJq6Xvr10MmDu0?iw)k4Y(lO=m;p_y&@Lw2ey%0l`0 zSNkeYEvh%(s$!Pk=ShuAG0uw&AA??36!Eoi>&h_S0AjrZLeeqigjr}h6r8&p7k{or z$|4vd45he2ZsAsu>LL$N?k>zjG3ynj=a&MNtDiO<=YKN^|4_sa(N zVRg%QWY=PMHC>hs=pKLoZx+!hZ=r}|!lPxt#fJ^PO;I$6?ASP`GH&UcqvgSOZ;#GT zz8s%lo*(~oIrx0|^X1uxi+_Ay%YP*n5gydDs`%q0A&A|z&Q`YUhdSU>)UXmrtLwO; z__B10w>ic! znfU{&2Hyb2|Ex6)DOfg_ksMM<)OX1^!hc1$ISCG|AN#;W5BAt{k5T~ZF_;X691J$68g_`33yHY zr`zdP>p$&YXZ!zOUZhpmF{$wHm%UVOSVVZ6~M5oX0Z zmG)ci8&6B7%=07ejel<>^Ez6#yJkdFltW7}M%{Q14MvsyZNENe zJ~*|QPm$SK(Dr-8fFUC^-gs08v)7GPE=|^roKb{~D7E1{H=Gkrk$>w)XfB)0duSx9 zhFd?aP*IaXJ?n&?#4r>>MwAylpl%H7w_itTwX5$61a;&6;Vr)7P;l-aDUT>0e+?h6 zT$%QV*-#r-ol;j#b533xWfy7NL!#?u@zq45?3}wC5+5JmV7`j?D7qA|Uwd!-JG4sv z7fg`Fbj*ds=Nn_){ePd`&R+HZe|37>`u~fx9XKP}xUBugj$3*6k35DcWhQJvLc%LD zMiqG3WdW5Y7J>pbLB?Rr#L$(eDIdQFiHvAEMSDmsJSRNzc7US;6yR+laYS!0a%Io| z`aXcqoXzb27)hK+ke~!cIR;*EG`JjSA<^4`LlMVFA{RmlsS`C?afbBiFWV+qQiJ?f(M+0RR7<8_Ns;SO5TEY5+(8 diff --git a/charts/latest/csi-driver-nfs/values.yaml b/charts/latest/csi-driver-nfs/values.yaml index 7e5a8f5f..4ea1987b 100755 --- a/charts/latest/csi-driver-nfs/values.yaml +++ b/charts/latest/csi-driver-nfs/values.yaml @@ -30,7 +30,7 @@ driver: feature: enableFSGroupPolicy: false - enableInlineVolume: true + enableInlineVolume: false controller: name: csi-nfs-controller diff --git a/deploy/example/nginx-pod-inline-volume.yaml b/deploy/example/nginx-pod-inline-volume.yaml index 2ac79d33..9022e10c 100644 --- a/deploy/example/nginx-pod-inline-volume.yaml +++ b/deploy/example/nginx-pod-inline-volume.yaml @@ -23,3 +23,4 @@ spec: volumeAttributes: server: nfs-server.default.svc.cluster.local # required share: / # required + mountOptions: "nfsvers=4.1,sec=sys" # optional diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index 1f43a981..af5c8331 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -38,7 +38,8 @@ type NodeServer struct { // NodePublishVolume mount the volume func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { - if req.GetVolumeCapability() == nil { + volCap := req.GetVolumeCapability() + if volCap == nil { return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") } volumeID := req.GetVolumeId() @@ -49,6 +50,10 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis if len(targetPath) == 0 { return nil, status.Error(codes.InvalidArgument, "Target path not provided") } + mountOptions := volCap.GetMount().GetMountFlags() + if req.GetReadonly() { + mountOptions = append(mountOptions, "ro") + } var server, baseDir string for k, v := range req.GetVolumeContext() { @@ -57,6 +62,10 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis server = v case paramShare: baseDir = v + case mountOptionsField: + if v != "" { + mountOptions = append(mountOptions, v) + } } } @@ -83,11 +92,6 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return &csi.NodePublishVolumeResponse{}, nil } - mountOptions := req.GetVolumeCapability().GetMount().GetMountFlags() - if req.GetReadonly() { - mountOptions = append(mountOptions, "ro") - } - 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 { diff --git a/test/e2e/dynamic_provisioning_test.go b/test/e2e/dynamic_provisioning_test.go index 3dd157e7..e44b822e 100644 --- a/test/e2e/dynamic_provisioning_test.go +++ b/test/e2e/dynamic_provisioning_test.go @@ -275,11 +275,12 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() { } test := testsuites.DynamicallyProvisionedInlineVolumeTest{ - CSIDriver: testDriver, - Pods: pods, - Server: nfsServerAddress, - Share: nfsShare, - ReadOnly: false, + CSIDriver: testDriver, + Pods: pods, + Server: nfsServerAddress, + Share: nfsShare, + MountOptions: "nfsvers=4.1,sec=sys", + ReadOnly: false, } test.Run(cs, ns) }) diff --git a/test/e2e/testsuites/dynamically_provisioned_inline_volume.go b/test/e2e/testsuites/dynamically_provisioned_inline_volume.go index eadacd13..5ce20aae 100644 --- a/test/e2e/testsuites/dynamically_provisioned_inline_volume.go +++ b/test/e2e/testsuites/dynamically_provisioned_inline_volume.go @@ -27,18 +27,19 @@ import ( // Waiting for the PV provisioner to create an inline volume // Testing if the Pod(s) Cmd is run with a 0 exit code type DynamicallyProvisionedInlineVolumeTest struct { - CSIDriver driver.DynamicPVTestDriver - Pods []PodDetails - Server string - Share string - ReadOnly bool + CSIDriver driver.DynamicPVTestDriver + Pods []PodDetails + Server string + Share string + MountOptions string + ReadOnly bool } func (t *DynamicallyProvisionedInlineVolumeTest) Run(client clientset.Interface, namespace *v1.Namespace) { for _, pod := range t.Pods { var tpod *TestPod var cleanup []func() - tpod, cleanup = pod.SetupWithCSIInlineVolumes(client, namespace, t.CSIDriver, t.Server, t.Share, t.ReadOnly) + tpod, cleanup = pod.SetupWithCSIInlineVolumes(client, namespace, t.CSIDriver, t.Server, t.Share, t.MountOptions, t.ReadOnly) // defer must be called here for resources not get removed before using them for i := range cleanup { defer cleanup[i]() diff --git a/test/e2e/testsuites/specs.go b/test/e2e/testsuites/specs.go index 025fed5f..c2215520 100644 --- a/test/e2e/testsuites/specs.go +++ b/test/e2e/testsuites/specs.go @@ -123,11 +123,11 @@ func (pod *PodDetails) SetupWithDynamicVolumes(client clientset.Interface, names return tpod, cleanupFuncs } -func (pod *PodDetails) SetupWithCSIInlineVolumes(client clientset.Interface, namespace *v1.Namespace, csiDriver driver.DynamicPVTestDriver, server, share string, readOnly bool) (*TestPod, []func()) { +func (pod *PodDetails) SetupWithCSIInlineVolumes(client clientset.Interface, namespace *v1.Namespace, csiDriver driver.DynamicPVTestDriver, server, share, mountOptions string, readOnly bool) (*TestPod, []func()) { tpod := NewTestPod(client, namespace, pod.Cmd) cleanupFuncs := make([]func(), 0) for n, v := range pod.Volumes { - tpod.SetupCSIInlineVolume(fmt.Sprintf("%s%d", v.VolumeMount.NameGenerate, n+1), fmt.Sprintf("%s%d", v.VolumeMount.MountPathGenerate, n+1), server, share, readOnly) + tpod.SetupCSIInlineVolume(fmt.Sprintf("%s%d", v.VolumeMount.NameGenerate, n+1), fmt.Sprintf("%s%d", v.VolumeMount.MountPathGenerate, n+1), server, share, mountOptions, readOnly) } return tpod, cleanupFuncs } diff --git a/test/e2e/testsuites/testsuites.go b/test/e2e/testsuites/testsuites.go index a7d8f730..321e99ca 100644 --- a/test/e2e/testsuites/testsuites.go +++ b/test/e2e/testsuites/testsuites.go @@ -596,7 +596,7 @@ func (t *TestPod) SetupVolumeMountWithSubpath(pvc *v1.PersistentVolumeClaim, nam t.pod.Spec.Volumes = append(t.pod.Spec.Volumes, volume) } -func (t *TestPod) SetupCSIInlineVolume(name, mountPath, server, share string, readOnly bool) { +func (t *TestPod) SetupCSIInlineVolume(name, mountPath, server, share, mountOptions string, readOnly bool) { volumeMount := v1.VolumeMount{ Name: name, MountPath: mountPath, @@ -610,8 +610,9 @@ func (t *TestPod) SetupCSIInlineVolume(name, mountPath, server, share string, re CSI: &v1.CSIVolumeSource{ Driver: nfs.DefaultDriverName, VolumeAttributes: map[string]string{ - "server": server, - "share": share, + "server": server, + "share": share, + "mountOptions": mountOptions, }, ReadOnly: &readOnly, },