[bug#42849] [PATCH 3/3] installer: Run the installation inside a container.

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

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

> When the store overlay is mounted, other processes such as kmscon, udev
> and guix-daemon may open files from the store, preventing the
> underlying install support from being umounted. See:
> https://lists.gnu.org/archive/html/guix-devel/2018-12/msg00161.html.
>
> To avoid this situation, mount the store overlay inside a container,
> and run the installation from within that container.
>
> * gnu/services/base.scm (guix-shepherd-service): Support an optional PID
> argument passed to the "start" method. If that argument is passed, ensure that
> guix-daemon enters the given PID MNT namespace.
> * gnu/installer/final.scm (umount-cow-store): Remove it,
> (install-system): run the installation from within a container.
> * gnu/installer/newt/final.scm (run-install-shell): Remove the display hack.

Smart!

> +    ;; When the store overlay is mounted, other processes such as kmscon, udev
> +    ;; and guix-daemon may open files from the store, preventing the
> +    ;; underlying install support from being umounted. See:
> +    ;; https://lists.gnu.org/archive/html/guix-devel/2018-12/msg00161.html.
> +    ;;
> +    ;; To avoid this situation, mount the store overlay inside a container,
> +    ;; and run the installation from within that container.
> +    (zero?
> +     (call-with-container '()
> +       (lambda ()
> +         (dynamic-wind
> +           (lambda ()
> +             ;; Save the database, so that it can be restored once the
> +             ;; cow-store is umounted.
> +             (copy-file database-file saved-database)
> +             (mount-cow-store (%installer-target-dir) backing-directory))
> +           (lambda ()
> +             ;; We need to drag the guix-daemon to the container MNT
> +             ;; namespace, so that it can operate on the cow-store.
> +             (stop-service 'guix-daemon)
> +             (start-service 'guix-daemon (list (number->string (getpid))))
> +
> +             (setvbuf (current-output-port) 'none)
> +             (setvbuf (current-error-port) 'none)
> +
> +             ;; If there are any connected clients, assume that we are running
> +             ;; installation tests. In that case, dump the standard and error
> +             ;; outputs to syslog.
> +             (set! ret
> +                   (if (not (null? (current-clients)))
> +                       (with-output-to-file "/dev/console"
> +                         (lambda ()
> +                           (with-error-to-file "/dev/console"
> +                             (lambda ()
> +                               (run-command install-command
> +                                            #:locale locale)))))
> +                       (run-command install-command #:locale locale))))
> +           (lambda ()
> +             ;; Restart guix-daemon so that it does no keep the MNT namespace
> +             ;; alive.
> +             (restart-service 'guix-daemon)
> +             (copy-file saved-database database-file)
> +
> +             ;; Finally umount the cow-store and exit the container.
> +             (umount-cow-store (%installer-target-dir) backing-directory)
> +             (assert-exit ret))))

Should ‘mount-cow-store’ also make an overlay for /var/guix/db?  That
way, changes to that directory would go to /mnt/var/guix/db and the
original database would remain unchanged.

> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -1558,36 +1558,50 @@ proxy of 'guix-daemon'...~%")
>             (provision '(guix-daemon))
>             (requirement '(user-processes))
>             (actions (list shepherd-set-http-proxy-action))
> -           (modules '((srfi srfi-1)))
> +           (modules '((srfi srfi-1)
> +                      (ice-9 match)))
>             (start
> -            #~(lambda _
> +            #~(lambda args
>                  (define proxy
>                    ;; HTTP/HTTPS proxy.  The 'http_proxy' variable is set by
>                    ;; the 'set-http-proxy' action.
>                    (or (getenv "http_proxy") #$http-proxy))
>  
>                  (fork+exec-command
> -                 (cons* #$(file-append guix "/bin/guix-daemon")
> -                        "--build-users-group" #$build-group
> -                        "--max-silent-time" #$(number->string max-silent-time)
> -                        "--timeout" #$(number->string timeout)
> -                        "--log-compression" #$(symbol->string log-compression)
> -                        #$@(if use-substitutes?
> -                               '()
> -                               '("--no-substitutes"))
> -                        "--substitute-urls" #$(string-join substitute-urls)
> -                        #$@extra-options
> -
> -                        ;; Add CHROOT-DIRECTORIES and all their dependencies
> -                        ;; (if these are store items) to the chroot.
> -                        (append-map (lambda (file)
> -                                      (append-map (lambda (directory)
> -                                                    (list "--chroot-directory"
> -                                                          directory))
> -                                                  (call-with-input-file file
> -                                                    read)))
> -                                    '#$(map references-file
> -                                            chroot-directories)))
> +                 ;; When running the installer, we need guix-daemon to operate
> +                 ;; from within the same MNT namespace as the installation
> +                 ;; container. In that case only, enter the namespace of the
> +                 ;; process PID passed as start argument.
> +                 (append
> +                  (match args
> +                    ((pid)
> +                     (list #$(file-append util-linux "/bin/nsenter")
> +                           "-t" pid "-m"))

We should use ‘container-excursion’ instead of nsenter.

> +                    (else '()))
> +                  (cons* #$(file-append guix "/bin/guix-daemon")
> +                         "--build-users-group" #$build-group
> +                         "--max-silent-time"
> +                         #$(number->string max-silent-time)
> +                         "--timeout" #$(number->string timeout)
> +                         "--log-compression"
> +                         #$(symbol->string log-compression)
> +                         #$@(if use-substitutes?
> +                                '()
> +                                '("--no-substitutes"))
> +                         "--substitute-urls" #$(string-join substitute-urls)
> +                         #$@extra-options
> +
> +                         ;; Add CHROOT-DIRECTORIES and all their dependencies
> +                         ;; (if these are store items) to the chroot.
> +                         (append-map
> +                          (lambda (file)
> +                            (append-map (lambda (directory)
> +                                          (list "--chroot-directory"
> +                                                directory))
> +                                        (call-with-input-file file
> +                                          read)))
> +                          '#$(map references-file

Hmm, that seems quite complex, and it’s not great that we have to tweak
guix-daemon-service “just” for this.

*scratches head*

Is there a way we can identify processes that have open overlay files,
so we could terminate them?

Alternately, something that might simplify the code would be to always
run guix-daemon in a separate mount namespace.  We could add a
‘fork+exec-command/container’ procedure in (gnu build shepherd) to help
with that.

That way, all we’d need to do is to run ‘guix system init’ in that same
mount namespace, which can be achieved using ‘container-excursion’.

Too bad we can’t use setns for a process other than the calling process.
:-/

Thoughts?

Ludo’.