bug#32665: Shepherd is stuck waiting for /var/run/nginx/pid

[Clément Lassieur]

Oh!  Sorry for the mess.  The patches both look good to me, thank you!

Clément

Ludovic Courtès <address@hidden> writes:

> Hello Clément,
>
> Commit 9fc2922794ffaae48e0a7c536e530ea2e0d46cf3 adds a loop to the nginx
> service that checks for /var/run/nginx/pid.  Unfortunately, on berlin
> that loop never ends because the file is not created (on berlin we use a
> “hand-written” nginx config file that lacks a “pid” directive; see
> guix-maintenance.git.)
>
> I think there are two things to address:
>
>   1. Don’t look for a PID file when passed a hand-written config file;
>
>   2. Make sure the loop always terminates, similar to what
>     ‘make-forkexec-constructor’ does.
>
> The patch below fixes that.  The second patch fixes the ‘stop’
> procedure.
>
> Thoughts?
>
> Ludo’.
>
> From c9daf228a69c24cff6ba5508a3b67ebe3c702a2b Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <address@hidden>
> Date: Sat, 8 Sep 2018 18:48:48 +0200
> Subject: [PATCH 1/2] services: nginx: Don't read PID file when passed a custom
>  config file.
>
> Fixes <https://bugs.gnu.org/XXX>.
>
> * gnu/services/web.scm (nginx-shepherd-service): Check whether FILE is
> true and don't read the PID file if it is; use 'read-pid-file' instead
> of a potentially endless loop.
> ---
>  gnu/services/web.scm | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/gnu/services/web.scm b/gnu/services/web.scm
> index 3778efd04..1c993b29f 100644
> --- a/gnu/services/web.scm
> +++ b/gnu/services/web.scm
> @@ -610,14 +610,12 @@ of index files."
>                   (match '#$args
>                     (("-s" . _) #t)
>                     (_
> -                    (let loop ((duration 0))
> -                      ;; 
> https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1581864/comments/7
> -                      (sleep duration)
> -                      (if (file-exists? #$pid-file)
> -                          (let ((pid (call-with-input-file #$pid-file read)))
> -                            ;; it could be #<eof>
> -                            (if (integer? pid) pid (loop 1)))
> -                          (loop 1)))))))))
> +                    ;; When FILE is true, we cannot be sure that PID-FILE 
> will
> +                    ;; be created, so assume it won't show up.  When FILE is
> +                    ;; false, read PID-FILE.
> +                    #$(if file
> +                          #~#t
> +                          #~(read-pid-file #$pid-file))))))))
>  
>       ;; TODO: Add 'reload' action.
>       (list (shepherd-service
> -- 
> 2.18.0
>
> From 211a820dbd37926f07f9245ab42cbaf6fb0264bb Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <address@hidden>
> Date: Sat, 8 Sep 2018 18:50:55 +0200
> Subject: [PATCH 2/2] services: nginx: 'stop' returns #f.
>
> Previously we'd return #t, which the Shepherd would consider a failure
> to stop the service.
>
> * gnu/services/web.scm (nginx-shepherd-service): In 'nginx-action',
> return #f when stopping the service.
> ---
>  gnu/services/web.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/services/web.scm b/gnu/services/web.scm
> index 1c993b29f..df82a6de6 100644
> --- a/gnu/services/web.scm
> +++ b/gnu/services/web.scm
> @@ -608,7 +608,7 @@ of index files."
>                                 (default-nginx-config config))
>                           address@hidden)
>                   (match '#$args
> -                   (("-s" . _) #t)
> +                   (("-s" . _) #f)
>                     (_
>                      ;; When FILE is true, we cannot be sure that PID-FILE 
> will
>                      ;; be created, so assume it won't show up.  When FILE is