GNU bug report logs - #29483
[PATCH] services: Add openntpd service.

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 29483 in the body.
You can then email your comments to 29483 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: Efraim Flashner <efraim <at> flashner.co.il>
To: guix-patches <at> gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH] services: Add openntpd service.
Date: Tue, 28 Nov 2017 11:04:43 +0200

* gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
set openntpd daemon's user and protected path. Add a custom phase to not
try to create said directory at install time.
* gnu/services/networking.scm (<openntpd-configuration>): New record type.
(openntpd-shepherd-service, openntpd-service-activation): New procedures.
(openntpd-service-type): New variable.
* doc/guix.texi (Networking Services): Add openntpd documentation.
---
 doc/guix.texi               | 11 ++++++
 gnu/packages/ntp.scm        | 12 ++++++
 gnu/services/networking.scm | 92 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2a6825682..f0a7dd958 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10498,6 +10498,17 @@ make an initial adjustment of more than 1,000 seconds.
 List of host names used as the default NTP servers.
 @end defvr
 
+@cindex Openntpd
+@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
+  [#:servers @var{%ntp-servers}] @
+  [#:allow-large-adjustment? #f]
+Return a service that runs the daemon from @var{openntpd}, the
+@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
+keep the system clock synchronized with that of @var{servers}.
+@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
+make an initial adjustment of more than 180 seconds."
+@end deffn
+
 @cindex inetd
 @deffn {Scheme variable} inetd-service-type
 This service runs the @command{inetd} (@pxref{inetd invocation,,,
diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
index d270f513d..619b9f998 100644
--- a/gnu/packages/ntp.scm
+++ b/gnu/packages/ntp.scm
@@ -107,6 +107,18 @@ computers over a network.")
                (base32
                 "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
     (build-system gnu-build-system)
+    (arguments
+     '(#:configure-flags '("--with-privsep-user=ntpd"
+                           "--with-privsep-path=/var/lib/openntpd"
+                           "--localstatedir=/var/lib/openntpd")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'modify-install-locations
+           (lambda _
+             ;; Don't try to create /var/lib/openntpd/run or /var/lib/openntpd/db
+             (substitute* "src/Makefile.in"
+               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
+             #t)))))
     (inputs
      `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
     (home-page "http://www.openntpd.org/")
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index b0c23aafc..82762738f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw <at> netris.org>
-;;; Copyright © 2016 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2016, 2017 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2016 John Darrington <jmd <at> gnu.org>
 ;;; Copyright © 2017 Clément Lassieur <clement <at> lassieur.org>
 ;;; Copyright © 2017 Thomas Danckaert <post <at> thomasdanckaert.be>
@@ -62,6 +62,11 @@
             ntp-service
             ntp-service-type
 
+            openntpd-configuration
+            openntpd-configuration?
+            openntpd-service
+            openntpd-service-type
+
             inetd-configuration
             inetd-entry
             inetd-service-type
@@ -447,6 +452,91 @@ make an initial adjustment of more than 1,000 seconds."
                               (allow-large-adjustment?
                                allow-large-adjustment?))))
 
+(define-record-type* <openntpd-configuration>
+  openntpd-configuration make-openntpd-configuration
+  openntpd-configuration?
+  (openntpd                openntpd-configuration-openntpd
+                           (default openntpd))
+  (servers                 openntpd-configuration-servers)
+  (allow-large-adjustment? openntpd-allow-large-adjustment?
+                           (default #f))) ; upstream default
+
+(define openntpd-shepherd-service
+  (match-lambda
+    (($ <openntpd-configuration> openntpd servers allow-large-adjustment?)
+     (let ()
+       (define config
+         (string-append (string-join (map (cut string-append "server " <>)
+                                          servers)
+                                     "\n")
+                        "
+# Only listen on localhost
+listen on 127.0.0.1
+listen on ::1
+
+# Query the 'Date' from trusted HTTPS servers via TLS.
+constraint from www.gnu.org\n"))
+
+       (define ntpd.conf
+         (plain-file "ntpd.conf" config))
+
+       (list (shepherd-service
+              (provision '(openntpd))
+              (documentation "Run the Network Time Protocol (NTP) daemon.")
+              (requirement '(user-processes networking))
+              (start #~(make-forkexec-constructor
+                        (list (string-append #$openntpd "/sbin/ntpd")
+                              "-f" #$ntpd.conf
+                              #$@(if allow-large-adjustment?
+                                     '("-s")
+                                     '()))))
+              (stop #~(make-kill-destructor))))))))
+
+(define (openntpd-service-activation config)
+  "Return the activation gexp for CONFIG."
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+        (define %user
+          (getpw "ntpd"))
+
+        (let ((directory "/var/lib/openntpd"))
+          (mkdir-p directory)
+          ;; and for the socket
+          (mkdir-p (string-append directory "/db"))
+          (mkdir-p (string-append directory "/run"))
+          (chown directory (passwd:uid %user) (passwd:gid %user))
+          (chmod directory #o755)))))
+
+(define openntpd-service-type
+  (service-type (name 'openntpd)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          openntpd-shepherd-service)
+                       (service-extension account-service-type
+                                          (const %ntp-accounts))
+                       (service-extension activation-service-type
+                                          openntpd-service-activation)))
+                (description
+                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
+daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
+implemented by OpenNTPD.  The daemon will keep the system clock synchronized
+with that of the given servers.")))
+
+(define* (openntpd-service #:key (openntpd openntpd)
+                           (servers %ntp-servers)
+                           allow-large-adjustment?)
+  "Return a service that runs the daemon from @var{openntpd}, the
+@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
+keep the system clock synchronized with that of @var{servers}.
+@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
+make an initial adjustment of more than 180 seconds."
+  (service openntpd-service-type
+           (openntpd-configuration (openntpd openntpd)
+                              (servers servers)
+                              (allow-large-adjustment?
+                               allow-large-adjustment?))))
+
 
 ;;;
 ;;; Inetd.
-- 
2.15.0

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Fri, 01 Dec 2017 11:19:57 +0100

Hello!

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and protected path. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.

Nice!

> +@cindex Openntpd

“OpenNTPD” maybe?  Or all lower case?

> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
> +  [#:servers @var{%ntp-servers}] @
> +  [#:allow-large-adjustment? #f]
> +Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +@end deffn

The convention now is to expose and document the configuration record
type and the service type, and to not provide a “foo-service” procedure.

Could you adjust accordingly?

> +(define-record-type* <openntpd-configuration>
> +  openntpd-configuration make-openntpd-configuration
> +  openntpd-configuration?
> +  (openntpd                openntpd-configuration-openntpd
> +                           (default openntpd))
> +  (servers                 openntpd-configuration-servers)

Probably with: (default %ntp-servers).

> +# Only listen on localhost
> +listen on 127.0.0.1
> +listen on ::1
> +
> +# Query the 'Date' from trusted HTTPS servers via TLS.
> +constraint from www.gnu.org\n"))

It would be nice to make that constraint server configurable too (not a
blocker though).

> +       (list (shepherd-service
> +              (provision '(openntpd))

Perhaps we should make that ‘ntpd’ so that it conflicts with the other
ntpd.

> +(define openntpd-service-type
> +  (service-type (name 'openntpd)
> +                (extensions
> +                 (list (service-extension shepherd-root-service-type
> +                                          openntpd-shepherd-service)
> +                       (service-extension account-service-type
> +                                          (const %ntp-accounts))

Are you sure that it uses those accounts?

> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
          ^---- remove -------------------------------------------^
> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
> +with that of the given servers.")))
> +
> +(define* (openntpd-service #:key (openntpd openntpd)
> +                           (servers %ntp-servers)
> +                           allow-large-adjustment?)

Remove.

Could you send an updated patch?

Thank you!

Ludo’.

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

From: Marius Bakke <mbakke <at> fastmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>, 29483 <at> debbugs.gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Sun, 03 Dec 2017 20:24:13 +0100

[Message part 1 (text/plain, inline)]

Efraim Flashner <efraim <at> flashner.co.il> writes:

> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and protected path. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.
> ---
>  doc/guix.texi               | 11 ++++++
>  gnu/packages/ntp.scm        | 12 ++++++
>  gnu/services/networking.scm | 92 ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 2a6825682..f0a7dd958 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -10498,6 +10498,17 @@ make an initial adjustment of more than 1,000 seconds.
>  List of host names used as the default NTP servers.
>  @end defvr
>  
> +@cindex Openntpd
> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
> +  [#:servers @var{%ntp-servers}] @
> +  [#:allow-large-adjustment? #f]
> +Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +@end deffn
> +
>  @cindex inetd
>  @deffn {Scheme variable} inetd-service-type
>  This service runs the @command{inetd} (@pxref{inetd invocation,,,
> diff --git a/gnu/packages/ntp.scm b/gnu/packages/ntp.scm
> index d270f513d..619b9f998 100644
> --- a/gnu/packages/ntp.scm
> +++ b/gnu/packages/ntp.scm
> @@ -107,6 +107,18 @@ computers over a network.")
>                 (base32
>                  "0fn12i4kzsi0zkr4qp3dp9bycmirnfapajqvdfx02zhr4hanj0kv"))))
>      (build-system gnu-build-system)
> +    (arguments
> +     '(#:configure-flags '("--with-privsep-user=ntpd"
> +                           "--with-privsep-path=/var/lib/openntpd"
> +                           "--localstatedir=/var/lib/openntpd")

Do we have to change localstatedir?  Would it work to create
/var/run/ntpd.sock and chown it?  Or is this the common way of
deployment?  No strong opinion though.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'modify-install-locations
> +           (lambda _
> +             ;; Don't try to create /var/lib/openntpd/run or /var/lib/openntpd/db
> +             (substitute* "src/Makefile.in"
> +               (("DESTDIR\\)\\$\\(localstatedir") "TMPDIR"))
> +             #t)))))
>      (inputs
>       `(("libressl" ,libressl))) ; enable TLS time constraints. See ntpd.conf(5).
>      (home-page "http://www.openntpd.org/")
> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
> index b0c23aafc..82762738f 100644
> --- a/gnu/services/networking.scm
> +++ b/gnu/services/networking.scm
> @@ -1,7 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
>  ;;; Copyright © 2015 Mark H Weaver <mhw <at> netris.org>
> -;;; Copyright © 2016 Efraim Flashner <efraim <at> flashner.co.il>
> +;;; Copyright © 2016, 2017 Efraim Flashner <efraim <at> flashner.co.il>
>  ;;; Copyright © 2016 John Darrington <jmd <at> gnu.org>
>  ;;; Copyright © 2017 Clément Lassieur <clement <at> lassieur.org>
>  ;;; Copyright © 2017 Thomas Danckaert <post <at> thomasdanckaert.be>
> @@ -62,6 +62,11 @@
>              ntp-service
>              ntp-service-type
>  
> +            openntpd-configuration
> +            openntpd-configuration?
> +            openntpd-service
> +            openntpd-service-type
> +
>              inetd-configuration
>              inetd-entry
>              inetd-service-type
> @@ -447,6 +452,91 @@ make an initial adjustment of more than 1,000 seconds."
>                                (allow-large-adjustment?
>                                 allow-large-adjustment?))))
>  
> +(define-record-type* <openntpd-configuration>
> +  openntpd-configuration make-openntpd-configuration
> +  openntpd-configuration?
> +  (openntpd                openntpd-configuration-openntpd
> +                           (default openntpd))
> +  (servers                 openntpd-configuration-servers)
> +  (allow-large-adjustment? openntpd-allow-large-adjustment?
> +                           (default #f))) ; upstream default
> +
> +(define openntpd-shepherd-service
> +  (match-lambda
> +    (($ <openntpd-configuration> openntpd servers allow-large-adjustment?)
> +     (let ()
> +       (define config
> +         (string-append (string-join (map (cut string-append "server " <>)
> +                                          servers)
> +                                     "\n")
> +                        "
> +# Only listen on localhost
> +listen on 127.0.0.1
> +listen on ::1
> +
> +# Query the 'Date' from trusted HTTPS servers via TLS.
> +constraint from www.gnu.org\n"))

It would be good if these options are configurable.  A user may want to
use a different constraint server, or none at all, and maybe also expose
this as an SNTP service.  IIRC constraints can also be specified
multiple times, so maybe add #:listen-on and #:constraints ?

It would also be great to have a system test that at least verifies that
the default configuration is okay.  Testing NTP functionality may be
trickier.

> +
> +       (define ntpd.conf
> +         (plain-file "ntpd.conf" config))
> +
> +       (list (shepherd-service
> +              (provision '(openntpd))
> +              (documentation "Run the Network Time Protocol (NTP) daemon.")
> +              (requirement '(user-processes networking))
> +              (start #~(make-forkexec-constructor
> +                        (list (string-append #$openntpd "/sbin/ntpd")
> +                              "-f" #$ntpd.conf
> +                              #$@(if allow-large-adjustment?
> +                                     '("-s")
> +                                     '()))))
> +              (stop #~(make-kill-destructor))))))))
> +
> +(define (openntpd-service-activation config)
> +  "Return the activation gexp for CONFIG."
> +  (with-imported-modules '((guix build utils))
> +    #~(begin
> +        (use-modules (guix build utils))
> +        (define %user
> +          (getpw "ntpd"))
> +
> +        (let ((directory "/var/lib/openntpd"))
> +          (mkdir-p directory)
> +          ;; and for the socket
> +          (mkdir-p (string-append directory "/db"))
> +          (mkdir-p (string-append directory "/run"))
> +          (chown directory (passwd:uid %user) (passwd:gid %user))
> +          (chmod directory #o755)))))
> +
> +(define openntpd-service-type
> +  (service-type (name 'openntpd)
> +                (extensions
> +                 (list (service-extension shepherd-root-service-type
> +                                          openntpd-shepherd-service)
> +                       (service-extension account-service-type
> +                                          (const %ntp-accounts))
> +                       (service-extension activation-service-type
> +                                          openntpd-service-activation)))
> +                (description
> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
> +with that of the given servers.")))
> +
> +(define* (openntpd-service #:key (openntpd openntpd)
> +                           (servers %ntp-servers)
> +                           allow-large-adjustment?)
> +  "Return a service that runs the daemon from @var{openntpd}, the
> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 180 seconds."
> +  (service openntpd-service-type
> +           (openntpd-configuration (openntpd openntpd)
> +                              (servers servers)
> +                              (allow-large-adjustment?
> +                               allow-large-adjustment?))))
> +
>  
>  ;;;
>  ;;; Inetd.
> -- 
> 2.15.0

[signature.asc (application/pgp-signature, inline)]

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Thu, 11 Jan 2018 22:44:03 +0100

Ping!

Let’s not let bitdust settle on this patch!

Ludo’.

ludo <at> gnu.org (Ludovic Courtès) skribis:

> Hello!
>
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
>
>> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
>> set openntpd daemon's user and protected path. Add a custom phase to not
>> try to create said directory at install time.
>> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
>> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
>> (openntpd-service-type): New variable.
>> * doc/guix.texi (Networking Services): Add openntpd documentation.
>
> Nice!
>
>> +@cindex Openntpd
>
> “OpenNTPD” maybe?  Or all lower case?
>
>> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
>> +  [#:servers @var{%ntp-servers}] @
>> +  [#:allow-large-adjustment? #f]
>> +Return a service that runs the daemon from @var{openntpd}, the
>> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
>> +keep the system clock synchronized with that of @var{servers}.
>> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
>> +make an initial adjustment of more than 180 seconds."
>> +@end deffn
>
> The convention now is to expose and document the configuration record
> type and the service type, and to not provide a “foo-service” procedure.
>
> Could you adjust accordingly?
>
>> +(define-record-type* <openntpd-configuration>
>> +  openntpd-configuration make-openntpd-configuration
>> +  openntpd-configuration?
>> +  (openntpd                openntpd-configuration-openntpd
>> +                           (default openntpd))
>> +  (servers                 openntpd-configuration-servers)
>
> Probably with: (default %ntp-servers).
>
>> +# Only listen on localhost
>> +listen on 127.0.0.1
>> +listen on ::1
>> +
>> +# Query the 'Date' from trusted HTTPS servers via TLS.
>> +constraint from www.gnu.org\n"))
>
> It would be nice to make that constraint server configurable too (not a
> blocker though).
>
>> +       (list (shepherd-service
>> +              (provision '(openntpd))
>
> Perhaps we should make that ‘ntpd’ so that it conflicts with the other
> ntpd.
>
>> +(define openntpd-service-type
>> +  (service-type (name 'openntpd)
>> +                (extensions
>> +                 (list (service-extension shepherd-root-service-type
>> +                                          openntpd-shepherd-service)
>> +                       (service-extension account-service-type
>> +                                          (const %ntp-accounts))
>
> Are you sure that it uses those accounts?
>
>> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
>> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
>           ^---- remove -------------------------------------------^
>> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
>> +with that of the given servers.")))
>> +
>> +(define* (openntpd-service #:key (openntpd openntpd)
>> +                           (servers %ntp-servers)
>> +                           allow-large-adjustment?)
>
> Remove.
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Sat, 20 Jan 2018 00:52:15 +0100

Ping again!  :-)

ludo <at> gnu.org (Ludovic Courtès) skribis:

> Hello!
>
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
>
>> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
>> set openntpd daemon's user and protected path. Add a custom phase to not
>> try to create said directory at install time.
>> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
>> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
>> (openntpd-service-type): New variable.
>> * doc/guix.texi (Networking Services): Add openntpd documentation.
>
> Nice!
>
>> +@cindex Openntpd
>
> “OpenNTPD” maybe?  Or all lower case?
>
>> +@deffn {Scheme Procedure} openntpd-service [#:openntpd @var{openntpd}] @
>> +  [#:servers @var{%ntp-servers}] @
>> +  [#:allow-large-adjustment? #f]
>> +Return a service that runs the daemon from @var{openntpd}, the
>> +@uref{http://www.openntpd.org, OpenNTPD package}.  The daemon will
>> +keep the system clock synchronized with that of @var{servers}.
>> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
>> +make an initial adjustment of more than 180 seconds."
>> +@end deffn
>
> The convention now is to expose and document the configuration record
> type and the service type, and to not provide a “foo-service” procedure.
>
> Could you adjust accordingly?
>
>> +(define-record-type* <openntpd-configuration>
>> +  openntpd-configuration make-openntpd-configuration
>> +  openntpd-configuration?
>> +  (openntpd                openntpd-configuration-openntpd
>> +                           (default openntpd))
>> +  (servers                 openntpd-configuration-servers)
>
> Probably with: (default %ntp-servers).
>
>> +# Only listen on localhost
>> +listen on 127.0.0.1
>> +listen on ::1
>> +
>> +# Query the 'Date' from trusted HTTPS servers via TLS.
>> +constraint from www.gnu.org\n"))
>
> It would be nice to make that constraint server configurable too (not a
> blocker though).
>
>> +       (list (shepherd-service
>> +              (provision '(openntpd))
>
> Perhaps we should make that ‘ntpd’ so that it conflicts with the other
> ntpd.
>
>> +(define openntpd-service-type
>> +  (service-type (name 'openntpd)
>> +                (extensions
>> +                 (list (service-extension shepherd-root-service-type
>> +                                          openntpd-shepherd-service)
>> +                       (service-extension account-service-type
>> +                                          (const %ntp-accounts))
>
> Are you sure that it uses those accounts?
>
>> +                 "Run the @command{ntpd}, the Network Time Protocol (NTP)
>> +daemon of the @uref{http://www.ntp.org, Network Time Foundation}, as
>           ^---- remove -------------------------------------------^
>> +implemented by OpenNTPD.  The daemon will keep the system clock synchronized
>> +with that of the given servers.")))
>> +
>> +(define* (openntpd-service #:key (openntpd openntpd)
>> +                           (servers %ntp-servers)
>> +                           allow-large-adjustment?)
>
> Remove.
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Fri, 2 Mar 2018 16:02:11 +0200

[Message part 1 (text/plain, inline)]

On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
> Heya Efraim,
> 
> > +(define openntpd-shepherd-service
> > +  (match-lambda
> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
> > +        openntpd-query-from openntpd-sensor openntpd-server
> > +        openntpd-servers openntpd-constraint-from
> > +        openntpd-constraints-from allow-large-adjustment?)
> 
> This is error prone (you could be matching the wrong fields), could you
> change that to ‘match-record’?
> 

I think this is the only thing left over. I compared my fields to
murmur, and for murmur we're looking at true/false or a single value.
Other than 'openntpd' and 'allow-large-adjustment?' each are lists
because they can all be lists, and I didn't want to make the logic phase
of generating the config file to be immensely long.

currently:

(match-lambda
  (($ <openntpd-configuration> openntpd openntpd-listen-on
      openntpd-query-from openntpd-sensor openntpd-server
      openntpd-servers openntpd-constraint-from
      openntpd-constraints-from allow-large-adjustment?)
   (let ()
     (define config
       (string-join
         (filter-map (lambda (field value)
                (string-join
                  (map (cut string-append field <> "\n")
                       value)))
              '("listen on " "query from " "sensor " "server " "servers "
                "constraint from ")
              (list openntpd-listen-on openntpd-query-from openntpd-sensor
                    openntpd-server openntpd-servers openntpd-constraint-from))
         ;; The 'constraints from' field needs to be enclosed in double quotes.
         (string-join
           (map (cut string-append "constraints from \"" <> "\"\n")
                openntpd-constraints-from))))

Other wise I suppose I'd be looking more at [the following] for most fields:

(match-record
...
  (if (not (null-list? openntpd-listen-on))
    (lambda (value)
      (string-append "listen on " value "\n")
     value)
     '())
...

currently to use the defaults I have
  (service openntpd-service-type (openntpd-configuration))
which obviously isn't ideal.
    

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[0001-services-Add-openntpd-service.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Fri, 02 Mar 2018 17:46:30 +0100

Hello,

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
>> Heya Efraim,
>> 
>> > +(define openntpd-shepherd-service
>> > +  (match-lambda
>> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
>> > +        openntpd-query-from openntpd-sensor openntpd-server
>> > +        openntpd-servers openntpd-constraint-from
>> > +        openntpd-constraints-from allow-large-adjustment?)
>> 
>> This is error prone (you could be matching the wrong fields), could you
>> change that to ‘match-record’?
>> 
>
> I think this is the only thing left over.

To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
entirely mechanical.  The above snippet would become:

  (define (openntpd-shepherd-service config)
    (match-record config <openntpd-configuration>
      (openntpd openntpd-listen-on
       openntpd-query-from openntpd-sensor openntpd-server
       openntpd-servers openntpd-constraint-from
       openntpd-constraints-from allow-large-adjustment?)
      …))

That’s all I was suggesting.  The body of that procedure can remain
unchanged.

Does that make sense?

Thanks,
Ludo’.

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Sun, 4 Mar 2018 20:02:43 +0200

[Message part 1 (text/plain, inline)]

On Fri, Mar 02, 2018 at 05:46:30PM +0100, Ludovic Courtès wrote:
> Hello,
> 
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> > On Mon, Feb 05, 2018 at 04:26:52PM +0100, Ludovic Courtès wrote:
> >> Heya Efraim,
> >> 
> >> > +(define openntpd-shepherd-service
> >> > +  (match-lambda
> >> > +    (($ <openntpd-configuration> openntpd openntpd-listen-on
> >> > +        openntpd-query-from openntpd-sensor openntpd-server
> >> > +        openntpd-servers openntpd-constraint-from
> >> > +        openntpd-constraints-from allow-large-adjustment?)
> >> 
> >> This is error prone (you could be matching the wrong fields), could you
> >> change that to ‘match-record’?
> >> 
> >
> > I think this is the only thing left over.
> 
> To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
> entirely mechanical.  The above snippet would become:
> 
>   (define (openntpd-shepherd-service config)
>     (match-record config <openntpd-configuration>
>       (openntpd openntpd-listen-on
>        openntpd-query-from openntpd-sensor openntpd-server
>        openntpd-servers openntpd-constraint-from
>        openntpd-constraints-from allow-large-adjustment?)
>       …))
> 
> That’s all I was suggesting.  The body of that procedure can remain
> unchanged.
> 
> Does that make sense?
> 

Yes, that does make sense. Switching helped me find that I mistakenly
used openntpd-<var> when it should've just been <var>, so I've fixed
that. I've also added a default value field, like the ones that were
recently added for the SQL services, and I tested that it worked with
(service openntpd-service-type) in my test config.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[0001-services-Add-openntpd-service.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 29483 <at> debbugs.gnu.org
Subject: Re: [bug#29483] [PATCH] services: Add openntpd service.
Date: Sun, 04 Mar 2018 23:21:21 +0100

Hello,

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> On Fri, Mar 02, 2018 at 05:46:30PM +0100, Ludovic Courtès wrote:

[...]

>> To be clear, the switch from ‘match-lambda’ to ‘match-record’ should be
>> entirely mechanical.  The above snippet would become:
>> 
>>   (define (openntpd-shepherd-service config)
>>     (match-record config <openntpd-configuration>
>>       (openntpd openntpd-listen-on
>>        openntpd-query-from openntpd-sensor openntpd-server
>>        openntpd-servers openntpd-constraint-from
>>        openntpd-constraints-from allow-large-adjustment?)
>>       …))
>> 
>> That’s all I was suggesting.  The body of that procedure can remain
>> unchanged.
>> 
>> Does that make sense?
>> 
>
> Yes, that does make sense. Switching helped me find that I mistakenly
> used openntpd-<var> when it should've just been <var>, so I've fixed
> that. I've also added a default value field, like the ones that were
> recently added for the SQL services, and I tested that it worked with
> (service openntpd-service-type) in my test config.

Awesome.

> From 0c4d07cce671ad9131416f51098082286f241046 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim <at> flashner.co.il>
> Date: Tue, 28 Nov 2017 10:19:11 +0200
> Subject: [PATCH] services: Add openntpd service.
>
> * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'configure-flags to
> set openntpd daemon's user and localstatedir. Add a custom phase to not
> try to create said directory at install time.
> * gnu/services/networking.scm (<openntpd-configuration>): New record type.
> (openntpd-shepherd-service, openntpd-service-activation): New procedures.
> (openntpd-service-type): New variable.
> * doc/guix.texi (Networking Services): Add openntpd documentation.

[...]

> +  (constraints-from        openntpd-constriants-from
                                             ^^
Typo.  :-)

Got for it!

Thank you,
Ludo’.

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: 29483-done <at> debbugs.gnu.org
Subject: re: [PATCH] services: Add openntpd service.
Date: Mon, 5 Mar 2018 11:32:32 +0200

[Message part 1 (text/plain, inline)]

done

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Previous Next

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