Remove MountFlags=slave from systemd unit definitions

[errordeveloper]

This prevents one from using mount propagation flags in Docker v1.10 (see moby/moby#19625).

[nathanleclaire]

Thanks @errordeveloper -- Can you elaborate a bit on your use case? e.g. some steps to reproduce that don't work without this patch. My notion of how this ties together is fairly hazy. If MountFlags=slave is what's set by the default Docker packages maybe we should consider an additional --engine-* type flag to enable this conditionally as well.

[errordeveloper]

@nathanleclaire what I'm working on is a thing that makes Kubernetes deployment very easy.

Here is the project itself: https://github.com/weaveworks/weave-kubernetes-anywhere

And here is the Docker Machine example: https://github.com/weaveworks/weave-kubernetes-anywhere/tree/master/examples/docker-machine

Feel free to look at the real thing, the failure surfaced for me on DigitalOcean, and seemed extremely obscure for a few hours. The code that actually fails is in setup-kubelet-volumes.sh, which is called from create-cluster.sh. The reason this is required is that kubelet happens to mount secrets in a tmpfs, for some extra security.

Here is the basics test that fails:

> docker-machine create -d digitalocean --digitalocean-access-token XYZ test-1
...
> docker-machine ssh test-1
...
# mount --make-rshared /
# docker run -v /mnt:/host-mnt:rw,rshared busybox
docker: Error response from daemon: Cannot start container 6859ea4fc2f23130da7e72f301c9a82528c6e78e406b6f68261d000410ad6960: Path /mnt is mounted on / but it is not a shared mount..

[errordeveloper]

It would be helpful to know when and why MountFlags=slave first appeared. It is certainly still shipped in official packages. I have asked moby/moby#19625 (comment) and haven't got an answer just yet. I am suspecting this a kind of thing that was added one day and people don't touch it cause they don't know what it is exactly, it really needs to be clarified now, as it stand on the way for someone needing to use a feature available in the latest release.

[AkihiroSuda]

@errordeveloper

Without the flag, the host-side /dev/* can be unexpectedly unmounted.

moby/moby#20670

[AkihiroSuda]

FYI, this commit introduced the flag
(You can find such information by clicking "blame" button in
GitHub web UI)
moby/moby@eb76cb2

[nathanleclaire]

Yeah, I think we need to err on the side of sticking with Docker upstream here, I'd be willing to consider proposals to customize turning these types of features on/off during provisioning, however it's a slippery slope so we need to be careful about what kind of customization we enable (supporting on all provisioners etc. is difficult).

[errordeveloper]

I think there should be a more explicit way to fix the issue with /dev, which might lie in the Docker daemon rather then the systemd unit...

[zhulinhong]

Remove MountFlags=slave
The systemd default flags is shared, mounts and unmounts are propagated from the host to the container and vice versa. Is there has the security issue.
Is user can unmount the volume in the container?

[errordeveloper]

I've submitted this change to Docker upstream, how is it mean to propagate to Machine?

[zhulinhong]

Probably the mounts and unmounts operations on the host will effect the containers.
And the mounts and unmounts operations in container will effect the host.

[errordeveloper]

@zhulinhong not that, I mean moby/moby#22806 has been merged, so the question is if I still need to make a PR to this repo or what?

[zhulinhong]

No need, thx a lot. It's configurable, I can change the config if I need.

[nathanleclaire]

I think we'll still need it, to ensure the upstream changes aren't over-written to be excluded, if you'd like to re-open.

[errordeveloper]

Ok, that's what I was wondering, i.e. whether I should copy upstream
version of the units here, there may be other flags too.

On Wed, 8 Jun 2016, 00:49 Nathan LeClaire, notifications@github.com wrote:

I think we'll still need it, to ensure the upstream changes aren't
over-written to be excluded, if you'd liek to re-open.

—
You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
#3029 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAPWS5xXE17w8M4cXUkkcxC5Vr4VP6IRks5qJgNtgaJpZM4HXa1Z
.

[nathanleclaire]

Ah, yes, let's stick as closely as possible to upstream please.

[pbecotte]

@errordeveloper @nathanleclaire This PR is still needed- the systemd units from machine overwrite the ones from upstream, so this is still setting this flag.