linux: relax filesystem requirements for container #666
Conversation
change MUST to SHOULD so containers are not required to have all these filesystems mounted. Signed-off-by: Daniel Dao <dqminh89@gmail.com>
|
I believe |
|
On Mon, Jan 23, 2017 at 08:00:44AM -0800, Justin Cormack wrote:
Also `/dev` seems curiously missing...
It does not have to be a separate filesystem. I'd floated devtmpfs in
the past [1,2], but folks can use other mechanisms to provide the
required devices [3].
If we're making these mounts optional for security reasons [4], we'd
be overturning an earlier decision on adding these (without a way to
opt-out [5]). For example, @philips has “considered making it
optional in appc but I don't think it hurts anything to have it” for
/dev/shm [6].
And if we do lift the requirement for these mounts, I'd rather just
drop the “Default Filesystems” section entirely. Users who need these
mounts would have to explicitly set them up in their configuration (as
runtime-tools continues to do, see opencontainers/runtime-tools#24) 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 section (vs. dropping it completely) helps either of
those workflows.
[1]: #95 (comment)
[2]: #164 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc3/config-linux.md#default-devices
[4]: opencontainers/runc#1176 (comment)
[5]: #95 (comment)
[6]: #95 (comment)
|
Makes sense. WDYT about:
|
|
@wking |
|
On Mon, Jan 23, 2017 at 09:38:46AM -0800, Daniel, Dao Quang Minh wrote:
- /dev/pts must be available if terminal= true ( dont we want to
take it out of process, i dont quite remember ?? )
You may be thinking about #663.
- leave out /dev/shm and the rest. I dont actually had any special
requirements to not have /dev/shm…
If we're rolling back the earlier “you won't need to opt out” position
in one case, I feel like we should roll it back for all of them.
Explicitly declaring these mounts is not particularly complicated
(opencontainers/runtime-spec#24) and we've already been wrong about
the need for opting-out once.
|
sysctl writes value to /proc/sys, not /sys. so it is not needed to mount sysfs to /sys. |
|
@Kxuan sysctl is not sysfs, please don't get those confused. sysfs should be mounted at /sys, if the container needs it. Depends on what is running in it. |
|
@gregkh yes, I know it. but they are discussing about "/sys if you specify sysctl config." |
|
LGTM The spec does not specify what you MUST populate in your own spec. If I'm writing a spec I can add whatever I want in it. This section is meant as a hint to spec authors that these filesystems should be made available. We need to stick to specing the schema for the fields in the spec and not spec what users put in it. |
With the phrasing in 279c3c0, it looks like we're going from “the runtime MUST supply these even if the config doesn't call for them in
|
|
LGTM |
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. [1]: opencontainers#666 (comment) 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>
Since 279c3c0 (linux: relax filesystem requirements for container, 2017-01-23, opencontainers#666) it's no longer garunteed that /proc will exist. And there doesn't seem to be much point in requiring symlinks which will be known broken. This commit also tightens the timing. Before it was just "after the container has `/proc` mounted", which could have happened during the 'delete' operation (if the container authors wanted to be especially ornery). With this commit, I've put the creation in step 2 of the lifecycle. And within step 2, it happens after 'mounts' has been processed. Signed-off-by: W. Trevor King <wking@tremily.us>
Since 279c3c0 (linux: relax filesystem requirements for container, 2017-01-23, opencontainers#666) it's no longer guaranteed that /proc will exist. And there doesn't seem to be much point in requiring symlinks which will be known broken. This commit also tightens the timing. Before it was just "after the container has `/proc` mounted", which could have happened during the 'delete' operation (if the container authors wanted to be especially ornery). With this commit, I've put the creation in step 2 of the lifecycle. And within step 2, it happens after 'mounts' has been processed. 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
change MUST to SHOULD so containers are not required to have all these filesystems mounted.
Related to discussions in opencontainers/runc#1176
cc @cyphar @crosbymichael @philips @justincormack