[bug#33893] [PATCH v5 3/4] services: Add docker.

["Ludovic Courtès" <ludo@gnu.org>]

Danny Milosavljevic <dannym@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’.