Require the mount destination to be created before hand #591
Conversation
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
|
Another alternative would be to add a flag to CLI API to make this optional. |
| @@ -47,6 +47,7 @@ The parameters are similar to the ones in [the Linux mount system call](http://m | |||
|
|
|||
| * **`destination`** (string, REQUIRED) Destination of mount point: path inside container. | |||
| For the Windows operating system, one mount destination MUST NOT be nested within another mount (e.g., c:\\foo and c:\\foo\\bar). | |||
| The destination MUST be present in the root filesystem. Runtime MUST NOT create the destination. | |||
There was a problem hiding this comment.
Also, one sentence per line.
|
a4695bc is fine with me, although it will make populating tmpfs
mounts difficult (e.g. if you want a tmpfs at /tmp and a fuse mount at
at /tmp/stuff). I don't have a use case for that sort of thing myself
though, and folks can always use hooks or post-create manipulation.
|
| @@ -47,6 +47,7 @@ The parameters are similar to the ones in [the Linux mount system call](http://m | |||
|
|
|||
| * **`destination`** (string, REQUIRED) Destination of mount point: path inside container. | |||
| For the Windows operating system, one mount destination MUST NOT be nested within another mount (e.g., c:\\foo and c:\\foo\\bar). | |||
| The destination MUST be present in the root filesystem. Runtime MUST NOT create the destination. | |||
There was a problem hiding this comment.
Why can't we just make this a restriction which only applies if readonly is set to true for the rootfs. I get why you'd want a sweeping statement like this, but as @wking mentioned it does make nested tmpfs mounts quite hard -- and in fact would make runC's method of handling cgroups not really possible anymore.
There was a problem hiding this comment.
Also, the "MUST be present in the root filesystem" means that you can't really have nested tmpfs mounts anyway. Because /tmp/a/b must already have existed in the host root filesystem in order for you to mount it (which would require making the directory). Maybe we should make it something like:
The destination MUST already be present if the destination mountpoint is on the root filesystem.
Runtimes MUST NOT modify the container's root filesystem to fulfil destination requirements.
|
I believe the goal here is that the base rootfs is readonly. Mounting multiple directories on top of a tmpfs should be allowed. Getting the wording correct on this is paramount. |
|
@rhatdan What do you think of my proposed wording in #591 (comment)? |
|
May be make it per mount property where callers can specify whether to create destination file/dir or not if it is not present. That should be generic enough to take care of wide variety of use cases. |
|
@cyphar I like your wording. |
|
Can we get more details on how this would break runC's cgroup On Wed, Oct 19, 2016 at 05:33:48AM -0700, Vivek Goyal wrote:
I like this better than @cyphar's wording 2, because it lets folks |
|
I think making it a per mount config may give most flexibility as @rhvgoyal suggested. |
|
I don't like having the option. Why can the runtime not just set to create it if it does not exist and if it cannot it errors. That still leaves it up to the high level to create these directories if its going to have an RO rootfs and if it does not the runtime gives a error. I think an additional field per mount just adds more complexity than just erroring if it does not exist and it cannot be created. |
|
Besides, you are still not solving the real problem of having a shared rootfs with volumes when volumes are a container level config and the rootfs is a shared object across containers. Even if you create these in some "init" layer, containers without a volume at the same path will still have these dirs/files showing up in other containers because the rootfs+init layer is shared. |
|
@crosbymichael I will not create directories for read-only containers and expect them to be part of the image. That way applications will not have any surprise. I think this requires little thought process shift. directories being mounted on need to be part of the image for read-only containers. Alternatively, we users could think of mounting a tmpfs and creating directories in that tmpfs and doing all the volume mounting there for read-only containers? |
|
@rhvgoyal ok, that makes more sense. If that is the case I don't see why we need a spec change as it raises more questions about nested mounts. The runtimes can stay the same and when you mount from the filesystem's you just make it readonly. If the path does not exist the runtime tries to create the dir and errors as a read-only filesystem error and the image author should know that they need to create these dirs or they need to remove a volume mount. Modifying the spec or adding a new field to tell the runtime to create the dir or not is just splitting the information in two places. As an image/container author I have to both, create a dir in the image. Tell the spec, dont create this dir. It should just work as expected. I don't see a spec change adding any value to solving this problem. |
|
@crosbymichael so you are suggesting that caller can leave rootfs read-only so that if runc tries to create a file/dir it fails? We can try it. Hopefully it works with pivot_root(".", ".") stuff. |
|
I mean, if we had this spec MUST how are you supposed to validate or what type of error are you going to return anyways? It would basically be the same with or without. |
This should work. It may require some changes to runc like the pivot_root change. |
|
@mrunalp ya, and doing a stat before a mkdirall. I cannot remember if mkdirall returns an error if the underlaying filesystem is RO but the dir exists. Just something to double check |
|
Closing this per the discussion on OCI weekly call today. We can handle this in the runtime (runc). Thanks! |
This makes it possible to mount content inside a tmpfs. For example, systemd wants a tmpfs at /dev [1] and you may want to also mount /dev/pts, /dev/shm, etc. Or you may want to put a tmpfs at /sys/fs/cgroup and a cgroup filesystem at /sys/fs/cgroup/systemd. There's also some background discussion in [2]. [1]: https://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/ [2]: opencontainers/runtime-spec#591 Subject: Require the mount destination to be created before hand
This makes it possible to mount content inside a tmpfs. For example, systemd wants a tmpfs at /dev [1] and you may want to also mount /dev/pts, /dev/shm, etc. Or you may want to put a tmpfs at /sys/fs/cgroup and a cgroup filesystem at /sys/fs/cgroup/systemd. There's also some background discussion in [2]. [1]: https://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/ [2]: opencontainers/runtime-spec#591 Subject: Require the mount destination to be created before hand
This will allow the rootfs to be truly read only (like a NFS read only mount for e.g.)
cc: @rhvgoyal @philips
Signed-off-by: Mrunal Patel mrunalp@gmail.com