| Message ID | 20190313093610.1071-3-arunisaac@systemreboot.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Support container network sharing | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | success | Successfully applied |
| cbaines/applying patch | success | Successfully applied |
Hello! Some comments below. Arun Isaac <arunisaac@systemreboot.net> skribis: > * gnu/services.scm (shared-network-service-type): New variable. > * gnu/services/base.scm (%base-services): Add shared-network-service. > * gnu/system.scm (essential-services): If shared-network-service exists, > extend it to add /etc/services, /etc/nsswitch.conf and /etc/hosts. > (operating-system-etc-service): Do not add /etc/services, /etc/nsswitch.conf > and /etc/hosts. > * gnu/system/linux-container.scm (container-script): Support returning a > container script that shares the host network. > * guix/scripts/system.scm (system-derivation-for-action, perform-action): Add > #:container-shared-network? argument. > (show-help): Add "-N, --network" help information. > (%options): Add network option. > (process-action): Call perform-action with #:container-shared-network?. > > Co-authored-by: Christopher Baines <mail@cbaines.net> [...] > +(define shared-network-service-type > + (service-type (name 'shared-network) > + (extensions (list (service-extension etc-service-type identity))) > + (compose concatenate) > + (extend append) > + (default-value '()))) I’d encourage you to add a ‘description’ field as well. :-) > --- a/gnu/system.scm > +++ b/gnu/system.scm > @@ -5,6 +5,7 @@ > ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com> > ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> > ;;; Copyright © 2019 Meiyo Peng <meiyo.peng@gmail.com> > +;;; Copyright © 2019 Arun Isaac <arunisaac@systemreboot.net> > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -501,7 +502,21 @@ a container or that of a \"bare metal\" system." > (list %containerized-shepherd-service) > (list %linux-bare-metal-service > (service firmware-service-type > - (operating-system-firmware os)))))))) > + (operating-system-firmware os)))) > + (if (find (lambda (service) > + (eq? (service-type-name (service-kind service)) > + 'shared-network)) > + (operating-system-user-services os)) > + (let ((nsswitch (plain-file "nsswitch.conf" > + (name-service-switch->string > + (operating-system-name-service-switch os))))) > + (list (simple-service 'shared-network-extension > + shared-network-service-type > + `(("services" ,(file-append net-base "/etc/services")) > + ("nsswitch.conf" ,#~#$nsswitch) > + ("hosts" ,#~#$(or (operating-system-hosts-file os) > + (default-/etc/hosts (operating-system-host-name os)))))))) > + (list)))))) A couple of things: 1. ‘service-type-name’ exists for debugging purposes, and I think we shouldn’t rely on it at all in our code. Instead, we should compare service types by identity, as in: (eq? (service-kind service) foo-service-type) 2. The notion of “shared network” is very much a container (or VM) thing, so somehow it still doesn’t feel right to me that (gnu system) has to be aware of these special cases. I think the ‘host-database-service-type’ wouldn’t have this problem, but maybe it has other issues. I guess this needs more experimentation, sorry for not coming up with clearer ideas! Ludo’.
>> +(define shared-network-service-type >> + (service-type (name 'shared-network) >> + (extensions (list (service-extension etc-service-type identity))) >> + (compose concatenate) >> + (extend append) >> + (default-value '()))) > > I’d encourage you to add a ‘description’ field as well. :-) Sure, will do. > 1. ‘service-type-name’ exists for debugging purposes, and I think we > shouldn’t rely on it at all in our code. Instead, we should > compare service types by identity, as in: > > (eq? (service-kind service) foo-service-type) Sure, will do. > 2. The notion of “shared network” is very much a container (or VM) > thing, so somehow it still doesn’t feel right to me that (gnu > system) has to be aware of these special cases. > > I think the ‘host-database-service-type’ wouldn’t have this problem, but > maybe it has other issues. I guess this needs more experimentation, > sorry for not coming up with clearer ideas! If these services (the shared-network service, the hosts-database service or indeed any other service) had access to the operating-system object `os', then they would be able to operate independently without having to be extended by `essential-services'. Is this possible somehow? Is it a good idea to give services access to the os fields?
Hi, Arun Isaac <arunisaac@systemreboot.net> skribis: >> 2. The notion of “shared network” is very much a container (or VM) >> thing, so somehow it still doesn’t feel right to me that (gnu >> system) has to be aware of these special cases. >> >> I think the ‘host-database-service-type’ wouldn’t have this problem, but >> maybe it has other issues. I guess this needs more experimentation, >> sorry for not coming up with clearer ideas! > > If these services (the shared-network service, the hosts-database > service or indeed any other service) had access to the operating-system > object `os', then they would be able to operate independently without > having to be extended by `essential-services'. Is this possible somehow? > Is it a good idea to give services access to the os fields? It’s not easily possible, and I think it would be a bad idea: if every service has access to every ‘operating-system’ field, that gives you more flexibility, but it’s also much harder to reason about what happens, compared to the current extension graph (the NixOS “module” system works like that: every service can access every bit of the whole configuration, but IMO that makes it quite hard to understand.) What could be useful is “self-referential” records, where a field can refer to the record it belongs do. So we’d do: (define-record-type* <operating-system> ;; … (services operating-system-services (self-referential? #t) (default essential-services))) whereby ‘essential-services’ would be passed the <operating-system> record somehow. That needs more thought… Thanks, Ludo’.
> It’s not easily possible, and I think it would be a bad idea: if every > service has access to every ‘operating-system’ field, that gives you > more flexibility, but it’s also much harder to reason about what > happens, compared to the current extension graph (the NixOS “module” > system works like that: every service can access every bit of the whole > configuration, but IMO that makes it quite hard to understand.) OK, I understand. Just out of curiosity: Why do we have special operating-system fields like host-name, hosts-file, etc. instead of just having services like host-name-service-type, hosts-file-service-type, etc.? Doesn't giving special status to these operating-system fields complicate things? For example, if we only had a hosts-file-service-type instead of a hosts-file operating-system field, we wouldn't have the problem that /etc/hosts could only be created from within essential-services. > What could be useful is “self-referential” records, where a field can > refer to the record it belongs do. So we’d do: > > (define-record-type* <operating-system> > ;; … > (services operating-system-services > (self-referential? #t) (default essential-services))) > > whereby ‘essential-services’ would be passed the <operating-system> > record somehow. > > That needs more thought… OK, I'll wait. Thanks!
Hi Arun & Chris, Arun Isaac <arunisaac@systemreboot.net> skribis: >> It’s not easily possible, and I think it would be a bad idea: if every >> service has access to every ‘operating-system’ field, that gives you >> more flexibility, but it’s also much harder to reason about what >> happens, compared to the current extension graph (the NixOS “module” >> system works like that: every service can access every bit of the whole >> configuration, but IMO that makes it quite hard to understand.) > > OK, I understand. Just out of curiosity: Why do we have special > operating-system fields like host-name, hosts-file, etc. instead of just > having services like host-name-service-type, hosts-file-service-type, > etc.? Doesn't giving special status to these operating-system fields > complicate things? For example, if we only had a hosts-file-service-type > instead of a hosts-file operating-system field, we wouldn't have the > problem that /etc/hosts could only be created from within > essential-services. You’re right, to some extent those fields complicate things (most of them were here before the service infrastructure, though.) OTOH I find it convenient to have a high-level view of the OS. >> What could be useful is “self-referential” records, where a field can >> refer to the record it belongs do. So we’d do: >> >> (define-record-type* <operating-system> >> ;; … >> (services operating-system-services >> (self-referential? #t) (default essential-services))) >> >> whereby ‘essential-services’ would be passed the <operating-system> >> record somehow. >> >> That needs more thought… > > OK, I'll wait. I didn’t mean to block you though because it was just an idea without code… but in the meantime I’ve sent code to <https://issues.guix.info/issue/34948>. It turned out to be easier than I thought! Ludo’.
> I didn’t mean to block you though because it was just an idea without > code… but in the meantime I’ve sent code to > <https://issues.guix.info/issue/34948>. It turned out to be easier than > I thought! It's not that you were blocking me. I was just at my wit's end about what to do. :-P
diff --git a/gnu/services.scm b/gnu/services.scm index f151bbaa9d..316b22eabb 100644 --- a/gnu/services.scm +++ b/gnu/services.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2015, 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com> +;;; Copyright © 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; ;;; This file is part of GNU Guix. ;;; @@ -95,6 +96,7 @@ profile-service-type firmware-service-type gc-root-service-type + shared-network-service-type %boot-service %activation-service @@ -651,6 +653,13 @@ as Wifi cards."))) "Register garbage-collector roots---i.e., store items that will not be reclaimed by the garbage collector."))) +(define shared-network-service-type + (service-type (name 'shared-network) + (extensions (list (service-extension etc-service-type identity))) + (compose concatenate) + (extend append) + (default-value '()))) + ;;; ;;; Service folding. diff --git a/gnu/services/base.scm b/gnu/services/base.scm index 67df4d1379..5f806fab35 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -2373,6 +2373,8 @@ to handle." (service special-files-service-type `(("/bin/sh" ,(file-append (canonical-package bash) - "/bin/sh")))))) + "/bin/sh")))) + + (service shared-network-service-type))) ;;; base.scm ends here diff --git a/gnu/system.scm b/gnu/system.scm index e6c86cb9ba..22f7e5d55d 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -5,6 +5,7 @@ ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com> ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> ;;; Copyright © 2019 Meiyo Peng <meiyo.peng@gmail.com> +;;; Copyright © 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; ;;; This file is part of GNU Guix. ;;; @@ -501,7 +502,21 @@ a container or that of a \"bare metal\" system." (list %containerized-shepherd-service) (list %linux-bare-metal-service (service firmware-service-type - (operating-system-firmware os)))))))) + (operating-system-firmware os)))) + (if (find (lambda (service) + (eq? (service-type-name (service-kind service)) + 'shared-network)) + (operating-system-user-services os)) + (let ((nsswitch (plain-file "nsswitch.conf" + (name-service-switch->string + (operating-system-name-service-switch os))))) + (list (simple-service 'shared-network-extension + shared-network-service-type + `(("services" ,(file-append net-base "/etc/services")) + ("nsswitch.conf" ,#~#$nsswitch) + ("hosts" ,#~#$(or (operating-system-hosts-file os) + (default-/etc/hosts (operating-system-host-name os)))))))) + (list)))))) (define* (operating-system-services os #:key container?) "Return all the services of OS, including \"internal\" services that do not @@ -592,10 +607,6 @@ directory." "/run/current-system/profile/sbin\n"))) (issue (plain-file "issue" (operating-system-issue os))) - (nsswitch (plain-file "nsswitch.conf" - (name-service-switch->string - (operating-system-name-service-switch os)))) - ;; Startup file for POSIX-compliant login shells, which set system-wide ;; environment variables. (profile (mixed-text-file "profile" "\ @@ -679,16 +690,12 @@ then source /run/current-system/profile/etc/profile.d/bash_completion.sh fi\n"))) (etc-service - `(("services" ,(file-append net-base "/etc/services")) - ("protocols" ,(file-append net-base "/etc/protocols")) + `(("protocols" ,(file-append net-base "/etc/protocols")) ("rpc" ,(file-append net-base "/etc/rpc")) ("login.defs" ,#~#$login.defs) ("issue" ,#~#$issue) - ("nsswitch.conf" ,#~#$nsswitch) ("profile" ,#~#$profile) ("bashrc" ,#~#$bashrc) - ("hosts" ,#~#$(or (operating-system-hosts-file os) - (default-/etc/hosts (operating-system-host-name os)))) ;; Write the operating-system-host-name to /etc/hostname to prevent ;; NetworkManager from changing the system's hostname when connecting ;; to certain networks. Some discussion at diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm index bceea41332..485623f563 100644 --- a/gnu/system/linux-container.scm +++ b/gnu/system/linux-container.scm @@ -1,6 +1,8 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2015 David Thompson <davet@gnu.org> ;;; Copyright © 2016, 2017 Ludovic Courtès <ludo@gnu.org> +;;; Copyright © 2019 Christopher Baines <mail@cbaines.net> +;;; Copyright © 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; ;;; This file is part of GNU Guix. ;;; @@ -60,11 +62,26 @@ containerized OS." %container-file-systems user-file-systems)))) -(define* (container-script os #:key (mappings '())) +(define* (container-script os #:key (mappings '()) container-shared-network?) "Return a derivation of a script that runs OS as a Linux container. MAPPINGS is a list of <file-system> objects that specify the files/directories that will be shared with the host system." - (let* ((os (containerized-operating-system os mappings)) + (let* ((os (containerized-operating-system + (operating-system + (inherit os) + (services (if container-shared-network? + (remove (lambda (service) + (case (service-type-name (service-kind service)) + ((nscd shared-network) #t) + (else #f))) + (operating-system-user-services os)) + (operating-system-user-services os)))) + (append + mappings + (if container-shared-network? + (cons %nscd-socket-mapping + %network-file-mappings) + '())))) (file-systems (filter file-system-needed-for-boot? (operating-system-file-systems os))) (specs (map file-system->spec file-systems))) @@ -93,6 +110,9 @@ that will be shared with the host system." ;; users and groups, which is sufficient for most cases. ;; ;; See: http://www.freedesktop.org/software/systemd/man/systemd-nspawn.html#--private-users= - #:host-uids 65536)))) + #:host-uids 65536 + #:namespaces (if #$container-shared-network? + (delq 'net %namespaces) + %namespaces))))) (gexp->script "run-container" script)))) diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm index d67b9f8185..c2fb1ebed5 100644 --- a/guix/scripts/system.scm +++ b/guix/scripts/system.scm @@ -4,6 +4,7 @@ ;;; Copyright © 2016, 2017, 2018 Chris Marusich <cmmarusich@gmail.com> ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com> ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net> +;;; Copyright © 2019 Christopher Baines <mail@cbaines.net> ;;; ;;; This file is part of GNU Guix. ;;; @@ -756,13 +757,16 @@ checking this by themselves in their 'check' procedure." (define* (system-derivation-for-action os action #:key image-size file-system-type - full-boot? mappings) + full-boot? mappings + container-shared-network?) "Return as a monadic value the derivation for OS according to ACTION." (case action ((build init reconfigure) (operating-system-derivation os)) ((container) - (container-script os #:mappings mappings)) + (container-script os + #:mappings mappings + #:container-shared-network? container-shared-network?)) ((vm-image) (system-qemu-image os #:disk-image-size image-size)) ((vm) @@ -817,6 +821,7 @@ and TARGET arguments." dry-run? derivations-only? use-substitutes? bootloader-target target image-size file-system-type full-boot? + container-shared-network? (mappings '()) (gc-root #f)) "Perform ACTION for OS. INSTALL-BOOTLOADER? specifies whether to install @@ -825,6 +830,8 @@ target root directory; IMAGE-SIZE is the size of the image to be built, for the 'vm-image' and 'disk-image' actions. The root file system is created as a FILE-SYSTEM-TYPE file system. FULL-BOOT? is used for the 'vm' action; it determines whether to boot directly to the kernel or to the bootloader. +CONTAINER-SHARED-NETWORK? determines if the container will use a separate +network namespace. When DERIVATIONS-ONLY? is true, print the derivation file name(s) without building anything. @@ -870,11 +877,13 @@ static checks." (check-initrd-modules os))) (mlet* %store-monad - ((sys (system-derivation-for-action os action - #:file-system-type file-system-type - #:image-size image-size - #:full-boot? full-boot? - #:mappings mappings)) + ((sys (system-derivation-for-action + os action + #:file-system-type file-system-type + #:image-size image-size + #:full-boot? full-boot? + #:container-shared-network? container-shared-network? + #:mappings mappings)) ;; For 'init' and 'reconfigure', always build BOOTCFG, even if ;; --no-bootloader is passed, because we then use it as a GC root. @@ -1011,6 +1020,8 @@ Some ACTIONS support additional ARGS.\n")) (display (G_ " --share=SPEC for 'vm', share host file system according to SPEC")) (display (G_ " + -N, --network for 'container', allow containers to access the network")) + (display (G_ " -r, --root=FILE for 'vm', 'vm-image', 'disk-image', 'container', and 'build', make FILE a symlink to the result, and register it as a garbage collector root")) @@ -1057,6 +1068,9 @@ Some ACTIONS support additional ARGS.\n")) (lambda (opt name arg result) (alist-cons 'image-size (size->number arg) result))) + (option '(#\N "network") #f #f + (lambda (opt name arg result) + (alist-cons 'container-shared-network? #t result))) (option '("no-bootloader" "no-grub") #f #f (lambda (opt name arg result) (alist-cons 'install-bootloader? #f result))) @@ -1173,6 +1187,8 @@ resulting from command-line parsing." #:file-system-type (assoc-ref opts 'file-system-type) #:image-size (assoc-ref opts 'image-size) #:full-boot? (assoc-ref opts 'full-boot?) + #:container-shared-network? + (assoc-ref opts 'container-shared-network?) #:mappings (filter-map (match-lambda (('file-system-mapping . m) m)