-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
linux: drop check for /proc as invalid dest #1832
linux: drop check for /proc as invalid dest #1832
Conversation
@giuseppe Looks like you missed some function calls in tests. |
3da0739
to
da6be08
Compare
tests fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@opencontainers/runc-maintainers PTAL |
@dqminh would you mind to take a quick look at this patch as well? :-) |
da6be08
to
de879c7
Compare
89bba2e
to
638fdda
Compare
638fdda
to
cc44090
Compare
Unfortunately this is not enough to get it working as runc doesn't allow to bind mount /proc. Depends on: opencontainers/runc#1832 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Unfortunately this is not enough to get it working as runc doesn't allow to bind mount /proc. Depends on: opencontainers/runc#1832 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Closes: #1349 Approved by: rhatdan
@cyphar @crosbymichael @dqminh PTAL |
libcontainer/rootfs_linux.go
Outdated
@@ -53,7 +53,7 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig) (err error) { | |||
} | |||
} | |||
|
|||
if err := mountToRootfs(m, config.Rootfs, config.MountLabel); err != nil { | |||
if err := mountToRootfs(m, config.Rootfs, config.MountLabel, config.Namespaces.Contains(configs.NEWNS)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you using NEWNS here when the description of the PR talks about NEWPID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry that was a leftover of a cleanup. The patch was originally written to address only the "chroot" case but I've changed it to address also another problem: /proc
could not be bind mounted from the host. This is useful for rootless containers when the PID namespace is shared with the host (and the container cannot mount a new /proc
).
I've pushed a new version which is much simpler now.
cc44090
to
fe0d9b1
Compare
it is now allowed to bind mount /proc. This is useful for rootless containers when the PID namespace is shared with the host. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
fe0d9b1
to
636b664
Compare
it is now allowed to bind mount /proc when the PID namespace is shared
with the host.
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com