@rhvgoyal

This brings in the mount propagation changes into docker but we are having some issues when running containers with --read-only. I think it has something due to the changes, maybe something around the root not being PRIVATE anymore or something else with the mounts and propagation flags. Could u please take a look?

@crosbymichael I looked briefly at my patch and can't think how it is leading to failure. Tomorrow I will spend more time on this. First I need to understand the test which is failing and that will give more clues.

BTW, my patch does not change the propagation property of rootfs and it still continues to be PRIVATE.

For me --read-only works with -ti option but not without it.

"docker run --red-only fedora ls" is failing while "docker run -ti --read-only fedora ls" succeeds.

I did strace on docker daemon and somebody is doing "fchown()" on a read only file system hence -EROFS is returned.

30347 fchown(0, 0, 0) = -1 EROFS (Read-only file system)

I am not sure who is doing fchown() yet.

@mrunalp Would you have any idea.

Ok, setupUser() if failing in libcontainer (init_linux.go)

    if err := setupUser(config); err != nil {
            return err
    }

Fchown is probably from the user ns related two patches.

Sent from my iPhone

On Sep 22, 2015, at 8:23 AM, Vivek Goyal [email protected] wrote:

@mrunalp Would you have any idea.

—
Reply to this email directly or view it on GitHub.

Following commit introduce Fchown()
commit 0dad64f7ad4d27a02aa0c6605304832e87a79974
Author: Michael Crosby [email protected]
Date: Fri Sep 18 13:55:49 2015 -0700

Fix STDIO permissions when container user not root

Fix the permissions of the container's main processes STDIO when the
process is not run as the root user.  This changes the permissions right
before switching to the specified user so that it's STDIO matches it's
UID and GID.

Add a test for checking that the STDIO of the process is owned by the
specified user.

Signed-off-by: Michael Crosby <[email protected]>

ok, thanks for taking a look at this. I was not sure about changing the propagation of the rootfs from PRIVATE to something else or not.

@crosbymichael rootfs propagation mode change patches have not been merged yet. I am reworking those patches now.

Ok, i figured out the issue, it's with -i and the readonly containers when there is no stdin.

michael|~ > docker run ubuntu ls -l /proc/1/fd/
total 0
lr-x------ 1 root root 64 Sep 22 17:41 0 -> /dev/null
l-wx------ 1 root root 64 Sep 22 17:41 1 -> pipe:[143849]
l-wx------ 1 root root 64 Sep 22 17:41 2 -> pipe:[143850]
lr-x------ 1 root root 64 Sep 22 17:41 3 -> /proc/1/fd
michael|~ > docker run -i ubuntu ls -l /proc/1/fd/
total 0
lr-x------ 1 root root 64 Sep 22 17:41 0 -> pipe:[142163]
l-wx------ 1 root root 64 Sep 22 17:41 1 -> pipe:[142167]
l-wx------ 1 root root 64 Sep 22 17:41 2 -> pipe:[142168]
lr-x------ 1 root root 64 Sep 22 17:41 3 -> /proc/1/fd

Oh, so without "-i", stdin is pointing to /dev/null and we try to change ownership of /dev/null instead? And that will fail as / is read only.

Naw, I just have to either ignore it if it's /dev/null or find a robust way to handle this case without being too specific.

I assume if we are going to pick up the update to handle /dev/null properly (opencontainers/runc#289), we should grab whatever gets merged to handle user ns join; currently I think we are leaning towards opencontainers/runc#288?

I think the combination of those (plus the ones included already in this PR) fix all known issues with user ns from a lower layer (libcontainer) perspective.

@icecrime @estesp @tiborvass this one should be good to go execpt for waiting on the apparmor test fixes. I can rebase once @icecrime is done doing what he is doing.

Looks like switching to the vendored netlink library kills Windows daemon build. I assume putting a dummy .go file in netlink/nl would fix the compilation problem, but is the getDefaultRouteMtu actually working today for Windows daemon operation? @jhowardmsft

@estesp i'm fixing that in runc now

@estesp - No, it's not used on Windows (yet)

LGTM