diff mbox

gnu: rottlog: rotate messages daily.

Message ID 87eg4epach.fsf@gnu.org
State New
Headers show

Commit Message

Jan Nieuwenhuizen Sept. 20, 2016, 8:47 p.m. UTC
Ludovic Courtès writes:

Hi Ludo,!

> 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 was thinking it does make some sense to have the installed
...-rottlog/etc/weekly file be functional and somewhat sensible for
GuixSD.  Otoh, patching rottlog with our specific configuration feels
wrong.

So let's do it like you suggest, I think that makes just a bit more
sense.

>> 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.

Ok, so no kill then for shepherd.log.

> Yes.  Instead, you have to write:
>
>   (services (cons* (service rottlog-service-type
>                             (rottlog-configuration …))
>                    …))

Okay.

> I hope the above sheds some light on this.  WDYT of this idea?

>> * 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.

Sure, removed.

>> +  (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 …
>     …")))

I did not get `.*/bin/kill' right.  There's a big FIXME in the patch.
My first naive attempt (string-append coreutils "/bin/kill") does not
work and I tried several #~ things but I don't really know what I'm
doing there...thats still mostly magic to me.

>   (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?

Yes, better.  Added something like this.

>> +  (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.

...moved to a function now...but I don't see what macro you mean (#~ ?)
and when it gets expanded and how that leads to duplication.

I'm not sure if we should export the %default-rotations or if we should
describe their contents in the manual.

> Use ‘system*’ instead of ‘system’ (the latter does “/bin/sh -c”, which
> is unnecessary here.)

Ok.

> 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.

Ahh.  Okay, (I think!).

Greetings,
Jan

Comments

Ludovic Courtès Sept. 21, 2016, 3:32 p.m. UTC | #1
Hello!

Jan Nieuwenhuizen <janneke@gnu.org> skribis:

> Ludovic Courtès writes:

[...]

>>> +  (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.
>
> ...moved to a function now...but I don't see what macro you mean (#~ ?)
> and when it gets expanded and how that leads to duplication.

The ‘rottlog-configuration’ is actually a macro.  So at every call site
where a default value is used, the default value code is duplicated,
leading to code bloat if that default value is a big expression.

> I'm not sure if we should export the %default-rotations or if we should
> describe their contents in the manual.

Probably worth exporting and documenting, so people can ‘cons’ on it.

> From ee1be88f60d70de46009069da020c1bdc4993fd8 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Thu, 8 Sep 2016 01:20:43 +0200
> Subject: [PATCH] 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.

[...]

> +(define (rotation-config file kill)
> +  (string-append file " {
> +	sharedscripts
> +	postrotate
> +" kill
> +"	endscript
> +	nocompress
> +}
> +"))
> +
> +(define (syslog-rotation-config file)
> +  (let ((coreutils "/gnu/store/56x9fvx59i300wav3c193h84cp80bslr-coreutils-8.25")) ;; FIXME

Good point…

> +    (rotation-config
> +     file
> +     (string-append
> +      "	"
> +      coreutils "/bin/kill -HUP $(cat /var/run/syslog.pid) 2> /dev/null
> +"))))
> +
> +(define (simple-rotation-config file)
> +  (rotation-config file ""))
> +
> +(define %default-rotations
> +  `(("weekly" ,(plain-file "rottlog.weekly"
> +                           (string-append (string-join
> +                                           (map syslog-rotation-config
> +                                                %rotated-files)
> +                                           "")
> +                                          (simple-rotation-config
> +                                           "/var/log/shepherd.log"))))))

I think we cannot use ‘plain-file’ here because of the computed ‘kill’
file name.  So instead, this would be something along the lines of
(moving ‘string-append’ from the host side to the build side):

  (define (syslog-rotation-config file)
    #~(string-append #$file " {\n" …
                     #$coreutils "/bin/kill -HUP …"
                     "}\n"))

