GNU bug report logs - #66160
[PATCH] gnu: Add oci-container-service-type.

Previous Next

Full log

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

From: paul <goodoldpaul <at> autistici.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 66160 <at> debbugs.gnu.org
Subject: Re: bug#66160: [PATCH] gnu: Add oci-container-service-type.
Date: Thu, 5 Oct 2023 19:30:41 +0200

Hi,

On 10/5/23 16:30, Ludovic Courtès wrote:
> Hi,
>
> Giacomo Leidi <goodoldpaul <at> autistici.org> 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.
> Neat!
>
>> +@cindex OCI-backed, Shepherd services
>> +@subsubheading OCI backed services
>> +
>> +Should you wish to manage your Docker containers with the same consistent
>> +interface you use for your other Shepherd services,
>> +@var{oci-container-service-type} is the tool to use.
> Perhaps expound a bit, like:
>
>    … is the tool to use: given an @acronym{Open Container Initiative,
>    OCI} container image, it will run it in a Shepherd service.  One
>    example where this is useful: it lets you run services that are
>    available as Docker/OCI images but not yet packaged for Guix.
nice thank you, fixed.
>
>> +@defvar oci-container-service-type
>> +
>> +This is a thin wrapper around Docker's CLI that wraps OCI images backed
>> +processes as Shepherd Services.
>> +
>> +@lisp
>> +(simple-service 'oci-grafana-service
>> +                (list
>> +                 (oci-container-configuration
> The second argument to ‘simple-service’ is missing.
Good catch, fixed.
>
>> +                  (image "prom/prometheus")
>> +                  (network "host")
>> +                  (ports
>> +                    '(("9000" . "9000")
>> +                      ("9090" . "9090"))))))
>> +                 (oci-container-configuration
>> +                  (image "grafana/grafana:10.0.1")
>> +                  (network "host")
>> +                  (volumes
>> +                    '("/var/lib/grafana:/var/lib/grafana"))))))
>> +@end lisp
> Please explain the example in one or two sentences.
>
> Personally, I’d like to know how the image names are resolved; would be
> nice to mention it in the doc.
[ ... ]
>
>> +@table @asis
>> +@item @code{command} (default: @code{()}) (type: list-of-strings)
>> +Overwrite the default CMD of the image.
> “… the default command (@code{CMD}) of the image.”
[ ... ]
>
>> +@item @code{entrypoint} (default: @code{""}) (type: string)
>> +Overwrite the default ENTRYPOINT of the image.
> Likewise.
Fixed, thank you.
>
>> +@item @code{environment} (default: @code{()}) (type: list)
>> +Set environment variables. This can be a list of pairs or strings, even mixed:
>> +
>> +@lisp
>> +(list '("LANGUAGE" . "eo:ca:eu")
>> +      "JAVA_HOME=/opt/java")
> I would choose one or the other, but not both.
I would like to allow some kind of escape (the same way the nice Guix 
configuration records provide an extra-content field which is literally 
appended to the config) in case there's some something I didn't foresee 
with this implementation. It may be paranoia, I don't have a strong 
opinion. are you strongly against supporting the two formats?
>
>> +@item @code{ports} (default: @code{()}) (type: list)
>> +Set the port or port ranges to expose from the spawned container. This can be a
>> +list of pairs or strings, even mixed:
>> +
>> +@lisp
>> +(list '("8080" . "80")
>> +      "10443:443")
> Likewise.
>
>> +(define (oci-sanitize-pair pair delimiter)
>> +  (cond ((file-like? (car pair))
>> +         (file-append (car pair) delimiter (cdr pair)))
> Please use ‘match’ instead of car/cdr (info "(guix) Data Types and
> Pattern Matching").
Thank you, fixed.
>
>> +         (error
>> +          (format #f "pair members must only contain gexps, file-like objects and strings but ~a was found" (car pair))))))
> Should be (raise (formatted-message (G_ …))).  That way we get i18n
> support and the message is presented like other error messages.

[ ... ]
>
>> +            (error
>> +             (format #f "~a members must be either a string or a pair but ~a was found!" name el)))))
> Ditto.

[ ... ]
>
>> +    (shepherd-service (provision `(,(string->symbol name)))
>> +                      (requirement '(dockerd))
> Actually: (requirement '(dockerd user-processes)).

[ ... ]
>
>> +                (description
>> +                 "This service provides allows the management of Docker
>> +containers as Shepherd services.")))
> “Docker and OCI containers”
Fixed.
> Could you send an updated patch?

I should have addressed all of your comments besides the one on the 
key-value format. I'm sending an updated patch.


Thank you for your time and effort,


giacomo

Previous Next

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