[bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure

[Carlo Zancanaro <carlo@zancanaro.id.au>]

Hey Clément,

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ément 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 'reload' action won't work, but the 
> service will keep running.

We do need to detect whether the binary has changed for the sake 
of security 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.

Broadly speaking, I conceptualise reconfigure as "bring my system 
into this state". Now, thus far we haven't been able to do that, 
because we have lacked the ability to restart services properly, 
but in my mind the ideal situation is that after running "guix 
system reconfigure" our system is completely put into the state 
specified by the config.scm file used.

Although, now that I type that out, I notice that there is one 
obvious way in which that is not true: the kernel. We can't 
hot-swap the kernel, so there can always be a difference between 
what the configuration file specifies and what the system is 
actually running.

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.

> 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 guess, "not running because of a failed restart").

> 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.

(1) actually subsumes (2), so I think I'll implement that. It 
actually ends up being slightly easier, because the restart 
strategy can just always be a procedure, with three predefined 
procedures: always-restart, manually-restart, 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 increased flexibility is 
useful to us.

> 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?

That is not the current implementation. Individual services can 
add a field 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).

Carlo