GNU bug report logs - #33893
[PATCH 0/2] Add docker.

Previous Next

Full log

Message #80 received at 33893 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 33893 <at> debbugs.gnu.org
Subject: Re: [bug#33893] [PATCH v5 3/4] services: Add docker.
Date: Sun, 06 Jan 2019 21:31:00 +0100

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/services/docker.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi (Miscellaneous Services): Document the service.

Nice!

> +@cindex docker

“Docker” with a capital.

> +@subsubheading Docker Service
> +
> +The @code{(gnu services docker)} module provides the following service.
> +
> +@defvr {Scheme Variable} docker-service-type
> +
> +This is a service that runs @url{http://www.docker.com,Docker}, a daemon that
> +provides container functionality.
> +

We’re missing “@end defvr” I guess.

I think we shouldn’t propagate the narrative that Docker = container.
So what about something like:

  This is the type of the service that runs @url{…, Docker}, a daemon
  that can execute application bundles (sometimes referred to as
  ``containers'') in isolated environments.

?

Also could you document ‘docker-configuration’ as well?


[...]

> +;; TODO: Refactor out into its own module?  How to depend on it then?
> +(define (containerd-shepherd-service config)
> +  (let* ((package (docker-configuration-containerd config)))
> +    (shepherd-service
> +           (documentation "containerd daemon.")
> +           (provision '(containerd))
> +           (start #~(make-forkexec-constructor
> +                     (list (string-append #$package "/bin/containerd"))))
> +           (stop #~(make-kill-destructor)))))

I suppose there could be a separate ‘containerd-service-type’ if it’s
useful; if it’s not, it’s OK to keep it this way.

As for the dependency, users would have to add both docker and
containerd to their service list, or docker-service-type could extend
containerd-service-type, which would ensure containerd-service-type is
automatically instantiated if it’s not already in the user’s service
list.

> +(define docker-service-type
> +  (service-type (name 'docker)
> +		(extensions
> +                 (list
> +                  (service-extension activation-service-type
> +                                     %docker-activation)
> +                  (service-extension shepherd-root-service-type
> +                                     (lambda args
> +                                       (list (apply containerd-shepherd-service args)
> +                                             (apply docker-shepherd-service args))))

You can make the above (lambda (config) …) instead of (lambda (args) …).

> +                  (service-extension account-service-type
> +                                     (const %docker-accounts))))
> +                (default-value (docker-configuration))))

Please add a ‘description’ field here, and please remove tabs from the
file.  :-)

Could you consider adding a system test for docker/containerd?  Perhaps
we could go as far as using ‘docker-image’ in (guix scripts pack) to
generate an image and make sure ‘docker load’ works, but maybe that’s
too much work.

Thank you,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.