Subject: Re: [bug#66160] [PATCH] gnu: Add oci-container-service-type.

From: Ludovic Courtès <[email-masked]>
Date: Sat, 14 Oct 2023 18:09:27 +0200

Hi Giacomo,

Giacomo Leidi <[email-masked]> skribis:

> * gnu/services/docker.scm (oci-container-configuration): New variable;
> (oci-container-shepherd-service): new variable;
> (oci-container-service-type): new variable.
> * doc/guix.texi: Document it.

We’re almost there!  There’s a couple of things I overlooked before (my
apologies), so here we go:

> +@table @asis
> +@item @code{command} (default: @code{()}) (type: list-of-strings)
> +Overwrite the default command (@code{CMD}) of the image.
> +
> +@item @code{entrypoint} (default: @code{""}) (type: string)
> +Overwrite the default entrypoint (@code{ENTRYPOINT}) of the image.

Apparently this doesn’t match the docstring that’s in
‘define-configuration’.

Could you make sure the docstring is the canonical source?  Then you can
use ‘generate-documentation’ to generate the bit that you’ll paste in
guix.texi (info "(guix) Complex Configurations").

> +  (entrypoint
> +   (string "")
> +   "Overwrite the default ENTRYPOINT of the image.")
> +  (environment
> +   (list '())
> +   "Set environment variables."
> +   (sanitizer oci-sanitize-environment))
> +  (image
> +   (string)
> +   "The image used to build the container.")
> +  (name
> +   (string "")
> +   "Set a name for the spawned container.")

Please use ‘maybe-string’ in cases where it’s either the Docker default
(default ENTRYPOINT, default CMD, etc.) or some user-provided value.
I find it clearer or at least more conventional than using the empty
string to denote default values.

> +(define oci-container-configuration->options
> +  (lambda (config)
> +    (let ((entrypoint
> +           (oci-container-configuration-entrypoint config))
> +          (network
> +           (oci-container-configuration-network config)))
> +      (apply append
> +             (filter (compose not unspecified?)
> +                     `(,(when (not (string-null? entrypoint))
> +                          (list "--entrypoint" entrypoint))
> +                       ,(append-map
> +                         (lambda (spec)
> +                           (list "--env" spec))
> +                         (oci-container-configuration-environment config))
> +                       ,(when (not (string-null? network))
> +                          (list "--network" network))

This would thus become:

   `(,@(if entrypoint
           `("--entrypoint" ,entrypoint)
           '())
     …)

> +                       #~(make-forkexec-constructor
> +                          ;; docker run [OPTIONS] IMAGE [COMMAND] [ARG...]
> +                          (list #$docker-command
> +                                "run"
> +                                "--rm"
> +                                "--name" #$name
> +                                #$@(oci-container-configuration->options config)
> +                                #$(oci-container-configuration-image config)
> +                                #$@(oci-container-configuration-command config))
> +                          #:user "root"
> +                          #:group "root"))

Does ‘docker run’ necessarily need to run as root, or are there cases
where one might want to run it as non-root?  (I expect the latter.)

> +(define oci-container-service-type
> +  (service-type (name 'oci-container)
> +                (extensions (list (service-extension profile-service-type
> +                                                     (lambda _ (list docker-cli)))
> +                                  (service-extension shepherd-root-service-type
> +                                                     configs->shepherd-services)))
> +                (default-value '())

I wonder if it should take a list of configs and be extensible, or
simply take a single config.  Users would write:

  (service oci-container-service-type
           (oci-container-configuration …))

WDYT?

Last thing: there’s no system test (something we normally require), but
since I forgot about it before and I’m already asking for more than I
should :-) I propose to leave it for later.

Thanks!

Ludo’.