diff mbox series

[bug#62802,1/4] services: syslog: Move configuration to /etc/syslog.conf.

Message ID 20230413012408.2759-1-maxim.cournoyer@gmail.com
State New
Headers show
Series Add reload action to syslog service. | expand

Commit Message

Maxim Cournoyer April 13, 2023, 1:24 a.m. UTC
Having the configuration live at a static location makes it possible to
hot-reload it.

* gnu/services/base.scm (syslog.conf): New variable.
(syslog-etc, syslog-shepherd-service): New procedures.
(syslog-service-type): Rewrite using the above new variable and procedures,
extending etc-service-type with its configuration file.
---

 gnu/services/base.scm | 61 ++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 24 deletions(-)


base-commit: 0fe2c78cac19acfb46c3bc365075293e51e0e5aa

Comments

Ludovic Courtès April 20, 2023, 3:22 p.m. UTC | #1
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Having the configuration live at a static location makes it possible to
> hot-reload it.
>
> * gnu/services/base.scm (syslog.conf): New variable.
> (syslog-etc, syslog-shepherd-service): New procedures.
> (syslog-service-type): Rewrite using the above new variable and procedures,
> extending etc-service-type with its configuration file.

I’m really not a fan of static configuration file names: you can never
be sure what config the service is using—compare this with the
unambiguous ‘--config=/gnu/store/…example.conf’.

Unfortunately there’s often no other option if we want to support live
reconfiguration—there’s only so much a signal can convey.

So I guess it’s a “weak accept” from me, because live reload is useful.

With the Shepherd in ‘master’, there’s a hook to change a service’s
“running value” so it should be possible to stop the previous process,
start a new one, and update the service’s running value (which is not
equivalent to SIGHUP, but maybe good enough for some cases).

A simpler approach might be run the service in a container with
/gnu/store/…conf mapped to a fixed location, and somehow update that
mapping as we go.  Food for thought!

Ludo’.
Maxim Cournoyer April 21, 2023, 12:50 p.m. UTC | #2
Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Having the configuration live at a static location makes it possible to
>> hot-reload it.
>>
>> * gnu/services/base.scm (syslog.conf): New variable.
>> (syslog-etc, syslog-shepherd-service): New procedures.
>> (syslog-service-type): Rewrite using the above new variable and procedures,
>> extending etc-service-type with its configuration file.
>
> I’m really not a fan of static configuration file names: you can never
> be sure what config the service is using—compare this with the
> unambiguous ‘--config=/gnu/store/…example.conf’.

Right; although now if you aren't sure what is used you can 'reload' it,
eh :-).

> Unfortunately there’s often no other option if we want to support live
> reconfiguration—there’s only so much a signal can convey.
>
> So I guess it’s a “weak accept” from me, because live reload is useful.

OK!

> With the Shepherd in ‘master’, there’s a hook to change a service’s
> “running value” so it should be possible to stop the previous process,
> start a new one, and update the service’s running value (which is not
> equivalent to SIGHUP, but maybe good enough for some cases).

Wouldn't that be equivalent to restarting the service?  I wasn't aware
of the new hook facility, I'll have to read on it, thanks!

> A simpler approach might be run the service in a container with
> /gnu/store/…conf mapped to a fixed location, and somehow update that
> mapping as we go.  Food for thought!

Interesting idea... although it'd only be compatible with Linux and I
dislike writing special cases in services (or anywhere if I can help
it).
Ludovic Courtès April 21, 2023, 2:03 p.m. UTC | #3
Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I’m really not a fan of static configuration file names: you can never
>> be sure what config the service is using—compare this with the
>> unambiguous ‘--config=/gnu/store/…example.conf’.
>
> Right; although now if you aren't sure what is used you can 'reload' it,
> eh :-).

True.  :-)  The other issue is that that makes it impossible to run
several instances of the service (not a problem for syslogd of course,
but could be an issue elsewhere).

>> With the Shepherd in ‘master’, there’s a hook to change a service’s
>> “running value” so it should be possible to stop the previous process,
>> start a new one, and update the service’s running value (which is not
>> equivalent to SIGHUP, but maybe good enough for some cases).
>
> Wouldn't that be equivalent to restarting the service?  I wasn't aware
> of the new hook facility, I'll have to read on it, thanks!

There’s nothing to read :-) and it wasn’t designed with that use case in
mind, but we’ll see.

>> A simpler approach might be run the service in a container with
>> /gnu/store/…conf mapped to a fixed location, and somehow update that
>> mapping as we go.  Food for thought!
>
> Interesting idea... although it'd only be compatible with Linux and I
> dislike writing special cases in services (or anywhere if I can help
> it).

Yeah.

Ludo’.
diff mbox series

Patch

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index e5c6bf5335..1ed874aa84 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -15,7 +15,7 @@ 
 ;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021 Hui Lu <luhuins@163.com>
-;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 muradm <mail@muradm.net>
 ;;; Copyright © 2022 Guillaume Le Vaillant <glv@posteo.net>
 ;;; Copyright © 2022 Justin Veilleux <terramorpha@cock.li>
@@ -1526,30 +1526,43 @@  (define-record-type* <syslog-configuration>
   (config-file          syslog-configuration-config-file
                         (default %default-syslog.conf)))
 
-(define syslog-service-type
-  (shepherd-service-type
-   'syslog
-   (lambda (config)
-     (define config-file
-       (syslog-configuration-config-file config))
+;;; Note: a static file name is used for syslog.conf so that the reload action
+;;; work as intended.
+(define syslog.conf "/etc/syslog.conf")
 
-     (shepherd-service
-      (documentation "Run the syslog daemon (syslogd).")
-      (provision '(syslogd))
-      (requirement '(user-processes))
-      (actions (list (shepherd-configuration-action config-file)))
-      (start #~(let ((spawn (make-forkexec-constructor
-                             (list #$(syslog-configuration-syslogd config)
-                                   "--rcfile" #$config-file)
-                             #:pid-file "/var/run/syslog.pid")))
-                 (lambda ()
-                   ;; Set the umask such that file permissions are #o640.
-                   (let ((mask (umask #o137))
-                         (pid  (spawn)))
-                     (umask mask)
-                     pid))))
-      (stop #~(make-kill-destructor))))
-   (syslog-configuration)
+(define (syslog-etc configuration)
+  (match-record configuration <syslog-configuration>
+    (config-file)
+    (list `(,(basename syslog.conf) ,config-file))))
+
+(define (syslog-shepherd-service config)
+  (define config-file
+    (syslog-configuration-config-file config))
+
+  (shepherd-service
+   (documentation "Run the syslog daemon (syslogd).")
+   (provision '(syslogd))
+   (requirement '(user-processes))
+   (actions (list (shepherd-configuration-action syslog.conf)))
+   (start #~(let ((spawn (make-forkexec-constructor
+                          (list #$(syslog-configuration-syslogd config)
+                                #$(string-append "--rcfile=" syslog.conf))
+                          #:pid-file "/var/run/syslog.pid")))
+              (lambda ()
+                ;; Set the umask such that file permissions are #o640.
+                (let ((mask (umask #o137))
+                      (pid  (spawn)))
+                  (umask mask)
+                  pid))))
+   (stop #~(make-kill-destructor))))
+
+(define syslog-service-type
+  (service-type
+   (name 'syslog)
+   (default-value (syslog-configuration))
+   (extensions (list (service-extension shepherd-root-service-type
+                                        (compose list syslog-shepherd-service))
+                     (service-extension etc-service-type syslog-etc)))
    (description "Run the syslog daemon, @command{syslogd}, which is
 responsible for logging system messages.")))