mountOptions support for local storage #1005
Conversation
k8s-ci-robot
added
area/local-volume
kind/feature
| @@ -255,12 +274,16 @@ func (d *Discoverer) discoverVolumesAtPath(class string, config common.MountConf | |||
| glog.Errorf("Path %q fs stats error: %v", filePath, err) | |||
| continue | |||
| } | |||
| if len(mountOptions) != 0 { | |||
| glog.Warningf("Path %q is already in filesystem mode, "+ | |||
There was a problem hiding this comment.
When we do the bind mount for filesystem mode, could you actually specify different mount options?
There was a problem hiding this comment.
Oh, thanks for pointing this out. I was mainly focus on the change this time, and did not think so thoroughly.
Then I wonder why we don't support mount options before, since we can also have the requirement in the bind mount scenario.
There was a problem hiding this comment.
Hmm, I think it's because the mount options here is kind of different from that of others.
It is not used in MountDevice or Provision phase.
Then I think it worth considering if users do have practical requirement for this, though it's supported to specify mount options in bind mount theoretically.
I see other kinds of storages did not take mount options into account in SetUp
But on the other hand, things can be different to local volume: for other storages, the options have take effect in other phase (MountDevice) of volume lifecycle, so can be ignored during SetUp, but this is not true for local volume, so perhaps we'll still need it.
There was a problem hiding this comment.
This PR updated all the volume plugins to take the same mount options for both MountDevice and SetUp: kubernetes/kubernetes#68945
There was a problem hiding this comment.
The fact that all volume plugins do not pass mount options when calling bind was an oversight - kubernetes/kubernetes#68945 which we fixed.
I know it is tricky for local volumes. We know that, file system mount options are ignored by bind mount and only userspace mount options do get applied. local volumes may not need too many userspace mount options so we might be safe.
lichuqiang
force-pushed
the
mount_options
branch
from
3a85fec to
7f6daf1
Compare
|
/lgtm |
Address kubernetes/kubernetes#68317
Follow-up kubernetes/kubernetes#69211
let the plugin pass the mountOptions specified in storageclasses into the PVs to make it effective
/area local-volume
/kind feature
/assign @msau42