* [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories.
@ 2022-04-23 13:11 Liliana Marie Prikler
2022-04-30 14:15 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Liliana Marie Prikler @ 2022-04-23 13:11 UTC (permalink / raw)
To: 55080; +Cc: ludo
* 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories.
2022-04-23 13:11 [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories Liliana Marie Prikler
@ 2022-04-30 14:15 ` Ludovic Courtès
2022-04-30 14:26 ` Liliana Marie Prikler
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2022-04-30 14:15 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 55080
Hi,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories.
2022-04-30 14:15 ` Ludovic Courtès
@ 2022-04-30 14:26 ` Liliana Marie Prikler
2022-05-01 13:32 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Liliana Marie Prikler @ 2022-04-30 14:26 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 55080
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories.
2022-04-30 14:26 ` Liliana Marie Prikler
@ 2022-05-01 13:32 ` Ludovic Courtès
2022-05-01 13:50 ` Liliana Marie Prikler
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2022-05-01 13:32 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 55080
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories.
2022-05-01 13:32 ` Ludovic Courtès
@ 2022-05-01 13:50 ` Liliana Marie Prikler
2022-08-29 15:18 ` bug#55080: " Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Liliana Marie Prikler @ 2022-05-01 13:50 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 55080
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#55080: [PATCH shepherd] service: Gracefully handle non-existing log directories.
2022-05-01 13:50 ` Liliana Marie Prikler
@ 2022-08-29 15:18 ` Ludovic Courtès
0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2022-08-29 15:18 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 55080-done
Hi Liliana,
Liliana Marie Prikler <liliana.prikler@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’.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-29 15:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-23 13:11 [bug#55080] [PATCH shepherd] service: Gracefully handle non-existing log directories Liliana Marie Prikler
2022-04-30 14:15 ` Ludovic Courtès
2022-04-30 14:26 ` Liliana Marie Prikler
2022-05-01 13:32 ` Ludovic Courtès
2022-05-01 13:50 ` Liliana Marie Prikler
2022-08-29 15:18 ` bug#55080: " Ludovic Courtès
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.