[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[guix-patches--- via]

This patch provides a new service type, which allows the creation of shepherd
services from docker containers.
---
Hi,

I have written a definition of the docker-container-service-type. It is
a service that allows you to convert docker containers into shepherd
services.

It has some limitations:

1. Containers are deleted when stopping.
   This makes it easier to manage them from shepherd. You can achieve
   persistence with volume binds.
   
2. Containers must be either attached or detached.
   Some containers simply run a command inside the container, and when
   run with docker run, they stay attached and the docker run command
   therefore behaves like a normal command. However, some containers use
   docker's "init system", that means that they won't block the docker
   run command. Sadly attached containers don't properly report that
   they are initialized, so to docker they are in the "starting" state
   until termination. This means that docker doesn't create a cid
   file (pid file for containers). To sum it up, there is no way to tell
   how will the container report it's state, and this process must be
   specified by the user, if the container runs attached or detached.

3. Images are not managed.
   Docker does manage images in a really stupid way. My original idea
   was to have a file-like object to define an image (from an image
   archive), but sadly it has been more complex than I initially
   thought. Docker uses 2 versions of archive manifests, and each
   archive can have multiple images, depending on the architecture. And
   those images can be composed from layers, which are also images. The
   daemon determines ad-hoc which images from the archive it will use,
   and there is no official tool (at leats when I looked for it) to get
   image ids reliably. As the docker load command can return multiple
   images. I will expand on the process on how the images could be used
   in a managed way, but I have to say something to the current
   interface. It works by expecting an image by name from the image-name
   field. That means that docker must already have the image in its
   database. There is currently no way to ensure that images will be in
   docker database before running shepherd services, but I expect they
   should simply fail to start.

There is currently no documentation outside of docstrings, I plan to
write it, but first I would welcome comments from you, maybe this
service isn't suitable for guix, as it does imply breaking of the
declarative guix model, but that goes for docker and flatpak too, so I
thought I can try it.

Now finally to images. The idea is that all images belonging to a
docker-container-configuration are tagged (via docker tag) with the name
o the docker-container-configuration-name (as this is unique). The
activation service would get the propper image-id (which is not easy to
get from the manifest, but with some json parsing, it can be done). Then
it would load the image into the docker database with docker load, and
tag the new image with the docker-configuration-name tag. This would
automatically update the image from which the container is running
without having to modify the shepherd service.

Sadly docker does not allow for containers to be ran directly from the
image archive and this is the most straightforward workaround I could
think of.

I hope this patch finds you in good mood,

Maya

PS. I found out that the original docker-service-type wasn't
indent-region. And it got snuck into the patch, I hope this will still
be a valid patch.

 gnu/services/docker.scm | 289 +++++++++++++++++++++++++++++++++-------
 1 file changed, 242 insertions(+), 47 deletions(-)

diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
index 741bab5a8c..b05804aa16 100644
--- a/gnu/services/docker.scm
+++ b/gnu/services/docker.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2020 Jesse Dowell <jessedowell@gmail.com>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2022 Maya Tomasek <maya.omase@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,6 +38,8 @@ (define-module (gnu services docker)
 
   #:export (docker-configuration
             docker-service-type
+            docker-container-configuration
+            docker-container-service-type
             singularity-service-type))
 
 (define-configuration docker-configuration
@@ -164,6 +167,198 @@ (define docker-service-type
                                      (const %docker-accounts))))
                 (default-value (docker-configuration))))
 
