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

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.
---
 gnu/installer/final.scm      | 125 +++++++++++++++++------------------
 gnu/installer/newt/final.scm |   7 --
 gnu/services/base.scm        |  60 ++++++++++-------
 3 files changed, 99 insertions(+), 93 deletions(-)

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’.

> 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.

I took the lazy path because it's just one file that keeps reasonably
small. Adding an extra overlay for /var/guix/db would make
sense here.

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

Yes I can't say I'm satisfied with all of this but I'm trying different
angles for this problem since months, with no proper outcome.

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

That's the current approach but it breaks very ofter because kmscon,
udev or any other processes that can't be killed, opens an overlay file.
I'd really like to avoid relying on this kind of solution.

> 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’.

Yes I tried that at first but there's a catch. While running guix-daemon
in it's own mount namespace, it won't 'see' the mounted file-systems
such as /mnt.

So that would mean that we would have to do spawn a containerized
process that would:

* Join guix-daemon mnt namespace
* Call "with-mounted-partitions"
* Mount the cow-store
* Run 'guix system init'

In this is end it still seem overly complex, but I can give it another
try. WDYT?

Thanks a lot for reviewing this!

Mathieu

Hi Mathieu!

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> 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.
>
> I took the lazy path because it's just one file that keeps reasonably
> small. Adding an extra overlay for /var/guix/db would make
> sense here.

Yeah, no big deal.

>> Hmm, that seems quite complex, and it’s not great that we have to tweak
>> guix-daemon-service “just” for this.
>
> Yes I can't say I'm satisfied with all of this but I'm trying different
> angles for this problem since months, with no proper outcome.

Yeah…  (I must say I really appreciate your commitment tackling hard
problems like this one, kudos!)

>> Is there a way we can identify processes that have open overlay files,
>> so we could terminate them?
>
> That's the current approach but it breaks very ofter because kmscon,
> udev or any other processes that can't be killed, opens an overlay file.
> I'd really like to avoid relying on this kind of solution.

OK, makes sense!

>> 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’.
>
> Yes I tried that at first but there's a catch. While running guix-daemon
> in it's own mount namespace, it won't 'see' the mounted file-systems
> such as /mnt.
>
> So that would mean that we would have to do spawn a containerized
> process that would:
>
> * Join guix-daemon mnt namespace
> * Call "with-mounted-partitions"
> * Mount the cow-store
> * Run 'guix system init'
>
> In this is end it still seem overly complex, but I can give it another
> try. WDYT?

It does seem complex indeed.

So perhaps we can settle on the solution you sent, but let’s see if we
can move complexity out of sight.  For example, if we can arrange to
have a ‘fork+exec-command/container’ procedure that can be passed the
PID of a namespace, such that the ‘start’ method of guix-daemon is just
a few more lines its current definition, I’ll be happy.

How does that sound?

Thank you!

Ludo’.

Hey Ludo,

> So perhaps we can settle on the solution you sent, but let’s see if we
> can move complexity out of sight.  For example, if we can arrange to
> have a ‘fork+exec-command/container’ procedure that can be passed the
> PID of a namespace, such that the ‘start’ method of guix-daemon is just
> a few more lines its current definition, I’ll be happy.
>
> How does that sound?

Sounds fine! I added a "fork+exec-command/container" in (gnu build
shepherd) module, that uses "container-excursion" to enter the
namespaces of the process passed as argument.

I also took your other remarks into account and pushed this
serie. Thanks a lot for diving into this harsh stuff :).

Locally, the installer tests behave fine, but I'll monitor the CI to see
how it goes.

Now, the only installation test failure I'm aware of is
https://issues.guix.gnu.org/41948. The good news, is that I have a Guile
patch that seem to solve it[1].

Thanks,

Mathieu

[1]: https://lists.gnu.org/archive/html/bug-guile/2020-08/msg00023.html

Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> So perhaps we can settle on the solution you sent, but let’s see if we
>> can move complexity out of sight.  For example, if we can arrange to
>> have a ‘fork+exec-command/container’ procedure that can be passed the
>> PID of a namespace, such that the ‘start’ method of guix-daemon is just
>> a few more lines its current definition, I’ll be happy.
>>
>> How does that sound?
>
> Sounds fine! I added a "fork+exec-command/container" in (gnu build
> shepherd) module, that uses "container-excursion" to enter the
> namespaces of the process passed as argument.
>
> I also took your other remarks into account and pushed this
> serie. Thanks a lot for diving into this harsh stuff :).

Awesome!

> Locally, the installer tests behave fine, but I'll monitor the CI to see
> how it goes.
>
> Now, the only installation test failure I'm aware of is
> https://issues.guix.gnu.org/41948. The good news, is that I have a Guile
> patch that seem to solve it[1].

Yes, it’s still on my radar (started looking into it the other day,
without any tangible result).

Ludo’.

Hey,

Mathieu Othacehe <othacehe@gnu.org> skribis:

> Sounds fine! I added a "fork+exec-command/container" in (gnu build
> shepherd) module, that uses "container-excursion" to enter the
> namespaces of the process passed as argument.

There’s a catch that I just discovered as I reconfigured berlin:

--8<---------------cut here---------------start------------->8---
root@berlin ~/maintenance/hydra# herd restart guix-daemon
Service cuirass-web has been stopped. 
Service cuirass has been stopped.
Service guix-publish has been stopped.
Service guix-daemon has been stopped. 
herd: exception caught while executing 'start' on service 'guix-daemon':
Unbound variable: fork+exec-command/container
root@berlin ~/maintenance/hydra# herd restart guix-daemon
Service guix-daemon is not running.   
herd: exception caught while executing 'start' on service 'guix-daemon':
Unbound variable: fork+exec-command/container
--8<---------------cut here---------------end--------------->8---

The workaround I found, which works nicely, is to run:

  herd eval root "(reload-module (resolve-module '(gnu build shepherd)))"

and then:

  herd restart guix-daemon
  herd restart guix-publish
  …

Not sure if there’s anything we can or should do.  It’s probably not too
common to restart guix-daemon after an upgrade, though.

Ludo’.