search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.34k stars 373 forks source link

Node mount paths exceed the 128 character limit #350

Open erikh opened 5 years ago

erikh commented 5 years ago

If the mount path during an attach exceeds 128 bytes, protobuf complains.

Since MAXPATHLEN is ... 1024 or something like that on linux, doesn't this seem a little short? We're shipping a UUID with our dynamically provisioned volumes. The k8s mountpath on our system is already over 90 bytes.

Any advice is welcome, thanks.

jdef commented 5 years ago

... protobuf complains.

Do you mean the CO or plugin is complaining (because they're actually validating the protobuf fields according to the CSI spec, as they should be)?

The spec defines a 128 byte limit for string fields, unless otherwise noted. It sounds like we should increase the limit for paths since they'll probably get quite longer than 128 in some cases.

erikh commented 5 years ago

Correct; the grpc connection is failing to serve because of the path limitations.

I'll get you some output in a few.

On Thu, Feb 21, 2019 at 2:21 PM James DeFelice notifications@github.com wrote:

... protobuf complains.

Do you mean the CO or plugin is complaining (because they're actually validating the protobuf fields according to the CSI spec, as they should be)?

The spec defines a 128 byte limit for string fields, unless otherwise noted. It sounds like we should increase the limit for paths since they'll probably get quite longer than 128 in some cases.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/350#issuecomment-466191095, or mute the thread https://github.com/notifications/unsubscribe-auth/AABJ66Fa3WOjjExaECCpHLhb_ldpB5prks5vPxv-gaJpZM4bIKJu .

erikh commented 5 years ago

Sorry for the late reply.

Here's the output coming back from kubectl describe:

  Warning  FailedMount  0s (x4 over 4s)  kubelet, appserv17  MountVolume.SetUp failed for volume "pvc-d41134f1-310b-11e9-a2ef-54ab3a2919d1" : rpc error: code = InvalidArgument desc = exceeds size limit: TargetPath: max=128, size=131

Please note that we are using https://github.com/rexray/gocsi for a framework, and while I cannot find where the final bits of the path are being set in the pipeline (we're using the volume ID), I think it's related to this library at least a little. I am still digging through it. If something in the k8s 1.13 pipe is not validating the length of the strings, this must be it.

Having some trouble finding the full attacher log info, but basically, this is my base path:

/var/lib/kubelet/pods/ed16ce55-310b-11e9-a2ef-54ab3a2919d1/volumes/

and then add another 60-70 bytes for a suffix on top of that for the actual volume dir (csi-%s) that I think gocsi is generating.

jdef commented 5 years ago

I think we should probably amend the spec and extend the length limit for path fields. Validation implementations could then follow suit.

On Thu, Feb 21, 2019 at 11:42 PM Erik Hollensbe notifications@github.com wrote:

Sorry for the late reply.

Here's the output coming back from kubectl describe:

Warning FailedMount 0s (x4 over 4s) kubelet, appserv17 MountVolume.SetUp failed for volume "pvc-d41134f1-310b-11e9-a2ef-54ab3a2919d1" : rpc error: code = InvalidArgument desc = exceeds size limit: TargetPath: max=128, size=131

Please note that we are using https://github.com/rexray/gocsi for a framework, and while I cannot find where the final bits of the path are being set in the pipeline (we're using the volume ID), I think it's related to this library at least a little. I am still digging through it. If something in the k8s 1.13 pipe is not validating the length of the strings, this must be it.

Having some trouble finding the full attacher log info, but basically, this is my base path:

/var/lib/kubelet/pods/ed16ce55-310b-11e9-a2ef-54ab3a2919d1/volumes/

and then add another 60-70 bytes for a suffix on top of that for the actual volume dir (csi-%s) that I think gocsi is generating.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/350#issuecomment-466271722, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLHARW39tnKxtDt_qcz03kNhPLE2sks5vP3VKgaJpZM4bIKJu .

saad-ali commented 5 years ago

Agreed 128 bytes is too small for target_path fields. I've heard the same complaint from others. Kubernetes doesn't enforce the limit, so it's not a huge deal. But we should loosen the spec here to permit standard Linux (and Windows) paths.

jdef commented 4 years ago

A quick Google search suggests that 4k is the length limit for common Linux filesystems. I guess Windows systems used to have a 255? 260? limit, but apparently that's overridable in Windows 10 - but I don't see what the new max length is in W10.

Suggest 4k as the limit for paths now.

Jiawei0227 commented 3 years ago

The change for length limitation has been merged with the new CSI spec. Please check with rexray/gocsi maintainer to see if they need to make any change on their side. Thanks