Allow services to implement a 'reload' action

OpenSubmitted by Clément Lassieur.
Details
4 participants
  • Clément Lassieur
  • Danny Milosavljevic
  • Ludovic Courtès
  • Mathieu Othacehe
Owner
unassigned
Severity
important
C
C
Clément Lassieur wrote on 8 May 2017 17:25
(address . guix-patches@gnu.org)
87d1bjtlpd.fsf@lassieur.org
Hi,
The first patch allows services to implement a 'reload' action, and theother three implement the 'reload' action for nginx, prosody anddovecot.
Services do not have to implement 'reload' and if, say, foo-daemondoesn't implement it, 'herd reload foo-daemon' will return 1 and displaya message saying that foo-deamon does not have an action 'reload'.That's the reason of the #f default value.
WDYT?
C
C
Clément Lassieur wrote on 8 May 2017 17:28
[PATCH 1/4] services: shepherd: Allow services to implement a 'reload' action.
(address . 26830@debbugs.gnu.org)
20170508152832.4797-1-clement@lassieur.org
* gnu/services/shepherd.scm (<shepherd-service>)[reload]: Add it.(shepherd-service-file): Add it to the Shepherd's service definition.* doc/guix.texi (Services): Update accordingly.--- doc/guix.texi | 7 ++++--- gnu/services/shepherd.scm | 9 ++++++++- 2 files changed, 12 insertions(+), 4 deletions(-)
Toggle diff (54 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex 4446909ed..3ccfa8d9e 100644--- a/doc/guix.texi+++ b/doc/guix.texi@@ -8671,9 +8671,10 @@ service: Run libc's name service cache daemon (nscd). @end example -The @command{start}, @command{stop}, and @command{restart} sub-commands-have the effect you would expect. For instance, the commands below stop-the nscd service and restart the Xorg display server:+The @command{start}, @command{stop}, @command{restart} and+@command{reload} sub-commands have the effect you would expect. For+instance, the commands below stop the nscd service and restart the Xorg+display server: @example # herd stop nscddiff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scmindex 7281746ab..17e53f774 100644--- a/gnu/services/shepherd.scm+++ b/gnu/services/shepherd.scm@@ -47,6 +47,7 @@ shepherd-service-respawn? shepherd-service-start shepherd-service-stop+ shepherd-service-reload shepherd-service-auto-start? shepherd-service-modules @@ -137,6 +138,8 @@ for a service that extends SHEPHERD-ROOT-SERVICE-TYPE and nothing else." (start shepherd-service-start) ;g-expression (procedure) (stop shepherd-service-stop ;g-expression (procedure) (default #~(const #f)))+ (reload shepherd-service-reload ;g-expression (procedure)+ (default #f)) (auto-start? shepherd-service-auto-start? ;Boolean (default #t)) (modules shepherd-service-modules ;list of module names@@ -214,7 +217,11 @@ stored." #:requires '#$(shepherd-service-requirement service) #:respawn? '#$(shepherd-service-respawn? service) #:start #$(shepherd-service-start service)- #:stop #$(shepherd-service-stop service))))))+ #:stop #$(shepherd-service-stop service)+ #:actions (make-actions+ (reload+ "Reload the service's configuration files."+ #$(shepherd-service-reload service)))))))) (define (shepherd-configuration-file services) "Return the shepherd configuration file for SERVICES."-- 2.12.2
C
C
Clément Lassieur wrote on 8 May 2017 17:28
[PATCH 2/4] gnu: services: nginx: Add a 'reload' action.
(address . 26830@debbugs.gnu.org)
20170508152832.4797-2-clement@lassieur.org
* gnu/services/web.scm (nginx-shepherd-service): Add it.--- gnu/services/web.scm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Toggle diff (30 lines)diff --git a/gnu/services/web.scm b/gnu/services/web.scmindex f85b41215..956aa1518 100644--- a/gnu/services/web.scm+++ b/gnu/services/web.scm@@ -4,6 +4,7 @@ ;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is> ;;; Copyright © 2016, 2017 Julien Lepiller <julien@lepiller.eu> ;;; Copyright © 2017 Christopher Baines <mail@cbaines.net>+;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org> ;;; ;;; This file is part of GNU Guix. ;;;@@ -262,13 +263,13 @@ of index files." run-directory server-blocks upstream-blocks)) #$@args)))))) - ;; TODO: Add 'reload' action. (list (shepherd-service (provision '(nginx)) (documentation "Run the nginx daemon.") (requirement '(user-processes loopback)) (start (nginx-action "-p" run-directory))- (stop (nginx-action "-s" "stop"))))))))+ (stop (nginx-action "-s" "stop"))+ (reload (nginx-action "-s" "reload")))))))) (define nginx-service-type (service-type (name 'nginx)-- 2.12.2
C
C
Clément Lassieur wrote on 8 May 2017 17:28
[PATCH 3/4] gnu: services: prosody: Add a 'reload' action.
(address . 26830@debbugs.gnu.org)
20170508152832.4797-3-clement@lassieur.org
* gnu/services/messaging.scm (prosody-shepherd-service): Add it.--- gnu/services/messaging.scm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Toggle diff (16 lines)diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scmindex 715d6181f..1f357c1c5 100644--- a/gnu/services/messaging.scm+++ b/gnu/services/messaging.scm@@ -582,7 +582,8 @@ See also @url{http://prosody.im/doc/modules/mod_muc}." (provision '(prosody xmpp-daemon)) (requirement '(networking syslogd user-processes)) (start (prosodyctl-action "start"))- (stop (prosodyctl-action "stop"))))))+ (stop (prosodyctl-action "stop"))+ (reload (prosodyctl-action "reload")))))) (define %prosody-accounts (list (user-group (name "prosody") (system? #t))-- 2.12.2
C
C
Clément Lassieur wrote on 8 May 2017 17:28
[PATCH 4/4] gnu: services: dovecot: Add a 'reload' action.
(address . 26830@debbugs.gnu.org)
20170508152832.4797-4-clement@lassieur.org
* gnu/services/mail.scm (dovecot-shepherd-service): Add it.--- gnu/services/mail.scm | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Toggle diff (18 lines)diff --git a/gnu/services/mail.scm b/gnu/services/mail.scmindex 6305f06f8..db0772c47 100644--- a/gnu/services/mail.scm+++ b/gnu/services/mail.scm@@ -1528,7 +1528,10 @@ greyed out, instead of only later giving \"not selectable\" popup error. "-F" "-c" #$config-file))) (stop #~(make-forkexec-constructor (list (string-append #$dovecot "/sbin/dovecot")- "-c" #$config-file "stop")))))))+ "-c" #$config-file "stop")))+ (reload #~(make-forkexec-constructor+ (list (string-append #$dovecot "/bin/doveadm")+ "-c" #$config-file "reload"))))))) (define %dovecot-pam-services (list (unix-pam-service "dovecot")))-- 2.12.2
M
M
Mathieu Othacehe wrote on 9 May 2017 17:37
Re: bug#26830: Allow services to implement a 'reload' action
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 26830@debbugs.gnu.org)
86vapa6nyi.fsf@gmail.com
Hi Clément,
Toggle quote (7 lines)> Services do not have to implement 'reload' and if, say, foo-daemon> doesn't implement it, 'herd reload foo-daemon' will return 1 and display> a message saying that foo-deamon does not have an action 'reload'.> That's the reason of the #f default value.>> WDYT?
Your whole serie LGTM for me !
I have just one small concern, there is a already a "reload" action onshepherd root service.
For instance you can call "herd reload root conf.scm".
Maybe it will be unclear for users how reload action differs on rootservice where it takes an argument and guix services where it does not.
You could maybe mention that in the documentation and/or in the code ?
Thanks,
Mathieu
C
C
Clément Lassieur wrote on 10 May 2017 21:31
[PATCH] services: shepherd: Allow services to implement a 'reload' action.
(address . 26830@debbugs.gnu.org)
20170510193137.846-1-clement@lassieur.org
* gnu/services/shepherd.scm (<shepherd-service>)[reload]: Add it.(shepherd-service-file): Add it to the Shepherd's service definition.* doc/guix.texi (Services, Shepherd Services): Update accordingly.--- doc/guix.texi | 14 +++++++++++--- gnu/services/shepherd.scm | 9 ++++++++- 2 files changed, 19 insertions(+), 4 deletions(-)
Toggle diff (68 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex 81aa957c6..2d2015df2 100644--- a/doc/guix.texi+++ b/doc/guix.texi@@ -8674,9 +8674,10 @@ service: Run libc's name service cache daemon (nscd). @end example -The @command{start}, @command{stop}, and @command{restart} sub-commands-have the effect you would expect. For instance, the commands below stop-the nscd service and restart the Xorg display server:+The @command{start}, @command{stop}, @command{restart} and+@command{reload} sub-commands have the effect you would expect. For+instance, the commands below stop the nscd service and restart the Xorg+display server: @example # herd stop nscd@@ -16204,6 +16205,13 @@ Constructors,,, shepherd, The GNU Shepherd Manual}). They are given as G-expressions that get expanded in the Shepherd configuration file (@pxref{G-Expressions}). +@item @code{reload} (default: @code{#f})+The @code{reload} field refers to the Shepherd's facilities to reload+the service's configuration files without restarting. They are+@code{actions} (@pxref{Slots of services,,, shepherd, The GNU Shepherd+Manual}) and are given as G-expressions that get expanded in the+Shepherd configuration file (@pxref{G-Expressions}).+ @item @code{documentation} A documentation string, as shown when running: diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scmindex 7281746ab..17e53f774 100644--- a/gnu/services/shepherd.scm+++ b/gnu/services/shepherd.scm@@ -47,6 +47,7 @@ shepherd-service-respawn? shepherd-service-start shepherd-service-stop+ shepherd-service-reload shepherd-service-auto-start? shepherd-service-modules @@ -137,6 +138,8 @@ for a service that extends SHEPHERD-ROOT-SERVICE-TYPE and nothing else." (start shepherd-service-start) ;g-expression (procedure) (stop shepherd-service-stop ;g-expression (procedure) (default #~(const #f)))+ (reload shepherd-service-reload ;g-expression (procedure)+ (default #f)) (auto-start? shepherd-service-auto-start? ;Boolean (default #t)) (modules shepherd-service-modules ;list of module names@@ -214,7 +217,11 @@ stored." #:requires '#$(shepherd-service-requirement service) #:respawn? '#$(shepherd-service-respawn? service) #:start #$(shepherd-service-start service)- #:stop #$(shepherd-service-stop service))))))+ #:stop #$(shepherd-service-stop service)+ #:actions (make-actions+ (reload+ "Reload the service's configuration files."+ #$(shepherd-service-reload service)))))))) (define (shepherd-configuration-file services) "Return the shepherd configuration file for SERVICES."-- 2.12.2
C
C
Clément Lassieur wrote on 10 May 2017 21:31
Re: bug#26830: Allow services to implement a 'reload' action
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 26830@debbugs.gnu.org)
87vap8h5jl.fsf@lassieur.org
Mathieu Othacehe <m.othacehe@gmail.com> writes:
Toggle quote (9 lines)>> Services do not have to implement 'reload' and if, say, foo-daemon>> doesn't implement it, 'herd reload foo-daemon' will return 1 and display>> a message saying that foo-deamon does not have an action 'reload'.>> That's the reason of the #f default value.>>>> WDYT?>> Your whole serie LGTM for me !
Hi Mathieu, thanks for reviewing :)
Toggle quote (8 lines)> I have just one small concern, there is a already a "reload" action on> shepherd root service.>> For instance you can call "herd reload root conf.scm".>> Maybe it will be unclear for users how reload action differs on root> service where it takes an argument and guix services where it does not.
They don't differ: 'root' is just another service, as 'nginx' is. Our'reload' action can handle many arguments as well. The only tinydifference is that the 'root' service is implemented by Shepherd, not byGuix.
Toggle quote (2 lines)> You could maybe mention that in the documentation and/or in the code ?
Sure, I updated the documentation. I had forgotten the "ShepherdServices" part and I think it helps understanding. But I didn't talkabout the 'root' service because it is a Shepherd thing and is alreadydescribed in the Shepherd manual.
WDYT?
C
C
Clément Lassieur wrote on 11 May 2017 09:13
Re: bug#26830: [PATCH] services: shepherd: Allow services to implement a 'reload' action.
(address . 26830@debbugs.gnu.org)
8760h7rhmm.fsf@lassieur.org
Clément Lassieur <clement@lassieur.org> writes:
Toggle quote (7 lines)> +@item @code{reload} (default: @code{#f})> +The @code{reload} field refers to the Shepherd's facilities to reload> +the service's configuration files without restarting. They are> +@code{actions} (@pxref{Slots of services,,, shepherd, The GNU Shepherd> +Manual}) and are given as G-expressions that get expanded in the> +Shepherd configuration file (@pxref{G-Expressions}).
With singular instead of plural...:
@item @code{reload} (default: @code{#f})The @code{reload} field refers to the Shepherd's facilities to reloadthe service's configuration files without restarting. It is an@code{action} (@pxref{Slots of services,,, shepherd, The GNU ShepherdManual}) and is given as a G-expression that gets expanded in theShepherd configuration file (@pxref{G-Expressions}).
C
C
Clément Lassieur wrote on 11 May 2017 14:40
(address . 26830@debbugs.gnu.org)
87vap7sh1i.fsf@lassieur.org
Clément Lassieur <clement@lassieur.org> writes:
Toggle quote (18 lines)> Clément Lassieur <clement@lassieur.org> writes:>>> +@item @code{reload} (default: @code{#f})>> +The @code{reload} field refers to the Shepherd's facilities to reload>> +the service's configuration files without restarting. They are>> +@code{actions} (@pxref{Slots of services,,, shepherd, The GNU Shepherd>> +Manual}) and are given as G-expressions that get expanded in the>> +Shepherd configuration file (@pxref{G-Expressions}).>> With singular instead of plural...:>> @item @code{reload} (default: @code{#f})> The @code{reload} field refers to the Shepherd's facilities to reload> the service's configuration files without restarting. It is an> @code{action} (@pxref{Slots of services,,, shepherd, The GNU Shepherd> Manual}) and is given as a G-expression that gets expanded in the> Shepherd configuration file (@pxref{G-Expressions}).
New version:
@item @code{reload} (default: @code{#f})The @code{reload} field allows Shepherd to reload the service'sconfiguration files without restarting. It is an @code{action}(@pxref{Slots of services,,, shepherd, The GNU Shepherd Manual}) and isgiven as a G-expression that gets expanded in the Shepherd configurationfile (@pxref{G-Expressions}).
M
M
Mathieu Othacehe wrote on 11 May 2017 14:57
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 26830@debbugs.gnu.org)
86shkbzh3d.fsf@gmail.com
Toggle quote (7 lines)> @item @code{reload} (default: @code{#f})> The @code{reload} field allows Shepherd to reload the service's> configuration files without restarting. It is an @code{action}> (@pxref{Slots of services,,, shepherd, The GNU Shepherd Manual}) and is> given as a G-expression that gets expanded in the Shepherd configuration> file (@pxref{G-Expressions}).
The new version looks better !
LGTM !
Mathieu
L
L
Ludovic Courtès wrote on 11 May 2017 23:24
Re: bug#26830: Allow services to implement a 'reload' action
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
87vap7kryj.fsf@gnu.org
Hello!
Mathieu Othacehe <m.othacehe@gmail.com> skribis:
Toggle quote (9 lines)>> Services do not have to implement 'reload' and if, say, foo-daemon>> doesn't implement it, 'herd reload foo-daemon' will return 1 and display>> a message saying that foo-deamon does not have an action 'reload'.>> That's the reason of the #f default value.>>>> WDYT?>> Your whole serie LGTM for me !
Same here, really happy to see this addressed!
Toggle quote (3 lines)> I have just one small concern, there is a already a "reload" action on> shepherd root service.
Right, but that’s just for ‘root’, not for the other services.
However, I think ‘reload’ might be confusing since in fact it doesn’tload Scheme code, contrary to what “herd load root foo.scm” does (maybethat’s what you meant?). In fact it’s closer to what “herd restartfoo” does.
What about changing the name to ‘reconfigure’ or ‘upgrade’ to avoid theconfusion?
The logical next step of this series will be to have the service upgradecode in ‘guix system reconfigure’ invoke this action when it is defined.That will be awesome.
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-#fvalue. That way we can distinguish between services that have a usefulreload/reconfigure/upgrade action and those that don’t; in the lattercase, we simply use ‘restart’ when upgrading.
Regarding nginx:
+ (stop (nginx-action "-s" "stop"))+ (reload (nginx-action "-s" "reload"))))))))
Is this of any use in practice? The nginx command line is somethinglike:
/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 -sreload” does nothing. 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.
Otherwise, I think we’d have to move the config to a fixed location, say/etc/nginx, for “nginx -s reload” to have any effect. However I don’tquite like the use of /etc. Thoughts?
Does Dovecot have the same problem?
Thank you!
Ludo’.
L
L
Ludovic Courtès wrote on 11 May 2017 23:24
control message for bug #26830
(address . control@debbugs.gnu.org)
87tw4rkry9.fsf@gnu.org
severity 26830 important
C
C
Clément Lassieur wrote on 12 May 2017 01:08
Re: bug#26830: Allow services to implement a 'reload' action
(name . Ludovic Courtès)(address . ludo@gnu.org)
874lwrq9em.fsf@lassieur.org
Hi Ludovic,
Thanks for commenting on this :)
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (28 lines)> Hello!>> Mathieu Othacehe <m.othacehe@gmail.com> skribis:>>>> Services do not have to implement 'reload' and if, say, foo-daemon>>> doesn't implement it, 'herd reload foo-daemon' will return 1 and display>>> a message saying that foo-deamon does not have an action 'reload'.>>> That's the reason of the #f default value.>>>>>> WDYT?>>>> Your whole serie LGTM for me !>> Same here, really happy to see this addressed!>>> I have just one small concern, there is a already a "reload" action on>> shepherd root service.>> Right, but that’s just for ‘root’, not for the other services.>> 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 initsystems (systemd, sysvinit) all call it 'reload'. And, well, people areprobably more familiar with Systemd's 'reload' than with Shepherd's'reload root' :) WDYT?
Toggle quote (4 lines)> The logical next step of this series will be to have the service upgrade> code in ‘guix system reconfigure’ invoke this action when it is defined.> That will be awesome.
Indeed!
Toggle quote (12 lines)> 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 systemreconfigure, probably because some services can't be restarted safely.Would that be why we need a 'reload/reconfigure/upgrade' for them?
Toggle quote (13 lines)> 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. Therewas going to be other patches on the certbot service as well :) Andafter a certificate renewal, the names of the certificates don't change(AFAIK they are still in /etc). So I think it would work. But indeed,I didn't think about the main configuration file :/
Toggle quote (10 lines)> 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 itdoesn't currently reload nor restart sevices...), why would a user wantto reload it again with the 'herd' command? Or maybe you want thisfeature as a workaround while the 'guix system reconfigure' that reloadsservices isn't implemented?
Anyway, I think the argument should be optional, so that if there arenone, the current configuration file is used. That will be useful forcertificates anyway, or for other kinds of configuration files thataren't in the store.
Toggle quote (4 lines)> Otherwise, I think we’d have to move the config to a fixed location, say> /etc/nginx, for “nginx -s reload” to have any effect. However I don’t> quite like the use of /etc. Thoughts?
I don't like it either :) 'reload' definitely has to supportconfiguration files that are in the store!
Toggle quote (2 lines)> Does Dovecot have the same problem?
Yes. (But Prosody doesn't.)
Clément
L
L
Ludovic Courtès wrote on 12 May 2017 10:25
(name . Clément Lassieur)(address . clement@lassieur.org)
87inl6trbu.fsf@gnu.org
Heya,
Clément Lassieur <clement@lassieur.org> skribis:
Toggle quote (2 lines)> Ludovic Courtès <ludo@gnu.org> writes:
[...]
Toggle quote (13 lines)>> 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 betweeninternal consistency (on the use of the word “load” in Shepherdcommands) and the rule of least surprise (choosing command names similarto those of other programs), I would favor internal consistency, Ithink. WDYT?
We have a nice bike shed to paint here. :-)
Toggle quote (15 lines)>> 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 arecurrently stopped, on the grounds that it would not be safe/desirable tosimply stop any running service.
Toggle quote (2 lines)> 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 unconditionallysince that action would semantically have the same effect hasstop/unload/load/start, except that it does that “live”, withoutstopping anything.
Toggle quote (18 lines)>> 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.
Toggle quote (3 lines)> 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. :-/
Toggle quote (17 lines)>> 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 somedo (like ‘herd start cow-store /mnt’ when installing GuixSD.) What I’msuggesting here is to add one/several arguments to this reload/upgradeaction. The meaning of these arguments would be defined by the serviceitself.
For nginx, there could be one argument (the config file) or two (theconfig file and the nginx executable file name). The reload/upgradeaction would do “nginx -s reload -c …” and so on.
The ‘upgrade-shepherd-services’ procedure would automatically call thereload/upgrade action with the right arguments. For that, it needs toknow what the arguments are. An option would be to add an‘upgrade-arguments’ field to <shepherd-service> that would return thearguments to pass to the upgrade action.
Does that make sense?
Toggle quote (5 lines)> 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 thisaction would be ‘guix system reconfigure’ and not the user.
Thanks,Ludo’.
C
C
Clément Lassieur wrote on 12 May 2017 10:57
(name . Ludovic Courtès)(address . ludo@gnu.org)
871sruqwok.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (27 lines)> 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?
Ok! I like 'upgrade'.
Toggle quote (35 lines)>>> 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?
Yes! Thank you :)
D
D
Danny Milosavljevic wrote on 28 Jan 2018 21:34
(name . Clément Lassieur)(address . clement@lassieur.org)
20180128213430.33175f51@scratchpost.org
Any news on this reload action patchset?
C
C
Clément Lassieur wrote on 29 Jan 2018 00:23
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
87inbl4lgh.fsf@lassieur.org
Danny Milosavljevic <dannym@scratchpost.org> writes:
Toggle quote (2 lines)> Any news on this reload action patchset?
I didn't find time to work on it. It's still on my todo list but ifanyone wants to do it, please go ahead.
L
L
Ludovic Courtès wrote on 12 Jul 2018 00:00
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
87bmbdigbj.fsf@gnu.org
Hello!
ludo@gnu.org (Ludovic Courtès) skribis:
Toggle quote (35 lines)> The logical next step of this series will be to have the service upgrade> code in ‘guix system reconfigure’ invoke this action when it is defined.> That will be awesome.>> 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.>> 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. 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
FWIW, with the patch at https://bugs.gnu.org/32128, adding suchactions becomes easy (it’s a generalization of what you did in thispatch series.)
Ludo’.
C
C
Clément Lassieur wrote on 12 Jul 2018 15:06
Re: [bug#26830] Allow services to implement a 'reload' action
(name . Ludovic Courtès)(address . ludo@gnu.org)
874lh438oj.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (6 lines)> FWIW, with the patch at https://bugs.gnu.org/32128, adding such> actions becomes easy (it’s a generalization of what you did in this> patch series.)>> Ludo’.
Hehe thanks for the reminder, I'll consider working on this again soon:-)
Clément
?