Fixes to ocitools generate to make it work with runc again #24
Conversation
|
@liangchenye PTAL |
| Destination: "/proc", | ||
| Type: "proc", | ||
| Source: "proc", | ||
| Options: nil, |
There was a problem hiding this comment.
@wking runc supports default devices but not mounts yet.
There was a problem hiding this comment.
On Wed, Mar 23, 2016 at 03:26:02PM -0700, Mrunal Patel wrote:
@@ -646,8 +648,53 @@ func getDefaultTemplate() specs.Spec {
},
},
Hostname: "shell",
Mounts: []specs.Mount{},Mounts: []specs.Mount{{Destination: "/proc",Type: "proc",Source: "proc",Options: nil,@wking runc supports default devices but not mounts yet.
Ah, got it. Maybe add a note to that effect in the commit message?
There was a problem hiding this comment.
@wking Updated the commit message.
There was a problem hiding this comment.
On Wed, Mar 23, 2016 at 04:04:14PM -0700, Mrunal Patel wrote:
@@ -646,8 +648,53 @@ func getDefaultTemplate() specs.Spec {
},
},
Hostname: "shell",
Mounts: []specs.Mount{},Mounts: []specs.Mount{{Destination: "/proc",Type: "proc",Source: "proc",Options: nil,@wking Updated the commit message.
Thanks. 85943a0 looks good to me.
This commit adds back the mounts since runc doesn't default these yet. Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
|
LGTM |
| Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k"}, | ||
| }, | ||
| { | ||
| Destination: "/dev/pts", |
There was a problem hiding this comment.
This one is causing trouble in my ccon-oci tests. The previous mount just put a tmpfs at /dev, so there is no directory at /dev/pts to attach the mount. Testing with a few patches to get ccon-oci and runtime_test.sh working together:
$ ./test_runtime.sh -r 'ccon-oci --verbose'
-----------------------------------------------------------------------------------
VALIDATING RUNTIME: ccon-oci --verbose
-----------------------------------------------------------------------------------
launched container process with PID 22796
write '0 1000 1' to /proc/22796/uid_map
write 'deny' to /proc/22796/setgroups
write '0 1000 1' to /proc/22796/gid_map
mount 0: /tmp/tmp.MZQVJfex0V/busybox/./rootfs to /tmp/tmp.MZQVJfex0V/busybox/./rootfs (type: (null), flags: 4096, data (null))
mount 1: /tmp/tmp.MZQVJfex0V/busybox/proc to /tmp/tmp.MZQVJfex0V/busybox/rootfs/proc (type: proc, flags: 0, data (null))
mount 2: /tmp/tmp.MZQVJfex0V/busybox/tmpfs to /tmp/tmp.MZQVJfex0V/busybox/rootfs/dev (type: tmpfs, flags: 16777218, data mode=755,size=65536k)
mount 3: /tmp/tmp.MZQVJfex0V/busybox/devpts to /tmp/tmp.MZQVJfex0V/busybox/rootfs/dev/pts (type: devpts, flags: 10, data newinstance,ptmxmode=0666,mode=0620,gid=5)
mount: No such file or directory
…
I don't see any language about “the runtime MUST create missing destinations” in the spec, and I don't see a way to support this default template without doing that. Are there suggestions for how other runtimes should handle entries like this until runC sorts out its required filesystems support?
There was a problem hiding this comment.
I don't see any language about “the runtime MUST create missing destinations” in the spec, and I don't see a way to support this default template without doing that.
The current spec still doesn't talk about this explicitly, but the intended approach is to have the runtime attempt to create the destination file/directory on its own.
Users who need these mounts would have to explicitly set them up in their configuration (as runtime-tools continues to do [1]) if they wanted to guarantee their presence. Users who don't need them can omit them from their configuration. I don't see how keeping a SHOULD-strength runtime requirement helps either of those workflows. [1]: opencontainers/runtime-tools#24 Signed-off-by: W. Trevor King <wking@tremily.us>
The MUST default-filesystem wording altered in 279c3c0 (linux: relax filesystem requirements for container, 2017-01-23, opencontainers#666) had read (to me, anyway) as: The runtime MUST supply these even if the config doesn't call for them in mounts. with 279c3c0 weaking it to: The runtime SHOULD supply these even if the config doesn't call for them in mounts. But that's not very useful (callers that *need* a given mount will still have to configure it explicitly). However, one interpretation of the 279c3c0 wording seems to be something like [1]: Config authors probably want to include mounts entries for these. That's fine, and this commit tries to make that interpretation more obvious by shifting the config recommendation over to the Linux 'mounts' example. The values I'm using are straight from [2]. [1]: opencontainers#666 (comment) [2]: opencontainers/runtime-tools#24 Signed-off-by: W. Trevor King <wking@tremily.us>
The mount-requirement was softened to a SHOULD in [1]. It's not clear
to me whether that SHOULD is directed at config authors ("you should
explicitly include mounts for these") or at runtimes ("you should
provide these even if the config doesn't ask for them"), but my
attempts to clarify that one way or the other were both rejected
[2,3]. The current runtime-tools and runC approach favors the
config-author direction [4], which is what I'd asked for in the
original thread post, so I'm tagging this obsolete.
[1]: opencontainers/runtime-spec#666
[2]: opencontainers/runtime-spec#679 (comment)
[3]: opencontainers/runtime-spec#678 (comment)
[4]: opencontainers/runtime-tools#24
Signed-off-by: Mrunal Patel mrunalp@gmail.com