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

[Clément Lassieur]

Hi Carlo,

Carlo Zancanaro <address@hidden> writes:

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

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

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

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

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

Ok thank you!

Clément