Message ID | 20221010043932.28384-1-eu@euandre.org |
---|---|
State | Accepted |
Headers | show |
Series | [bug#58405] services: nginx: Add reload action | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
EuAndreh via Guix-patches via <guix-patches@gnu.org> writes: > In a new "reload" shepherd-action, send a SIGHUP to the NGINX master > process, so that it can re-read the configuration file and start new > worker processes. > > * gnu/services/web.scm (nginx-shepherd-service): Add the "reload" > shepherd-action > --- > gnu/services/web.scm | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) With the NGinx service currently, you need to restart it to change the NGinx binary or configuration file. What's the purpose of the reload action here given that neither the binary or configuration file being used will change? Thanks, Chris
> With the NGinx service currently, you need to restart it to change the > NGinx binary or configuration file. It is true that you need to restart to change the NGINX binary, but this is not true for changing the configuration file. NGINX's master process reloads the configuration file, which could have an "include" line that points to ad-hoc files in /etc. So even though the NGINX service is using the immutable file inside /gnu/store, reloading it can have it change its runtime behaviour. The same behaviour is relied upon for certbot certificates: the current certificate lives in /etc/letsencrypt/live, but it is a symlink that points to /etc/letsencrypt/archive. When a certificate is renewed, a SIGHUP ought to be sent to NGINX in order to reload the configuration file, so that the certificates themselves can be reloaded, even though neither the NGINX binary nor the configuration file changed, but only what they point to did. > What's the purpose of the reload action here given that neither the > binary or configuration file being used will change? I'm doing blue/green deployments on a web service. I have the equivalent of /etc/my-service/{blue,green,active}.conf files, and an "include" line in the main NGINX configuration that includes the "active" one. Doing a deploy from blue to green is done by changing the `active.conf` symlink to point to `green.conf` instead, and sending a SIGHUP to NGINX.
EuAndreh <eu@euandre.org> writes: >> With the NGinx service currently, you need to restart it to change the >> NGinx binary or configuration file. > > It is true that you need to restart to change the NGINX binary, but this > is not true for changing the configuration file. > > NGINX's master process reloads the configuration file, which could have > an "include" line that points to ad-hoc files in /etc. So even though > the NGINX service is using the immutable file inside /gnu/store, > reloading it can have it change its runtime behaviour. > > The same behaviour is relied upon for certbot certificates: the current > certificate lives in /etc/letsencrypt/live, but it is a symlink that > points to /etc/letsencrypt/archive. When a certificate is renewed, a > SIGHUP ought to be sent to NGINX in order to reload the configuration > file, so that the certificates themselves can be reloaded, even though > neither the NGINX binary nor the configuration file changed, but only > what they point to did. That makes sense. I do think this still might cause confusion, since I think some will expect this to change NGinx to use the configuration defined in the system configuration. I'm not quite sure how to address that, but I think this can still be merged. Chris
Christopher Baines <mail@cbaines.net> writes: > I'm not quite sure how to address that, but I think this can still be > merged. I've gone ahead and pushed this as 10d429f2fce321d8285684503094694ec3979865. Thanks, Chris
Hi, A late comment… EuAndreh <eu@euandre.org> skribis: > + (shepherd-action > + (name 'reload) > + (documentation "Reload NGINX configuration file and restart worker processes.") > + (procedure > + #~(lambda (pid) > + (if pid > + (begin > + (kill pid SIGHUP) Isn’t ‘nginx -s reload’ the documented way to do that? Or maybe it’s completely equivalent? > + (format #t "Service NGINX (PID ~a) has been reloaded." pid)) > + (format #t "Service NGINX is not running.")))))))))))) Nitpick: According to <https://nginx.org/en/> it seems that the correct spelling is “nginx”, lowercase. :-) Thanks, Ludo’.
> That makes sense. I do think this still might cause confusion, since I > think some will expect this to change NGinx to use the configuration > defined in the system configuration. How about being more explicit in the action documentation about the scope of the reload?
They're compleltely equivalent :) Both are documented ways of doing the same, see: - https://nginx.org/en/docs/control.html - https://nginx.org/en/docs/beginners_guide.html > Nitpick: According to <https://nginx.org/en/> it seems that the correct > spelling is “nginx”, lowercase. :-) Oh, TIL. I'll fix that.
I think it would address your concerns on confusion of users. I'm up for doing it if you agree.
EuAndreh <eu@euandre.org> writes: >> That makes sense. I do think this still might cause confusion, since I >> think some will expect this to change NGinx to use the configuration >> defined in the system configuration. > > How about being more explicit in the action documentation about the scope of the > reload? Yeah, that sounds good to me.
diff --git a/gnu/services/web.scm b/gnu/services/web.scm index e5ab1a1180..227a577de3 100644 --- a/gnu/services/web.scm +++ b/gnu/services/web.scm @@ -807,7 +807,6 @@ (define (nginx-shepherd-service config) #~#t #~(read-pid-file #$pid-file)))))))) - ;; TODO: Add 'reload' action. (list (shepherd-service (provision '(nginx)) (documentation "Run the nginx daemon.") @@ -815,7 +814,19 @@ (define (nginx-shepherd-service config) (modules `((ice-9 match) ,@%default-modules)) (start (nginx-action "-p" run-directory)) - (stop (nginx-action "-s" "stop"))))))) + (stop (nginx-action "-s" "stop")) + (actions + (list + (shepherd-action + (name 'reload) + (documentation "Reload NGINX configuration file and restart worker processes.") + (procedure + #~(lambda (pid) + (if pid + (begin + (kill pid SIGHUP) + (format #t "Service NGINX (PID ~a) has been reloaded." pid)) + (format #t "Service NGINX is not running.")))))))))))) (define nginx-service-type (service-type (name 'nginx)