graphdriver: promote overlay driver to first #12354

Closed
wants to merge 1 commit into
from

Projects

None yet
@vbatts
Contributor
vbatts commented Apr 14, 2015

Upgrade overlayfs to first place, now that this will not break driver
usage on existing installs.

Splitting this PR out from #11999 (comment)

Issue #12327 and #12080 flagged overlay for further review.

Notable overlay issues to sort out:

  • expected behavior of unix sockets
  • open("foo.txt", O_RDONLY); open("foo.txt", O_RDWR)

@LK4D4 @jfrazelle @crosbymichael

Signed-off-by: Vincent Batts vbatts@redhat.com

@vbatts vbatts graphdriver: promote overlay driver to first
Upgrade overlayfs to first place, now that this will not break driver
usage on existing installs.

Signed-off-by: Vincent Batts <vbatts@redhat.com>
035a267
@ahmetalpbalkan
Contributor

@vbatts should we also update install.sh?

# aufs is preferred over devicemapper; try to ensure the driver is available.
if ! grep -q aufs /proc/filesystems && ! $sh_c 'modprobe aufs'; then
        kern_extras="linux-image-extra-$(uname -r)"

        apt_get_update
        ( set -x; $sh_c 'sleep 3; apt-get install -y -q '"$kern_extras" ) || true

        if ! grep -q aufs /proc/filesystems && ! $sh_c 'modprobe aufs'; then
                echo >&2 'Warning: tried to install '"$kern_extras"' (for AUFS)'
                echo >&2 ' but we still have no AUFS.  Docker may not work. Proceeding anyways!'
                ( set -x; sleep 10 )
        fi
fi
@vbatts
Contributor
vbatts commented Apr 14, 2015

