From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gShfO-0007Th-T2 for guix-patches@gnu.org; Fri, 30 Nov 2018 07:13:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gShfK-00016f-Rk for guix-patches@gnu.org; Fri, 30 Nov 2018 07:13:06 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:51350) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gShfK-00016P-N8 for guix-patches@gnu.org; Fri, 30 Nov 2018 07:13:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1gShfK-0007qA-7i for guix-patches@gnu.org; Fri, 30 Nov 2018 07:13:02 -0500 Subject: [bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure Resent-Message-ID: References: <87efb8m5gy.fsf@zancanaro.id.au> <871s78ypr9.fsf@lassieur.org> <875zwj8uqz.fsf@zancanaro.id.au> <87k1kzd02u.fsf@lassieur.org> <87lg5fsdof.fsf@zancanaro.id.au> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87lg5fsdof.fsf@zancanaro.id.au> Date: Fri, 30 Nov 2018 13:12:57 +0100 Message-ID: <87bm66higm.fsf@lassieur.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: 33508@debbugs.gnu.org Hi Carlo, Carlo Zancanaro writes: > Hey Cl=C3=A9ment, > > Thanks for your thoughts! I think you're right that the approach I've > implemented isn't flexible enough. I potentially haven't thought through = the > failure cases enough. I was more thinking of reload as providing "zero > downtime" upgrades, rather than providing a safer way to upgrade. > > I'll respond more specifically inline. > > On Tue, Nov 27 2018, Cl=C3=A9ment Lassieur wrote: >> I don't think it needs to detect whether the binary has changed, because >> 'reload' signals are usually implemented so that they can safely fail. = So, >> if the configuration file has changed in an incompatible way, the 'reloa= d' >> action won't work, but the service will keep running. > > We do need to detect whether the binary has changed for the sake of secur= ity > updates, or similar. It would be bad if a user reconfigured their system = and > it didn't upgrade the version of nginx (or its dependencies) that they're > running. If there is a risk for a service to be broken on reconfigure, a user might want to do a safe reconfigure, and later on deal with each critical service one after another, so to avoid having several services down at the same time. I think we should at least allow a user to do a 'safe reconfigure' if they want. > Broadly speaking, I conceptualise reconfigure as "bring my system into th= is > state". Now, thus far we haven't been able to do that, because we have la= cked > the ability to restart services properly, but in my mind the ideal situat= ion > is that after running "guix system reconfigure" our system is completely = put > into the state specified by the config.scm file used. This is ideal, but most services depend on a state (Cuirass, mail servers...). > Although, now that I type that out, I notice that there is one obvious wa= y in > which that is not true: the kernel. We can't hot-swap the kernel, so ther= e can > always be a difference between what the configuration file specifies and = what > the system is actually running. And I don't think you can restart Xorg either... > At any rate, even if we give services the ability to reload without > restarting, they would need to print out a message to prompt the user to > manually restart them if the binary has changed. I would also then expect= the > --restart-services flag to fully restart those services, rather than just > reloading them. Agreed :-) >> Your patch also leads to this inconsistency, because it allows a service= to >> either be restarted or not, in my opinion :-) > > Yes, that's true, but then there's no middle-ground. Reloading is "new > configuration, old binary", whereas the current options are "old > configuration, old binary" or "new configuration, new binary" (or, I gues= s, > "not running because of a failed restart"). > >> I think both (1) and (2) make sense because both kind of services (the o= nes >> pointing to configuration files in the store and the ones using >> /etc/some-file.conf) already exist. Ideally, the mechanism should be >> generic enough to handle both cases. > > (1) actually subsumes (2), so I think I'll implement that. It actually en= ds up > being slightly easier, because the restart strategy can just always be a > procedure, with three predefined procedures: always-restart, manually-res= tart, > and never-restart. > >> That being said, I agree that adding support for 'reload' would lead to = more >> complexity, and I would understand if you don't add it :-), but I still >> think it's a very useful feature. > > I think you've convinced me that there's value in having more flexibility > around restarts. I'll change the restart-strategy value to be a procedure > rather than a bare symbol. The downside is that we'll lose the ability to > statically analyse how services will behave when restarting, but the incr= eased > flexibility is useful to us. It could also detect whether you pass a symbol or a procedure. In most cases that would be a symbol which would allow static analysis. But one could still customize it with a procedure. >> One question though: my understanding is that the default value for >> 'restart-strategy' is set in the Guix repository, but a user would be ab= le >> to customize it in their config.scm. Can you confirm it? > > That is not the current implementation. Individual services can add a fie= ld to > their configuration objects if it's meaningful for them to customise their > restart behaviour, but there isn't a general-purpose mechanism for a user= to > change the restart behaviour of any service (beyond the ability to write > arbitrary Scheme to do it). Ok thank you! Cl=C3=A9ment