Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mount propagation for btrfs #38026

Merged
merged 3 commits into from
Oct 12, 2018
Merged

Conversation

kolyshkin
Copy link
Contributor

For some reason, shared mount propagation between the host
and a container does not work for btrfs, unless container
root directory (i.e. graphdriver home) is a bind mount.

The above issue was reproduced on SLES 12sp3 + btrfs using
the following script:

        #!/bin/bash
        set -eux -o pipefail

        # DIR should not be under a subvolume
        DIR=${DIR:-/lib}
        MNT=$DIR/my-mnt
        FILE=$MNT/file

        ID=$(docker run -d --privileged -v $DIR:$DIR:rshared ubuntu sleep 24h)
        docker exec $ID mkdir -p $MNT
        docker exec $ID mount -t tmpfs tmpfs $MNT
        docker exec $ID touch $FILE
        ls -l $FILE
        umount $MNT
        docker rm -f $ID

which fails this way:

        + ls -l /lib/my-mnt/file
        ls: cannot access '/lib/my-mnt/file': No such file or directory

meaning the mount performed inside a priviledged container is not
propagated back to the host (even if all the mounts have "shared"
propagation mode).

The remedy to the above is to make graphdriver home a bind mount.

1. There is no need to specify rw argument -- bind mounts are
   read-write by default.

2. There is no point in parsing /proc/self/mountinfo after performing
   a mount, especially if we don't check whether the fs is mounted or
   not -- the only outcome from it could be an error from our mountinfo
   parser, which makes no sense in this context.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function ensures the argument is the mount point
(i.e. if it's not, it bind mounts it to itself).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For some reason, shared mount propagation between the host
and a container does not work for btrfs, unless container
root directory (i.e. graphdriver home) is a bind mount.

The above issue was reproduced on SLES 12sp3 + btrfs using
the following script:

	#!/bin/bash
	set -eux -o pipefail

	# DIR should not be under a subvolume
	DIR=${DIR:-/lib}
	MNT=$DIR/my-mnt
	FILE=$MNT/file

	ID=$(docker run -d --privileged -v $DIR:$DIR:rshared ubuntu sleep 24h)
	docker exec $ID mkdir -p $MNT
	docker exec $ID mount -t tmpfs tmpfs $MNT
	docker exec $ID touch $FILE
	ls -l $FILE
	umount $MNT
	docker rm -f $ID

which fails this way:

	+ ls -l /lib/my-mnt/file
	ls: cannot access '/lib/my-mnt/file': No such file or directory

meaning the mount performed inside a priviledged container is not
propagated back to the host (even if all the mounts have "shared"
propagation mode).

The remedy to the above is to make graphdriver home a bind mount.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

@nikitych @dmcgowan @cpuguy83 PTAL

@codecov
Copy link

codecov bot commented Oct 12, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@07ccc6d). Click here to learn what that means.
The diff coverage is 35.29%.

@@            Coverage Diff            @@
##             master   #38026   +/-   ##
=========================================
  Coverage          ?    36.1%           
=========================================
  Files             ?      610           
  Lines             ?    45190           
  Branches          ?        0           
=========================================
  Hits              ?    16318           
  Misses            ?    26634           
  Partials          ?     2238

@crosbymichael
Copy link
Contributor

LGTM

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants