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

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

[Zheng Junjie]

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(-)

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

base-commit: 473cdecd8965a301ef6817027090bc61c6907a18
-- 
2.41.0

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

[Fabio Natali via Guix-patches via]

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.

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

[Fabio Natali via Guix-patches via]

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.


-- 
Fabio Natali
https://fabionatali.com

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

[Zheng Junjie]

[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]