@ahmetalpbalkan i don't think so.

  • If the install.sh does install aufs-tools, and the kernel supports overlay, then overlay wins`.
  • if aufs-tools installs and the kernel does not support overlay, then aufs will win

to my knowledge kernels that are supporting overlay are not packaging it into a module to ship separately. (but i've been wrong before)

@ahmetalpbalkan
Contributor

ah ok we're cool then

@icecrime icecrime removed the dco/yes label Apr 23, 2015
@calavera
Contributor

The code LGTM and I'm 👍 on having overlay as first option. @vbatts is this PR ready to ship as it is? Do we need to update some documentation to reflect this change?

@vbatts
Contributor
vbatts commented Apr 29, 2015

@calavera
Now that #11999 is merged, this change would be a no-op for existing users, and only make fresh installs use overlay when supported.

I'm ready for overlay to get promoted, though there are a couple of outstanding questions about it before bumping it to first choice for general consumption.

@thaJeztah
Member

Should this be added to the 1.7 milestone, to try and get all outstanding questions resolved before 1.7 (don't know if that's general practice this early)?

@vbatts
Contributor
vbatts commented Apr 30, 2015

Would love to.
On Apr 29, 2015 8:32 PM, "Sebastiaan van Stijn" notifications@github.com
wrote:

Should this be added to the 1.7 milestone, to try and get all outstanding
questions resolved before 1.7 (don't know if that's general practice this
early)?


Reply to this email directly or view it on GitHub
#12354 (comment).

@thaJeztah
Member

@vbatts oh! Just noticed something, hint: 💯 🍰

@vbatts
Contributor
vbatts commented May 2, 2015

I don't follow.
On May 2, 2015 12:14 AM, "Sebastiaan van Stijn" notifications@github.com
wrote:

@vbatts https://github.com/vbatts oh! Just noticed something, hint: [image:
💯] [image: 🍰]


Reply to this email directly or view it on GitHub
#12354 (comment).

@thaJeztah
Member

@vbatts it's your 100th PR 😄

@vbatts
Contributor
vbatts commented May 2, 2015

Oh! Cute.
On May 2, 2015 8:14 AM, "Sebastiaan van Stijn" notifications@github.com
wrote:

@vbatts https://github.com/vbatts it's your 100th PR [image: 😄]


Reply to this email directly or view it on GitHub
#12354 (comment).

@thaJeztah
Member

Oh, shucks had the sort order wrong;12814 is the actual 100th (sorry for the off-topic noise)

@LK4D4
Contributor
LK4D4 commented May 18, 2015

@vbatts We're waiting for something fixed before merging this?

@vbatts
Contributor
vbatts commented May 18, 2015

@LK4D4 I don't think overlay is ready for first position... from the /system/overlay labeled issues, I've been working to get concise preproductions to get upstream traction on fixes. I think we should hold on this for a bit.

@LK4D4
Contributor
LK4D4 commented May 18, 2015

@vbatts Thanks for answer!

@unclejack
Contributor

+1 on not doing this now. We've had a lot of activity regarding overlay problems and making this change would only increase the number of reports for those issues (and, perhaps, others).

@thaJeztah
Member

If the "experimental builds" materialize, perhaps make it the default for those builds; that will give a bigger audience for testing it, and those running "experimental" should be aware of possible risks.

@vbatts
Contributor
vbatts commented May 18, 2015

well, users have the option (and do) pass -s overlay to the daemon. I
wouldn't clutter the experimental build with so many things like this such
that it's a bad time.

On Mon, May 18, 2015 at 4:12 PM, Sebastiaan van Stijn <
notifications@github.com> wrote:

If the "experimental builds" materialize, perhaps make it the default for
those builds; that will give a bigger audience for testing it, and those
running "experimental" should be aware of possible risks.


Reply to this email directly or view it on GitHub
#12354 (comment).

@tianon
Member
tianon commented May 29, 2015

@vbatts what are the plans with respect to high inode usage? 😢

@tianon
Member
tianon commented May 29, 2015

(I had to reformat my partition with less "bytes per inode" to stop exhausting them -- you can't even runtime change that, it has to be reformat, so is really disruptive.)

@SamSaffron

@tianon is there a bug open regarding the high inode usage?

@tianon
Member
tianon commented May 29, 2015

Not that I know of -- it only happens if you've got lots of images (or large images), and as I understand it relates to the way overlayfs has to manage hardlinks between layers? The details are fuzzy from when @vbatts first explained it to me.

@tianon
Member
tianon commented May 29, 2015

"High inode usage with overlay" isn't going to be a very actionable issue, though, to be fair. 😄

@jessfraz
Contributor

we are adding it to the docs as a known issue, its not really something we
can control

On Thu, May 28, 2015 at 5:16 PM, Tianon Gravi notifications@github.com
wrote:

"High inode usage with overlay" isn't going to be a very actionable issue,
though, to be fair. [image: 😄]


Reply to this email directly or view it on GitHub
#12354 (comment).

@SamSaffron

coreos/bugs#264

@jfrazelle docker could potentially warn if bytes per inode is too high, probably the simplest actionable item short term.

@tianon
Member
tianon commented Jun 8, 2015

How many "bytes per inode" is too high though? It depends pretty heavily not just on the number of images you use, but even which images you use, so it's going to vary pretty wildly.

@SamSaffron

Docker itself can be helpful here, if it notices you are using say 80% of
your inodes it could issue a warning in the CLI letting you know that
stuff, soon, is going to explode.

Resizing is a huge pita though, I guess if you are on overlay you best be
way prepared.

Found this writeup kind of interesting
http://blog.cloud66.com/docker-with-overlayfs-first-impression/

On Tue, Jun 9, 2015 at 8:03 AM, Tianon Gravi notifications@github.com
wrote:

How many "bytes per inode" is too high though? It depends pretty heavily
not just on the number of images you use, but even which images you use, so
it's going to vary pretty wildly.


Reply to this email directly or view it on GitHub
#12354 (comment).

@rhvgoyal
Contributor

@vbatts, so all the issues with overlay have been resolved now that it is considered safer default? Specifically this one.

#10180

@unclejack
Contributor

@rhvgoyal https://github.com/docker/docker/labels/%2Fsystem%2Foverlay There are still a lot of issues that are open.

@SamSaffron

@unclejack @jfrazelle I just don't get how device mapper is before overlay in the list. device mapper is 100% broken for Discourse installs as it is, you can not bootstrap, overlay works, inode count is solvable (with warnings from Docker ideally)

boot time of our rails app goes down from 4.3 seconds to 3.2 using overlay.

@rhvgoyal
Contributor

@SamSaffron

What's broken about device mapper? What do you mean by you can not bootstrap?

@unclejack
Contributor

@SamSaffron Devicemapper requires a dynamically linked Docker binary in order to have a stable environment. Devicemapper should also not be used with loopback mounted files, otherwise you run into all sorts of weird things.

Overlay isn't particularly stable right now. There are multiple rather severe bugs and I wouldn't recommend it to be used in production.

@SamSaffron

https://meta.discourse.org/t/cannot-start-container/16049/4

https://meta.discourse.org/t/discourse-docker-installation-without-aufs/15639/6

https://meta.discourse.org/t/discourse-on-centos-7/20701/4

https://meta.discourse.org/t/cant-bootstrap-discourse-docker-on-digitalocean-arch-linux/20541/5

we still get these reports on a regular basis, probably one every few
weeks.

On Mon, Jun 15, 2015 at 11:18 PM, Vivek Goyal notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron

What's broken about device mapper? What do you mean by you can not
bootstrap?


Reply to this email directly or view it on GitHub
#12354 (comment).

@SamSaffron

@unclejack perhaps devicemapper should require "-s devicemapper" same as
overlay, I guess my big objection is that Docker can start up in a broken
state and it feels like overlay is less broken... but in all means remove
the drivers that you know are unstable and force people to opt in with -s

On Mon, Jun 15, 2015 at 11:26 PM, unclejack notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron Devicemapper requires a
dynamically linked Docker binary in order to have a stable environment.
Devicemapper should also not be used with loopback mounted files, otherwise
you run into all sorts of weird things.

Overlay isn't particularly stable right now. There are multiple rather
severe bugs and I wouldn't recommend it to be used in production.


Reply to this email directly or view it on GitHub
#12354 (comment).

@rhvgoyal
Contributor

@SamSaffron

In above links, I can't see where is the issue with devicemapper. Instead you somehow seems to be little adamant to prove devicemapper is the issue.

aufs is not the solution as it is not upstream and all the distributions don't support it.

If you have a dynamically built docker binary, devicemapper works very well and is a stable solution.

@rhvgoyal
Contributor

@SamSaffron

Stop making assertion that devicemapper is unstable hence should require -s. it is not unstable. If you want to make that assertion, back it up with some data and analysis.

@SamSaffron

But you don't ship a dynamically built docker binary, I can repro this
broken on the shipped binary in about 15 minutes.

On Mon, Jun 15, 2015 at 11:45 PM, Vivek Goyal notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron

In above links, I can't see where is the issue with devicemapper. Instead
you somehow seems to be little adamant to prove devicemapper is the issue.

aufs is not the solution as it is not upstream and all the distributions
don't support it.

If you have a dynamically built docker binary, devicemapper works very
well and is a stable solution.


Reply to this email directly or view it on GitHub
#12354 (comment).

@SamSaffron

Tomorrow, its getting late today, will open a ticket with a repro and ping
you on it.

On Mon, Jun 15, 2015 at 11:46 PM, Vivek Goyal notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron

Stop making assertion that devicemapper is unstable hence should require
-s. it is not unstable. If you want to make that assertion, back it up with
some data and analysis.


Reply to this email directly or view it on GitHub
#12354 (comment).

@rhvgoyal
Contributor

@SamSaffron Static docker binary does not work well with devicemapper. That's a known issue. Dynamic binary works just fine. And distributions are shipping dynamic binary which works well with devicemapper graph driver.

@cpuguy83
Contributor

@SamSaffron Docker 1.7 will refuse to start if it's statically compiled and devicemapper is selected.

@SamSaffron

@vivek I stand corrected I am not able to trivially crash devicemapper on
latest, so there has been progress there. I wonder how many of the issues
we were seeing were related to older kernels and older versions of docker.

on ubuntu 14.04 with latest docker I can not seem to trivially crash this

On Mon, Jun 15, 2015 at 11:53 PM, Vivek Goyal notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron Static docker binary does not
work well with devicemapper. That's a known issue. Dynamic binary works
just fine. And distributions are shipping dynamic binary which works well
with devicemapper graph driver.


Reply to this email directly or view it on GitHub
#12354 (comment).

@SamSaffron

@brian awesome, that is a good change

I still think the inode warning would be super cool for overlay ... once
inodes are say 90% used up warn user

On Tue, Jun 16, 2015 at 12:17 AM, Sam Saffron sam.saffron@gmail.com wrote:

@vivek I stand corrected I am not able to trivially crash devicemapper on
latest, so there has been progress there. I wonder how many of the issues
we were seeing were related to older kernels and older versions of docker.

on ubuntu 14.04 with latest docker I can not seem to trivially crash this

On Mon, Jun 15, 2015 at 11:53 PM, Vivek Goyal notifications@github.com
wrote:

@SamSaffron https://github.com/SamSaffron Static docker binary does
not work well with devicemapper. That's a known issue. Dynamic binary works
just fine. And distributions are shipping dynamic binary which works well
with devicemapper graph driver.


Reply to this email directly or view it on GitHub
#12354 (comment).

@rhvgoyal
Contributor

@SamSaffron

I think we have made lot of progress on improving devicemapper graph driver. Lot of issues are because of static binary and I think many of the issues are configuration issues. Based on configuraiton, one can easily switch between loop based thin pool, or thin pool on block devices or
usage of external lvm thin pool. If such switch happens, data in /var/lib/docker/devmapper/ is stale
and all sorts of errors will happen.

To make things little better and detect configuation issues early, I have created following PR. This should help a bit I think.

#13896

@icecrime
Member
icecrime commented Jul 7, 2015

@vbatts Considering the comments here, WDYT: too soon, close it?

@vbatts
Contributor
vbatts commented Jul 7, 2015

too soon

@vbatts vbatts closed this Jul 7, 2015
@rhatdan
Contributor
rhatdan commented Jul 16, 2015

Red Hat kernel engineers have been working hard on Overlayfs, (Perhaps others on the outside, but I am not familiar). We hope to turn it on for docker in rhel7.2 BUT their are many concerns around it.

  • First off (From my perspective) still no SELinux support for labeling containers differently. There are many patches for this but none have gotten into the upstream kernel.
  • Second Very limited testing has been done on file system support on top of OVERLAYFS, IE How well does ext3, ext4, xfs ... run on top of Overlayfs.
  • Third, we have found applications (rpm) that open a file for read/only and later open for Read/Write which end up with two different files and lots of stuff gets screwed up. While their are fixes being developed for this in rpm/yum/dnf. We are not sure if their are others. These are touch to diagnose so overlayfs has added logging to discover when this happens and report it to dmesg.

I agree that it is too soon for default.

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