[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#66160] [PATCH] gnu: Add oci-container-service-type.
From: |
paul |
Subject: |
[bug#66160] [PATCH] gnu: Add oci-container-service-type. |
Date: |
Sat, 14 Oct 2023 23:29:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 |
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
[bug#66160] [PATCH] gnu: Add oci-container-service-type., Giacomo Leidi, 2023/10/13
[bug#66160] [PATCH] gnu: Add oci-container-service-type., Giacomo Leidi, 2023/10/14
[bug#66160] [PATCH] gnu: Add oci-container-service-type., Giacomo Leidi, 2023/10/14