bug#22039: 'guix system reconfigure' must start/restart/stop services

bug#22039: 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hello!

Currently ‘guix system reconfigure’ doesn’t try to dynamically update
the set of running services, which is a shame.

A simple strategy would be to have it:

  1. Stop and unregister services currently known to dmd that are
     missing in the new configuration.

  2. Load and start (if they have ‘auto-start?’) services that are in
     the new configuration and currently unknown to dmd.

  3. The rest is the most difficult part: dealing with services that
     already exist but that have changed (see below.)

One step towards this has been the fact that each service has its code
in a module of its own (commit fae685b), making it easy to have dmd load
it.

For #3, the difficulty is that we cannot do deco stop/load/start for
core services like udev or file-system-root because stopping these would
effectively halt the system.

However, we can safely restart services that are leaves of the dmd graph
(unless the user explicitly asks not to do it.)  Here’s what it would
mean on my system, which uses ‘%desktop-services’ and a few more:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix)
scheme@(guile-user)> ,use(gnu)
scheme@(guile-user)> ,use(gnu services dmd)
scheme@(guile-user)> (define os (load "/home/ludo/src/configuration/pluto-configuration.scm"))
scheme@(guile-user)> ,use(gnu services)
scheme@(guile-user)> (define dmds (fold-services (operating-system-services os)
						 #:target-type dmd-root-service-type))
scheme@(guile-user)> ,use(gnu services)
scheme@(guile-user)> (length (service-parameters dmds))
$2 = 49
scheme@(guile-user)> (define back-edges (dmd-service-back-edges (service-parameters dmds)))
scheme@(guile-user)> ,use(srfi srfi-1)
scheme@(guile-user)> (map dmd-service-provision
			  (filter (lambda (s)
				    (null? (back-edges s)))
				(service-parameters dmds)))
$3 = ((swap-/dev/sda4) (nscd) (guix-daemon) (console-font-tty6) (console-font-tty5) (console-font-tty4) (console-font-tty3) (console-font-tty2) (console-font-tty1) (ntpd) (elogind) (upower-daemon) (avahi-daemon) (xorg-server) (tor) (ssh-daemon) (bitlbee))
scheme@(guile-user)> (length $3)
$4 = 17
--8<---------------cut here---------------end--------------->8---

17 out of 49 services could be restarted.

As a first step, we could ignore the other services.

As a second step, we could maybe have an ‘upgrade’ action that would
mutate their <service> instance in place, but without actually
restarting them, such that the changes would only take effect on the
next restart.

Roughly, we’d be doing, say:

  deco upgrade udev /gnu/store/…-dmd-udev.scm

where …-dmd-udev.scm is the service file that contains:

  (make <service> #:provides '(udev) …)

The ‘upgrade’ action would ‘set!’ all the fields of the old service
instance to those of the new instance, such that they are ‘equal?’ (but
not ‘eq?’.)  The caveat is that this is not atomic.

Thoughts?

The prerequisite to all this work is to make the dmd RPCs
machine-processable, which is not too much work.

Thanks,
Ludo’.

bug#22039: 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> The prerequisite to all this work is to make the dmd RPCs
> machine-processable, which is not too much work.

Commit 841b009 in dmd does one step in that direction: it’s now possible
to get the status of services as an sexp.

Ludo’.

bug#22039: 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> Currently ‘guix system reconfigure’ doesn’t try to dynamically update
> the set of running services, which is a shame.
>
> A simple strategy would be to have it:
>
>   1. Stop and unregister services currently known to dmd that are
>      missing in the new configuration.
>
>   2. Load and start (if they have ‘auto-start?’) services that are in
>      the new configuration and currently unknown to dmd.
>
>   3. The rest is the most difficult part: dealing with services that
>      already exist but that have changed (see below.)

Commit 240b57f implements #1 and #2.

Ludo’.

bug#22039: 'guix system reconfigure' must start/restart/stop services

[Thompson, David]

On Wed, Feb 3, 2016 at 4:32 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> ludo@gnu.org (Ludovic Courtès) skribis:
>
>> Currently ‘guix system reconfigure’ doesn’t try to dynamically update
>> the set of running services, which is a shame.
>>
>> A simple strategy would be to have it:
>>
>>   1. Stop and unregister services currently known to dmd that are
>>      missing in the new configuration.
>>
>>   2. Load and start (if they have ‘auto-start?’) services that are in
>>      the new configuration and currently unknown to dmd.
>>
>>   3. The rest is the most difficult part: dealing with services that
>>      already exist but that have changed (see below.)
>
> Commit 240b57f implements #1 and #2.

Awesome!  This is very good progress.

- Dave

bug#22039: ‘guix system reconfigure’ does not always load new services

[Ludovic Courtès]

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

Forwarded from
<https://lists.gnu.org/archive/html/guix-devel/2018-01/msg00187.html>.


[-- Attachment #2: Type: message/rfc822, Size: 6740 bytes --]

From: ludo@gnu.org (Ludovic Courtès)
To: guix-devel@gnu.org
Subject: ‘guix system reconfigure’ does not always load new services
Date: Sat, 13 Jan 2018 22:05:07 +0100
Message-ID: <87inc5mpvg.fsf_-_@gnu.org>

Hello,

ng0 <ng0@n0.is> skribis:

> I noticed a `herd restart guix-daemon` after reconfigure wasn't
> enough, I had to reboot to make use of the new guix. Could this
> be improved?

Yes, that’s a problem (Andreas also reported it to me a couple of days
ago.)

The problem is that ‘guix system reconfigure’ reloads and restarts
services that were not currently running.  For other services, such as
guix-daemon, it does nothing—it does not even load the new version.

This is because the Shepherd currently doesn’t offer a way to “hot-swap”
services without stopping them.  Instead it first unloads the service,
loads the new one, and (optionally) starts it.  See (gnu services herd).

So what we would need is to somehow overwrite the current service.  The
problem is that it may be incorrect, for instance because we’d like to
use the ‘stop’ action of the previous service when we eventually stop
it, rather than the new ‘stop’ action.  So perhaps we should register
the new service so that it replaces the old one only when the old one is
stopped.

Thoughts?

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

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

When the next release of the Shepherd is made (including commit 
9ec5c0000e9a45441417a6ee4138cdcbf1b1f2b2) we should have the 
capability to resolve this ticket.

Attached is my proposed patch from the Guix side. I have tested it 
on my machine by grafting the Shepherd with the appropriate patch 
and it seems to work as expected.

I tested it by changing the substitute-urls in my guix-daemon 
configuration. The output of `ps aux | grep guix-daemon` after 
`guix system reconfigure` showed the substitute-urls were 
unchanged. After `herd restart guix-daemon` the updated 
substitute-urls appeared in `ps aux | grep guix-daemon`. I did not 
need to reboot my system.

One possible improvement would be to print out the services that 
need to be restarted to be upgraded.


[-- Attachment #1.2: 0001-gnu-services-Load-all-services-on-reconfigure-not-ju.patch --]
[-- Type: text/x-patch, Size: 2391 bytes --]

From 162bd298563201ebf6eda87d46ae1b64671397da Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Sun, 26 Aug 2018 21:54:14 +1000
Subject: [PATCH] gnu: services: Load all services on reconfigure, not just
 stopped ones

* gnu/services/shepherd.scm (shepherd-service-upgrade): Remove checks for
running services.
---
 gnu/services/shepherd.scm | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index 4cd224984..efeb82c86 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2018 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -338,20 +339,6 @@ needs to be loaded."
     (shepherd-service-lookup-procedure target
                                        shepherd-service-provision))
 
-  (define lookup-live
-    (shepherd-service-lookup-procedure live
-                                       live-service-provision))
-
-  (define (running? service)
-    (and=> (lookup-live (shepherd-service-canonical-name service))
-           live-service-running))
-
-  (define (stopped service)
-    (match (lookup-live (shepherd-service-canonical-name service))
-      (#f #f)
-      (service (and (not (live-service-running service))
-                    service))))
-
   (define live-service-dependents
     (shepherd-service-back-edges live
                                  #:provision live-service-provision
@@ -363,14 +350,12 @@ needs to be loaded."
       (_  #f)))
 
   (define to-load
-    ;; Only load services that are either new or currently stopped.
-    (remove running? target))
+    ;; Load all of the new services.
+    target)
 
   (define to-unload
-    ;; Unload services that are (1) no longer required, or (2) are in TO-LOAD.
-    (remove essential?
-            (append (filter obsolete? live)
-                    (filter-map stopped to-load))))
+    ;; Unload services that are no longer required.
+    (remove essential? (filter obsolete? live)))
 
   (values to-unload to-load))
 
-- 
2.18.0


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

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi Carlo,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> When the next release of the Shepherd is made (including commit
> 9ec5c0000e9a45441417a6ee4138cdcbf1b1f2b2) we should have the
> capability to resolve this ticket.

Woohoo!

I’d like to make sure we understand the story with ‘EINTR-safe’, but
after that I’m happy to push a release.

> Attached is my proposed patch from the Guix side. I have tested it on
> my machine by grafting the Shepherd with the appropriate patch and it
> seems to work as expected.
>
> I tested it by changing the substitute-urls in my guix-daemon
> configuration. The output of `ps aux | grep guix-daemon` after `guix
> system reconfigure` showed the substitute-urls were unchanged. After
> `herd restart guix-daemon` the updated substitute-urls appeared in `ps
> aux | grep guix-daemon`. I did not need to reboot my system.

Perfect.

> One possible improvement would be to print out the services that need
> to be restarted to be upgraded.

Yes, that’d be nice.

> From 162bd298563201ebf6eda87d46ae1b64671397da Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Sun, 26 Aug 2018 21:54:14 +1000
> Subject: [PATCH] gnu: services: Load all services on reconfigure, not just
>  stopped ones
>
> * gnu/services/shepherd.scm (shepherd-service-upgrade): Remove checks for
> running services.

Could you adjust the manual, where it currently says “if a service is
currently running, it does not attempt to upgrade it”?

Other than that LGTM!

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

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

Hey Ludo’,

On Sat, Sep 01 2018, Ludovic Courtès wrote:
> I’d like to make sure we understand the story with ‘EINTR-safe’, 
> but after that I’m happy to push a release.

Do you have any thoughts about why it could be failing, or things 
I could investigate? I don't know where to start.

>> One possible improvement would be to print out the services 
>> that need to be restarted to be upgraded.
>
> Yes, that’d be nice.

I have done this, but now it seems a bit overwhelming how many 
services would need to be manually restarted. My modified code 
writes a message like this:

To complete the upgrade, restart the following services:
    file-systems
    user-file-systems
    file-system-/boot/efi
    file-system-/dev/pts
    file-system-/dev/shm
    file-system-/gnu/store
    file-system-/run/systemd
    file-system-/run/user
    file-system-/sys/fs/cgroup/elogind
    file-system-/sys/fs/cgroup
    file-system-/sys/fs/cgroup/cpuset
    file-system-/sys/fs/cgroup/cpu
    file-system-/sys/fs/cgroup/cpuacct
    file-system-/sys/fs/cgroup/memory
    file-system-/sys/fs/cgroup/devices
    file-system-/sys/fs/cgroup/freezer
    file-system-/sys/fs/cgroup/blkio
    file-system-/sys/fs/cgroup/perf_event
    root-file-system
    user-processes
    host-name
    udev
    nscd
    guix-daemon
    urandom-seed
    syslogd
    loopback
    term-tty6
    term-tty5
    term-tty4
    term-tty3
    term-tty2
    term-tty1
    console-font-tty1
    console-font-tty2
    console-font-tty3
    console-font-tty4
    console-font-tty5
    console-font-tty6
    virtual-terminal
    ntpd
    dbus-system
    elogind
    upower-daemon
    avahi-daemon
    wpa-supplicant
    networking
    xorg-server
    cups

The same list is printed every time on my system, because the 
diffing is only on the level of the canonical-name. Most of these 
services are being "replaced" by services that are exactly the 
same, so they don't really need to be restarted. I don't really 
know what to do about this, Even if it were fixed, on an actual 
upgrade I assume many of these services would be different, and 
thus would be printed legitimately.

I'm also confused why some of these things are services (like 
host-name).

I'll send through an updated patch once I've cleaned it up a bit, 
but I'm not as positive about it as I was initially.

Carlo

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

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

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

On Sat, Sep 01 2018, Carlo Zancanaro wrote:
> I'll send through an updated patch once I've cleaned it up a 
> bit, [ ... ]

Updated patch attached.


[-- Attachment #1.2: 0001-gnu-services-Load-all-services-on-reconfigure-not-ju.patch --]
[-- Type: text/x-patch, Size: 7125 bytes --]

From 47647b767930d16ab5a3b855993daf6ddf98f230 Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Sun, 26 Aug 2018 21:54:14 +1000
Subject: [PATCH] gnu: services: Load all services on reconfigure, not just
 stopped ones

* doc/guix.texi (Invoking guix system): Document the new behaviour.
* gnu/services/shepherd.scm (shepherd-service-upgrade): Return a list of
  services that need to be restarted to complete their upgrade.
* guix/scripts/system.scm (call-with-service-upgrade-info): Rename an internal
  variable to reflect the change to shepherd-service-upgrade.
  (upgrade-shepherd-services): Print the names of services that need to be
  restarted in order to be upgraded.
---
 doc/guix.texi             |  8 ++++----
 gnu/services/shepherd.scm | 23 ++++++++---------------
 guix/scripts/system.scm   | 20 ++++++++++++--------
 3 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index d2d278df4..421762122 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33,7 +33,7 @@ Copyright @copyright{} 2016 Alex ter Weele@*
 Copyright @copyright{} 2017, 2018 Clément Lassieur@*
 Copyright @copyright{} 2017 Mathieu Othacehe@*
 Copyright @copyright{} 2017 Federico Beffa@*
-Copyright @copyright{} 2017 Carlo Zancanaro@*
+Copyright @copyright{} 2017, 2018 Carlo Zancanaro@*
 Copyright @copyright{} 2017 Thomas Danckaert@*
 Copyright @copyright{} 2017 humanitiesNerd@*
 Copyright @copyright{} 2017 Christopher Allan Webber@*
@@ -21143,9 +21143,9 @@ systems already running GuixSD.}.
 This effects all the configuration specified in @var{file}: user
 accounts, system services, global package list, setuid programs, etc.
 The command starts system services specified in @var{file} that are not
-currently running; if a service is currently running, it does not
-attempt to upgrade it since this would not be possible without stopping it
-first.
+currently running; if a service is currently running this command will
+arrange for it to be upgraded the next time it is stopped (eg. by
+@code{herd stop X} or @code{herd restart X}).
 
 This command creates a new generation whose number is one greater than
 the current generation (as reported by @command{guix system
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index 4cd224984..4c7e72049 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2018 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -329,7 +330,7 @@ symbols provided/required by a service."
 (define (shepherd-service-upgrade live target)
   "Return two values: the subset of LIVE (a list of <live-service>) that needs
 to be unloaded, and the subset of TARGET (a list of <shepherd-service>) that
-needs to be loaded."
+need to be restarted to complete their upgrade."
   (define (essential? service)
     (memq (first (live-service-provision service))
           '(root shepherd)))
@@ -346,12 +347,6 @@ needs to be loaded."
     (and=> (lookup-live (shepherd-service-canonical-name service))
            live-service-running))
 
-  (define (stopped service)
-    (match (lookup-live (shepherd-service-canonical-name service))
-      (#f #f)
-      (service (and (not (live-service-running service))
-                    service))))
-
   (define live-service-dependents
     (shepherd-service-back-edges live
                                  #:provision live-service-provision
@@ -362,16 +357,14 @@ needs to be loaded."
       (#f (every obsolete? (live-service-dependents service)))
       (_  #f)))
 
-  (define to-load
-    ;; Only load services that are either new or currently stopped.
-    (remove running? target))
+  (define to-restart
+    ;; Restart services that are currently running.
+    (filter running? target))
 
   (define to-unload
-    ;; Unload services that are (1) no longer required, or (2) are in TO-LOAD.
-    (remove essential?
-            (append (filter obsolete? live)
-                    (filter-map stopped to-load))))
+    ;; Unload services that are no longer required.
+    (remove essential? (filter obsolete? live)))
 
-  (values to-unload to-load))
+  (values to-unload to-restart))
 
 ;;; shepherd.scm ends here
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 69bd05b51..41a348a5b 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -310,9 +310,9 @@ names of services to load (upgrade), and the list of names of services to
 unload."
   (match (current-services)
     ((services ...)
-     (let-values (((to-unload to-load)
+     (let-values (((to-unload to-restart)
                    (shepherd-service-upgrade services new-services)))
-       (mproc to-load
+       (mproc to-restart
               (map (compose first live-service-provision)
                    to-unload))))
     (#f
@@ -335,21 +335,25 @@ bring the system down."
   ;; Arrange to simply emit a warning if the service upgrade fails.
   (with-shepherd-error-handling
    (call-with-service-upgrade-info new-services
-     (lambda (to-load to-unload)
+     (lambda (to-restart to-unload)
         (for-each (lambda (unload)
                     (info (G_ "unloading service '~a'...~%") unload)
                     (unload-service unload))
                   to-unload)
 
         (with-monad %store-monad
-          (munless (null? to-load)
-            (let ((to-load-names  (map shepherd-service-canonical-name to-load))
-                  (to-start       (filter shepherd-service-auto-start? to-load)))
-              (info (G_ "loading new services:~{ ~a~}...~%") to-load-names)
+          (munless (null? new-services)
+            (let ((new-service-names  (map shepherd-service-canonical-name new-services))
+                  (to-restart-names   (map shepherd-service-canonical-name to-restart))
+                  (to-start           (filter shepherd-service-auto-start? new-services)))
+              (info (G_ "loading new services:~{ ~a~}...~%") new-service-names)
+              (unless (null? to-restart-names)
+                (format #t (G_ "To complete the upgrade, restart the following services:~%"))
+                (for-each (cut format #t "~4t~a~%" <>) to-restart-names))
               (mlet %store-monad ((files (mapm %store-monad
                                                (compose lower-object
                                                         shepherd-service-file)
-                                               to-load)))
+                                               new-services)))
                 ;; Here we assume that FILES are exactly those that were computed
                 ;; as part of the derivation that built OS, which is normally the
                 ;; case.
-- 
2.18.0


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

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Heya,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Sat, Sep 01 2018, Ludovic Courtès wrote:
>> I’d like to make sure we understand the story with ‘EINTR-safe’, but
>> after that I’m happy to push a release.
>
> Do you have any thoughts about why it could be failing, or things I
> could investigate? I don't know where to start.

First, could you check (in a VM) whether the boot failure is
reproducible when that patch that removes ‘EINTR-safe’ is applied?

If it’s 100% reproducible, could you share the VM’s output?

I don’t know what the problem might be but hopefully that’ll give us a
starting point.

> I have done this, but now it seems a bit overwhelming how many
> services would need to be manually restarted. My modified code writes
> a message like this:

[...]

> The same list is printed every time on my system, because the diffing
> is only on the level of the canonical-name. Most of these services are
> being "replaced" by services that are exactly the same, so they don't
> really need to be restarted. I don't really know what to do about
> this, Even if it were fixed, on an actual upgrade I assume many of
> these services would be different, and thus would be printed
> legitimately.

Indeed.  In addition, some low-level services such as file system mounts
cannot be restarted without rebooting, so it’s not useful to mention
them.  Perhaps we should simply print (1) the list of services that were
restarted, and (2) a message saying that users should explicitly run
“herd restart SERVICE” to upgrade other services.

WDYT?

> I'm also confused why some of these things are services (like
> host-name).

‘host-name’ could (should?) be an activation snippet.

Thank you!

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

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


On Sun, Sep 02 2018, Ludovic Courtès wrote:
> First, could you check (in a VM) whether the boot failure is 
> reproducible when that patch that removes ‘EINTR-safe’ is 
> applied?

As far as I can tell it's completely reproducible.

> If it’s 100% reproducible, could you share the VM’s output?

Sure. It's attached.


[-- Attachment #1.2: vm-output --]
[-- Type: application/octet-stream, Size: 42762 bytes --]

[-- Attachment #1.3: Type: text/plain, Size: 802 bytes --]


> Indeed.  In addition, some low-level services such as file 
> system mounts cannot be restarted without rebooting, so it’s not 
> useful to mention them.  Perhaps we should simply print (1) the 
> list of services that were restarted, and (2) a message saying 
> that users should explicitly run “herd restart SERVICE” to 
> upgrade other services.
>
> WDYT?

If there are services that must never be restarted, then maybe we 
don't want to indiscriminately print out a message to restart 
everything. We need some way to mark services that must not be 
restarted. If that's the case, then we might as well just 
automatically restart the services that we can rather than 
printing a message saying to do so. What do we gain by adding an 
extra step to that process?

Carlo

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

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi Carlo,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> [   18.924085] shepherd[1]: Service root-file-system has been started.
> [   18.932361] shepherd[1]: 
> [   18.939972] shepherd[1]: Service user-file-systems has been started.
> [   18.947889] shepherd[1]: 
> [   18.989611] shepherd[1]: waiting for udevd...
> [   19.001396] shepherd[1]: 
> [   19.229174] udevd[267]: starting version 3.2.5
> failed to start service 'file-systems'
> could not create '/dev/autofs': File exists
> could not create '/dev/fuse': File exists
> could not create '/dev/cuse': File exists
> [   19.525763] udevd[267]: starting eudev-3.2.5

[...]

> [   19.553794] udevd[267]: no sender credentials received, message ignored
> failed to start service 'file-system-/dev/pts'

[...]

> [   19.633995] udevd[267]: no sender credentials received, message ignored
> failed to start service 'file-system-/dev/shm'

[...]

> [   19.741025] udevd[267]: no sender credentials received, message ignored
> failed to start service 'user-processes'
> [   19.773968] shepherd[1]: Service host-name has been started.
> [   19.784495] udevd[268]: starting version 3.2.5
> [   19.797674] shepherd[1]: 
> could not create '/dev/autofs': File exists
> could not create '/dev/fuse': File exists
> [   19.846310] udevd[269]: starting version 3.2.5

It looks as if udev failed to start initially, hence the subsequent
“failed to start 'file-system-*'” messages, but then we appear to have
several competing udevd processes, as if (exec-command (list udevd)) had
been executed multiple times.  Hmm not sure what’s going on…

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi Carlo,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Sun, Sep 02 2018, Ludovic Courtès wrote:
>> First, could you check (in a VM) whether the boot failure is
>> reproducible when that patch that removes ‘EINTR-safe’ is applied?
>
> As far as I can tell it's completely reproducible.

Commit c4ba8c79db0aa4ba3517acc82ebafe16105fbb97 reinstates the commit
and removes the leftover #:replace, which was responsible for the
problem: in the context of the ‘start’ method of udev, ‘system*’ was
unbound, to ‘start’ would throw an exception and shepherd would call it
again (thinking udev had failed to start), indefinitely.

If there’s nothing left to add to Shepherd, we can release 0.5.0 within
a few days and then commit the Guix side of this change.

WDYT?

Thanks,
Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

Hey Ludo’,

On Thu, Sep 20 2018, Ludovic Courtès wrote:
> Commit c4ba8c79db0aa4ba3517acc82ebafe16105fbb97 reinstates the 
> commit and removes the leftover #:replace, which was responsible 
> for the problem: ...

That's great! I didn't even know about the #:replace option, so 
I'm glad you were able to find it.

> If there’s nothing left to add to Shepherd, we can release 0.5.0 
> within a few days and then commit the Guix side of this change.

This seems like the sort of thing that shouldn't have been this 
tricky. Is the exception printed somewhere? If not, then I think 
we should print the exception, or at least some information, when 
a service fails to load.

Carlo

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Thu, Sep 20 2018, Ludovic Courtès wrote:
>> Commit c4ba8c79db0aa4ba3517acc82ebafe16105fbb97 reinstates the
>> commit and removes the leftover #:replace, which was responsible for
>> the problem: ...
>
> That's great! I didn't even know about the #:replace option, so I'm
> glad you were able to find it.
>
>> If there’s nothing left to add to Shepherd, we can release 0.5.0
>> within a few days and then commit the Guix side of this change.
>
> This seems like the sort of thing that shouldn't have been this
> tricky. Is the exception printed somewhere? If not, then I think we
> should print the exception, or at least some information, when a
> service fails to load.

I agree.  Note that ‘herd start foo’ prints at least a one-line message
showing the exception when that happens.  The problem here is that
failure happens when ‘start’ is called from the shepherd config file.
At that point there’s no client connected and syslogd either around
either, so presumably messages go to /dev/kmsg and/or the console.

I wouldn’t consider it a blocker for 0.5.0 though.  WDYT?

Thanks,
Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

> [...] so presumably messages go to /dev/kmsg and/or the console.

I don't remember seeing anything about the exception in any of the 
output that I looked at. I'm a bit confused about where different 
bits of output go, so I'll take a look at how output is handled in 
a few weeks, when the rest of life settles down a bit.

> I wouldn’t consider it a blocker for 0.5.0 though.  WDYT?

Yeah, I agree. We should try to improve it, but as long as we 
haven't made things worse (which we haven't) then it shouldn't 
block a release.

We still need to work out what we want to do on the Guix side once 
the Shepherd is released. Do we want to restart services that we 
can, or print a message telling users how to do so? Maybe 
individual services should be able to specify their preference?

Carlo

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> We still need to work out what we want to do on the Guix side once the
> Shepherd is released. Do we want to restart services that we can, or
> print a message telling users how to do so? Maybe individual services
> should be able to specify their preference?

I would reload and restart services currently stopped (what ‘guix system
reconfigure’ currently does), and replace all the other services.  This
is what the patch you sent at <https://issues.guix.info/issue/22039#24>
does.

AIUI the only remaining issue is whether/how to print hints about
services that need to be manually restarted.  In
<https://issues.guix.info/issue/22039#36> I wrote:

> Perhaps we should simply print (1) the list of services that were
> restarted, and (2) a message saying that users should explicitly run
> “herd restart SERVICE” to upgrade other services.

To which you replied:

> If there are services that must never be restarted, then maybe we 
> don't want to indiscriminately print out a message to restart 
> everything. We need some way to mark services that must not be 
> restarted. If that's the case, then we might as well just 
> automatically restart the services that we can rather than 
> printing a message saying to do so. What do we gain by adding an 
> extra step to that process?

From the POV of the Shepherd, services carry no semantics.  The Shepherd
cannot guess that restarting ‘udev’ or ‘file-system-xyz’ is impractical
(try it :-)).  Leaf services like ‘ssh-daemon’ can generally be
restarted, but whether or not now is a good time to do it is something
only the user can decide.  That’s why the only services which are safe
to restart right away are those currently stopped (and those that can be
hot-swapped like nginx.)

Thus I think it’s reasonable to print a message along the lines of:

  The following services were upgraded: …
  Please run “herd restart SERVICE” to stop, upgrade, and restart
  services that were not automatically upgraded.

WDYT?

Thanks,
Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

Hey Ludo’,

> From the POV of the Shepherd, services carry no semantics.

In Guix we have as much information as possible about the 
services. We should be know which services should be upgraded 
automatically, which ones we should prompt the user to upgrade, 
and which ones are never safe to upgrade. Maybe we could add a 
"restart-strategy" to the shepherd-service object?

> Thus I think it’s reasonable to print a message along the lines 
> of:
>
>   The following services were upgraded: …
>   Please run “herd restart SERVICE” to stop, upgrade, and 
>   restart services that were not automatically upgraded.
>
> WDYT?

The main reasons I'm not super happy with this are that it's not 
discoverable (which is bad for new users), and it requires 
interaction (so cannot be an unattended upgrade). In particular 
for discoverability, some of our services don't take advantage of 
the Shepherd's ability to have multiple "provision" values. For 
instance, I just have to know that to restart wicd I have to run 
"herd restart networking".

Maybe this should be a separate ticket. Replacing the services and 
printing a generic message will still be an improvement on what 
Guix currently does, and I don't want to hold that up just because 
I think we can do better.

Carlo

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hello,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

>> From the POV of the Shepherd, services carry no semantics.
>
> In Guix we have as much information as possible about the services. We
> should be know which services should be upgraded automatically, which
> ones we should prompt the user to upgrade, and which ones are never
> safe to upgrade. Maybe we could add a "restart-strategy" to the
> shepherd-service object?

What would you put there?  Do you have concrete examples?

Note that FHS distros don’t do better: either the service is
hot-replaceable (nginx; I don’t know of any other) or can at least
reload its config (sshd, etc.), and then it’s dynamically upgraded, or
it’ll be upgraded next time you restart it.

That’s because fundamentally only the user can tell whether now is a
good time to restart, say, sshd.

In Debian, “apt-get dist-upgrade” opens a dialog box asking the user
whether services can be restarted right away, IIRC.

>> Thus I think it’s reasonable to print a message along the lines of:
>>
>>   The following services were upgraded: …
>>   Please run “herd restart SERVICE” to stop, upgrade, and   restart
>> services that were not automatically upgraded.
>>
>> WDYT?
>
> The main reasons I'm not super happy with this are that it's not
> discoverable (which is bad for new users), and it requires interaction
> (so cannot be an unattended upgrade).

I agree, but I don’t think full unattended upgrades exist out there.
I’m not saying this is good, but rather that this is hard and beyond
the scope of this patch.

> In particular for discoverability, some of our services don't take
> advantage of the Shepherd's ability to have multiple "provision"
> values. For instance, I just have to know that to restart wicd I have
> to run "herd restart networking".

There’s ‘guix system search’ that provides this kind of info (see
<https://issues.guix.info/issue/29707>), but I agree we could do better.

> Maybe this should be a separate ticket. Replacing the services and
> printing a generic message will still be an improvement on what Guix
> currently does, and I don't want to hold that up just because I think
> we can do better.

Yes, I think this should be a separate ticket.  We can go with your
patch and a message along the lines of what we discussed above, and then
work on the improvements you mentioned, one at a time.  That way we’ll
have the warm feeling of having achieved something, even if there’s more
to come.  :-)

Thank you!

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Efraim Flashner]

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

On Fri, Sep 21, 2018 at 01:58:03PM +0200, Ludovic Courtès wrote:
> Hello,
> 
> Carlo Zancanaro <carlo@zancanaro.id.au> skribis:
> 
> >> From the POV of the Shepherd, services carry no semantics.
> >
> > In Guix we have as much information as possible about the services. We
> > should be know which services should be upgraded automatically, which
> > ones we should prompt the user to upgrade, and which ones are never
> > safe to upgrade. Maybe we could add a "restart-strategy" to the
> > shepherd-service object?
> 
> What would you put there?  Do you have concrete examples?

Restart/reload/whatever unless explicitly disabled?

> 
> Note that FHS distros don’t do better: either the service is
> hot-replaceable (nginx; I don’t know of any other) or can at least
> reload its config (sshd, etc.), and then it’s dynamically upgraded, or
> it’ll be upgraded next time you restart it.
> 
> That’s because fundamentally only the user can tell whether now is a
> good time to restart, say, sshd.

Not exactly the point, but Debian regularly restarts sshd for me on a
remote box (somehow) without me losing the connection.

> 
> In Debian, “apt-get dist-upgrade” opens a dialog box asking the user
> whether services can be restarted right away, IIRC.
> 
> >> Thus I think it’s reasonable to print a message along the lines of:
> >>
> >>   The following services were upgraded: …
> >>   Please run “herd restart SERVICE” to stop, upgrade, and   restart
> >> services that were not automatically upgraded.
> >>
> >> WDYT?
> 

This sounds like a really good idea, especially if we limit to ones that
are less likely to cause problems if restarted (like filesystems). We
still have to figure out something for databases and upgrading them from
one version to another.


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Fri, Sep 21, 2018 at 01:58:03PM +0200, Ludovic Courtès wrote:

[...]

>> Note that FHS distros don’t do better: either the service is
>> hot-replaceable (nginx; I don’t know of any other) or can at least
>> reload its config (sshd, etc.), and then it’s dynamically upgraded, or
>> it’ll be upgraded next time you restart it.
>> 
>> That’s because fundamentally only the user can tell whether now is a
>> good time to restart, say, sshd.
>
> Not exactly the point, but Debian regularly restarts sshd for me on a
> remote box (somehow) without me losing the connection.

Good point!  I think sshd opens child processes for new sessions, and
thanks to that it falls into the category of service that can be
hot-replaced.

For hot-swappable daemons, I think we should provide a specific ‘reload’
or ‘upgrade’ action as was discussed at
<https://issues.guix.info/issue/26830>.  That way, to figure out the
right strategy, we would just check whether the service supports that
action.

Thanks,
Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

Hey Ludo’,

On Fri, Sep 21 2018, Ludovic Courtès wrote:
> What would you put there?  Do you have concrete examples?

I would have three possible values: 'never, 'always, 'manual.

'never would mean that the service should never be restarted. This 
is for things like udev, or the filesystems, which should never be 
restarted on a running system.

'always would mean that the service is always safe to restart. I 
don't immediately know what services would fit in this category 
(maybe sshd, given Efraim's comment; maybe ntpd? I'm sure there 
are others). Things like nginx will probably not fall into this 
category, because they involve some downtime when restarting. 
Reloading their configuration (via a "reload" action, or similar) 
is not enough because the binary and/or libraries might have 
changed (and, in the worst case, might have an incompatible 
configuration format, although I would expect that to be 
exceedingly rare).

'manual would mean that the service should be restarted, but it 
need to be done at an appropriate time. This should prompt the 
user with the names of the services, and we should provide an 
option to guix system reconfigure to restart these services as 
part of the reconfigure. We could call the option 
"--restart-services".

>> [ ... ] I just have to know that to restart wicd I have to run 
>> "herd restart networking".
>
> There’s ‘guix system search’ that provides this kind of info 
> (see <https://issues.guix.info/issue/29707>), but I agree we 
> could do better.

I actually checked this before sending my previous message, but I 
didn't see that it includes "shepherdnames". I tested with "guix 
system search wicd" which didn't show any, but I see now that 
searching "guix system search xmpp" does helpfully show how to 
restart the service.

> We can go with your patch and a message along the lines of what 
> we discussed above, and then work on the improvements you 
> mentioned, one at a time.  That way we’ll have the warm feeling 
> of having achieved something, even if there’s more to come.  :-)

I won't be able to look at writing the code for this for a few 
weeks, but hopefully I'll get to it around mid- to late-October.

Carlo

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hi Carlo,

Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

> On Fri, Sep 21 2018, Ludovic Courtès wrote:
>> What would you put there?  Do you have concrete examples?
>
> I would have three possible values: 'never, 'always, 'manual.
>
> 'never would mean that the service should never be restarted. This is
> for things like udev, or the filesystems, which should never be
> restarted on a running system.
>
> 'always would mean that the service is always safe to restart. I don't
> immediately know what services would fit in this category (maybe sshd,
> given Efraim's comment; maybe ntpd? I'm sure there are others). Things
> like nginx will probably not fall into this category, because they
> involve some downtime when restarting. Reloading their configuration
> (via a "reload" action, or similar) is not enough because the binary
> and/or libraries might have changed (and, in the worst case, might
> have an incompatible configuration format, although I would expect
> that to be exceedingly rare).
>
> 'manual would mean that the service should be restarted, but it need
> to be done at an appropriate time. This should prompt the user with
> the names of the services, and we should provide an option to guix
> system reconfigure to restart these services as part of the
> reconfigure. We could call the option "--restart-services".

OK, I see.

>> We can go with your patch and a message along the lines of what we
>> discussed above, and then work on the improvements you mentioned,
>> one at a time.  That way we’ll have the warm feeling of having
>> achieved something, even if there’s more to come.  :-)
>
> I won't be able to look at writing the code for this for a few weeks,
> but hopefully I'll get to it around mid- to late-October.

If that’s fine with you, I can apply the patch you initially posted so
we can start taking advantage of it (I’d like to push a Guix release by
the end of October.)  WDYT?

Thanks!

Ludo’.

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Carlo Zancanaro]

Hey Ludo’,

On Mon, Sep 24 2018, Ludovic Courtès wrote:
> If that’s fine with you, I can apply the patch you initially 
> posted so we can start taking advantage of it (I’d like to push 
> a Guix release by the end of October.)  WDYT?

That sounds good to me!

Thanks for your patience through this. It's taken a bit of time 
for my ideas to fully form, but I think it's coming together.

Carlo

bug#22039: [PATCH] 'guix system reconfigure' must start/restart/stop services

[Ludovic Courtès]

Hello!

I went ahead and pushed the patch as
4245ddcbc9f935804c17c97872b90ec1050c2d75.

One modification I had to make and which I hadn’t though of before is
the new ‘load-services/safe’ procedure I added: it makes sure it DTRT
when talking to shepherd < 0.15.0.

I’ve reconfigured from master, and so far so good!  :-)

I’m closing this issue.  I suggest opening new ones for specific
improvements we discussed.

Thank you!

Ludo’.