GNU bug report logs - #68073
[PATCH] Add config-file configuration option to dockerd

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68073 in the body.
You can then email your comments to 68073 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Connor Clark <connor <at> psyleft.com>
To: guix-patches <at> gnu.org
Cc: Connor Clark <connor <at> psyleft.com>
Subject: [PATCH] Add config-file configuration option to dockerd
Date: Wed, 27 Dec 2023 15:20:21 -0500

---

This is my first time submitting a patch here, so please let me know if I'm
missing something important, or my email is formatted incorrectly, or anything
else.

This patch adds an option to pass a json configuration file directly into
dockerd for options which aren't available in the docker-configuration record,
of which there are many. From what I've seen, some other packages use a
raw-configuration-string that gets appended into a file to accomplish similar
things, but I figured a file-like was better here because the rest of the
options are passed into the command invocation and not serialized into a file.

 gnu/services/docker.scm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
index 72ef7d74db..4d32b96847 100644
--- a/gnu/services/docker.scm
+++ b/gnu/services/docker.scm
@@ -61,6 +61,8 @@ (define-module (gnu services docker)
             oci-container-service-type
             oci-container-shepherd-service))
 
+(define-maybe file-like)
+
 (define-configuration docker-configuration
   (docker
    (file-like docker)
@@ -87,6 +89,9 @@ (define-configuration docker-configuration
   (environment-variables
    (list '())
    "Environment variables to set for dockerd")
+  (config-file
+   (maybe-file-like)
+   "JSON configuration file to pass to dockerd")
   (no-serialization))
 
 (define %docker-accounts
@@ -131,7 +136,8 @@ (define (docker-shepherd-service config)
          (enable-iptables? (docker-configuration-enable-iptables? config))
          (environment-variables (docker-configuration-environment-variables config))
          (proxy (docker-configuration-proxy config))
-         (debug? (docker-configuration-debug? config)))
+         (debug? (docker-configuration-debug? config))
+         (config-file (docker-configuration-config-file config)))
     (shepherd-service
            (documentation "Docker daemon.")
            (provision '(dockerd))
@@ -144,6 +150,10 @@ (define (docker-shepherd-service config)
            (start #~(make-forkexec-constructor
                      (list (string-append #$docker "/bin/dockerd")
                            "-p" "/var/run/docker.pid"
+                           #$@(if (not (eq? config-file %unset-value))
+                                  (list #~(string-append
+                                           "--config-file=" #$config-file))
+                                  '())
                            #$@(if debug?
                                   '("--debug" "--log-level=debug")
                                   '())

base-commit: 5bd80ccd69047b1777749e24d4adf2c951b5d14b
-- 
2.41.0

Message #8 received at 68073 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Connor Clark <connor <at> psyleft.com>
Cc: zimon.toutoune <at> gmail.com, paren <at> disroot.org, ludo <at> gnu.org, me <at> tobias.gr,
 rekado <at> elephly.net, rg <at> raghavgururajan.name, jgart <at> dismail.de,
 guix <at> cbaines.net, 68073 <at> debbugs.gnu.org
Subject: Re: [bug#68073] [PATCH] Add config-file configuration option to
 dockerd
Date: Thu, 28 Dec 2023 10:35:40 +0100

Hello Connor,

Thanks for this first submission and welcome :)

> This is my first time submitting a patch here, so please let me know
> if I'm missing something important, or my email is formatted
> incorrectly, or anything else.

This looks OK, a few remarks below:

> This patch adds an option to pass a json configuration file directly into
> dockerd for options which aren't available in the docker-configuration record,
> of which there are many. From what I've seen, some other packages use a
> raw-configuration-string that gets appended into a file to accomplish similar
> things, but I figured a file-like was better here because the rest of the
> options are passed into the command invocation and not serialized into a file.

You are missing a commit message. Its format is described here:
https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs

Here that would look like:

--8<---------------cut here---------------start------------->8---
* gnu/services/docker.scm (docker-configuration)[config-file]: New
file-like field.
...
--8<---------------cut here---------------end--------------->8---

You can have a look to the git log of that specific file for examples.

If you want extra points, you can extend the system test in
gnu/tests/docker.scm to check that the docker-service-type is honoring
this new config-file field. Here is the documentation of system tests:
https://guix.gnu.org/manual/en/html_node/Running-the-Test-Suite.html

Could you please send a v2?

Thanks,

Mathieu

Message #11 received at 68073 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Connor Clark <connor <at> psyleft.com>
Cc: zimon.toutoune <at> gmail.com, paren <at> disroot.org, ludo <at> gnu.org, me <at> tobias.gr,
 rekado <at> elephly.net, rg <at> raghavgururajan.name, jgart <at> dismail.de,
 guix <at> cbaines.net, 68073 <at> debbugs.gnu.org
Subject: Re: [bug#68073] [PATCH] Add config-file configuration option to
 dockerd
Date: Thu, 28 Dec 2023 10:39:48 +0100

Last but not least, this new field needs to be documented in
doc/guix.texi in the following section:

--8<---------------cut here---------------start------------->8---
@cindex Docker
@subsubheading Docker Service
--8<---------------cut here---------------end--------------->8---

Thanks,

Mathieu

Message #14 received at 68073 <at> debbugs.gnu.org (full text, mbox):

From: Connor Clark <connor <at> psyleft.com>
To: 68073 <at> debbugs.gnu.org
Cc: Connor Clark <connor <at> psyleft.com>
Subject: [PATCH v2] services: docker: Add config-file option.
Date: Thu, 28 Dec 2023 23:47:37 -0500

* gnu/services/docker.scm (docker-configuration)[config-file] Add file-like
field.
* doc/guix.texi (Docker Service): Add information about config-file.
---

Thanks for responding! This revision should fix the issues you raised. I added
documentation in guix.texi and revised the commit message to fit in with the
others. The tests file was a bit complex though, I couldn't figure out where I
would extend it if I wanted to :).

 doc/guix.texi           |  3 +++
 gnu/services/docker.scm | 12 +++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index a9a9272c35..a9488ff4b5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -39701,6 +39701,9 @@ This must be a list of strings where each string has the form
       "TMPDIR=/tmp/dockerd")
 @end lisp
 
+@item @code{config-file} (type: maybe-file-like)
+JSON configuration file pass to @command{dockerd}.
+
 @end table
 @end deftp
 
diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
index 72ef7d74db..4d32b96847 100644
--- a/gnu/services/docker.scm
+++ b/gnu/services/docker.scm
@@ -61,6 +61,8 @@ (define-module (gnu services docker)
             oci-container-service-type
             oci-container-shepherd-service))
 
+(define-maybe file-like)
+
 (define-configuration docker-configuration
   (docker
    (file-like docker)
@@ -87,6 +89,9 @@ (define-configuration docker-configuration
   (environment-variables
    (list '())
    "Environment variables to set for dockerd")
+  (config-file
+   (maybe-file-like)
+   "JSON configuration file to pass to dockerd")
   (no-serialization))
 
 (define %docker-accounts
@@ -131,7 +136,8 @@ (define (docker-shepherd-service config)
          (enable-iptables? (docker-configuration-enable-iptables? config))
          (environment-variables (docker-configuration-environment-variables config))
          (proxy (docker-configuration-proxy config))
-         (debug? (docker-configuration-debug? config)))
+         (debug? (docker-configuration-debug? config))
+         (config-file (docker-configuration-config-file config)))
     (shepherd-service
            (documentation "Docker daemon.")
            (provision '(dockerd))
@@ -144,6 +150,10 @@ (define (docker-shepherd-service config)
            (start #~(make-forkexec-constructor
                      (list (string-append #$docker "/bin/dockerd")
                            "-p" "/var/run/docker.pid"
+                           #$@(if (not (eq? config-file %unset-value))
+                                  (list #~(string-append
+                                           "--config-file=" #$config-file))
+                                  '())
                            #$@(if debug?
                                   '("--debug" "--log-level=debug")
                                   '())

base-commit: 5bd80ccd69047b1777749e24d4adf2c951b5d14b
-- 
2.41.0

Message #19 received at 68073-done <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Connor Clark <connor <at> psyleft.com>
Cc: 68073-done <at> debbugs.gnu.org
Subject: Re: [bug#68073] [PATCH v2] services: docker: Add config-file option.
Date: Wed, 03 Jan 2024 15:31:32 +0100

> * gnu/services/docker.scm (docker-configuration)[config-file] Add file-like
> field.
> * doc/guix.texi (Docker Service): Add information about config-file.

Applied, thanks,

Mathieu

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.