From debbugs-submit-bounces@debbugs.gnu.org Mon Nov 26 16:02:22 2018 Received: (at 33508) by debbugs.gnu.org; 26 Nov 2018 21:02:22 +0000 Received: from localhost ([127.0.0.1]:50184 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gRO1O-0003jl-4i for submit@debbugs.gnu.org; Mon, 26 Nov 2018 16:02:22 -0500 Received: from mail.lassieur.org ([83.152.10.219]:40934) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gRO1M-0003jc-67 for 33508@debbugs.gnu.org; Mon, 26 Nov 2018 16:02:20 -0500 Received: from rodion (88.191.118.83 [88.191.118.83]) by mail.lassieur.org (OpenSMTPD) with ESMTPSA id 2191bfc6 (TLSv1.2:ECDHE-RSA-CHACHA20-POLY1305:256:NO); Mon, 26 Nov 2018 21:02:18 +0000 (UTC) References: <87efb8m5gy.fsf@zancanaro.id.au> <871s78ypr9.fsf@lassieur.org> <875zwj8uqz.fsf@zancanaro.id.au> User-agent: mu4e 1.0; emacs 26.1 From: =?utf-8?Q?Cl=C3=A9ment?= Lassieur To: Carlo Zancanaro Subject: Re: [bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure In-reply-to: <875zwj8uqz.fsf@zancanaro.id.au> Date: Mon, 26 Nov 2018 22:02:17 +0100 Message-ID: <87k1kzd02u.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 33508 Cc: 33508@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hey Carlo! Carlo Zancanaro writes: > I can add this, but I don't think this is as useful as it initially > sounds. Most of our services are a specific version of a service pointing= to a > specific version of a configuration file (ie. that's in the store). That = means > that a "reload" shepherd action won't be able to know where the new > configuration file is to load it. > > We could solve this in one of two ways: > > 1) by allowing an arbitrary procedure as the value of restart-strategy, > because it can then call a shepherd action with the appropriate configura= tion > file > but then our action will have to detect whether the binary has been > changed (which would also detect any dependencies changing). 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 'reload' action won't work, but the service will keep running. > This may also lead to an inconsistent user experience where a > "reconfigure" might lead to a reload, or might lead to a restart, and > it's not obvious which it will be. Your patch also leads to this inconsistency, because it allows a service to either be restarted or not, in my opinion :-) > 2) by changing our services to create configuration files in a known loca= tion > (ie. /etc/nginx/nginx.conf). This would make it so a simple "reload" acti= on in > the service could meaningfully reload the service, but only if the binary= was > unchanged (because the old binary might not be able to read the new > configuration format, for instance). This still leads to the above problem > around the inconsistent user experience, and adds some complexity in term= s of > how configuration files are managed. > > I lean towards option (1), because it gives us the ability to call a shep= herd > action if we want, but also allows us to do arbitrary other things with t= he > extra knowledge on the Guix side. I think both (1) and (2) make sense because both kind of services (the ones 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. > In the end, though, I'm unconvinced that this is a useful thing to add. W= hat > do you think? I don't agree :-). A 'restart' is inherently dangerous because there is a chance for the restart to fail (say, if the new configuration file is erroneous), whereas the 'reload' action cannot fail (if it is correctly implemented). 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. One question though: my understanding is that the default value for 'restart-strategy' is set in the Guix repository, but a user would be able to customize it in their config.scm. Can you confirm it? Thank you, Cl=C3=A9ment