bug#26830: Allow services to implement a 'reload' action

[ludo@gnu.org (Ludovic Courtès)]

Heya,

Clément Lassieur <clement@lassieur.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> However, I think ‘reload’ might be confusing since in fact it doesn’t
>> load Scheme code, contrary to what “herd load root foo.scm” does (maybe
>> that’s what you meant?).  In fact it’s closer to what “herd restart
>> foo” does.
>>
>> What about changing the name to ‘reconfigure’ or ‘upgrade’ to avoid the
>> confusion?
>
> I think it's going to be even more confusing because the other init
> systems (systemd, sysvinit) all call it 'reload'.  And, well, people are
> probably more familiar with Systemd's 'reload' than with Shepherd's
> 'reload root' :)  WDYT?

I think it’s a valid argument!  However, if the choice is between
internal consistency (on the use of the word “load” in Shepherd
commands) and the rule of least surprise (choosing command names similar
to those of other programs), I would favor internal consistency, I
think.  WDYT?

We have a nice bike shed to paint here.  :-)

>> Some comments:
>>
>> +                      #:actions (make-actions
>> +                                 (reload
>> +                                  "Reload the service's configuration files."
>> +                                  #$(shepherd-service-reload service))))))))
>>
>> Here I think we should only define the action when it has a non-#f
>> value.  That way we can distinguish between services that have a useful
>> reload/reconfigure/upgrade action and those that don’t; in the latter
>> case, we simply use ‘restart’ when upgrading.
>
> Ok.  But right now IIRC we don't even use restart after a system
> reconfigure, probably because some services can't be restarted safely.

Currently ‘guix system reconfigure’ (specifically
‘upgrade-shepherd-services’) reloads and starts all services that are
currently stopped, on the grounds that it would not be safe/desirable to
simply stop any running service.

> Would that be why we need a 'reload/reconfigure/upgrade' for them?

‘upgrade-shepherd-services’ could check whether a service has an
‘upgrade’ action.  If it does, it could call that action unconditionally
since that action would semantically have the same effect has
stop/unload/load/start, except that it does that “live”, without
stopping anything.

>> Regarding nginx:
>>
>> +              (stop (nginx-action "-s" "stop"))
>> +              (reload (nginx-action "-s" "reload"))))))))
>>
>> Is this of any use in practice?  The nginx command line is something
>> like:
>>
>>   /gnu/store/74kz9m850ycxpzkg6dvn9wbd3xjkwwrb-nginx-1.12.0/sbin/nginx -c /gnu/store/5w11ahw113fndvab3xmwcjzs2rw56sbh-nginx-config/bayfront.conf -p /var/run/nginx
>>
>> and the configuration file in /gnu/store is immutable, so “nginx -s
>> reload” does nothing.
>
> Actually, my goal was to use this after a certificate renewal.  There
> was going to be other patches on the certbot service as well :)  And
> after a certificate renewal, the names of the certificates don't change
> (AFAIK they are still in /etc).

Indeed.

> So I think it would work.  But indeed, I didn't think about the main
> configuration file :/

Yeah.  Sorry for dropping a fly in the ointment.  :-/

>> If the action took an argument, we could do:
>>
>>   herd reconfigure nginx /gnu/store/…-new-config.conf
>>
>> which would translate to:
>>
>>   nginx -s reload -c /gnu/store/…-new-config.conf
>>
>> Probably our best option.
>
> I don't see the point.  If the service has already been reloaded by the
> 'guix system reconfigure' command (let's assume it does, but I know it
> doesn't currently reload nor restart sevices...), why would a user want
> to reload it again with the 'herd' command?  Or maybe you want this
> feature as a workaround while the 'guix system reconfigure' that reloads
> services isn't implemented?

Sorry, I wasn’t clear.  Action can take arguments; most don’t, but some
do (like ‘herd start cow-store /mnt’ when installing GuixSD.)  What I’m
suggesting here is to add one/several arguments to this reload/upgrade
action.  The meaning of these arguments would be defined by the service
itself.

For nginx, there could be one argument (the config file) or two (the
config file and the nginx executable file name).  The reload/upgrade
action would do “nginx -s reload -c …” and so on.

The ‘upgrade-shepherd-services’ procedure would automatically call the
reload/upgrade action with the right arguments.  For that, it needs to
know what the arguments are.  An option would be to add an
‘upgrade-arguments’ field to <shepherd-service> that would return the
arguments to pass to the upgrade action.

Does that make sense?

> Anyway, I think the argument should be optional, so that if there are
> none, the current configuration file is used.  That will be useful for
> certificates anyway, or for other kinds of configuration files that
> aren't in the store.

We could do that too.  My thought was that the primary consumer of this
action would be ‘guix system reconfigure’ and not the user.

Thanks,
Ludo’.