[bug#34917,v2] gnu: docker: Use fewer modprobes.

Message ID 20190319182648.20666-1-dannym@scratchpost.org
State Accepted
Headers show
Series [bug#34917,v2] gnu: docker: Use fewer modprobes. | expand

Checks

Context Check Description
cbaines/applying patch success Successfully applied

Commit Message

Danny Milosavljevic March 19, 2019, 6:26 p.m. UTC
Fixes <https://bugs.gnu.org/34333>.
Reported by Allan Adair <allan@adair.io>.

* gnu/packages/patches/docker-use-fewer-modprobes.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/docker.scm (docker)[source]: Use it.
---
 gnu/local.mk                                  |   1 +
 gnu/packages/docker.scm                       |   5 +-
 .../patches/docker-use-fewer-modprobes.patch  | 116 ++++++++++++++++++
 3 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/docker-use-fewer-modprobes.patch

Comments

Ludovic Courtès March 22, 2019, 9:48 p.m. UTC | #1
Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Fixes <https://bugs.gnu.org/34333>.
> Reported by Allan Adair <allan@adair.io>.
>
> * gnu/packages/patches/docker-use-fewer-modprobes.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/docker.scm (docker)[source]: Use it.

> --- /dev/null
> +++ b/gnu/packages/patches/docker-use-fewer-modprobes.patch
> @@ -0,0 +1,116 @@
> +This patch makes docker find out whether a filesystem type is supported
> +by trying to mount a filesystem of that type rather than invoking "modprobe".

Nice!  What this patch does looks like the right thing to me, certainly
better than invoking modprobe (who thought Docker would do that?!).

Did you submit it upstream?  It’d be nice to get their feedback, and
also have a tracking URL in the patch.

Otherwise LGTM, thanks!

Ludo’.
Danny Milosavljevic March 25, 2019, 7:27 p.m. UTC | #2
Hi Ludo,

On Fri, 22 Mar 2019 22:48:02 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> Nice!  What this patch does looks like the right thing to me, certainly
> better than invoking modprobe (who thought Docker would do that?!).
> 
> Did you submit it upstream?  It’d be nice to get their feedback, and
> also have a tracking URL in the patch.

See https://github.com/moby/moby/pull/38930

The changes were approved upstream.
Ludovic Courtès March 25, 2019, 9:27 p.m. UTC | #3
Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Fri, 22 Mar 2019 22:48:02 +0100
> Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Nice!  What this patch does looks like the right thing to me, certainly
>> better than invoking modprobe (who thought Docker would do that?!).
>> 
>> Did you submit it upstream?  It’d be nice to get their feedback, and
>> also have a tracking URL in the patch.
>
> See https://github.com/moby/moby/pull/38930
>
> The changes were approved upstream.

Awesome, thank you!

Can you just add this URL to the patch and push?

Ludo’.
Ludovic Courtès March 30, 2019, 10:41 a.m. UTC | #4
This was pushed as 516f6f55eb, closing!

Patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 0a7e9bbc6..46bd83e50 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -723,6 +723,7 @@  dist_patch_DATA =						\
   %D%/packages/patches/doc++-segfault-fix.patch			\
   %D%/packages/patches/docker-engine-test-noinstall.patch	\
   %D%/packages/patches/docker-fix-tests.patch			\
+  %D%/packages/patches/docker-use-fewer-modprobes.patch		\
   %D%/packages/patches/dovecot-trees-support-dovecot-2.3.patch	\
   %D%/packages/patches/doxygen-test.patch			\
   %D%/packages/patches/dropbear-CVE-2018-15599.patch		\
diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
index 88fc7fc6e..a11ce266d 100644
--- a/gnu/packages/docker.scm
+++ b/gnu/packages/docker.scm
@@ -227,6 +227,8 @@  network attachments.")
     (home-page "http://containerd.io/")
     (license license:asl2.0)))
 
