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

[paul via Guix-patches via <guix-patches@gnu.org>]

Hi Ludo’ ,


> We’re almost there!  There’s a couple of things I overlooked before (my
> apologies), so here we go:
Thank you for your help and the time you spent reviewing this!
>> +@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").
I should have aligned the code and documentation.
>
>> +  (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.
I don't know why I didn't do it in the first place, thank you. fixed
>> +                       #~(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.)

yes you are right, it's only required to be in the docker group or in 
general have enough permission to operate on the docker daemon socket. I 
added a new service extension setting up an oci-container user, that 
it's just in the docker group and can not login, that runs oci backed 
services. it is also overridable by the user


>
>> +(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?

I get that it's not super consistent with other services but for example 
Nextcloud in some case require 3 containers: nextcloud itself, redis and 
a cron container. in this case i suppose one would define an 
oci-nextcloud-service-type which maybe extends an 
oci-redis-service-type. in this case the oci-nextcloud-service-type 
would define the 2 shepherd services for nextcloud and the cron process 
and the oci-redis-service-type would define one redis service.

Right now we can already do:

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

and i updated the documentation accordingly. do you have any suggestion 
for changing the api of oci-container-configuration to support having 
different shepherd oci backed services behind a guix system service? 
This way we could remove the (list) call.

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

I actually looked around but didn't find them, but it was a long time 
ago and I certainly didn't look very well. I'm definitely up for testing 
this (maybe it's possible to use swineherd? could we use it for 
automating integration tests?), could you point me to something similar 
i can look to as an example?


Thank you for your time and effort, i'm sending an updated patch


giacomo