From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fspw6-0007Be-8F for guix-patches@gnu.org; Thu, 23 Aug 2018 09:46:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fspw2-0006fv-Qo for guix-patches@gnu.org; Thu, 23 Aug 2018 09:46:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:53349) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fspw2-0006fj-LD for guix-patches@gnu.org; Thu, 23 Aug 2018 09:46:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fspw2-0004bS-Fv for guix-patches@gnu.org; Thu, 23 Aug 2018 09:46:02 -0400 Subject: [bug#32408] [PATCH shepherd] Allow replacement of services Resent-Message-ID: References: <87wosz4spx.fsf@zancanaro.id.au> <87h8jobwwp.fsf@gnu.org> <87k1okzqkf.fsf@zancanaro.id.au> <87in44ovzm.fsf@gnu.org> From: Carlo Zancanaro In-reply-to: <87in44ovzm.fsf@gnu.org> Date: Thu, 23 Aug 2018 23:45:05 +1000 Message-ID: <87o9dtqjry.fsf@zancanaro.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 32408@debbugs.gnu.org --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Hey Ludo=E2=80=99, I've attached an updated patch. I couldn't think of any unwanted=20 consequences, so I took your idea of making register-services=20 handle most of the details of replacement. With my patch,=20 something like > herd eval root '(register-services (load "a.scm") (load=20 > "b.scm"))' will deal with a conflict by either replacing the old service (if=20 it's not running), arranging for the old service to be replaced=20 when it's stopped, or raising an error. This seems like a sensible=20 way for things to function. >> At the very least we need to control the inherent race=20 >> condition [...] > > Indeed. Despite my desire to deal with the race condition, I haven't done=20 anything about it in this patch. The modification of %services=20 that was done in register-services was already racy, and I don't=20 think this patch will make it worse. If it hasn't been a problem=20 up until now, then I don't think this will make it a problem. Carlo --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline; filename=0001-service-Add-a-replacement-slot-for-delayed-service-r.patch Content-Transfer-Encoding: quoted-printable From=209ec5c0000e9a45441417a6ee4138cdcbf1b1f2b2 Mon Sep 17 00:00:00 2001 From: Carlo Zancanaro Date: Thu, 9 Aug 2018 22:30:38 +1000 Subject: [PATCH] service: Add a replacement slot for delayed service replacement. * modules/shepherd/service.scm (): Add replacement slot (replace-service): New procedure. (stop): Call replace-service after stopping a service. (register-services): Replace existing services where possible, setting the = new replacement slot if they are currently running. * tests/replacement.sh: Add a test for it. * Makefile.am (TESTS): Add the new test. * doc/shepherd.texi (Slots of services): Document it. =2D-- Makefile.am | 1 + doc/shepherd.texi | 9 +++ modules/shepherd/service.scm | 68 ++++++++++++++++++----- tests/replacement.sh | 105 +++++++++++++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 tests/replacement.sh diff --git a/Makefile.am b/Makefile.am index 8dad006..4322d7f 100644 =2D-- a/Makefile.am +++ b/Makefile.am @@ -184,6 +184,7 @@ SUFFIXES =3D .go =20 TESTS =3D \ tests/basic.sh \ + tests/replacement.sh \ tests/respawn.sh \ tests/respawn-throttling.sh \ tests/misbehaved-client.sh \ diff --git a/doc/shepherd.texi b/doc/shepherd.texi index 7946f8b..1de6d80 100644 =2D-- a/doc/shepherd.texi +++ b/doc/shepherd.texi @@ -708,6 +708,15 @@ handler will not start it again. =20 otherwise @code{#f}. =20 +@item +@vindex replacement (slot of ) +@code{replacement} specifies a service to be used to replace this one +when it is stopped. This service will continue to function normally +until the @code{stop} action is invoked. After the service has been +successfully stopped, its definition will be replaced by the value of +this slot, which must itself be a service. This slot is ignored if +its value is @code{#f}. + @end itemize =20 @c @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm index 5653388..006309c 100644 =2D-- a/modules/shepherd/service.scm +++ b/modules/shepherd/service.scm @@ -205,7 +205,10 @@ respawned, shows that it has been respawned more than = TIMES in SECONDS." (stop-delay? #:init-keyword #:stop-delay? #:init-value #f) ;; The times of the last respawns, most recent first. =2D (last-respawns #:init-form '())) + (last-respawns #:init-form '()) + ;; A replacement for when this service is stopped. + (replacement #:init-keyword #:replacement + #:init-value #f)) =20 (define (service? obj) "Return true if OBJ is a service." @@ -341,6 +344,20 @@ wire." (canonical-name obj))))) (slot-ref obj 'running)) =20 +(define (replace-service old-service new-service) + "Replace OLD-SERVICE with NEW-SERVICE in the services registry. This +completely removes all references to OLD-SERVICE before registering +NEW-SERVICE." + (define (remove-service name) + (let* ((old (hashq-ref %services name)) + (new (delete old-service old))) + (if (null? new) + (hashq-remove! %services name) + (hashq-set! %services name new)))) + (when new-service + (for-each remove-service (provided-by old-service)) + (register-services new-service))) + ;; Stop the service, including services that depend on it. If the ;; latter fails, continue anyway. Return `#f' if it could be stopped. (define-method (stop (obj ) . args) @@ -385,6 +402,11 @@ wire." ;; Reset the list of respawns. (slot-set! obj 'last-respawns '()) =20 + ;; Replace the service with its replacement, if it has one + (let ((replacement (slot-ref obj 'replacement))) + (when replacement + (replace-service obj replacement))) + ;; Status message. (let ((name (canonical-name obj))) (if (running? obj) @@ -1038,25 +1060,41 @@ then disable it." =20 ;; Add NEW-SERVICES to the list of known services. (define (register-services . new-services) + "Add NEW-SERVICES to the list of known services. If a service has alrea= dy +been registered, arrange to have it replaced when it is next stopped. If = it +is currently stopped, replace it immediately." (define (register-single-service new) ;; Sanity-checks first. (assert (list-of-symbols? (provided-by new))) (assert (list-of-symbols? (required-by new))) (assert (boolean? (respawn? new))) =2D ;; Canonical name actually must be canonical. (FIXME: This test =2D ;; is incomplete, since we may add a service later that makes it =2D ;; non-cannonical.) =2D (assert (null? (lookup-services (canonical-name new)))) =2D ;; FIXME: Verify consistency: Check that there are no circular =2D ;; dependencies, check for bogus conflicts/dependencies, whatever =2D ;; else makes sense. =2D =2D ;; Insert into the hash table. =2D (for-each (lambda (name) =2D (let ((old (lookup-services name))) =2D ;; Actually add the new service now. =2D (hashq-set! %services name (cons new old)))) =2D (provided-by new))) + + ;; FIXME: Just because we have a unique canonical name now doesn't mea= n it + ;; will remain unique as other services are added. Whenever a service = is + ;; added it should check that it's not conflicting with any already + ;; registered canonical names. + (match (lookup-services (canonical-name new)) + (() ;; empty, so we can safely add ourselves + (for-each (lambda (name) + (let ((old (lookup-services name))) + (hashq-set! %services name (cons new old)))) + (provided-by new))) + ((old) ;; one service registered, so it may be an old version of us + (cond + ((not (eq? (canonical-name new) (canonical-name old))) + (local-output + "Cannot register service ~a: canonical name is not unique." + (canonical-name new)) + (throw 'non-canonical-name)) + ((running? old) + (slot-set! old 'replacement new)) + (#:else + (replace-service old new)))) + (_ ;; in any other case, there are too many services to register + (local-output + "Cannot register service ~a: canonical name is not unique." + (canonical-name new)) + (throw 'non-canonical-name)))) =20 (for-each register-single-service new-services)) =20 diff --git a/tests/replacement.sh b/tests/replacement.sh new file mode 100644 index 0000000..e06cb93 =2D-- /dev/null +++ b/tests/replacement.sh @@ -0,0 +1,105 @@ +# GNU Shepherd --- Ensure replacing services works properly +# Copyright =C2=A9 2014, 2016 Ludovic Court=C3=A8s +# Copyright =C2=A9 2018 Carlo Zancanaro +# +# This file is part of the GNU Shepherd. +# +# The GNU Shepherd is free software; you can redistribute it and/or modify= it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or (at +# your option) any later version. +# +# The GNU Shepherd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with the GNU Shepherd. If not, see . + +shepherd --version +herd --version + +socket=3D"t-socket-$$" +conf=3D"t-conf-$$" +rconf=3D"t-rconf-$$" +log=3D"t-log-$$" +stamp=3D"t-stamp-$$" +pid=3D"t-pid-$$" + +herd=3D"herd -s $socket" + +trap "rm -f $socket $conf $rconf $stamp $log; + test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT + +cat > "$conf"< + #:provides '(test) + #:start (const #t) + #:actions (make-actions + (say-hello (lambda _ + (call-with-output-file "$stamp" + (lambda (port) + (display "Hello" port)))))) + #:respawn? #f)) +EOF + +rm -f "$pid" "$stamp" "$socket" +shepherd -I -s "$socket" -c "$conf" --pid=3D"$pid" --log=3D"$log" & + +while ! test -f "$pid"; do sleep 0.5 ; done + +$herd start test + +if ! $herd say-hello test; then + echo "say-hello failed" + exit 1 +fi + +cat > "$rconf"< + #:provides '(test) + #:start (const #t) + #:actions (make-actions + (say-goodbye (lambda _ + (call-with-output-file "$stamp" + (lambda (port) + (display "Goodbye" port)))))) + #:respawn? #f)) +EOF + +$herd load root "$rconf" + +if ! $herd say-hello test; then + echo "say-hello failed after setting replacement" + exit 1 +fi + +if test "`cat $stamp`" !=3D "Hello"; then + echo "Output file had the wrong contents! Was:" + cat $stamp + exit 1 +fi + +$herd stop test + +$herd start test + +if $herd say-hello test; then + echo "say-hello should have failed after stop/start" + exit 1 +fi + +if ! $herd say-goodbye test; then + echo "say-goodbye failed after replacement" + exit 1 +fi + +if test "`cat $stamp`" !=3D "Goodbye"; then + echo "Output file had the wrong contents! Was:" + cat $stamp + exit 1 +fi =2D-=20 2.18.0 --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEE1lpncq7JnOkt+LaeqdyPv9awIbwFAlt+umEACgkQqdyPv9aw Ibz9VQ/8CcWPyF3dAAxTQVS20gJs/b3NpTgQdTf4Vj+p9PTDrzY9NJgh2HMyvZJ4 OvoEZEzS7LfRV3InERRwJbEH8npN1PUrJz196EAD5AVwAynDHrwe/KwZDqk38ScP BIEpcOr34Jzn/CZ2c+CnXyYZsXf7omJtcP6LHLdq2yBW/KkKB5UagAxNjNzf/wPZ qzoZ1zb4XJ+tMzCaupUm+J4OyVgnEjqCvWy96L47QSD3TP9dl4701oJaZ7Vvzsm9 08+8ikDJ71ZKWhQ9PniKLj9pWrKibcBMMZ9PTY6zw69NJ2DHOH1AONWPzCmRKiy3 c776j3vkFXhQ2SUyprphzF7CfZ7OEgH2xwgy2TAphwNPZtHtamngQcR6MLECOQIP fKlGzxYdQ44u8TLCKkeR/ngMrJLMy5FuCymKuMqO3NUOHopp8qnK5qiUnp8RcaFV LFYOaWcugGqt0eugKTaGnHap5UtMVMXukmoQcLAzv6fBYYiyFOnFvXxCKOi0VXw0 6b+UDmjs2EuC5lmx/mMf3HlS3FxvyDfo41PdPoeKPXQBfDRzVepapBb+4nw6lqJj WLu+h6YsYrwWJxJx+9vP9mmGCTLDCXIiEfCmry+pkZsMndC+d0Tt0X/xYd6p3yHh bOW7NbNUG0/C3aYVjV62nIYTWSdpVQ3X14hd4OVYuAtYbrV7Vuk= =yrpC -----END PGP SIGNATURE----- --==-=-=--