[bug#35895] [PATCH 0/1] linux-container: Remove networking service when network is shared with host.

[bug#35895] [PATCH 0/1] linux-container: Remove networking service when network is shared with host.

[Arun Isaac]

When the container network is shared with the host, the loopback and
networking services fail. This causes other services, like nginx, which depend
on the loopback and networking services to also fail. Hence, when the network
is to be shared with the host, we must replace the
static-networking-service-type with a new dummy-networking-service-type that
does nothing but simply provide loopback and networking.

Arun Isaac (1):
  linux-container: Remove networking service when network is shared with
    host.

 gnu/system/linux-container.scm | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

-- 
2.21.0

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

[Arun Isaac]

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

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

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

[Christopher Baines]

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


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)


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

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

[Danny Milosavljevic]

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

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.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Arun Isaac]

[-- Attachment #1.1: Type: text/plain, Size: 282 bytes --]


Thank you both for the review. I have made the suggested changes. Please
find attached an updated patch.

In the future, please address me in the Cc or To fields so that I take
note sooner. I found your mails only a couple of days ago while going
through all my unread mails. :-(


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-linux-container-Remove-networking-service-when-netwo.patch --]
[-- Type: text/x-patch, Size: 3732 bytes --]

From a7b795d9af3347330b48470d3988d43b8038c2c1 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Sat, 25 May 2019 11:49:42 +0530
Subject: [PATCH] linux-container: Remove networking service when network 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 | 36 +++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
index c1e963d047..95b56b6f4f 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
+           (const (list dummy-networking-shepherd-service)))))
+   (default-value #f)))
+
 (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,18 @@ 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))
+               ;; Many Guix services 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.
+               (if shared-network?
+                   (list (service dummy-networking-service-type))
+                   (list))))
     (file-systems (append (map mapping->fs
                                (if shared-network?
                                    (append %network-file-mappings mappings)
-- 
2.21.0


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

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

[Ludovic Courtès]

Hello,

Arun Isaac <arunisaac@systemreboot.net> skribis:

> From a7b795d9af3347330b48470d3988d43b8038c2c1 Mon Sep 17 00:00:00 2001
> From: Arun Isaac <arunisaac@systemreboot.net>
> Date: Sat, 25 May 2019 11:49:42 +0530
> Subject: [PATCH] linux-container: Remove networking service when network 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.

[...]

> +(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
> +           (const (list dummy-networking-shepherd-service)))))
> +   (default-value #f)))

You can use ‘shepherd-service-type’ as a shorthand for these two
definitions.

> +    (services (append
> +               (remove (lambda (service)
> +                         (memq (service-kind service)
> +                               useless-services))
> +                       (operating-system-user-services os))
> +               ;; Many Guix services 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.
> +               (if shared-network?
> +                   (list (service dummy-networking-service-type))
> +                   (list))))

I’m really nitpicking here, but I like to have the first argument to
‘append’ on the same line, and I also visually recognize '() more easily
than (list).

Anyway, LGTM!

Thanks,
Ludo’.

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

[Arun Isaac]

[-- Attachment #1.1: Type: text/plain, Size: 74 bytes --]


I've made the suggested changes. Please find attached an updated patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-linux-container-Remove-networking-service-when-netwo.patch --]
[-- Type: text/x-patch, Size: 3595 bytes --]

From 4b50e35e3d2b8adea1e496e0336d23f35d0c9def Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Sat, 25 May 2019 11:49:42 +0530
Subject: [PATCH] linux-container: Remove networking service when network 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 | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
index c1e963d047..61248c62b9 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,16 @@ from OS that are needed on the bare metal and not in a container."
                          files)))
             base)))
 
+(define dummy-networking-service-type
+  (shepherd-service-type
+   'dummy-networking
+   (const (shepherd-service
+           (documentation "Provide loopback and networking without actually
+doing anything.")
+           (provision '(loopback networking))
+           (start #~(const #t))))
+   #f))
+
 (define* (containerized-operating-system os mappings
                                          #:key
                                          shared-network?
@@ -96,7 +107,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 +117,17 @@ 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))
+                      ;; Many Guix services 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.
+                      (if shared-network?
+                          (list (service dummy-networking-service-type))
+                          '())))
     (file-systems (append (map mapping->fs
                                (if shared-network?
                                    (append %network-file-mappings mappings)
-- 
2.22.0


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

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

[Ludovic Courtès]

Hi,

Arun Isaac <arunisaac@systemreboot.net> skribis:

> From 4b50e35e3d2b8adea1e496e0336d23f35d0c9def Mon Sep 17 00:00:00 2001
> From: Arun Isaac <arunisaac@systemreboot.net>
> Date: Sat, 25 May 2019 11:49:42 +0530
> Subject: [PATCH] linux-container: Remove networking service when network 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.

LGTM, thanks!

Ludo’.

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

[Arun Isaac]

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


I had forgotten to update the commit message after the last set of
changes. I updated the commit message and pushed. Thanks for the peer
review, everyone! :-)

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