diff mbox series

[bug#71254] services: oci-container: fix provided image is string.

Message ID 91c850e563f05fb1fa10aea1758786dce3f79157.1716956272.git.zhengjunjie@iscas.ac.cn
State New
Headers show
Series [bug#71254] services: oci-container: fix provided image is string. | expand

Commit Message

Zheng Junjie May 29, 2024, 4:17 a.m. UTC
gnu/services/docker.scm (oci-container-shepherd-service): when image is
oci-image, call %oci-image-loader.

Change-Id: I26105e82643affe9e7037975e42ec9690089545b
---
 gnu/services/docker.scm | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)


base-commit: 473cdecd8965a301ef6817027090bc61c6907a18

Comments

Fabio Natali May 31, 2024, 2:14 p.m. UTC | #1
Hi Zheng,

Thanks for your patch!

For what it's worth, this is to confirm that the patch works for
me. Without the patch, I don't seem to be able to use
'oci-container-service-type', at least when the image is provided as a
string.

I can't comment on the stylistic consistency of the patch (i.e. the use
of '(when #$ ...)' vs '#$@(if ...)'), but it's a +1 for me in terms of
its effectiveness.

Just my 2 cents for other fellow reviewers and the final committer.

Best, Fabio.
Fabio Natali June 5, 2024, 11:12 a.m. UTC | #2
Hi,

Just a quick follow-up to mention the way I tested this. Possibly a bit
convoluted, but it worked for me.

Save this to `/tmp/operating-system-test.scm'.

,----
| (define-module (operating-system-test)
|   #:use-module (gnu)
|   #:use-module (gnu services admin)
|   #:use-module (gnu services dbus)
|   #:use-module (gnu services desktop)
|   #:use-module (gnu services docker)
|   #:use-module (gnu services networking)
|   #:export (operating-system-test))
| 
| (define operating-system-test
|   (operating-system
|     (host-name "host")
|     (timezone "Europe/London")
|     (locale "en_US.UTF-8")
|     (bootloader (bootloader-configuration
|                  (bootloader grub-bootloader)
|                  (targets '("/dev/vda"))))
|     (file-systems (cons
|                    (file-system
|                      (device "/dev/vda2")
|                      (mount-point "/")
|                      (type "ext4"))
|                    %base-file-systems))
| 
|     (services (cons*
|                 (service dhcp-client-service-type)
|                 (service docker-service-type)
|                 (service elogind-service-type)
|                 (service oci-container-service-type
|                          (list
|                           (oci-container-configuration
|                            (image "nginx")
|                            (provision "nginx")
|                            (network "host"))))
|                 %base-services))))
| 
| operating-system-test
`----

Then run this to build a system image.

,----
| guix system image \
|     --load-path=build \
|     --image-size=20GB \
|     --image-type=qcow2 \
|     /tmp/operating-system-test.scm
`----

If you are on or around '2e53fa5346bf52f6d6d26e035bc905ebd410dabb', the
above command will fail with error `In procedure struct-vtable: Wrong
type argument in position 1 (expecting struct): "nginx"'.

Now try instead from a Guix checkout with Zheng's patch.

,----
| orig=$(./pre-inst-env guix system image \
|     --load-path=build \
|     --image-size=20GB \
|     --image-type=qcow2 \
|     /tmp/operating-system-test.scm)
`----

The command will succeed, and this should be sufficient proof of the
patch effectiveness. One can of course go on and test the image itself:

,----
| dest=/tmp/operating-system-media.qcow2
| cp "${orig}" "${dest}"
| chown user:users "${dest}"
| chmod u+w "${dest}"
| qemu-system-x86_64 \
|     -enable-kvm \
|     -m 4096 \
|     -smp 2 \
|     -device e1000,netdev=net0 \
|     -netdev user,id=net0,hostfwd=tcp::8080-:80 \
|     -drive if=none,file="${dest}",id=myhd,index=0 \
|     -device virtio-blk,drive=myhd
`----

