[PATCH] gnu: docker: Use fewer modprobes.

  • Done
  • quality assurance status badge
Details
2 participants
  • Danny Milosavljevic
  • Ludovic Courtès
Owner
unassigned
Submitted by
Danny Milosavljevic
Severity
normal

Debbugs page

Danny Milosavljevic wrote 6 years ago
(address . guix-patches@gnu.org)(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
20190319182053.20524-1-dannym@scratchpost.org
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 | 100 ++++++++++++++++++
3 files changed, 105 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/docker-use-fewer-modprobes.patch

Toggle diff (141 lines)
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..77605d4c0 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, xfrm_user, xfrm_algo, nf_conntrack,
+;; 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..c631400cb
--- /dev/null
+++ b/gnu/packages/patches/docker-use-fewer-modprobes.patch
@@ -0,0 +1,100 @@
+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)
+ }
+ }
+
Danny Milosavljevic wrote 6 years ago
[PATCH v2] gnu: docker: Use fewer modprobes.
(address . 34917@debbugs.gnu.org)(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
20190319182648.20666-1-dannym@scratchpost.org
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

Toggle diff (157 lines)
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
+ }
+
Ludovic Courtès wrote 6 years ago
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 34917@debbugs.gnu.org)
87mulm1sz1.fsf@gnu.org
Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (13 lines)
> 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 wrote 6 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34917@debbugs.gnu.org)
20190325202656.3901a92d@scratchpost.org
Hi Ludo,

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

Toggle quote (6 lines)
> 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.


The changes were approved upstream.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAlyZK4UACgkQ5xo1VCww
uqVPvQf+OonRD7Km3qfTLRxxpYyFwnSfltdZxufDL1HVgqDSBGQyHtcs5zyDg2OV
w9t6drc/5+8Q2XTMUUcg4SP1wDvIBpL3lze266hxMvBM/lVLr0WIxKIf8/47+yCt
b37Rls6zPDK68R3rux/qBoUpVcBpjHDSYJe8rG09gGvTAGmEvs353pn2UwyH2cLL
uBaAdPWph/O8Gd87M20sceE9TOFM76FqaLApLHMlMdrohFy5zPQGzb+tBrGiQTsp
MWNRBDH1Ilk6znVlTUWJcWbw2OOJnC3nfvlEmxT/aKJU5/+LT3bqY2hiplmVT2rI
wSNI8g3W8iZQnboJVigfgKFyEo71Lw==
=GFb7
-----END PGP SIGNATURE-----


Ludovic Courtès wrote 6 years ago
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 34917@debbugs.gnu.org)
87bm1yve57.fsf@gnu.org
Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (13 lines)
> 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 wrote 6 years ago
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 34917-done@debbugs.gnu.org)
87y34w4pc1.fsf@gnu.org
This was pushed as 516f6f55eb, closing!
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 34917@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 34917
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help