From: ludo@gnu.org (Ludovic Courtès)
To: Jan Nieuwenhuizen <janneke@gnu.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: rottlog: rotate messages daily.
Date: Tue, 20 Sep 2016 14:15:58 +0900 [thread overview]
Message-ID: <87wpi7p2wh.fsf@gnu.org> (raw)
In-Reply-To: <874m5itjv5.fsf@gnu.org> (Jan Nieuwenhuizen's message of "Wed, 14 Sep 2016 20:30:06 +0200")
Hello Jan!
Jan Nieuwenhuizen <janneke@gnu.org> skribis:
> Ludovic Courtès writes:
>
>> I would add /var/log/{secure,shepherd.log}, but weekly is probably
>> enough.
>
> Ok. I implemented this by changing the rottlog package that already
> modifies etc/weekly.
The current ‘tweak-rc-weekly’ phase “fixes a bug” in etc/weekly, but I
don’t think we should do customization in the package itself. Instead,
it would be more appropriate to generate/provide the configuration files
that we need, without further modifying the package itself. WDYT?
> I'm not sure if sending SIGHUP to syslog is ok for shepherd.log or
> that would need to be a kill 1? See attached.
GuixSD runs syslogd from Inetutils, so it’s not PID 1, fortunately. ;-)
However, shepherd.log is written by PID_1, indeed, and we shouldn’t send
SIGHUP to PID 1. However, I don’t think anything bad will happen if
rottlog moves shepherd.log and Shepherd doesn’t reopen this file.
>>> +(define* (rottlog-service
>>
>> This can be omitted. It’s enough to expose ‘rottlog-service-type’ and
>> ‘rottlog-configuration’.
>
> Hmm. I removed all the defaulting here and then removed the function
> alltogether, but then I cannot seem to do
>
> (services (cons* (mcron-service) (rottlog-service) %base-services)))
>
> anymore. Is that right?
Yes. Instead, you have to write:
(services (cons* (service rottlog-service-type
(rottlog-configuration …))
…))
> I'm feeling a bit stubborn keeping this but I thought that's what we
> want and my test showed me that this is needed. So I kept a minimal of
> this function. Please just remove+enlighten me how to use it if this
> can really go :-)
I hope the above sheds some light on this. WDYT of this idea?
> From e8e489db62337c6e8ef03e745a56938566e078c8 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Wed, 14 Sep 2016 19:04:38 +0200
> Subject: [PATCH 1/2] gnu: update rottlog: install guix-specific etc/weekly.
>
> * gnu/packages/admin.scm (rottlog): Install guix-specific etc/weekly
> for rotating /var/log/{messages,secureshepherd.log}.
As discussed above, I think this is the wrong place for this.
> From 59213cce5d6d4e41f9b6f321e3fef056cccc7c22 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Thu, 8 Sep 2016 01:20:43 +0200
> Subject: [PATCH 2/2] gnu: services: add rottlog.
>
> * gnu/services/admin.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi (Log Rotation): Document it.
[...]
> + (periodic-rotations rottlog-periodic-rotations ; list of (name file) tuples
> + (default `(("weekly"
> + ,(file-append rottlog "/etc/weekly")))))
Here I would suggest adding somewhere:
(define %rotated-files
'("/var/log/messages" …))
(define (syslog-rotation-config file)
(string-append file " {
…
kill -HUP …
…")))
(define (simple-rotation-config file)
;; Same as above, but without “kill -HUP”.
…)
(define %default-rotations
`(("weekly" . ,(plain-file "rottlog.weekly"
(string-append (string-join
(map syslog-rotation-config
'("/var/log/messages"…)))
(simple-rotation-config
"shepherd.log"))))))
Does that make sense?
> + (jobs rottlog-jobs ; list of <mcron-job>
> + (default
> + (list #~(job
> + '(next-hour '(0))
> + (lambda ()
> + (system (string-append #$rottlog "/sbin/rottlog"))))
> + #~(job
> + '(next-hour '(12))
> + (lambda ()
> + (system (string-append #$rottlog "/sbin/rottlog"))))))))
Please move (list …) to a global variable, to avoid code duplication
when the macro is expanded.
Use ‘system*’ instead of ‘system’ (the latter does “/bin/sh -c”, which
is unnecessary here.)
Also, this should use the ‘rottlog’ package of <rottlog-configuration>,
which cannot be referred to from the default value, which is a constant.
Thus, you may have to change the default to #f, and generate the default
value upon #f.
TIA!
Ludo’.
next prev parent reply other threads:[~2016-09-20 13:43 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-11 10:52 [PATCH] gnu: rottlog: rotate messages daily Jan Nieuwenhuizen
2016-09-07 9:52 ` Ludovic Courtès
2016-09-07 12:32 ` Jan Nieuwenhuizen
2016-09-09 5:26 ` Jan Nieuwenhuizen
2016-09-11 13:30 ` Ludovic Courtès
2016-09-12 21:35 ` Jan Nieuwenhuizen
2016-09-13 21:41 ` Ludovic Courtès
2016-09-14 18:30 ` Jan Nieuwenhuizen
2016-09-20 5:15 ` Ludovic Courtès [this message]
2016-09-20 20:47 ` Jan Nieuwenhuizen
2016-09-21 15:32 ` Ludovic Courtès
2016-09-22 20:51 ` Jan Nieuwenhuizen
2016-10-02 22:39 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wpi7p2wh.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=guix-devel@gnu.org \
--cc=janneke@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).