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

default to slave mounting #158

Merged
merged 1 commit into from
May 27, 2016

Conversation

runcom
Copy link
Collaborator

@runcom runcom commented May 27, 2016

Fix BZ https://bugzilla.redhat.com/show_bug.cgi?id=1339146

@rhatdan @rhvgoyal @mrunalp @ncdc PTAL could this clash with @rhvgoyal's patches we backported here?

Signed-off-by: Antonio Murdaca runcom@redhat.com

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented May 27, 2016

LGTM

@rhatdan
Copy link
Member

rhatdan commented May 27, 2016

@rh-atomic-bot r+

@rhatdan rhatdan merged commit 3787b39 into projectatomic:rhel7-1.10.3 May 27, 2016
@cgwalters
Copy link
Member

Enabling Homu for this repo is possible but we need to define the test suite.

@runcom runcom deleted the mount-prop-rslave branch May 27, 2016 14:38
@runcom
Copy link
Collaborator Author

runcom commented May 27, 2016

I'd love if we could test this before building for rhel, I have also a question for Vivek in the first comment

@cgwalters
Copy link
Member

I have been working on both PR and Homu in CentOS/sig-atomic-buildscripts#81

However it doesn't yet cover Docker because docker.spec: https://github.com/cgwalters/rpmdistro-gitoverlay/issues/24

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

@ncdc @rhatdan this has broken docker when running integration tests:

Step 2 : RUN apt-get update && apt-get install -y   gcc     libc6-dev   --no-install-recommends     && rm -rf /var/lib/apt/lists/*
 ---> Running in e7f6029e0e2c
Cannot start container e7f6029e0e2ce235e496a5a7bec7377eac1cad45f8c6f634b27be0b16d468249: Path /var/lib/docker/containers/e7f6029e0e2ce235e496a5a7bec7377eac1cad45f8c6f634b27be0b16d468249/resolv.conf is mounted on /var/lib/docker but it is not a shared or slave mount.
/go/src/github.com/docker/docker/hack/make.sh: line 258: local: can only be used in a function

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

@rhvgoyal ^^

@rhatdan
Copy link
Member

rhatdan commented May 31, 2016

Seems like it is a bogus test?

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

I don't think so, it's not starting the container at all:

Cannot start container e7f6029e0e2ce235e496a5a7bec7377eac1cad45f8c6f634b27be0b16d468249: Path /var/lib/docker/containers/e7f6029e0e2ce235e496a5a7bec7377eac1cad45f8c6f634b27be0b16d468249/resolv.conf is mounted on /var/lib/docker but it is not a shared or slave mount.

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

docker run seems to work but you can't run "make test-integration-cli" because of that error - thus we cannot validate tests

@rhatdan
Copy link
Member

rhatdan commented May 31, 2016

So containers won't start or just this test is failing?

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

make test-integration-cli isn't working - if you docker run it works - but something is going on with tests and it's related to this patch :/

@rhatdan
Copy link
Member

rhatdan commented May 31, 2016

Ok so if your system is not setup as a slave mount then this blows up this test.

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

@rhatdan alright, is this patch needed in fedora also? Because I didn't cherry-pick it, should I?

@rhatdan
Copy link
Member

rhatdan commented May 31, 2016

I think so since OpenShift can not switch to the new method until the next release.

@rhatdan
Copy link
Member

rhatdan commented May 31, 2016

We need to talk to Vivek when he gets in.

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

alright

@rhvgoyal
Copy link

It is complaining that you are trying to make a volume "slave" but it will not work because its parent mount has to be shared or slave for propagation to work. In this case looks like /var/lib/docker is the parent mount and looks like it is not "shared" or "slave". It must be "private". And that's why it fails.

Who is mounting /var/lib/docker/ as "private"?

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

I think that's the default on my Fedora 24 box Vivek (updated from F23)

@rhvgoyal
Copy link

I think easiest fix is to convert these volume mounts to "rprivate". I don't think anybody is expecting any mount propagations on "resolv.conf"

@rhvgoyal
Copy link

So default will be "rshared" but make these internally defined individual volumes to be "rprivate". That will ensure whatever user is mounting using "-v" will be mounted as "rshared".

@runcom
Copy link
Collaborator Author

runcom commented May 31, 2016

The same it's happening with shm, should I do the same for shm?

@rhvgoyal
Copy link

I think so. Either we need to make "/var/lib/docker" "slave" or convert these individual volumes to "rprivate". I think later is safer approach as we are not expecting these docker defined volumes to be "shared". So yes, do the same for "shm" too.

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

Successfully merging this pull request may close these issues.

4 participants