unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#68073] [PATCH] Add config-file configuration option to dockerd
@ 2023-12-27 20:20 Connor Clark via Guix-patches via
  2023-12-28  9:35 ` Mathieu Othacehe
  2023-12-29  4:47 ` [bug#68073] [PATCH v2] services: docker: Add config-file option Connor Clark via Guix-patches via
  0 siblings, 2 replies; 5+ messages in thread
From: Connor Clark via Guix-patches via @ 2023-12-27 20:20 UTC (permalink / raw)
  To: 68073
  Cc: Connor Clark, paren, guix, ludo, othacehe, rg, rekado,
	zimon.toutoune, me, jgart

---

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





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [bug#68073] [PATCH] Add config-file configuration option to dockerd
  2023-12-27 20:20 [bug#68073] [PATCH] Add config-file configuration option to dockerd Connor Clark via Guix-patches via
@ 2023-12-28  9:35 ` Mathieu Othacehe
  2023-12-28  9:39   ` Mathieu Othacehe
  2023-12-29  4:47 ` [bug#68073] [PATCH v2] services: docker: Add config-file option Connor Clark via Guix-patches via
  1 sibling, 1 reply; 5+ messages in thread
From: Mathieu Othacehe @ 2023-12-28  9:35 UTC (permalink / raw)
  To: Connor Clark
  Cc: zimon.toutoune, paren, ludo, me, rekado, rg, jgart, guix, 68073


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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#68073] [PATCH] Add config-file configuration option to dockerd
  2023-12-28  9:35 ` Mathieu Othacehe
@ 2023-12-28  9:39   ` Mathieu Othacehe
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Othacehe @ 2023-12-28  9:39 UTC (permalink / raw)
  To: Connor Clark
  Cc: zimon.toutoune, paren, ludo, me, rekado, rg, jgart, guix, 68073


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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#68073] [PATCH v2] services: docker: Add config-file option.
  2023-12-27 20:20 [bug#68073] [PATCH] Add config-file configuration option to dockerd Connor Clark via Guix-patches via
  2023-12-28  9:35 ` Mathieu Othacehe
@ 2023-12-29  4:47 ` Connor Clark via Guix-patches via
  2024-01-03 14:31   ` bug#68073: " Mathieu Othacehe
  1 sibling, 1 reply; 5+ messages in thread
From: Connor Clark via Guix-patches via @ 2023-12-29  4:47 UTC (permalink / raw)
  To: 68073; +Cc: Connor Clark

* 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





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#68073: [PATCH v2] services: docker: Add config-file option.
  2023-12-29  4:47 ` [bug#68073] [PATCH v2] services: docker: Add config-file option Connor Clark via Guix-patches via
@ 2024-01-03 14:31   ` Mathieu Othacehe
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Othacehe @ 2024-01-03 14:31 UTC (permalink / raw)
  To: Connor Clark; +Cc: 68073-done


> * 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-01-03 14:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-27 20:20 [bug#68073] [PATCH] Add config-file configuration option to dockerd Connor Clark via Guix-patches via
2023-12-28  9:35 ` Mathieu Othacehe
2023-12-28  9:39   ` Mathieu Othacehe
2023-12-29  4:47 ` [bug#68073] [PATCH v2] services: docker: Add config-file option Connor Clark via Guix-patches via
2024-01-03 14:31   ` bug#68073: " Mathieu Othacehe

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).