and:

  (define %default-rotations
    `(("weekly" ,(computed-file "rottlog.weekly"
                                #~(call-with-output-file #$output
                                    (lambda (port)
                                      (display #$(syslog-rotation-file …)
                                               port)))))))
HTH!

Thanks again for taking the time and coping with half-baked advice!
;-)

Ludo’.
diff mbox

Patch

From ee1be88f60d70de46009069da020c1bdc4993fd8 Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen <janneke@gnu.org>
Date: Thu, 8 Sep 2016 01:20:43 +0200
Subject: [PATCH] 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.
---
 doc/guix.texi          |  55 ++++++++++++++++++++++-
 gnu/local.mk           |   1 +
 gnu/services/admin.scm | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+), 1 deletion(-)
 create mode 100644 gnu/services/admin.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index d5ece55..735e8ac 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26,7 +26,8 @@  Copyright @copyright{} 2016 Ben Woodcroft@*
 Copyright @copyright{} 2016 Chris Marusich@*
 Copyright @copyright{} 2016 Efraim Flashner@*
 Copyright @copyright{} 2016 John Darrington@*
-Copyright @copyright{} 2016 ng0
+Copyright @copyright{} 2016 ng0@*
+Copyright @copyright{} 2016 Jan Nieuwenhuizen
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -213,6 +214,7 @@  Services
 
 * Base Services::               Essential system services.
 * Scheduled Job Execution::     The mcron service.
+* Log Rotation::                The rottlog service.
 * Networking Services::         Network setup, SSH daemon, etc.
 * X Window::                    Graphical display.
 * Desktop Services::            D-Bus and desktop services.
@@ -7622,6 +7624,7 @@  declaration.
 @menu
 * Base Services::               Essential system services.
 * Scheduled Job Execution::     The mcron service.
+* Log Rotation::                The rottlog service.
 * Networking Services::         Network setup, SSH daemon, etc.
 * X Window::                    Graphical display.
 * Desktop Services::            D-Bus and desktop services.
@@ -8096,6 +8099,56 @@  specifications,, mcron, GNU@tie{}mcron}).
 @end deftp
 
 
+@node Log Rotation
+@subsubsection Log Rotation
+
+@cindex rottlog
+@cindex log rotation
+The @code{(gnu services admin)} module provides an interface to
+GNU@tie{}rottlog, a log rotator @pxref{rottlog,,,GNU Rot[t]log Manual}.
+
+The example below defines an operating system that provides
+log rotation.
+
+@lisp
+(use-modules (guix) (gnu) (gnu services admin) (gnu services mcron))
+(use-package-modules base idutils)
+
+(operating-system
+  ;; @dots{}
+  (services (cons* (mcron-service)
+                   (service rottlog-service-type (rottlog-configuration)) 
+                   %base-services)))
+@end lisp
+
+@defvr {Scheme Variable} rottlog-service-type
+This is the type of the @code{rottlog} service, whose value is an
+@code{rottlog-configuration} object.
+
+This service type can define @var{mcron}-jobs (@pxref{Scheduled Job
+Execution}) to run the rottlog service.
+@end defvr
+
+@deftp {Data Type} rottlog-configuration
+Data type representing the configuration of rottlog.
+
+@table @asis
+@item @code{rottlog} (default: @var{rottlog})
+The rottlog package to use.
+
+@item @code{rc-file} (default: (file-append rottlog "/etc/rc")
+The rottlog @var{rc-file} to use.
+
+@item @code{periodic-rotations} (default: `(("weekly" %default-rotatations)))
+A list of rottlog period-name/period-config tuples.
+
+@item @code{jobs}
+This is a list of gexps (@pxref{G-Expressions}), where each gexp
+corresponds to an mcron job specification (@pxref{Syntax, mcron job
+specifications,, mcron, GNU@tie{}mcron}).
+@end table
+@end deftp
+
 @node Networking Services
 @subsubsection Networking Services
 