+(define (pair-of-strings? val)
+  (and (pair val)
+       (string? (car val))
+       (string? (cdr val))))
+
+(define (list-of-pair-of-strings? val)
+  (list-of pair-of-strings?))
+
+(define-configuration/no-serialization docker-container-configuration
+  (name
+   (symbol '())
+   "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
+We recommend, that the name of the container is prefixed with @code{docker-}.")
+  (comment
+   (string "")
+   "A documentation on the docker container.")
+  (image-name
+   (string)
+   "A name of the image that will be used. (Note that the existence of the image
+is not guaranteed by this daemon.)")
+  (volumes
+   (list-of-pair-of-strings '())
+   "A list of volume binds. In (HOST_PATH CONTAINER_PATH) format.")
+  (ports
+   (list-of-pair-of-strings '())
+   "A list of port binds. In (HOST_PORT CONTAINER_PORT) or (HOST_PORT CONTAINER_PORT OPTIONS) format.
+For example, both port bindings are valid:
+
+@lisp
+(ports '((\"2222\" \"22\") (\"21\" \"21\" \"tcp\")))
+@end lisp"
+   (environments
+    (list-of-pair-of-strings '())
+    "A list of variable binds, inside the container enviornment. In (VARIABLE VALUE) format."))
+  (network
+   (string "none")
+   "Network type.")
+  (additional-arguments
+   (list-of-strings '())
+   "Additional arguments to the docker command line interface.")
+  (container-command
+   (list-of-strings '())
+   "Command to send into the container.")
+  (attached?
+   (boolean #t)
+   "Run the container as an normal attached process (sending SIGTERM).
+Or run the container as a isolated environment that must be stopped with @code{docker stop}.
+
+Please verify first, that you container is indeed not attached, otherwise @code{shepherd} might
+assume the process is dead, even when it is not.
+
+You can do that, by first running your container with @code{docker run image-name}.
+
+Then check @code{docker ps}, if the command shows beside your container the word @code{running}.
+Your container is indeed detached, but if it shows @code{starting}, and it doesn't flip to
+@code{running} after a while, it means that you container is attached, and you need to keep this
+option turned @code{#t}."))
+
+(define (serialize-volumes config)
+  "Serialize list of pairs into flat list of @code{(\"-v\" \"HOST_PATH:CONTAINER_PATH\" ...)}"
+  (append-map
+   (lambda (volume-bind)
+     (list "-v" (format #f "~?" "~a:~a" volume-bind)))
+   (docker-container-configuration-volumes config)))
+
+(define (serialize-ports config)
+  "Serialize list of either pairs, or lists into flat list of
+@code{(\"-p\" \"NUMBER:NUMBER\" \"-p\" \"NUMBER:NUMBER/PROTOCOL\" ...)}"
+  (append-map
+   (lambda (port-bind)
+     (list "-p" (format #f "~?" "~a:~a~^/~a" port-bind)))
+   (docker-container-configuration-ports config)))
+
+(define (serialized-environments config)
+  "Serialize list of pairs into flat list of @code{(\"-e\" \"VAR=val\" \"-e\" \"VAR=val\" ...)}."
+  (append-map
+   (lambda (env-bind)
+     (list "-e" (format #f "~?" "~a=~a" env-bind)))
+   (docker-container-configuration-environments config)))
+
+(define (docker-container-startup-script docker-cli container-name config)
+  "Return a program file, that executes the startup sequence of the @code{docker-container-shepherd-service}."
+  (let* ((attached? (docker-container-configuration-attached? config))
+         (image-name (docker-container-configuration-image config))
+         (volumes (serialize-volumes config))
+         (ports (serialize-ports config))
+         (envs (serialize-environments config))
+         (network (docker-container-configuration-network config))
+         (additional-arguments (docker-container-configuration-additional-arguments config))
+         (container-command (docker-container-configuration-container-command config)))
+    (program-file
+     (string-append "start-" container-name "-container")
+     #~(let ((docker (string-append #$docker-cli "/bin/docker")))
+         (system* docker "stop" #$container-name)
+         (system* docker "rm" #$container-name)
+         (apply system* `(,docker
+                          "run"
+                          ,(string-append "--name=" #$container-name)
+                          ;; Automatically remove the container when stopping
+                          ;; If you want persistent data, you need to use
+                          ;; volume binds or other methods.
+                          "--rm"
+                          ,(string-append "--network=" #$network)
+                          ;; TODO:
+                          ;; Write to a cid file the container id, this allows
+                          ;; for shepherd to manage container even when the process
+                          ;; itself gets detached from the container
+                          ,@(if (not #$attached) '("--cidfile" #$cid-file) '())
+                          #$@volumes
+                          #$@ports
+                          #$@envs
+                          #$@additional-arguments
+                          ,#$image-name
+                          #$@container-command))))))
+
+(define (docker-container-shepherd-service docker-cli config)
+  "Return a shepherd-service that runs CONTAINER."
+  (let* ((container-name (symbol->string (docker-container-configuration-name config)))
+         (cid-file (string-append "/var/run/docker/" container-name ".pid"))
+         (attached? (docker-container-configuration-attached? config)))
+    (shepherd-service
+     (provision (list (docker-container-configuration-name config)))
+     (requirement `(dockerd))
+     (start #~(make-forkexec-constructor
+               (list #$(docker-container-startup-script docker-cli container-name config))
+               ;; Watch the cid-file instead of the docker run command, as the daemon can
+               ;; still be running even when the command terminates
+               (if (not #$attached?)
+                   #:pid-file #$cid-file)))
+     (stop (if #$attached?
+               #~(make-kill-destructor)
+               #~(lambda _
+                   (exec-command (list
+                                  (string-append #$docker-cli "/bin/docker")
+                                  "stop" #$container-name))
+                   #f))))))
+
+
+(define (list-of-docker-container-configurations? val)
+  (list-of docker-container-configuration?))
+
+(define-configuration/no-serialization docker-container-service-configuration
+  (docker-cli
+   (file-like docker-cli)
+   "The docker package to use.")
+  (containers
+   (list-of-docker-container-configurations '())
+   "The docker containers to run."))
+
+(define (docker-container-shepherd-services config)
+  "Return shepherd services for all containers inside config."
+  (let ((docker-cli (docker-container-service-configuration-docker-cli config)))
+    (map
+     (lambda (container)
+       (docker-container-shepherd-service
+        docker-cli
+        container))
+     (docker-container-service-configuration-containers config))))
+
+(define docker-container-service-type
+  "This is the type of the service that runs docker containers using GNU Shepherd.
+It allows for declarative management of running containers inside the Guix System.
+
+This service can be extended with list of @code{<docker-container-configuration>} objects.
+
+The following is an example @code{docker-container-service-type} configuration.
+
+@lisp
+(service docker-container-service-type
+  (containers (list
+                (docker-container-configuration
+                  (name 'docker-example)
+                  (comment \"An example docker container configuration\")
+                  (image-name \"example/example:latest\") ;; Note that images must be provided separately.
+                  (volumes '((\"/mnt\" \"/\") (\"/home/example\" \"/home/user\")))
+                  (ports '((\"21\" \"21\" \"tcp\") (\"22\" \"22\")))
+                  (network \"host\")))))
+@end lisp"
+  (service-type
+   (name 'docker-container)
+   (description "Manage docker containers with shepherd.")
+   (extensions
+    (list (service-extension shepherd-root-service-type docker-container-shepherd-services)))
+   (compose concatenate)
+   (extend (lambda (config containers)
+             (let ((docker-cli (docker-container-service-configuration-docker-cli config))
+                   (initial-containers (docker-container-service-configuration-containers config)))
+               (docker-container-service-configuration
+                (docker-cli docker-cli)
+                (containers (append initial-containers containers))))))
+   (default-value (docker-container-service-configuration))))
+
 \f
 ;;;
 ;;; Singularity.
-- 
2.37.3

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 10291 bytes --]



On 27-09-2022 19:16, guix-patches--- via wrote:
> 
> This patch provides a new service type, which allows the creation of shepherd
> services from docker containers.
> ---
> Hi,
> 
> I have written a definition of the docker-container-service-type. It is
> a service that allows you to convert docker containers into shepherd
> services.  [...]
> 
> There is currently no documentation outside of docstrings, I plan to
> write it, but first I would welcome comments from you, maybe this
> service isn't suitable for guix, as it does imply breaking of the
> declarative guix model, but that goes for docker and flatpak too, so I
> thought I can try it.

We already have a docker-service-type, why not a 
docker-container-service-type, though I wouldn't recommend docker 
myself.  Can't really document on the docker bits, but some mostly 
superficial comments:

>
> +(define (pair-of-strings? val)
> +  (and (pair val)

I think you meant 'pair?' here, not 'pair'.

> +       (string? (car val))
> +       (string? (cdr val))))
> +
> +(define (list-of-pair-of-strings? val)
> +  (list-of pair-of-strings?))
> +
> +(define-configuration/no-serialization docker-container-configuration
> +  (name
> +   (symbol '())
> +   "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
> +We recommend, that the name of the container is prefixed with @code{docker-}.")
> +  (comment
> +   (string "")
> +   "A documentation on the docker container.")

I don't think documentation is countable, maybe

"Documentation on the Docker container (optional)."

?  (I don't know what casing is appropriate here).

> +  (image-name
> +   (string)
> +   "A name of the image that will be used. (Note that the existence of the image
> +is not guaranteed by this daemon.)")
> +  (volumes
> +   (list-of-pair-of-strings '())
> +   "A list of volume binds. In (HOST_PATH CONTAINER_PATH) format.")

binds -> bindings and HOST_PATH CONTAINER_PATH -> (HOST-PATH 
CONTAINER-PATH) per Scheme conventions.

> +  (ports
> +   (list-of-pair-of-strings '())
> +   "A list of port binds. In (HOST_PORT CONTAINER_PORT) or (HOST_PORT CONTAINER_PORT OPTIONS) format.
> +For example, both port bindings are valid:
> +
> +@lisp
> +(ports '((\"2222\" \"22\") (\"21\" \"21\" \"tcp\")))
> +@end lisp"

* binds -> bindings

> +   (environments
> +    (list-of-pair-of-strings '())
> +    "A list of variable binds, inside the container enviornment. In (VARIABLE VALUE) format."))

'enviornment' -> 'environment', 'variable binds' -> 'environment 
variables', '. In' -> ', in'.

> +  (network
> +   (string "none")
> +   "Network type.")

Can the available network types be listed or a reference to the Docker 
documentation be added, to help users with determining what to set it to?

> +  (additional-arguments
> +   (list-of-strings '())
> +   "Additional arguments to the docker command line interface.")
> +  (container-command
> +   (list-of-strings '())
> +   "Command to send into the container.")
> +  (attached?
> +   (boolean #t)
> +   "Run the container as an normal attached process (sending SIGTERM).
> +Or run the container as a isolated environment that must be stopped with @code{docker stop}.
> +
> +Please verify first, that you container is indeed not attached, otherwise @code{shepherd} might
> +assume the process is dead, even when it is not.
> +
> +You can do that, by first running your container with @code{docker run image-name}.
> +
> +Then check @code{docker ps}, if the command shows beside your container the word @code{running}.
> +Your container is indeed detached, but if it shows @code{starting}, and it doesn't flip to
> +@code{running} after a while, it means that you container is attached, and you need to keep this
> +option turned @code{#t}."))
> +
> +(define (serialize-volumes config)
> +  "Serialize list of pairs into flat list of @code{(\"-v\" \"HOST_PATH:CONTAINER_PATH\" ...)}"
> +  (append-map
> +   (lambda (volume-bind)
> +     (list "-v" (format #f "~?" "~a:~a" volume-bind)))
> +   (docker-container-configuration-volumes config)))

See following about pairs and simplification.

> +
> +(define (serialize-ports config)
> +  "Serialize list of either pairs, or lists into flat list of
> +@code{(\"-p\" \"NUMBER:NUMBER\" \"-p\" \"NUMBER:NUMBER/PROTOCOL\" ...)}"
> +  (append-map
> +   (lambda (port-bind)
> +     (list "-p" (format #f "~?" "~a:~a~^/~a" port-bind)))
> +   (docker-container-configuration-ports config)))

See following about pairs and simplification.

> +
> +(define (serialized-environments config)
> +  "Serialize list of pairs into flat list of @code{(\"-e\" \"VAR=val\" \"-e\" \"VAR=val\" ...)}."
> +  (append-map
> +   (lambda (env-bind)
> +     (list "-e" (format #f "~?" "~a=~a" env-bind)))
> +   (docker-container-configuration-environments config)))

I tried this out in a REPL, but found that it doesn't accept pairs but 
2-element lists:

scheme@(guile-user)> (format #f "~?" "~a=~a" '("x" . "y"))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure length: Wrong type argument in position 1: ("x" . "y")

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> ,q
scheme@(guile-user)> (format #f "~?" "~a=~a" '("x" "y"))
$1 = "x=y"

Also, the 'format' can be simplified:

(apply format #f "~a=~a" env-bind)


> +
> +(define (docker-container-startup-script docker-cli container-name config)
> +  "Return a program file, that executes the startup sequence of the @code{docker-container-shepherd-service}."
> +  (let* ((attached? (docker-container-configuration-attached? config))
> +         (image-name (docker-container-configuration-image config))
> +         (volumes (serialize-volumes config))
> +         (ports (serialize-ports config))
> +         (envs (serialize-environments config))
> +         (network (docker-container-configuration-network config))
> +         (additional-arguments (docker-container-configuration-additional-arguments config))
> +         (container-command (docker-container-configuration-container-command config)))
> +    (program-file
> +     (string-append "start-" container-name "-container")
> +     #~(let ((docker (string-append #$docker-cli "/bin/docker")))
> +         (system* docker "stop" #$container-name)
> +         (system* docker "rm" #$container-name) > +         (apply system* `(,docker
> +                          "run"
> +                          ,(string-append "--name=" #$container-name)
> +                          ;; Automatically remove the container when stopping
> +                          ;; If you want persistent data, you need to use
> +                          ;; volume binds or other methods.
> +                          "--rm"
> +                          ,(string-append "--network=" #$network)
> +                          ;; TODO:
> +                          ;; Write to a cid file the container id, this allows
> +                          ;; for shepherd to manage container even when the process
> +                          ;; itself gets detached from the container
> +                          ,@(if (not #$attached) '("--cidfile" #$cid-file) '())
> +                          #$@volumes
> +                          #$@ports
> +                          #$@envs
> +                          #$@additional-arguments
> +                          ,#$image-name
> +                          #$@container-command))))))

'system*' can fail, which it does by returning some number (and not by 
an exception).  I recommend using 'invoke' from (guix build utils) 
(which uses exceptions) instead; you may need use-modules + 
with-imported-modules to use that module.


> +
> +(define (docker-container-shepherd-service docker-cli config)
> +  "Return a shepherd-service that runs CONTAINER."
> +  (let* ((container-name (symbol->string (docker-container-configuration-name config)))
> +         (cid-file (string-append "/var/run/docker/" container-name ".pid"))

This sounds like ".", ".." and anything containing a "/" or "\x00" would 
be invalid container names, I recommend refining the type check for 
'container-name' a little.  It also looks like container names must be 
unique within a system, that sounds like something to mention in its 
docstring to me.

> +         (attached? (docker-container-configuration-attached? config)))
> +    (shepherd-service
> +     (provision (list (docker-container-configuration-name config)))
> +     (requirement `(dockerd))
> +     (start #~(make-forkexec-constructor
> +               (list #$(docker-container-startup-script docker-cli container-name config))
> +               ;; Watch the cid-file instead of the docker run command, as the daemon can
> +               ;; still be running even when the command terminates
> +               (if (not #$attached?)
> +                   #:pid-file #$cid-file)))

I don't think this does what you want it to do -- when attached, it will 
evaluate to #$cid-file, when not, it will evaluate to #:pid-flile.

Try apply+list instead:

(apply
   make-forkexec-constructor
   (list ...)
   #$(if $attached
         #~()
         #~(list #:pid-file #$cid-file)))

(Changing the staging is not required, though myself I prefer it this way.)

I recommend writing a system test (in gnu/tests/docker.scm), to prevent 
such problems, though I don't know how feasible it would be.

> +     (stop (if #$attached?
> +               #~(make-kill-destructor)
> +               #~(lambda _
> +                   (exec-command (list
> +                                  (string-append #$docker-cli "/bin/docker")
> +                                  "stop" #$container-name))
> +                   #f))))))

Not very familiar with how Shepherd works here, but I think that the 
'return #false' dseserves a command.

Also, on (string-append #$docker-cli "/bin/docker"): we have
#$(file-append docker-cli "/bin/docker") for that, though in this case 
it doesn't matter (for uses inside 'quote', it does, but here, not really).

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[guix-patches--- via]

Hi

> On 27-09-2022 19:16, guix-patches--- via wrote:
>> 
>> This patch provides a new service type, which allows the creation of shepherd
>> services from docker containers.
>> ---
>> Hi,
>> 
>> I have written a definition of the docker-container-service-type. It is
>> a service that allows you to convert docker containers into shepherd
>> services.  [...]
>> 
>> There is currently no documentation outside of docstrings, I plan to
>> write it, but first I would welcome comments from you, maybe this
>> service isn't suitable for guix, as it does imply breaking of the
>> declarative guix model, but that goes for docker and flatpak too, so I
>> thought I can try it.
>
> We already have a docker-service-type, why not a 
> docker-container-service-type, though I wouldn't recommend docker 
> myself.  Can't really document on the docker bits, but some mostly 
> superficial comments:

I was thinking that this isn't exactly a "needed" use and can be thought
of as separate, but probably I was wrong.

>>
>> +(define (pair-of-strings? val)
>> +  (and (pair val)
>
> I think you meant 'pair?' here, not 'pair'.
>

Yes.

>> +       (string? (car val))
>> +       (string? (cdr val))))
>> +
>> +(define (list-of-pair-of-strings? val)
>> +  (list-of pair-of-strings?))
>> +
>> +(define-configuration/no-serialization docker-container-configuration
>> +  (name
>> +   (symbol '())
>> +   "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
>> +We recommend, that the name of the container is prefixed with @code{docker-}.")
>> +  (comment
>> +   (string "")
>> +   "A documentation on the docker container.")
>
> I don't think documentation is countable, maybe
>
> "Documentation on the Docker container (optional)."
>
> ?  (I don't know what casing is appropriate here).
>
>> +  (image-name
>> +   (string)
>> +   "A name of the image that will be used. (Note that the existence of the image
>> +is not guaranteed by this daemon.)")
>> +  (volumes
>> +   (list-of-pair-of-strings '())
>> +   "A list of volume binds. In (HOST_PATH CONTAINER_PATH) format.")
>
> binds -> bindings and HOST_PATH CONTAINER_PATH -> (HOST-PATH 
> CONTAINER-PATH) per Scheme conventions.
>
>> +  (ports
>> +   (list-of-pair-of-strings '())
>> +   "A list of port binds. In (HOST_PORT CONTAINER_PORT) or (HOST_PORT CONTAINER_PORT OPTIONS) format.
>> +For example, both port bindings are valid:
>> +
>> +@lisp
>> +(ports '((\"2222\" \"22\") (\"21\" \"21\" \"tcp\")))
>> +@end lisp"
>
> * binds -> bindings
>
>> +   (environments
>> +    (list-of-pair-of-strings '())
>> +    "A list of variable binds, inside the container enviornment. In (VARIABLE VALUE) format."))
>
> 'enviornment' -> 'environment', 'variable binds' -> 'environment 
> variables', '. In' -> ', in'.
>
>> +  (network
>> +   (string "none")
>> +   "Network type.")
>
> Can the available network types be listed or a reference to the Docker 
> documentation be added, to help users with determining what to set it to?
>
>> +  (additional-arguments
>> +   (list-of-strings '())
>> +   "Additional arguments to the docker command line interface.")
>> +  (container-command
>> +   (list-of-strings '())
>> +   "Command to send into the container.")
>> +  (attached?
>> +   (boolean #t)
>> +   "Run the container as an normal attached process (sending SIGTERM).
>> +Or run the container as a isolated environment that must be stopped with @code{docker stop}.
>> +
>> +Please verify first, that you container is indeed not attached, otherwise @code{shepherd} might
>> +assume the process is dead, even when it is not.
>> +
>> +You can do that, by first running your container with @code{docker run image-name}.
>> +
>> +Then check @code{docker ps}, if the command shows beside your container the word @code{running}.
>> +Your container is indeed detached, but if it shows @code{starting}, and it doesn't flip to
>> +@code{running} after a while, it means that you container is attached, and you need to keep this
>> +option turned @code{#t}."))
>> +
>> +(define (serialize-volumes config)
>> +  "Serialize list of pairs into flat list of @code{(\"-v\" \"HOST_PATH:CONTAINER_PATH\" ...)}"
>> +  (append-map
>> +   (lambda (volume-bind)
>> +     (list "-v" (format #f "~?" "~a:~a" volume-bind)))
>> +   (docker-container-configuration-volumes config)))
>
> See following about pairs and simplification.
>
>> +
>> +(define (serialize-ports config)
>> +  "Serialize list of either pairs, or lists into flat list of
>> +@code{(\"-p\" \"NUMBER:NUMBER\" \"-p\" \"NUMBER:NUMBER/PROTOCOL\" ...)}"
>> +  (append-map
>> +   (lambda (port-bind)
>> +     (list "-p" (format #f "~?" "~a:~a~^/~a" port-bind)))
>> +   (docker-container-configuration-ports config)))
>
> See following about pairs and simplification.
>
>> +
>> +(define (serialized-environments config)
>> +  "Serialize list of pairs into flat list of @code{(\"-e\" \"VAR=val\" \"-e\" \"VAR=val\" ...)}."
>> +  (append-map
>> +   (lambda (env-bind)
>> +     (list "-e" (format #f "~?" "~a=~a" env-bind)))
>> +   (docker-container-configuration-environments config)))
>
> I tried this out in a REPL, but found that it doesn't accept pairs but 
> 2-element lists:
>
> scheme@(guile-user)> (format #f "~?" "~a=~a" '("x" . "y"))
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> In procedure length: Wrong type argument in position 1: ("x" . "y")
>
> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
> scheme@(guile-user) [1]> ,q
> scheme@(guile-user)> (format #f "~?" "~a=~a" '("x" "y"))
> $1 = "x=y"
>
> Also, the 'format' can be simplified:
>
> (apply format #f "~a=~a" env-bind)
>
>
>> +
>> +(define (docker-container-startup-script docker-cli container-name config)
>> +  "Return a program file, that executes the startup sequence of the @code{docker-container-shepherd-service}."
>> +  (let* ((attached? (docker-container-configuration-attached? config))
>> +         (image-name (docker-container-configuration-image config))
>> +         (volumes (serialize-volumes config))
>> +         (ports (serialize-ports config))
>> +         (envs (serialize-environments config))
>> +         (network (docker-container-configuration-network config))
>> +         (additional-arguments (docker-container-configuration-additional-arguments config))
>> +         (container-command (docker-container-configuration-container-command config)))
>> +    (program-file
>> +     (string-append "start-" container-name "-container")
>> +     #~(let ((docker (string-append #$docker-cli "/bin/docker")))
>> +         (system* docker "stop" #$container-name)
>> +         (system* docker "rm" #$container-name) > +         (apply system* `(,docker
>> +                          "run"
>> +                          ,(string-append "--name=" #$container-name)
>> +                          ;; Automatically remove the container when stopping
>> +                          ;; If you want persistent data, you need to use
>> +                          ;; volume binds or other methods.
>> +                          "--rm"
>> +                          ,(string-append "--network=" #$network)
>> +                          ;; TODO:
>> +                          ;; Write to a cid file the container id, this allows
>> +                          ;; for shepherd to manage container even when the process
>> +                          ;; itself gets detached from the container
>> +                          ,@(if (not #$attached) '("--cidfile" #$cid-file) '())
>> +                          #$@volumes
>> +                          #$@ports
>> +                          #$@envs
>> +                          #$@additional-arguments
>> +                          ,#$image-name
>> +                          #$@container-command))))))
>
> 'system*' can fail, which it does by returning some number (and not by 
> an exception).  I recommend using 'invoke' from (guix build utils) 
> (which uses exceptions) instead; you may need use-modules + 
> with-imported-modules to use that module.
>
>
>> +
>> +(define (docker-container-shepherd-service docker-cli config)
>> +  "Return a shepherd-service that runs CONTAINER."
>> +  (let* ((container-name (symbol->string (docker-container-configuration-name config)))
>> +         (cid-file (string-append "/var/run/docker/" container-name ".pid"))
>
> This sounds like ".", ".." and anything containing a "/" or "\x00" would 
> be invalid container names, I recommend refining the type check for 
> 'container-name' a little.  It also looks like container names must be 
> unique within a system, that sounds like something to mention in its 
> docstring to me.
>

There actually is mention of it!

"Name of the docker container. Will be used to denote service to Shepherd and must be unique!
We recommend, that the name of the container is prefixed with
@code{docker-}."

But I agree I need to modify it a bit.

>> +         (attached? (docker-container-configuration-attached? config)))
>> +    (shepherd-service
>> +     (provision (list (docker-container-configuration-name config)))
>> +     (requirement `(dockerd))
>> +     (start #~(make-forkexec-constructor
>> +               (list #$(docker-container-startup-script docker-cli container-name config))
>> +               ;; Watch the cid-file instead of the docker run command, as the daemon can
>> +               ;; still be running even when the command terminates
>> +               (if (not #$attached?)
>> +                   #:pid-file #$cid-file)))
>
> I don't think this does what you want it to do -- when attached, it will 
> evaluate to #$cid-file, when not, it will evaluate to #:pid-flile.
>
> Try apply+list instead:
>
> (apply
>    make-forkexec-constructor
>    (list ...)
>    #$(if $attached
>          #~()
>          #~(list #:pid-file #$cid-file)))
>
> (Changing the staging is not required, though myself I prefer it this way.)
>
> I recommend writing a system test (in gnu/tests/docker.scm), to prevent 
> such problems, though I don't know how feasible it would be.
>

I overlooked some issues, as I was sending the patch after last minute
changes and I forgot to check it, thank you for noticing that mistake.

>> +     (stop (if #$attached?
>> +               #~(make-kill-destructor)
>> +               #~(lambda _
>> +                   (exec-command (list
>> +                                  (string-append #$docker-cli "/bin/docker")
>> +                                  "stop" #$container-name))
>> +                   #f))))))
>
> Not very familiar with how Shepherd works here, but I think that the 
> 'return #false' dseserves a command.
>

Well, I looked through the source code, and this is shepherd's own
definition:


(define* (make-kill-destructor #:optional (signal SIGTERM))
  "Return a procedure that sends SIGNAL to the process group of the PID given
as argument, where SIGNAL defaults to `SIGTERM'."
  (lambda (pid . args)
    ;; Kill the whole process group PID belongs to.  Don't assume that PID is
    ;; a process group ID: that's not the case when using #:pid-file, where
    ;; the process group ID is the PID of the process that "daemonized".  If
    ;; this procedure is called, between the process fork and exec, the PGID
    ;; will still be zero (the Shepherd PGID). In that case, use the PID.
    (let ((pgid (getpgid pid)))
      (if (= (getpgid 0) pgid)
          (kill pid signal) ;don't kill ourself
          (kill (- pgid) signal)))
    #f))


So I think that returning #f works. docker stop will send SIGKILL if the
container takes too long, so it should succeed.

I will send a new patch with this service moved into the
docker-service-type and addressed your criticisms.

Maya

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 2827 bytes --]


>>> +
>>> +(define (docker-container-shepherd-service docker-cli config)
>>> +  "Return a shepherd-service that runs CONTAINER."
>>> +  (let* ((container-name (symbol->string (docker-container-configuration-name config)))
>>> +         (cid-file (string-append "/var/run/docker/" container-name ".pid"))
>>
>> This sounds like ".", ".." and anything containing a "/" or "\x00" would
>> be invalid container names, I recommend refining the type check for
>> 'container-name' a little.  It also looks like container names must be
>> unique within a system, that sounds like something to mention in its
>> docstring to me.
>>
> 
> There actually is mention of it!
> 
> "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
> We recommend, that the name of the container is prefixed with
> @code{docker-}."

Oops, didn't notice that.  However, you could insert a check somewhere 
for uniqueness, to avoid accidents.

>>> +     (stop (if #$attached?
>>> +               #~(make-kill-destructor)
>>> +               #~(lambda _
>>> +                   (exec-command (list
>>> +                                  (string-append #$docker-cli "/bin/docker")
>>> +                                  "stop" #$container-name))
>>> +                   #f))))))
>>
>> Not very familiar with how Shepherd works here, but I think that the
>> 'return #false' dseserves a command.
>>
> 
> Well, I looked through the source code, and this is shepherd's own
> definition:
> 
> 
> (define* (make-kill-destructor #:optional (signal SIGTERM))
>    "Return a procedure that sends SIGNAL to the process group of the PID given
> as argument, where SIGNAL defaults to `SIGTERM'."
>    (lambda (pid . args)
>      ;; Kill the whole process group PID belongs to.  Don't assume that PID is
>      ;; a process group ID: that's not the case when using #:pid-file, where
>      ;; the process group ID is the PID of the process that "daemonized".  If
>      ;; this procedure is called, between the process fork and exec, the PGID
>      ;; will still be zero (the Shepherd PGID). In that case, use the PID.
>      (let ((pgid (getpgid pid)))
>        (if (= (getpgid 0) pgid)
>            (kill pid signal) ;don't kill ourself
>            (kill (- pgid) signal)))
>      #f))
> 
> 
> So I think that returning #f works. docker stop will send SIGKILL if the
> container takes too long, so it should succeed.

Not saying it won't work, just that it deserves a comment (though 
apparently I misspelled 'comment'), even if it's only something like 
"return #false as done by 'make-kill-destructor'".

However, 'exec-command' runs 'exec' (replacing the shepherd process), I 
think you need something like 'invoke' or 'system*' instead.

Greetings,
MaximE.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 510 bytes --]



On 30-09-2022 15:40, Mája Tomášek wrote:
> +(define-configuration/no-serialization docker-container-configuration
> +  (name
> +   (symbol '())
> +   "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
> +We recommend, that the name of the container is prefixed with @code{docker-}.")

On uniqueness: on second thought, that's probably already handled by the 
general service code, probably no need for an additional check here.

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#58123]

[guix-patches--- via]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

I have applied the changes as you suggested. Thank you for your (as you
said) "superficial comments", they were really helpful! And I am happy
that you made them, as I'm sometimes too happy that I have made a
contribution and I forget that I don't write only for myself, but for
others.


[-- Attachment #2: 0001-Add-docker-container-management-with-shepherd.patch --]
[-- Type: text/x-patch, Size: 13525 bytes --]

---
 gnu/services/docker.scm | 240 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 222 insertions(+), 18 deletions(-)

diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
index 741bab5a8c..f3a347981f 100644
--- a/gnu/services/docker.scm
+++ b/gnu/services/docker.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2020 Jesse Dowell <jessedowell@gmail.com>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2022 Maya Tomasek <maya.omase@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,7 +22,9 @@
 ;;; You should have received a copy of the GNU General Public License
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
-(define-module (gnu services docker)
+(define-module (magi system docker)
+  #:use-module (srfi srfi-1)
+  #:use-module (ice-9 format)
   #:use-module (gnu services)
   #:use-module (gnu services configuration)
   #:use-module (gnu services base)
@@ -36,9 +39,191 @@ (define-module (gnu services docker)
   #:use-module (guix packages)
 
   #:export (docker-configuration
+            docker-container
             docker-service-type
             singularity-service-type))
 
+(define (pair-of-strings? val)
+  (and (pair? val)
+       (string? (car val))
+       (string? (cdr val))))
+
+(define (list-of-pair-of-strings? val)
+  (list-of pair-of-strings?))
+
+(define-configuration/no-serialization docker-container
+  (name
+   (symbol '())
+   "Name of the docker container. Will be used to denote service to Shepherd and must be unique!
+We recommend, that the name of the container is prefixed with @code{docker-}.")
+  (documentation
+   (string "")
+   "Documentation on the docker container (optional). It will be used for the shepherd service.")
+  (image-name
+   (string #f)
+   "A name of the image that will be used. (Note that the existence of the image
+is not guaranteed by this daemon.)")
+  (volumes
+   (list-of-pair-of-strings '())
+   "A list of volume bindings. In (HOST-PATH CONTAINER-PATH) format.")
+  (ports
+   (list-of-pair-of-strings '())
+   "A list of port bindings. In (HOST-PORT CONTAINER-PORT) or (HOST-PORT CONTAINER-PORT OPTIONS) format.
+For example, both port bindings are valid:
+
+@lisp
+(ports '((\"2222\" \"22\") (\"21\" \"21\" \"tcp\")))
+@end lisp")
+  (environments
+   (list-of-pair-of-strings '())
+   "A list of environment variables, inside the container environment, in (VARIABLE VALUE) format.")
+  (network
+   (string "none")
+   "Network type.
+
+Available types are:
+@table @code
+@c Copied from https://docs.docker.com/network/
+
+@item none
+
+The default option. For this container, disable all networking. Usually used in
+conjunction with a custom network driver. none is not available for swarm services.
+
+@item bridge
+
+Bridge networks are usually used when your applications run in standalone
+containers that need to communicate.
+
+@item host
+
+For standalone containers, remove network isolation between the container and the Docker host, 
+and use the host’s networking directly.
+
+@item overlay
+
+Overlay networks connect multiple Docker daemons together and enable swarm services to
+communicate with each other. You can also use overlay networks to facilitate
+communication between a swarm service and a standalone container, or between
+two standalone containers on different Docker daemons. This strategy removes
+the need to do OS-level routing between these containers.
+
+@item ipvlan
+
+IPvlan networks give users total control over both IPv4 and IPv6 addressing.
+The VLAN driver builds on top of that in giving operators complete control of
+layer 2 VLAN tagging and even IPvlan L3 routing for users interested in underlay
+network integration.
+
+@item macvlan
+
+Macvlan networks allow you to assign a MAC address to a container, making it appear
+as a physical device on your network. The Docker daemon routes traffic to containers
+by their MAC addresses. Using the macvlan driver is sometimes the best choice when
+dealing with legacy applications that expect to be directly connected to the physical
+network, rather than routed through the Docker host’s network stack.
+
+@end table")
+  (additional-arguments
+   (list-of-strings '())
+   "Additional arguments to the docker command line interface.")
+  (container-command
+   (list-of-strings '())
+   "Command to send into the container.")
+  (pid-file-timeout
+   (number 5)
+   "If the docker container does not show up in @code{docker ps} as @code{running} in less than pid-file-timeout seconds, the container is considered as failing to start.
+
+Note that some containers take a really long time to start, so you should adjust it accordingly."))
+
+(define (serialize-volumes config)
+  "Serialize list of pairs into flat list of @code{(\"-v\" \"HOST_PATH:CONTAINER_PATH\" ...)}"
+  (append-map
+   (lambda (volume-bind)
+     (list "-v" (apply format #f "~a:~a~^:~a" volume-bind)))
+   (docker-container-volumes config)))
+
+(define (serialize-ports config)
+  "Serialize list of either pairs, or lists into flat list of
+@code{(\"-p\" \"NUMBER:NUMBER\" \"-p\" \"NUMBER:NUMBER/PROTOCOL\" ...)}"
+  (append-map
+   (lambda (port-bind)
+     (list "-p" (apply format #f "~a:~a~^/~a" port-bind)))
+   (docker-container-ports config)))
+
+(define (serialize-environments config)
+  "Serialize list of pairs into flat list of @code{(\"-e\" \"VAR=val\" \"-e\" \"VAR=val\" ...)}."
+  (append-map
+   (lambda (env-bind)
+     (list "-e" (apply format #f "~a=~a" env-bind)))
+   (docker-container-environments config)))
+
+(define (docker-container-startup-script docker-cli container-name cid-file config)
+  "Return a program file, that executes the startup sequence of the @code{docker-container-shepherd-service}."
+  (let* ((image-name (docker-container-image-name config))
+         (volumes (serialize-volumes config))
+         (ports (serialize-ports config))
+         (envs (serialize-environments config))
+         (network (docker-container-network config))
+         (additional-arguments (docker-container-additional-arguments config))
+         (container-command (docker-container-container-command config)))
+    (with-imported-modules
+     '((guix build utils))
+     (program-file
+      (string-append "start-" container-name "-container")
+      #~(let ((docker (string-append #$docker-cli "/bin/docker")))
+          (use-modules (guix build utils))
+          ;; These two commands should fail
+          ;; they are there as a failsafe to
+          ;; prevent contamination from unremoved containers
+          (system* docker "stop" #$container-name)
+          (system* docker "rm" #$container-name)
+          (apply invoke `(,docker
+                           "run"
+                           ,(string-append "--name=" #$container-name)
+                           ;; Automatically remove the container when stopping
+                           ;; If you want persistent data, you need to use
+                           ;; volume binds or other methods.
+                           "--rm"
+                           ,(string-append "--network=" #$network)
+                           ;; Write to a cid file the container id, this allows
+                           ;; for shepherd to manage container even when the process
+                           ;; itself gets detached from the container
+                           "--cidfile" #$cid-file
+                           #$@volumes
+                           #$@ports
+                           #$@envs
+                           #$@additional-arguments
+                           ,#$image-name
+                           #$@container-command)))))))
+
+(define (docker-container-shepherd-service docker-cli config)
+  "Return a shepherd-service that runs CONTAINER."
+  (let* ((container-name (symbol->string (docker-container-name config)))
+         (cid-file (string-append "/var/run/docker/" container-name ".pid"))
+         (pid-file-timeout (docker-container-pid-file-timeout config)))
+    (shepherd-service
+     (provision (list (docker-container-name config)))
+     (requirement `(dockerd))
+     (documentation (docker-container-documentation config))
+     (start #~(apply make-forkexec-constructor
+                     `(,(list #$(docker-container-startup-script docker-cli container-name cid-file config))
+                       ;; Watch the cid-file instead of the docker run command, as the daemon can
+                       ;; still be running even when the command terminates
+                       #:pid-file #$cid-file
+                       #:pid-file-timeout #$pid-file-timeout)))
+     (stop #~(lambda _
+               (invoke
+                (string-append #$docker-cli "/bin/docker")
+                "stop"
+                #$container-name)
+               ;; Shepherd expects the stop command to return #f if it succeeds
+               ;; docker stop should always succeed
+               #f)))))
+
+(define (list-of-docker-containers? val)
+  (list-of docker-container?))
+
 (define-configuration docker-configuration
   (docker
    (file-like docker)
@@ -65,8 +250,21 @@ (define-configuration docker-configuration
   (environment-variables
    (list '())
    "Environment variables to set for dockerd")
+  (containers
+   (list-of-docker-containers '())
+   "List of docker containers to run as shepherd services.")
   (no-serialization))
 
+(define (docker-container-shepherd-services config)
+  "Return shepherd services for all containers inside config."
+  (let ((docker-cli (docker-configuration-docker-cli config)))
+    (map
+     (lambda (container)
+       (docker-container-shepherd-service
+        docker-cli
+        container))
+     (docker-configuration-containers config))))
+
 (define %docker-accounts
   (list (user-group (name "docker") (system? #t))))
 
@@ -88,20 +286,20 @@ (define (containerd-shepherd-service config)
          (debug? (docker-configuration-debug? config))
          (containerd (docker-configuration-containerd config)))
     (shepherd-service
-           (documentation "containerd daemon.")
-           (provision '(containerd))
-           (start #~(make-forkexec-constructor
-                     (list (string-append #$package "/bin/containerd")
-                           #$@(if debug?
-                                  '("--log-level=debug")
-                                  '()))
-                     ;; For finding containerd-shim binary.
-                     #:environment-variables
-                     (list (string-append "PATH=" #$containerd "/bin"))
-                     #:pid-file "/run/containerd/containerd.pid"
-                     #:pid-file-timeout 300
-                     #:log-file "/var/log/containerd.log"))
-           (stop #~(make-kill-destructor)))))
+     (documentation "containerd daemon.")
+     (provision '(containerd))
+     (start #~(make-forkexec-constructor
+               (list (string-append #$package "/bin/containerd")
+                     #$@(if debug?
+                            '("--log-level=debug")
+                            '()))
+               ;; For finding containerd-shim binary.
+               #:environment-variables
+               (list (string-append "PATH=" #$containerd "/bin"))
+               #:pid-file "/run/containerd/containerd.pid"
+               #:pid-file-timeout 300
+               #:log-file "/var/log/containerd.log"))
+     (stop #~(make-kill-destructor)))))
 
 (define (docker-shepherd-service config)
   (let* ((docker (docker-configuration-docker config))
@@ -148,7 +346,7 @@ (define (docker-shepherd-service config)
 (define docker-service-type
   (service-type (name 'docker)
                 (description "Provide capability to run Docker application
-bundles in Docker containers.")
+bundles in Docker containers and optionally wrap those containers in shepherd services.")
                 (extensions
                  (list
                   ;; Make sure the 'docker' command is available.
@@ -158,10 +356,16 @@ (define docker-service-type
                                      %docker-activation)
                   (service-extension shepherd-root-service-type
                                      (lambda (config)
-                                       (list (containerd-shepherd-service config)
-                                             (docker-shepherd-service config))))
+                                       (cons* (containerd-shepherd-service config)
+                                              (docker-shepherd-service config)
+                                              (docker-container-shepherd-services config))))
                   (service-extension account-service-type
                                      (const %docker-accounts))))
+                (compose concatenate)
+                (extend (lambda (config containers)
+                          (docker-configuration
+                           (inherit config)
+                           (containers (append containers (docker-configuration-containers config))))))
                 (default-value (docker-configuration))))
 
 \f
-- 
2.37.3

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Ludovic Courtès]

Hi Mája,

Mája Tomášek <maya.tomasek@disroot.org> skribis:

> I have applied the changes as you suggested. Thank you for your (as you
> said) "superficial comments", they were really helpful! And I am happy
> that you made them, as I'm sometimes too happy that I have made a
> contribution and I forget that I don't write only for myself, but for
> others.

Thanks for the nice and useful service!

It looks pretty good already (in part thanks to Maxime’s guidance :-)).
I would have two more asks:

  1. Could you update doc/guix.texi to document the new service?  You
     can mostly use ‘generate-documentation’ to produce the reference of
     the configuration record, and then add a paragraph giving some
     context and a documented example.

  2. Could you add a test under (gnu tests *)?  That would ensure the
     service does not bitrot going forward.

Let us know if you need guidance on these things.  When you’re done,
please send an updated patch with those changes here.

Thank you!

Ludo’.

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[guix-patches--- via]

Hi Ludo',

>> I have applied the changes as you suggested. Thank you for your (as you
>> said) "superficial comments", they were really helpful! And I am happy
>> that you made them, as I'm sometimes too happy that I have made a
>> contribution and I forget that I don't write only for myself, but for
>> others.
>
> Thanks for the nice and useful service!
>
> It looks pretty good already (in part thanks to Maxime’s guidance :-)).
> I would have two more asks:
>
>   1. Could you update doc/guix.texi to document the new service?  You
>      can mostly use ‘generate-documentation’ to produce the reference of
>      the configuration record, and then add a paragraph giving some
>      context and a documented example.

Is that a command from make? I'm sorry I have never used it, I can
update it if I can generate it :)

>   2. Could you add a test under (gnu tests *)?  That would ensure the
>      service does not bitrot going forward.

I'm not exactly sure what would that mean. Test that creates a container
and then runs it or...?

> Let us know if you need guidance on these things.  When you’re done,
> please send an updated patch with those changes here.

Will do!

Maya

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Ludovic Courtès]

Hello,

Mája Tomášek <maya.tomasek@disroot.org> skribis:

>> I would have two more asks:
>>
>>   1. Could you update doc/guix.texi to document the new service?  You
>>      can mostly use ‘generate-documentation’ to produce the reference of
>>      the configuration record, and then add a paragraph giving some
>>      context and a documented example.
>
> Is that a command from make? I'm sorry I have never used it, I can
> update it if I can generate it :)

‘generate-documentation’ is a procedure in (gnu services configuration):

  https://guix.gnu.org/manual/devel/en/html_node/Complex-Configurations.html#index-generate_002ddocumentation

>>   2. Could you add a test under (gnu tests *)?  That would ensure the
>>      service does not bitrot going forward.
>
> I'm not exactly sure what would that mean. Test that creates a container
> and then runs it or...?

I guess the test would start the Shepherd service, ensure it’s running,
and attempt to talk to the running container one way or another.

Does that make sense?

Thanks,
Ludo’.

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Ludovic Courtès]

Hi Mája,

Did you have a chance to look into this?

  https://issues.guix.gnu.org/58123

Thanks in advance,
Ludo’.

Ludovic Courtès <ludo@gnu.org> skribis:

> Hello,
>
> Mája Tomášek <maya.tomasek@disroot.org> skribis:
>
>>> I would have two more asks:
>>>
>>>   1. Could you update doc/guix.texi to document the new service?  You
>>>      can mostly use ‘generate-documentation’ to produce the reference of
>>>      the configuration record, and then add a paragraph giving some
>>>      context and a documented example.
>>
>> Is that a command from make? I'm sorry I have never used it, I can
>> update it if I can generate it :)
>
> ‘generate-documentation’ is a procedure in (gnu services configuration):
>
>   https://guix.gnu.org/manual/devel/en/html_node/Complex-Configurations.html#index-generate_002ddocumentation
>
>>>   2. Could you add a test under (gnu tests *)?  That would ensure the
>>>      service does not bitrot going forward.
>>
>> I'm not exactly sure what would that mean. Test that creates a container
>> and then runs it or...?
>
> I guess the test would start the Shepherd service, ensure it’s running,
> and attempt to talk to the running container one way or another.
>
> Does that make sense?
>
> Thanks,
> Ludo’.

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[guix-patches--- via]

Hi Ludo',

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Mája,
>
> Did you have a chance to look into this?
>

I am sorry I have been busy lately. I have briefly looked into (gnu
tests docker) but I need to study it more closely. I hope that I will
have more time to spend on this during the holidays.

I haven't forgot about this! I have been using it on my machines all
this time, so this isn't something I want to abandon, but I need to test
it more thoroughly and really study how tests work. The docker one is
pretty complex, and I need to do something at least that complicated.

Best regards,
Maya

[bug#58123] [PATCH] gnu: services: docker: Add docker-container-service-type

[Sergey Trofimov]

Mája Tomášek <maya.tomasek@disroot.org> writes:

Hi, guix has oci-container-service-type nowadays. Is this patch 
now obsolete?

> Hi Ludo',
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi Mája,
>>
>> Did you have a chance to look into this?
>>
>
> I am sorry I have been busy lately. I have briefly looked into 
> (gnu
> tests docker) but I need to study it more closely. I hope that I 
> will
> have more time to spend on this during the holidays.
>
> I haven't forgot about this! I have been using it on my machines 
> all
> this time, so this isn't something I want to abandon, but I need 
> to test
> it more thoroughly and really study how tests work. The docker 
> one is
> pretty complex, and I need to do something at least that 
> complicated.
>
> Best regards,
> Maya

bug#58123: [PATCH] gnu: services: docker: Add docker-container-service-type

[Ludovic Courtès]

Hi Sergey and all,

Sergey Trofimov <sarg@sarg.org.ru> skribis:

> Mája Tomášek <maya.tomasek@disroot.org> writes:
>
> Hi, guix has oci-container-service-type nowadays. Is this patch now
> obsolete?

Yes, looks like it (I had completely forgotten about it!).

I’m closing this issue but Mája, please let us know if you think of
changes that should be made to ‘oci-container-service-type’ to match
what you had proposed.

Thanks,
Ludo’.