[bug#35895] linux-container: Remove networking service whennetwork is shared with host.

* gnu/system/linux-container.scm (dummy-networking-shepherd-service): New
procedure.
(dummy-networking-service-type): New variable.
(containerized-operating-system): If network is shared with host, replace
static-networking-service-type with dummy-networking-service-type.
---
 gnu/system/linux-container.scm | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Arun Isaac <arunisaac@systemreboot.net> writes:

> * gnu/system/linux-container.scm (dummy-networking-shepherd-service): New
> procedure.
> (dummy-networking-service-type): New variable.
> (containerized-operating-system): If network is shared with host, replace
> static-networking-service-type with dummy-networking-service-type.

Sounds good. It would be good to have the motivation/reasoning behind
this change in the commit message though.

> ---
>  gnu/system/linux-container.scm | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
> index c1e963d047..ee2a476e4c 100644
> --- a/gnu/system/linux-container.scm
> +++ b/gnu/system/linux-container.scm
> @@ -30,6 +30,7 @@
>    #:use-module (gnu build linux-container)
>    #:use-module (gnu services)
>    #:use-module (gnu services base)
> +  #:use-module (gnu services shepherd)
>    #:use-module (gnu system)
>    #:use-module (gnu system file-systems)
>    #:export (system-container
> @@ -65,6 +66,22 @@ from OS that are needed on the bare metal and not in a container."
>                           files)))
>              base)))
>  
> +(define (dummy-networking-shepherd-service _)
> +  (shepherd-service
> +   (documentation "Provide loopback and networking without actually doing
> +anything.")
> +   (provision '(loopback networking))
> +   (start #~(const #t))))
> +
> +(define dummy-networking-service-type
> +  (service-type
> +   (name 'dummy-networking)
> +   (extensions
> +    (list (service-extension
> +           shepherd-root-service-type
> +           (compose list dummy-networking-shepherd-service))))
> +   (default-value #f)))
> +

Something like this seems a little neater to me:

  (define dummy-networking-service-type
    (service-type
     (name 'dummy-networking)
     (extensions
      (list (service-extension
             shepherd-root-service-type
             (const
              (list
               (shepherd-service
                (documentation
                 "Provide loopback and networking without actually doing anything.")
                (provision '(loopback networking))
                (start #~(const #t))))))))
     (default-value #f)))

Just becasue const is being used. Although maybe the shepherd-service
itself could do with being extracted to a variable.

>  (define* (containerized-operating-system os mappings
>                                           #:key
>                                           shared-network?
> @@ -96,7 +113,8 @@ containerized OS.  EXTRA-FILE-SYSTEMS is a list of file systems to add to OS."
>                    agetty-service-type)
>              ;; Remove nscd service if network is shared with the host.
>              (if shared-network?
> -                (list nscd-service-type)
> +                (list nscd-service-type
> +                      static-networking-service-type)
>                  (list))))
>  
>    (operating-system
> @@ -105,10 +123,14 @@ containerized OS.  EXTRA-FILE-SYSTEMS is a list of file systems to add to OS."
>      (essential-services (container-essential-services
>                           this-operating-system
>                           #:shared-network? shared-network?))
> -    (services (remove (lambda (service)
> -                        (memq (service-kind service)
> -                              useless-services))
> -                      (operating-system-user-services os)))
> +    (services (append
> +               (remove (lambda (service)
> +                         (memq (service-kind service)
> +                               useless-services))
> +                       (operating-system-user-services os))
> +               (if shared-network?
> +                   (list (service dummy-networking-service-type))
> +                   (list))))
>      (file-systems (append (map mapping->fs
>                                 (if shared-network?
>                                     (append %network-file-mappings mappings)

On Sat, 25 May 2019 13:37:51 +0100
Christopher Baines <mail@cbaines.net> wrote:

> Arun Isaac <arunisaac@systemreboot.net> writes:
> 
> > * gnu/system/linux-container.scm (dummy-networking-shepherd-service): New
> > procedure.
> > (dummy-networking-service-type): New variable.
> > (containerized-operating-system): If network is shared with host, replace
> > static-networking-service-type with dummy-networking-service-type.  
> 
> Sounds good. It would be good to have the motivation/reasoning behind
> this change in the commit message though.

IMO in a comment, not in a commit message :)

Let's not make commit messages the documentation--except when it's impossible
to document otherwise.

In this case it's pretty clear what the form in containerized-operating-system
does, but yeah, maybe a comment like the following:

;; Many Guix services (which?) depend on a 'networking' shepherd service, so
;; make sure to provide a dummy 'networking' service when we are sure that
;; networking is already set up in the host and can be used.
;; That prevents double-setup.