libcontainer: Support mounting tmpfs #16
Conversation
| } | ||
|
|
||
| ts := []syscall.Timespec{stat.Atim, stat.Mtim} | ||
| // syscall.UtimesNano doesn't support a NOFOLLOW flag atm, and |
There was a problem hiding this comment.
missing last part in comment?
By adding a mount type of "tmpfs" you can tell libcontainer to mount a fresh tmpfs instance at some point in the container. If CopyContent is also set the existing files at the mountpoint will be copied into the tmpfs. Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
|
rebased and fixed nits |
| @@ -30,6 +30,7 @@ type Mount struct { | |||
| Destination string `json:"destination,omitempty"` // Destination path, in the container | |||
| Writable bool `json:"writable,omitempty"` | |||
| Private bool `json:"private,omitempty"` | |||
| CopyContent bool `json:"copycontent,omitempty"` // Copy original content into tmpfs (for tmpfs mounts) | |||
There was a problem hiding this comment.
So I guess the question is why is this only for tmpfs? Only concern I have is from an API perspective where some of these things start to become arbitrary. I am fine saying that we only do it for tmpfs for the moment.
|
Overall LGTM I think. I'm not a huge fan of having to do this and we're probably gonna run into some issues with it. But it is opt-in for now which makes it more okay. Will wait to hear from @crosbymichael since he had some thoughts around this last time. |
|
Ya, I'm not a fan. I don't think we have any time constraints on this so maybe we can find a better way and not have to bake something like copy into libcontainer |
|
Here we are trying to salvage the times that the user has already mounted some files in /var/run (or elsewhere that we want a tmpfs). What about failing those times and asking the user to "fix" their image? Is that too high a bar for something to ask of the user? |
|
Or change systemd to not try to mount this stuff |
|
There are a lot of people with pre populated /run dirs, postgres and sshd are two that I know of and we cannot break them for this one usecase. |
|
Well, all new distros already mount /run unpopulated at boot. Services that need dirs there need to create them in their init script if the service themself doesn't do it as needed. Any service that works on "bare metal" should work in docker with an empty /run, although its possible that it doesn't becasue people launch the service directly rather than via any init scripts. Here are some details about how /run works: https://wiki.debian.org/ReleaseGoals/RunDirectory Systemd mounts it for two reasons. First of all systemd is itself a heavy user of /run by using it as a store for all transient state (see /run/systemd on a systemd host). Secondly it sets it up as other services may rely on it being a tmpfs, as well as supporting a readonly root boot. Other services may rely on tmpfs being transient and fast, and although its unlikely it not being a tmpfs will completely break things it may well cause performance issues. |
|
Here is what the linked debian policy says: §9.3.2. Writing the scripts '/var/run' and '/var/lock' should be symlinks to '/run' and '/run/lock', respectively. This arrangement may also be satisfied through equivalent means, for example bind or nullfs mounts. Files and directories residing in '/run' should be stored on a temporary filesystem and not be persistent across a reboot, and hence the presence of files or directories in any of these directories is not guaranteed and 'init.d' scripts must handle this correctly. This will typically amount to creating any required subdirectories dynamically when the 'init.d' script is run, rather than including them in the package and relying on 'dpkg' to create them. |
|
👍 on tmpfs support; I was already planning to put together a PR for it. File copying, though, seems like an anti-feature to me... it adds significant complexity here, presumably to support containers which were built under false assumptions. As a libcontainer consumer, I would not expect pre-populated tmpdirs to be supported; that kind of behavior seems better handled elsewhere (e.g., using aufs and throwing away modifications after exit). |
|
The copy is a workaround for existing images that break with a /run tmpfs mounted. I agree that such images are broken, but they exist and we can't break them. Another way would be to make /run mount optional. |
|
@alexlarsson I'd certainly expect mounting /run to be optional at the libcontainer layer. Higher layers should be able to deal with this in their own way -- e.g., docker might default to mounting /run as a tmpfs only if /run is empty, and otherwise issue a warning. This would ensure that all new images use ephemeral /run directories, but maintain the existing behavior for containers relying on it. |
|
I don't think they're broken at all, and that this isn't just for backwards For normal systems, setting up /run happens in the init script. In Docker, |
| if err := system.Mount("tmpfs", dest, "tmpfs", uintptr(defaultMountFlags), mountLabel); err != nil { | ||
| return fmt.Errorf("mounting %s into %s %s", m.Source, dest, err) | ||
| } | ||
| return fmt.Errorf("mounting %s into %s %s", m.Source, dest, err) |
There was a problem hiding this comment.
This always returns an error when mounting a tmpfs and not copying the content.
By adding a mount type of "tmpfs" you can tell libcontainer to mount a
fresh tmpfs instance at some point in the container. If CopyContent is
also set the existing files at the mountpoint will be copied into the
tmpfs.
This will be used from docker to implement /run.
Docker-DCO-1.1-Signed-off-by: Alexander Larsson alexl@redhat.com (github: alexlarsson)