[shepherd] branch master updated: service: Restart replacements rather than original services upon "restart".

[Ludovic Courtès]

This is an automated email from the git hooks/post-receive script.

civodul pushed a commit to branch master
in repository shepherd.

The following commit(s) were added to refs/heads/master by this push:
     new 63af9d7  service: Restart replacements rather than original services 
upon "restart".
63af9d7 is described below

commit 63af9d7c4460b55953bfa199ea44ac0114289b64
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Thu May 25 15:40:32 2023 +0200

    service: Restart replacements rather than original services upon "restart".
    
    Fixes <https://issues.guix.gnu.org/63717>.
    
    * modules/shepherd/service.scm (stop-service): When REPLACEMENT is true,
    add REPLACEMENT rather than SERVICE to the returned list.
    * tests/replacement.sh: Raise an error if the initial 'start' method is
    called more than once.  Use 'herd restart' rather than 'start' followed
    by 'stop'.  Ensure the replacement's 'start' method is called.
    * NEWS: Update.
---
 NEWS                         | 12 ++++++++++++
 modules/shepherd/service.scm |  4 ++--
 tests/replacement.sh         | 21 +++++++++++++++------
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 7fdfdba..516244a 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,18 @@ Please send Shepherd bug reports to bug-guix@gnu.org.
 The ‘endpoint’ procedure takes a new ‘#:bind-attempts’ parameter.  Its default
 value is (default-bind-attempts), itself a new SRFI-39 parameter.
 
+** ‘herd restart SERVICE’ starts the replacement, not the original service
+   <https://issues.guix.gnu.org/63717>
+
+In 0.10.0, when a service had received a replacement (for instance via ‘guix
+system reconfigure’), using ‘herd restart’ would invoke the ‘start’ method of
+the original service while installing its replacement in the registry.
+
+This would lead to an inconsistency where the registry would show the new
+service (typically failing to start) while the original service would still be
+running “in the shadows” (responding to SIGCHLD or to incoming connections,
+and so on).
+
 * Changes in 0.10.0
 
 ** Distinguish ‘starting’ and ‘stopping’ intermediate service statuses
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index e3ca910..340ce67 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -925,9 +925,9 @@ in a list."
         ;; Replace the service with its replacement, if it has one.
         (let ((replacement (service-replacement service)))
           (when replacement
-            (replace-service service replacement)))
+            (replace-service service replacement))
 
-        (cons service stopped-dependents))))
+          (cons (or replacement service) stopped-dependents)))))
 
 (define (perform-service-action service the-action . args)
   "Perform @var{the-action} (a symbol such as @code{'restart} or 
@code{'status})
diff --git a/tests/replacement.sh b/tests/replacement.sh
index c4796e3..31776fe 100644
--- a/tests/replacement.sh
+++ b/tests/replacement.sh
@@ -1,5 +1,5 @@
 # GNU Shepherd --- Ensure replacing services works properly
-# Copyright © 2014, 2016 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2014, 2016, 2023 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2018 Carlo Zancanaro <carlo@zancanaro.id.au>
 #
 # This file is part of the GNU Shepherd.
@@ -29,7 +29,7 @@ pid="t-pid-$$"
 
 herd="herd -s $socket"
 
-trap "rm -f $socket $conf $rconf $stamp $log;
+trap "cat $log || true; rm -f $socket $conf $rconf $stamp $log;
       test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
 
 cat > "$conf"<<EOF
@@ -37,7 +37,13 @@ cat > "$conf"<<EOF
 (register-services
  (list (service
         '(test)
-        #:start (const #t)
+        #:start (let ((started? #f))
+                   (lambda ()
+                     (let ((first-time? (not started?)))
+                      (unless first-time?
+                        (error "Already started once!\n"))
+                       (set! started? #t)
+                      first-time?)))
         #:actions (actions
                    (say-hello (lambda _
                                (call-with-output-file "$stamp"
@@ -62,7 +68,9 @@ cat > "$rconf"<<EOF
 (register-services
  (list (service
         '(test)
-        #:start (const #t)
+        #:start (lambda _
+                   (display "The replacement is starting.\n")
+                   #t)
         #:actions (actions
                    (say-goodbye (lambda _
                                   (call-with-output-file "$stamp"
@@ -84,9 +92,10 @@ if test "`cat $stamp`" != "Hello"; then
     exit 1
 fi
 
-$herd stop test
+$herd restart test
 
-$herd start test
+$herd status test | grep running
+grep "The replacement is starting" "$log"
 
 if $herd say-hello test; then
     echo "say-hello should have failed after stop/start"