From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1frqsF-0008PL-Er for guix-patches@gnu.org; Mon, 20 Aug 2018 16:34:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1frqsE-00046b-Cn for guix-patches@gnu.org; Mon, 20 Aug 2018 16:34:03 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:50914) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1frqsE-00045W-4X for guix-patches@gnu.org; Mon, 20 Aug 2018 16:34:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1frqsD-0002rl-WC for guix-patches@gnu.org; Mon, 20 Aug 2018 16:34:02 -0400 Subject: [bug#32408] [PATCH shepherd] Allow replacement of services Resent-Message-ID: From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <87wosz4spx.fsf@zancanaro.id.au> Date: Mon, 20 Aug 2018 22:33:42 +0200 In-Reply-To: <87wosz4spx.fsf@zancanaro.id.au> (Carlo Zancanaro's message of "Thu, 09 Aug 2018 22:42:02 +1000") Message-ID: <87h8jobwwp.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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: Carlo Zancanaro Cc: 32408@debbugs.gnu.org Hi Carlo, Carlo Zancanaro skribis: > This is a relatively simple patch adding a replacement slot to > services in the Shepherd. When stopping a service, the replacement > slot is checked and, if it has a value, is used to upgrade the current > service. > > I've chosen to modify the existing service, rather than creating a new > one, but that was mostly because it was easier for me to implement > quickly, and I didn't have a huge amount of time. > > I'm hopeful that this, or something like it, can be used by GuixSD to > allow people to restart services after reconfiguring without rebooting > (or remembering to stop, reconfigure, start). Awesome! This is exactly what we need to address the problem. We=E2=80=99ll still want to be able to special-case things like nginx that = can be =E2=80=9Chot-replaced=E2=80=9D, though. So perhaps, in addition to this= patch on the Shepherd side, we=E2=80=99ll need extra stuff in (gnu services shepherd). For instance, the =E2=80=98actions=E2=80=99 field of cou= ld, by default, include an =E2=80=9Cupgrade=E2=80=9D action that simply sets the = =E2=80=98replacement=E2=80=99 slot. For nginx, we=E2=80=99d provide a custom =E2=80=9Cupgrade=E2=80=9D a= ction that does =E2=80=9Cnginx -s restart=E2=80=9D or whatever it is that needs to be done. =E2=80=98guix system reconfigure=E2=80=99 would automatically invoke the = =E2=80=98upgrade=E2=80=99 action for each new service. WDYT? > From e03290041c91813f1a301c7e9c4dbb9ee768b400 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. > * tests/replacement.sh: Add a test for it. > * Makefile.am (TESTS): Add the new test. > * doc/shepherd.texi (Slots of services): Document it. Overall LGTM. Some comments: > +(define (replace-service service) Please add a docstring. > + (let ((replacement (slot-ref service 'replacement))) > + (define (copy-slot! slot) > + (slot-set! service slot (slot-ref replacement slot))) > + (when replacement > + (copy-slot! 'provides) > + (copy-slot! 'requires) > + (copy-slot! 'respawn?) > + (copy-slot! 'start) > + (copy-slot! 'stop) > + (copy-slot! 'actions) > + (copy-slot! 'running) > + (copy-slot! 'docstring)) > + service)) Having a hardcoded list of slots sounds error-prone=E2=80=94surely we=E2=80= =99ll forget to update it down the road. I wonder what else could be done. One option would be to grab the block asyncs and atomically replace the service in the =E2=80=98%services=E2=80=99 hash table. Then we only need t= o copy the =E2=80=98last-respawns=E2=80=99 slot to the new service, I believe. (This = changes the object identity of the service but I think its OK.) Another option would be to use GOOPS tricks to iterate over the list of slots and have a list of slots *not* to copy. I=E2=80=99m not a big fan of= this option, though. > +cat - > "$rconf"< +(let ((service (lookup-running 'test))) > + (slot-set! service 'replacement > + (make I wonder if we should let users fiddle with =E2=80=98replacement=E2=80=99 d= irectly, or if we should provide a higher-level construct. For instance, =E2=80=98register-services=E2=80=99 could transparently set t= he =E2=80=98replacement=E2=80=99 field for services already registered instead= of doing: (assert (null? (lookup-services (canonical-name new)))) Not sure if there are cases where this behavior would be undesirable, though. Thoughts? > +if test `cat $stamp` !=3D "Goodbye"; then Nitpick: "`cat $stamp`". Thanks! Ludo=E2=80=99.