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

  • Done
  • quality assurance status badge
Details
2 participants
  • Fabio Natali
  • Zheng Junjie
Owner
unassigned
Submitted by
Zheng Junjie
Severity
normal
Z
Z
Zheng Junjie wrote on 29 May 06:17 +0200
(address . guix-patches@gnu.org)
91c850e563f05fb1fa10aea1758786dce3f79157.1716956272.git.zhengjunjie@iscas.ac.cn
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(-)

Toggle diff (40 lines)
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
F
F
Fabio Natali wrote on 31 May 16:14 +0200
(address . 71254@debbugs.gnu.org)
87plt283wc.fsf@fabionatali.com
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.
F
F
Fabio Natali wrote on 5 Jun 13:12 +0200
(address . 71254@debbugs.gnu.org)
87a5jzd4ny.fsf@fabionatali.com
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
Z
Z
Zheng Junjie wrote on 6 Jun 07:57 +0200
Re: [bug#71254] [PATCH] services: oci-container: fix provided image is string.
(name . Fabio Natali via Guix-patches via)(address . guix-patches@gnu.org)
87sexqvcjx.fsf@iscas.ac.cn
Fabio Natali via Guix-patches via <guix-patches@gnu.org> writes:

Toggle quote (98 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEfr6klGDOXiwIdX/bO1qpk+Gi3/AFAmZhT7IACgkQO1qpk+Gi
3/AzCA//cBpqPs5Oyy4pdRFno7V6x4TaBYmVnytugvXggY5ZqDIrrva8NGUKbb+i
GTXEKY2jV+NeaoqMXLdbTo5Xk36roXxoct4d5Gw0CA34caPa9lBL5H2pdrF23bXT
MTRo5S2zfGcord7kLHF30q7/MKFip+C9brL5fBha6lvmmqoOEkuP+uL7eCX4X16m
XHd6ra0O1P1PiMGe63wJglkZHKEucimmtIZxczMb9uZc1oIN52aUcsDqiXRZu1PH
TUK9QfW2y2UYBV1YD+NkMyoQ41xpXN9F+s+enzW0qB0YW6N3mVwQHtYwfZjMeLag
awH0ggIYrXjwkgF+X9z26K25JWau7U6HiH4P0yW5wuNQe2tO0RVDy53Kh0pAU2dm
Fm4oRJH2Pk5aZp/d/vZQoD4iCb2y2lt368yTOy9m67a9mzzGsD46rfG1LIoTtr7b
tIhOvb0Mw9uXTB0JISa8HfAD5YdAN5BJ8JRDAJY4Vn7XAsZOvVrl0gqoHQocX2eU
iEkibBfFpgynBFsvVGq3nfz2EhEkthNMpE9E5cBgSpd8DgYp7x54MWx6wC9e13lF
XLYCK4UprpNDe8Q83Go7xzU/0TWIHstVEPO+DGjNTlLZQ/lOhhKiEAhwtwUBoz1H
RttYrG6ANMi7IaaROqVdtHi9FuSA3wYK5DSjQJDzvqt2nYu8fJM=
=f51t
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 71254@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71254
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch