GNU bug report logs - #55080
[PATCH shepherd] service: Gracefully handle non-existing log directories.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55080 in the body.
You can then email your comments to 55080 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: ludo <at> gnu.org
Subject: [PATCH shepherd] service: Gracefully handle non-existing log
 directories.
Date: Sat, 23 Apr 2022 15:11:50 +0200

* gnu/packages/services.scm (%service-file-logger): New variable,
implementing...
(service-file-logger): ... the old behaviour of this variable.  Catch system
errors from %service-file-logger and handle them.
---
 modules/shepherd/service.scm | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 013347b..567a08b 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -873,9 +873,9 @@ daemon writing FILE is running in a separate PID namespace."
               (try-again)
               (apply throw args)))))))
 
-(define (service-file-logger file input)
-  "Return a thunk meant to run as a fiber that reads from INPUT and logs it to
-FILE."
+(define (%service-file-logger file input)
+  "Like 'service-file-logger', but doesn't handle the case in which FILE does
+not exist."
   (let* ((fd     (open-fdes file (logior O_CREAT O_WRONLY O_APPEND) #o640))
          (output (fdopen fd "al")))
     (set-port-encoding! output "UTF-8")
@@ -894,6 +894,19 @@ FILE."
                  (format output "~a~a~%" prefix line)
                  (loop))))))))))
 
+(define (service-file-logger file input)
+  "Return a thunk meant to run as a fiber that reads from INPUT and logs it to
+FILE."
+  (catch 'system-error
+    (lambda ()
+      (%service-file-logger file input))
+    (lambda args
+      (if (= ENOENT (system-error-errno args))
+          (begin
+            (mkdir-p (dirname file))
+            (%service-file-logger file input))
+          (apply throw args)))))
+
 (define (service-builtin-logger command input)
   "Return a thunk meant to run as a fiber that reads from INPUT and logs to
 'log-output-port'."
-- 
2.35.1

Message #8 received at 55080 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 55080 <at> debbugs.gnu.org
Subject: Re: bug#55080: [PATCH shepherd] service: Gracefully handle
 non-existing log directories.
Date: Sat, 30 Apr 2022 16:15:33 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> * gnu/packages/services.scm (%service-file-logger): New variable,
> implementing...
> (service-file-logger): ... the old behaviour of this variable.  Catch system
> errors from %service-file-logger and handle them.

[...]

> +(define (service-file-logger file input)
> +  "Return a thunk meant to run as a fiber that reads from INPUT and logs it to
> +FILE."
> +  (catch 'system-error
> +    (lambda ()
> +      (%service-file-logger file input))
> +    (lambda args
> +      (if (= ENOENT (system-error-errno args))
> +          (begin
> +            (mkdir-p (dirname file))
> +            (%service-file-logger file input))
> +          (apply throw args)))))

I wonder to what extent automatically creating log directories is a good
idea.  A potential drawback is if shepherd creates them with unexpected
ownership or permissions.

Did you encounter this issue while working on services?

Am I right that the Shepherd 0.8 had the same problem?

Thanks,
Ludo’.

Message #11 received at 55080 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 55080 <at> debbugs.gnu.org
Subject: Re: bug#55080: [PATCH shepherd] service: Gracefully handle
 non-existing log directories.
Date: Sat, 30 Apr 2022 16:26:29 +0200

Am Samstag, dem 30.04.2022 um 16:15 +0200 schrieb Ludovic Courtès:
> 
> > +(define (service-file-logger file input)
> > +  "Return a thunk meant to run as a fiber that reads from INPUT
> > and logs it to
> > +FILE."
> > +  (catch 'system-error
> > +    (lambda ()
> > +      (%service-file-logger file input))
> > +    (lambda args
> > +      (if (= ENOENT (system-error-errno args))
> > +          (begin
> > +            (mkdir-p (dirname file))
> > +            (%service-file-logger file input))
> > +          (apply throw args)))))
> 
> I wonder to what extent automatically creating log directories is a
> good idea.  A potential drawback is if shepherd creates them with
> unexpected ownership or permissions.
As far as I know, those logs should be managed by shepherd, no?  It
just redirects stdout/stderr there, or is there something special going
on?

> Did you encounter this issue while working on services?
> 
> Am I right that the Shepherd 0.8 had the same problem?
It might be, I don't know.  I've encountered this for non-existing log
directory, so a reproducer would be setting #:log-file to $test-tmp-
directory/does-not-exist/log and check for each service.

Cheers 

Message #14 received at 55080 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 55080 <at> debbugs.gnu.org
Subject: Re: bug#55080: [PATCH shepherd] service: Gracefully handle
 non-existing log directories.
Date: Sun, 01 May 2022 15:32:35 +0200

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Samstag, dem 30.04.2022 um 16:15 +0200 schrieb Ludovic Courtès:

[...]

>> Did you encounter this issue while working on services?
>> 
>> Am I right that the Shepherd 0.8 had the same problem?
> It might be, I don't know.  I've encountered this for non-existing log
> directory, so a reproducer would be setting #:log-file to $test-tmp-
> directory/does-not-exist/log and check for each service.

Usually /var/log and similar directories are created not by shepherd but
by Guix System, the distro being used, or whatever.  That’s why I wonder
if it’s shepherd’s job to do that.

I was asking how you encountered it to better understand in which
circumstances the problem can occur in practice and what the failure
more is like.

Thanks,
Ludo’.

Message #17 received at 55080 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 55080 <at> debbugs.gnu.org
Subject: Re: bug#55080: [PATCH shepherd] service: Gracefully handle
 non-existing log directories.
Date: Sun, 01 May 2022 15:50:25 +0200

Am Sonntag, dem 01.05.2022 um 15:32 +0200 schrieb Ludovic Courtès:
> > 
> > > Did you encounter this issue while working on services?
> > > 
> > > Am I right that the Shepherd 0.8 had the same problem?
> > It might be, I don't know.  I've encountered this for non-existing
> > log directory, so a reproducer would be setting #:log-file to
> > $test-tmp-directory/does-not-exist/log and check for each service.
> 
> Usually /var/log and similar directories are created not by shepherd
> but by Guix System, the distro being used, or whatever.  That’s why I
> wonder if it’s shepherd’s job to do that.
Hmm, it might not be.  Still, I wouldn't like shepherd to fail in such
a weird manner if the log file can't be created.  Should we write a
warning to shepherd's log and redirect to /dev/null instead?  Should we
just kill the service?

Message #22 received at 55080-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 55080-done <at> debbugs.gnu.org
Subject: Re: bug#55080: [PATCH shepherd] service: Gracefully handle
 non-existing log directories.
Date: Mon, 29 Aug 2022 17:18:58 +0200

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Sonntag, dem 01.05.2022 um 15:32 +0200 schrieb Ludovic Courtès:
>> > 
>> > > Did you encounter this issue while working on services?
>> > > 
>> > > Am I right that the Shepherd 0.8 had the same problem?
>> > It might be, I don't know.  I've encountered this for non-existing
>> > log directory, so a reproducer would be setting #:log-file to
>> > $test-tmp-directory/does-not-exist/log and check for each service.
>> 
>> Usually /var/log and similar directories are created not by shepherd
>> but by Guix System, the distro being used, or whatever.  That’s why I
>> wonder if it’s shepherd’s job to do that.
> Hmm, it might not be.  Still, I wouldn't like shepherd to fail in such
> a weird manner if the log file can't be created.

I reread this thread and I concur.  Patch finally pushed as
b0d3f625543bcb32e94167c27cba153f9fc03acd.

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.