+;; TODO: Patch out modprobes for ip_vs, nf_conntrack,
+;; brige, nf_conntrack_netlink, aufs.
 (define-public docker
   (package
     (name "docker")
@@ -242,7 +244,8 @@  network attachments.")
        (base32 "06yr5xwr181lalh8z1lk07nxlp7hn38aq8cyqjk617dfy4lz0ixx"))
       (patches
        (search-patches "docker-engine-test-noinstall.patch"
-                       "docker-fix-tests.patch"))))
+                       "docker-fix-tests.patch"
+                       "docker-use-fewer-modprobes.patch"))))
     (build-system gnu-build-system)
     (arguments
      `(#:modules
diff --git a/gnu/packages/patches/docker-use-fewer-modprobes.patch b/gnu/packages/patches/docker-use-fewer-modprobes.patch
new file mode 100644
index 000000000..ebee83329
--- /dev/null
+++ b/gnu/packages/patches/docker-use-fewer-modprobes.patch
@@ -0,0 +1,116 @@ 
+This patch makes docker find out whether a filesystem type is supported
+by trying to mount a filesystem of that type rather than invoking "modprobe".
+--- docker-18.09.0-checkout/daemon/graphdriver/overlay/overlay.go.orig	1970-01-01 01:00:00.000000000 +0100
++++ docker-18.09.0-checkout/daemon/graphdriver/overlay/overlay.go	2019-03-19 09:16:03.487087490 +0100
+@@ -8,7 +8,6 @@
+ 	"io"
+ 	"io/ioutil"
+ 	"os"
+-	"os/exec"
+ 	"path"
+ 	"path/filepath"
+ 	"strconv"
+@@ -201,9 +200,16 @@
+ }
+ 
+ func supportsOverlay() error {
+-	// We can try to modprobe overlay first before looking at
+-	// proc/filesystems for when overlay is supported
+-	exec.Command("modprobe", "overlay").Run()
++	// Access overlay filesystem so that Linux loads it (if possible).
++	mountTarget, err := ioutil.TempDir("", "supportsOverlay")
++	if err != nil {
++		logrus.WithField("storage-driver", "overlay2").Error("Could not create temporary directory, so assuming that 'overlay' is not supported.")
++		return graphdriver.ErrNotSupported
++	} else {
++		/* The mounting will fail--after the module has been loaded.*/
++		defer os.RemoveAll(mountTarget)
++		unix.Mount("overlay", mountTarget, "overlay", 0, "")
++	}
+ 
+ 	f, err := os.Open("/proc/filesystems")
+ 	if err != nil {
+--- docker-18.09.0-checkout/daemon/graphdriver/overlay2/overlay.go.orig	2019-03-18 23:42:23.728525231 +0100
++++ docker-18.09.0-checkout/daemon/graphdriver/overlay2/overlay.go	2019-03-19 08:54:31.411906113 +0100
+@@ -10,7 +10,6 @@
+ 	"io"
+ 	"io/ioutil"
+ 	"os"
+-	"os/exec"
+ 	"path"
+ 	"path/filepath"
+ 	"strconv"
+@@ -261,9 +260,16 @@
+ }
+ 
+ func supportsOverlay() error {
+-	// We can try to modprobe overlay first before looking at
+-	// proc/filesystems for when overlay is supported
+-	exec.Command("modprobe", "overlay").Run()
++	// Access overlay filesystem so that Linux loads it (if possible).
++	mountTarget, err := ioutil.TempDir("", "supportsOverlay")
++	if err != nil {
++		logrus.WithField("storage-driver", "overlay2").Error("Could not create temporary directory, so assuming that 'overlay' is not supported.")
++		return graphdriver.ErrNotSupported
++	} else {
++		/* The mounting will fail--after the module has been loaded.*/
++		defer os.RemoveAll(mountTarget)
++		unix.Mount("overlay", mountTarget, "overlay", 0, "")
++	}
+ 
+ 	f, err := os.Open("/proc/filesystems")
+ 	if err != nil {
+--- docker-18.09.0-checkout/daemon/graphdriver/devmapper/deviceset.go.orig	2019-03-19 09:19:16.592844887 +0100
++++ docker-18.09.0-checkout/daemon/graphdriver/devmapper/deviceset.go	2019-03-19 09:21:18.019361761 +0100
+@@ -540,8 +539,14 @@
+ 		return err // error text is descriptive enough
+ 	}
+ 
+-	// Check if kernel supports xfs filesystem or not.
+-	exec.Command("modprobe", "xfs").Run()
++        mountTarget, err := ioutil.TempDir("", "supportsOverlay")
++        if err != nil {
++		return errors.Wrapf(err, "error checking for xfs support")
++        } else {
++                /* The mounting will fail--after the module has been loaded.*/
++                defer os.RemoveAll(mountTarget)
++                unix.Mount("none", mountTarget, "xfs", 0, "")
++        }
+ 
+ 	f, err := os.Open("/proc/filesystems")
+ 	if err != nil {
+--- docker-18.09.0-checkout/vendor/github.com/docker/libnetwork/iptables/iptables.go.orig	2019-03-19 09:47:19.430111170 +0100
++++ docker-18.09.0-checkout/vendor/github.com/docker/libnetwork/iptables/iptables.go	2019-03-19 10:38:01.445136177 +0100
+@@ -72,11 +71,12 @@
+ }
+ 
+ func probe() {
+-	if out, err := exec.Command("modprobe", "-va", "nf_nat").CombinedOutput(); err != nil {
+-		logrus.Warnf("Running modprobe nf_nat failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
++	path, err := exec.LookPath("iptables")
++	if err != nil {
++		return
+ 	}
+-	if out, err := exec.Command("modprobe", "-va", "xt_conntrack").CombinedOutput(); err != nil {
+-		logrus.Warnf("Running modprobe xt_conntrack failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
++	if out, err := exec.Command(path, "--wait", "-t", "nat", "-L", "-n").CombinedOutput(); err != nil {
++		logrus.Warnf("Running iptables --wait -t nat -L -n failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
+ 	}
+ }
+ 
+--- docker-18.09.0-checkout/vendor/github.com/docker/libnetwork/ns/init_linux.go.orig	2019-03-19 11:23:20.738316699 +0100
++++ docker-18.09.0-checkout/vendor/github.com/docker/libnetwork/ns/init_linux.go	2019-03-19 11:27:57.149753073 +0100
+@@ -100,12 +100,7 @@
+ }
+ 
+ func loadXfrmModules() error {
+-	if out, err := exec.Command("modprobe", "-va", "xfrm_user").CombinedOutput(); err != nil {
+-		return fmt.Errorf("Running modprobe xfrm_user failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
+-	}
+-	if out, err := exec.Command("modprobe", "-va", "xfrm_algo").CombinedOutput(); err != nil {
+-		return fmt.Errorf("Running modprobe xfrm_algo failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
+-	}
++	// Those are automatically loaded when someone opens the socket anyway.
+ 	return nil
+ }
+