There's a bit of manual work to be done here, as in my tests the docker
container doesn't start straightaway. Log in as root, wait for Docker to
finish pulling the Nginx image (see `tail -f /var/log/messages' and
possibly `herd restart dockerd'), then re-enable and start the Nginx
service (`herd enable nginx && herd start nginx'). After that,
everything should work as expected, i.e. the Docker Nginx contained in
the VM answers requests from the host.

Cheers, Fabio.
Zheng Junjie June 6, 2024, 5:57 a.m. UTC | #3
Fabio Natali via Guix-patches via <guix-patches@gnu.org> writes:

> Hi,
>
> Just a quick follow-up to mention the way I tested this. Possibly a bit
> convoluted, but it worked for me.
>
> Save this to `/tmp/operating-system-test.scm'.
>
> ,----
> | (define-module (operating-system-test)
> |   #:use-module (gnu)
> |   #:use-module (gnu services admin)
> |   #:use-module (gnu services dbus)
> |   #:use-module (gnu services desktop)
> |   #:use-module (gnu services docker)
> |   #:use-module (gnu services networking)
> |   #:export (operating-system-test))
> | 
> | (define operating-system-test
> |   (operating-system
> |     (host-name "host")
> |     (timezone "Europe/London")
> |     (locale "en_US.UTF-8")
> |     (bootloader (bootloader-configuration
> |                  (bootloader grub-bootloader)
> |                  (targets '("/dev/vda"))))
> |     (file-systems (cons
> |                    (file-system
> |                      (device "/dev/vda2")
> |                      (mount-point "/")
> |                      (type "ext4"))
> |                    %base-file-systems))
> | 
> |     (services (cons*
> |                 (service dhcp-client-service-type)
> |                 (service docker-service-type)
> |                 (service elogind-service-type)
> |                 (service oci-container-service-type
> |                          (list
> |                           (oci-container-configuration
> |                            (image "nginx")
> |                            (provision "nginx")
> |                            (network "host"))))
> |                 %base-services))))
> | 
> | operating-system-test
> `----
>
> Then run this to build a system image.
>
> ,----
> | guix system image \
> |     --load-path=build \
> |     --image-size=20GB \
> |     --image-type=qcow2 \
> |     /tmp/operating-system-test.scm
> `----
>
> If you are on or around '2e53fa5346bf52f6d6d26e035bc905ebd410dabb', the
> above command will fail with error `In procedure struct-vtable: Wrong
> type argument in position 1 (expecting struct): "nginx"'.
>
> Now try instead from a Guix checkout with Zheng's patch.
>
> ,----
> | orig=$(./pre-inst-env guix system image \
> |     --load-path=build \
> |     --image-size=20GB \
> |     --image-type=qcow2 \
> |     /tmp/operating-system-test.scm)
> `----
>
> The command will succeed, and this should be sufficient proof of the
> patch effectiveness. One can of course go on and test the image itself:
>
> ,----
> | dest=/tmp/operating-system-media.qcow2
> | cp "${orig}" "${dest}"
> | chown user:users "${dest}"
> | chmod u+w "${dest}"
> | qemu-system-x86_64 \
> |     -enable-kvm \
> |     -m 4096 \
> |     -smp 2 \
> |     -device e1000,netdev=net0 \
> |     -netdev user,id=net0,hostfwd=tcp::8080-:80 \
> |     -drive if=none,file="${dest}",id=myhd,index=0 \
> |     -device virtio-blk,drive=myhd
> `----
>
> There's a bit of manual work to be done here, as in my tests the docker
> container doesn't start straightaway. Log in as root, wait for Docker to
> finish pulling the Nginx image (see `tail -f /var/log/messages' and
> possibly `herd restart dockerd'), then re-enable and start the Nginx
> service (`herd enable nginx && herd start nginx'). After that,
> everything should work as expected, i.e. the Docker Nginx contained in
> the VM answers requests from the host.
>
> Cheers, Fabio.
thanks, now pushed, see https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2b2337f275a6421a0d0964c54987df4ac74162e6
diff mbox series

Patch

diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
index 7aff8dcc5f..cc1201508c 100644
--- a/gnu/services/docker.scm
+++ b/gnu/services/docker.scm
@@ -687,18 +687,19 @@  (define (oci-container-shepherd-service config)
                         (if (oci-image? image) name image) "."))
                       (start
                        #~(lambda ()
-                          (when #$(oci-image? image)
-                            (invoke #$(%oci-image-loader
-                                       name image image-reference)))
-                          (fork+exec-command
-                           ;; docker run [OPTIONS] IMAGE [COMMAND] [ARG...]
-                           (list #$docker "run" "--rm" "--name" #$name
-                                 #$@options #$@extra-arguments
-                                 #$image-reference #$@command)
-                           #:user #$user
-                           #:group #$group
-                           #:environment-variables
-                           (list #$@host-environment))))
+                           #$@(if (oci-image? image)
+                                  #~((invoke #$(%oci-image-loader
+                                                name image image-reference)))
+                                  #~())
+                           (fork+exec-command
+                            ;; docker run [OPTIONS] IMAGE [COMMAND] [ARG...]
+                            (list #$docker "run" "--rm" "--name" #$name
+                                  #$@options #$@extra-arguments
+                                  #$image-reference #$@command)
+                            #:user #$user
+                            #:group #$group
+                            #:environment-variables
+                            (list #$@host-environment))))
                       (stop
                        #~(lambda _
                            (invoke #$docker "rm" "-f" #$name)))