diff --git a/gnu/local.mk b/gnu/local.mk
index 0da41f7..baa10f9 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -381,6 +381,7 @@  GNU_SYSTEM_MODULES =				\
   %D%/packages/zip.scm				\
 						\
   %D%/services.scm				\
+  %D%/services/admin.scm			\
   %D%/services/avahi.scm			\
   %D%/services/base.scm				\
   %D%/services/databases.scm			\
diff --git a/gnu/services/admin.scm b/gnu/services/admin.scm
new file mode 100644
index 0000000..08c9ab7
--- /dev/null
+++ b/gnu/services/admin.scm
@@ -0,0 +1,116 @@ 
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of thye GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services admin)
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages base)
+  #:use-module (gnu services)
+  #:use-module (gnu services mcron)
+  #:use-module (gnu services shepherd)
+  #:use-module (guix gexp)
+  #:use-module (guix packages)
+  #:use-module (guix records)
+  #:export (rottlog-configuration
+            rottlog-configuration?
+            rottlog-service
+            rottlog-service-type))
+
+;;; Commentary:
+;;;
+;;; This module implements configuration of rottlog by writing
+;;; /etc/rottlog/{rc,hourly|daily|weekly}.  Example usage
+;;; 
+;;;     (mcron-service)
+;;;     (service rottlog-service-typeo (rottlog-configuration))
+;;;
+;;; Code:
+
+(define %rotated-files
+  '("/var/log/messages" "/var/log/secure"))
+
+(define (rotation-config file kill)
+  (string-append file " {
+	sharedscripts
+	postrotate
+" kill
+"	endscript
+	nocompress
+}
+"))
+
+(define (syslog-rotation-config file)
+  (let ((coreutils "/gnu/store/56x9fvx59i300wav3c193h84cp80bslr-coreutils-8.25")) ;; FIXME
+    (rotation-config
+     file
+     (string-append
+      "	"
+      coreutils "/bin/kill -HUP $(cat /var/run/syslog.pid) 2> /dev/null
+"))))
+
+(define (simple-rotation-config file)
+  (rotation-config file ""))
+
+(define %default-rotations
+  `(("weekly" ,(plain-file "rottlog.weekly"
+                           (string-append (string-join
+                                           (map syslog-rotation-config
+                                                %rotated-files)
+                                           "")
+                                          (simple-rotation-config
+                                           "/var/log/shepherd.log"))))))
+
+(define (default-jobs rottlog)
+  (list #~(job
+           '(next-hour '(0))
+           (lambda ()
+             (system* (string-append #$rottlog "/sbin/rottlog"))))
+        #~(job
+           '(next-hour '(12))
+           (lambda ()
+             (system* (string-append #$rottlog "/sbin/rottlog"))))))
+
+(define-record-type* <rottlog-configuration>
+  rottlog-configuration make-rottlog-configuration
+  rottlog-configuration?
+  (rottlog            rottlog-rottlog            ; package
+                      (default rottlog))
+  (rc-file            rottlog-rc-file            ; file
+                      (default (file-append rottlog "/etc/rc")))
+  (periodic-rotations rottlog-periodic-rotations ; list of (name file) tuples
+                      (default %default-rotations))
+  (jobs               rottlog-jobs               ; list of <mcron-job>
+                      (default #f)))
+
+(define (rottlog-etc config)
+  `(("rottlog" ,(file-union "rottlog"
+                            (cons `("rc" ,(rottlog-rc-file config))
+                                  (rottlog-periodic-rotations config))))))
+
+(define (rottlog-jobs-or-default config)
+  (or (rottlog-jobs config)
+      (default-jobs (rottlog-rottlog config))))
+
+(define rottlog-service-type
+  (service-type
+   (name 'rottlog)
+   (extensions
+    (list
+     (service-extension etc-service-type rottlog-etc)
+     (service-extension mcron-service-type rottlog-jobs-or-default)))))
+
+;;; admin.scm ends here
-- 
2.10.0