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

Detect overlay2 support on pre-4.0 kernels #35527

Merged
merged 1 commit into from Nov 29, 2017

Conversation

Projects
None yet
6 participants
@thaJeztah
Member

thaJeztah commented Nov 16, 2017

fixes #34368

The overlay2 storage-driver requires multiple lower dir support for overlayFs. Support for this feature was added in kernel 4.x, but some distros (RHEL 7.4, CentOS 7.4) ship with an older kernel with this feature backported.

This patch adds feature-detection for multiple lower dirs, and will perform this feature-detection on pre-4.x kernels with overlayFS support.

With this patch applied, daemons running on a kernel with multiple lower dir support will now select "overlay2" as storage-driver, instead of falling back to "overlay".

- Description for the changelog

+ Add support for `overlay2` storage driver on CentOS/RHEL 7.4 [moby/moby#35527](https://github.com/moby/moby/pull/35527)
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 16, 2017

Member

@kolyshkin @friism @dmcgowan I think this os roughly what's needed, but would need testing, and can use some eyes; opened this so that we can discuss

Member

thaJeztah commented Nov 16, 2017

@kolyshkin @friism @dmcgowan I think this os roughly what's needed, but would need testing, and can use some eyes; opened this so that we can discuss

Show outdated Hide outdated daemon/graphdriver/overlay2/overlay.go
opts := fmt.Sprintf("lowerdir=%s:%s,upperdir=%s,workdir=%s", path.Join(td, "lower2"), path.Join(td, "lower1"), path.Join(td, "upper"), path.Join(td, "work"))
if err := unix.Mount("overlay", filepath.Join(td, "merged"), "overlay", 0, opts); err != nil {
return errors.Wrap(err, "failed to mount overlay")

This comment has been minimized.

@dmcgowan

dmcgowan Nov 16, 2017

Member

What is the error that gets returned when it is not supported?

@dmcgowan

dmcgowan Nov 16, 2017

Member

What is the error that gets returned when it is not supported?

This comment has been minimized.

@thaJeztah

thaJeztah Nov 18, 2017

Member

I saw @kolyshkin tested this;

CentOS version Kernel version Compile date Error
7.1 3.10.0-229.1.2.el7.x86_64 ? failed to mount overlay: invalid argument
7.1 3.10.0-229.20.1.el7.x86_64 ? failed to mount overlay: invalid argument
7.2 3.10.0-327.el7.x86_64 Nov 19 2015 nil
7.2 3.10.0-327.36.3.el7.x86_64 Oct 24 2016 nil
7.3 3.10.0-514.26.2.el7.x86_64 Jul 4 2017 nil
@thaJeztah

thaJeztah Nov 18, 2017

Member

I saw @kolyshkin tested this;

CentOS version Kernel version Compile date Error
7.1 3.10.0-229.1.2.el7.x86_64 ? failed to mount overlay: invalid argument
7.1 3.10.0-229.20.1.el7.x86_64 ? failed to mount overlay: invalid argument
7.2 3.10.0-327.el7.x86_64 Nov 19 2015 nil
7.2 3.10.0-327.36.3.el7.x86_64 Oct 24 2016 nil
7.3 3.10.0-514.26.2.el7.x86_64 Jul 4 2017 nil
Show outdated Hide outdated daemon/graphdriver/overlay2/check.go
Show outdated Hide outdated daemon/graphdriver/overlay2/check.go
Show outdated Hide outdated daemon/graphdriver/overlay2/check.go
Show outdated Hide outdated daemon/graphdriver/overlay2/check.go
Show outdated Hide outdated daemon/graphdriver/overlay2/overlay.go
@dmcgowan

LGTM

// CentOS 3.x kernels (3.10.0-693.el7.x86_64 and up). This function is to detect
// support on those kernels, without doing a kernel version compare.
func supportsMultipleLowerDir(d string) error {
td, err := ioutil.TempDir(d, "multiple-lowerdir-check")

This comment has been minimized.

@andrewhsu

andrewhsu Nov 27, 2017

Contributor

@kolyshkin curious to know if this was tested with selinux enabled? just wanted to make sure the writing to tmp works.

@andrewhsu

andrewhsu Nov 27, 2017

Contributor

@kolyshkin curious to know if this was tested with selinux enabled? just wanted to make sure the writing to tmp works.

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Nov 28, 2017

Contributor

curious to know if this was tested with selinux enabled? just wanted to make sure the writing to tmp works.

@andrewhsu good question! I did my checks on the box with selinux disabled, but by looking at the code I see that directories are created under the graphdriver "home" dir which should succeed as long as docker+selinux is confgured properly.

The above is purely theoretical though; I'll do some real checks

Contributor

kolyshkin commented Nov 28, 2017

curious to know if this was tested with selinux enabled? just wanted to make sure the writing to tmp works.

@andrewhsu good question! I did my checks on the box with selinux disabled, but by looking at the code I see that directories are created under the graphdriver "home" dir which should succeed as long as docker+selinux is confgured properly.

The above is purely theoretical though; I'll do some real checks

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Nov 28, 2017

Contributor

Oops

Nov 28 04:37:42 kir-ce71 dockerd[669]: time="2017-11-28T04:37:42.349701814Z" level=debug msg="Multiple lower dirs not supported: stat /var/lib/docker/overlay2: no such file or directory"

I guess we should either do something like

-                        if err := supportsMultipleLowerDir(home); err != nil {
+                        if err := supportsMultipleLowerDir(filepath.Dir(home)); err != nil {

or move the creation of the driver home to above this check (and make sure to remove the dir on error path). I like the former version more.

Update: in fact at this point even the driver home's parent dir (i.e. /var/lib/docker) might not be available, so maybe it's easy to create a dir and do the check

Contributor

kolyshkin commented Nov 28, 2017

Oops

Nov 28 04:37:42 kir-ce71 dockerd[669]: time="2017-11-28T04:37:42.349701814Z" level=debug msg="Multiple lower dirs not supported: stat /var/lib/docker/overlay2: no such file or directory"

I guess we should either do something like

-                        if err := supportsMultipleLowerDir(home); err != nil {
+                        if err := supportsMultipleLowerDir(filepath.Dir(home)); err != nil {

or move the creation of the driver home to above this check (and make sure to remove the dir on error path). I like the former version more.

Update: in fact at this point even the driver home's parent dir (i.e. /var/lib/docker) might not be available, so maybe it's easy to create a dir and do the check

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 28, 2017

Member

@kolyshkin looking at the code, if /var/lib/docker (--data-root) isn't there yet, how does the check for fsmagic work in this case?

fsMagic, err := graphdriver.GetFSMagic(home)
Member

thaJeztah commented Nov 28, 2017

@kolyshkin looking at the code, if /var/lib/docker (--data-root) isn't there yet, how does the check for fsmagic work in this case?

fsMagic, err := graphdriver.GetFSMagic(home)
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 28, 2017

Member

Oh, never mind that takes the parent directory

Member

thaJeztah commented Nov 28, 2017

Oh, never mind that takes the parent directory

Detect overlay2 support on pre-4.0 kernels
The overlay2 storage-driver requires multiple lower dir
support for overlayFs. Support for this feature was added
in kernel 4.x, but some distros (RHEL 7.4, CentOS 7.4) ship with
an older kernel with this feature backported.

This patch adds feature-detection for multiple lower dirs,
and will perform this feature-detection on pre-4.x kernels
with overlayFS support.

With this patch applied, daemons running on a kernel
with multiple lower dir support will now select "overlay2"
as storage-driver, instead of falling back to "overlay".

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 28, 2017

Member

@kolyshkin updated; could you check with SELinux? (given that it now will check /var/lib instead of /var/lib/docker).

Alternatively, we should create /var/lib/docker before the graphdriver checks are performed

Member

thaJeztah commented Nov 28, 2017

@kolyshkin updated; could you check with SELinux? (given that it now will check /var/lib instead of /var/lib/docker).

Alternatively, we should create /var/lib/docker before the graphdriver checks are performed

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Nov 29, 2017

Contributor

(given that it now will check /var/lib instead of /var/lib/docker)

By looking at the code, it seems it is checking /var/lib/docker (as it is a parent dir of driver's home, /var/lib/docker/overlay2).

Anyway, it works!

Contributor

kolyshkin commented Nov 29, 2017

(given that it now will check /var/lib instead of /var/lib/docker)

By looking at the code, it seems it is checking /var/lib/docker (as it is a parent dir of driver's home, /var/lib/docker/overlay2).

Anyway, it works!

@kolyshkin

This comment has been minimized.

Show comment
Hide comment
@kolyshkin

kolyshkin Nov 29, 2017

Contributor

The test to start dockerd was done

  • on a CentOS 7.4 machine with kernel downgraded to 3.10.0-327.el7.x86_64
  • with selinux turned on
  • with /var/lib/docker removed

A relevant quote from the logs

Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.898726143Z" level=info msg="Loading containers: done."
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.905776790Z" level=warning msg="Not using native diff for overlay2, this may cause degraded performance for building images: opaque flag erroneously copied up, consider update to kernel 4.8 or later to fix"
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.915391003Z" level=info msg="Docker daemon" commit=bb370f5 graphdriver(s)=overlay2 version=dev
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.915733174Z" level=info msg="Daemon has completed initialization"

Test sequence:

# systemctl stop docker
# mv /var/lib/docker{,-moved}
# systemctl start docker

System info:

# uname -a
Linux kir-ce71 3.10.0-327.el7.x86_64 #1 SMP Thu Nov 19 22:10:57 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
[root@kir-ce71 ~]# selinuxenabled ; echo $?
0
Contributor

kolyshkin commented Nov 29, 2017

The test to start dockerd was done

  • on a CentOS 7.4 machine with kernel downgraded to 3.10.0-327.el7.x86_64
  • with selinux turned on
  • with /var/lib/docker removed

A relevant quote from the logs

Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.898726143Z" level=info msg="Loading containers: done."
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.905776790Z" level=warning msg="Not using native diff for overlay2, this may cause degraded performance for building images: opaque flag erroneously copied up, consider update to kernel 4.8 or later to fix"
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.915391003Z" level=info msg="Docker daemon" commit=bb370f5 graphdriver(s)=overlay2 version=dev
Nov 29 02:46:02 kir-ce71 dockerd[7988]: time="2017-11-29T02:46:02.915733174Z" level=info msg="Daemon has completed initialization"

Test sequence:

# systemctl stop docker
# mv /var/lib/docker{,-moved}
# systemctl start docker

System info:

# uname -a
Linux kir-ce71 3.10.0-327.el7.x86_64 #1 SMP Thu Nov 19 22:10:57 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
[root@kir-ce71 ~]# selinuxenabled ; echo $?
0
@kolyshkin

LGTM (IANAM)

@vieux

vieux approved these changes Nov 29, 2017

LGTM

@vieux vieux merged commit 11e07e7 into moby:master Nov 29, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 38076 has succeeded
Details
janky Jenkins build Docker-PRs 46792 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 7202 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 18349 has succeeded
Details
z Jenkins build Docker-PRs-s390x 7023 has succeeded
Details

@thaJeztah thaJeztah deleted the thaJeztah:feature-detect-overlay2 branch Nov 29, 2017

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