[bug#36555] [PATCH 1/2] guix system: Add 'reconfigure' module.

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

Hello!

zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis:

> * guix/scripts/system/reconfigure.scm: New file.
> * Makefile.am (MODULES): Add it.
> * guix/scripts/system.scm (bootloader-installer-script): Export variable.
> * gnu/machine/ssh.scm (switch-to-system, upgrade-shepherd-services)
> (install-bootloader): Delete variable.
> * gnu/machine/ssh.scm (deploy-managed-host): Rewrite procedure.

[...]

> +  (define (run-switch-to-system machine)
> +    "Monadic procedure serializing the items in MACHINE necessary to build a
> +G-Expression with 'switch-to-system'."
> +    (mlet %store-monad ((script (switch-system-program (machine-system machine))))
> +        (machine-remote-eval machine #~(primitive-load #$script))))
> +
> +  (define (run-upgrade-shepherd-services machine)
> +    "Monadic procedure serializing the items in MACHINE necessary to build a
> +G-Expression with 'upgrade-shepherd-services'."
> +    (mlet* %store-monad ((target-services target-services)
> +                         (script (upgrade-services-program target-services)))
> +      (machine-remote-eval machine #~(primitive-load #$script))))

These would look nicer if ‘switch-system-program’ and
‘upgrade-services-program’ returns a <program-file> because you could
just write:

  (machine-remote-eval #~(primitive-load #$(switch-system-program …))
                       machine)

(I realize the order of arguments is reversed; to stick to what ‘eval’
does, I’d tend to put the ‘machine’ argument second—but that’s a
separate issue.  :-))

> +(define (switch-system-program os)
> +  "Return as a monadic value a derivation to build a scheme file that, upon
> +being evaluated, will create a new generation for SYSTEM-DERIVATION and
> +execute ACTIVATION-SCRIPT."
> +  (gexp->script
> +   "switch-to-system.scm"
> +   (with-extensions (list guile-gcrypt)
> +     (with-imported-modules (source-module-closure '((guix config)
> +                                                     (guix profiles)
> +                                                     (guix utils)))
> +       #~(begin
> +           (use-modules (guix config)
> +                        (guix profiles)
> +                        (guix utils))
> +
> +           (define %system-profile
> +             (string-append %state-directory "/profiles/system"))
> +
> +           (let* ((number (1+ (generation-number %system-profile)))
> +                  (generation (generation-file-name %system-profile number)))
> +             (switch-symlinks generation #$os)
> +             (switch-symlinks %system-profile generation)
> +             (setenv "GUIX_NEW_SYSTEM" #$os)
> +             (with-output-to-string
> +               (lambda ()
> +                 (primitive-load
> +                  #$(operating-system-activation-script os))))))))))

Can we remove ‘with-output-to-string’?  I’d rather see what’s going on.
:-)

If that’s too verbose, we can use ‘invoke/quiet’.

> +;; XXX: Currently, this does NOT attempt to restart running services. See
> +;; <https://issues.guix.info/issue/33508> for details.
> +(define (upgrade-services-program target-services)
> +  "Return as a monadic value a derivation to build a scheme file that, upon
> +being evaluated, will use TARGET-SERVICES, a list
> +of (shepherd-service-canonical-name, shepherd-service-file) pairs to determine
> +which services are obsolete and need to be unloaded, as well as which services
> +are new and need to be started."
> +  (gexp->script
> +   "upgrade-shepherd-services.scm"
> +   (with-imported-modules '((gnu services herd))
> +    #~(begin
> +        (use-modules (gnu services herd)
> +                     (srfi srfi-1))
> +
> +        (define running
> +          (filter live-service-running (current-services)))
> +
> +        (define (essential? service)
> +          ;; Return #t if SERVICE is essential and should not be unloaded
> +          ;; under any circumstance.
> +          (memq (first (live-service-provision service))
> +                '(root shepherd)))
> +
> +        (define (obsolete? service)
> +          ;; Return #t if SERVICE can be safely unloaded.
> +          (and (not (essential? service))
> +               (every (lambda (requirements)
> +                        (not (memq (first (live-service-provision service))
> +                                   requirements)))
> +                      (map live-service-requirement running))))
> +
> +        (define to-unload
> +          (filter obsolete?
> +                  (remove (lambda (service)
> +                            (memq (first (live-service-provision service))
> +                                  (map first '#$target-services)))
> +                          running)))
> +
> +        (define to-start
> +          (remove (lambda (service-pair)
> +                    (memq (first service-pair)
> +                          (map (compose first live-service-provision)
> +                               running)))
> +                  '#$target-services))
> +
> +        ;; Unload obsolete services.
> +        (for-each (lambda (service)
> +                    (false-if-exception
> +                     (unload-service service)))
> +                  to-unload)
> +
> +        ;; Load the service files for any new services and start them.
> +        (load-services/safe (map second to-start))
> +        (for-each start-service (map first to-start))))))

It seems that this sort-of inlines parts of ‘shepherd-service-upgrade’
but without traversing the service dependency graph to determine the
compilete set of obsolete services, no?  I feel that we should be
reusing ‘shepherd-service-upgrade’ or similar bits.  (I realize this is
already in ‘master’ for ‘guix deploy’, but since this is going to be
shared with ‘guix system’, we’d rather be extra cautious.)

Also, I think we should remove ‘false-if-exception’ around
‘unload-service’.

> +(define (install-bootloader-program installer-script bootcfg bootcfg-file target)
> +  "Return as a monadic value a derivation to build a scheme file that, upon
> +being evaluated, will install BOOTCFG to BOOTCFG-FILE, a target path, on
> +TARGET, a mount point, and subsequently run INSTALLER-SCRIPT."
> +  (gexp->script
> +   "install-bootloader.scm"
> +   (with-extensions (list guile-gcrypt)
> +     (with-imported-modules (source-module-closure '((gnu build install)
> +                                                     (guix store)
> +                                                     (guix utils)))
> +       #~(begin
> +           (use-modules (gnu build install)
> +                        (guix store)
> +                        (guix utils))
> +           (let* ((gc-root (string-append "/" %gc-roots-directory "/bootcfg"))
> +                  (temp-gc-root (string-append gc-root ".new")))
> +
> +             (switch-symlinks temp-gc-root gc-root)
> +
> +             (let ((installer-result
> +                    (false-if-exception
> +                     (begin
> +                       (install-boot-config #$bootcfg #$bootcfg-file #$target)
> +                       (with-output-to-string
> +                         (lambda ()
> +                           (primitive-load #$installer-script)))))))
> +               (unless installer-result
> +                 (delete-file temp-gc-root)
> +                 (error "failed to install bootloader"))
> +               (rename-file temp-gc-root gc-root)
> +               installer-result)))))))

I’d rather not swallow stdout and not use ‘error’.  Or at least, code
that runs ‘install-bootloader-program’ should be able to produce a
meaningful (and i18n’d) error message.  So the caller could do something
like:

  (define result
    (machine-eval #~(…
                     (guard (c ((message-condition? c)
                                (cons 'error (condition-message c))))
                       (invoke/quiet #$(install-bootloader-program …))
                       '(success)))
                  machine))

  (match result
    (('error message)
     (leave (G_ "failed to install bootloader:~%~a~%") message))
    (('success)
     #t))

Does that make sense?

That’s quite some boilerplate to the challenge will be to factorize it.

Ultimately, the code in (guix scripts system reconfigure) should be
parameterized by an evaluation procedure that would be either
‘machine-eval’ or some hypothetical ‘local-eval’ procedure to evaluate
things locally.

Thanks,
Ludo’.