Message ID | 20220728113422.6739-1-guillaume.bressaix@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | package/ntpsec: add systemd unit file | expand |
Guillaume, All, On 2022-07-28 13:34 +0200, Guillaume W. Bres spake thusly: > Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com> > --- > Use -d to daemonize, use -g to allow a "big leap". > Prior synchronization, system date is usually set to 01/01/1970. > By default, ntpsec does not allow that big of a leap, > and we never obtain synchronization without -g. > Ideally, you want some sort of mechanism to only use > -g on first (ever) boot. Thos explanations should not be after a --- line, as they rally explain the change, so really must be oin the commit log, not a post-commit note. For the -g option, I would rephrase as such: Devices that have no RTC, or one that is so bad that it drifts very fast, the system time prior to synchronisation is ways off, so that ntpsec refuses to apply a giant leap to the current time, unless forced with -g. Idally, the user should be able to easy override that, so I'd add an environment file where the user can just set additional options (or remove them); see what it does with my final proposal, below... However, I think using the -d option as you did is incorrect. Indeed, the default for a systemd service unit, is Type=simple. This means that systemd does expect the program in ExecStart to be ready as soon as systemd calls execve() on it. If the program forks and the parent exits, then system will consider the system to be terminated, but will whine that there is a lingering process in the cgroup (to be confirmed). With Type=simple, its systemd that is responsible for the daemonisation of the program. The correct solution in this is either one of the following: 1. ntpsec is left in the foreground, and Type is explicitly set to 'simple' (or better yet, 'exec', and we do not use -d 2. ntpsec is set to fork with -d, but then Type is set to 'forking' The second solution is nice when the service is going to be used by dependent units (e.g. a DB server that will be used by further units later in the boot); in tha case, the program is suposed to fork after it is ready to server requests, so solution 2 is nice. However, for ntpsec, I don't think it makes much sense, because it won't "serve" anything locally. Still, I think it is still better to use solution 2, because, supposedly, ntpsec only forks when it has eventually done its initialisation step, so we know it is mostly ready. So: [Service] Type=forking EnvironmentFile=/etc/defaults/ntpsec ExecStart=/usr/sbin/ntpd -d $NTPSEC_OPTIONS and then in /etc/defaults/ntpsec: NTPSEC_OPTIONS="-g" Care to check, test, and resubmit, please. Regards, Yann E. MORIN. > --- > package/ntpsec/ntpd.service | 10 ++++++++++ > package/ntpsec/ntpsec.mk | 5 +++++ > 2 files changed, 15 insertions(+) > create mode 100644 package/ntpsec/ntpd.service > > diff --git a/package/ntpsec/ntpd.service b/package/ntpsec/ntpd.service > new file mode 100644 > index 0000000..3987085 > --- /dev/null > +++ b/package/ntpsec/ntpd.service > @@ -0,0 +1,10 @@ > +[Unit] > +Description=NTPSec > +After=network.target > +Conflicts=systemd-timesyncd.service > + > +[Service] > +ExecStart=/usr/sbin/ntpd -d -g > + > +[Install] > +WantedBy=multi-user.target > diff --git a/package/ntpsec/ntpsec.mk b/package/ntpsec/ntpsec.mk > index a0d0662..a0fc49b 100644 > --- a/package/ntpsec/ntpsec.mk > +++ b/package/ntpsec/ntpsec.mk > @@ -62,6 +62,11 @@ define NTPSEC_INSTALL_INIT_SYSV > $(INSTALL) -D -m 755 package/ntpsec/S49ntpd $(TARGET_DIR)/etc/init.d/S49ntpd > endef > > +define NTPSEC_INSTALL_INIT_SYSTEMD > + $(INSTALL) -D -m 0644 package/ntpsec/ntpd.service \ > + $(TARGET_DIR)/usr/lib/systemd/system/ntpd.service > +endef > + > define NTPSEC_USERS > ntp -1 ntp -1 * - - - ntpd user > endef > -- > 1.8.3.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
On Thu, Jul 28, 2022 at 5:35 AM Guillaume W. Bres <guillaume.bressaix@gmail.com> wrote: > > Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com> > --- > Use -d to daemonize, use -g to allow a "big leap". > Prior synchronization, system date is usually set to 01/01/1970. > By default, ntpsec does not allow that big of a leap, > and we never obtain synchronization without -g. > Ideally, you want some sort of mechanism to only use > -g on first (ever) boot. > --- > package/ntpsec/ntpd.service | 10 ++++++++++ > package/ntpsec/ntpsec.mk | 5 +++++ > 2 files changed, 15 insertions(+) > create mode 100644 package/ntpsec/ntpd.service > > diff --git a/package/ntpsec/ntpd.service b/package/ntpsec/ntpd.service > new file mode 100644 > index 0000000..3987085 > --- /dev/null > +++ b/package/ntpsec/ntpd.service > @@ -0,0 +1,10 @@ > +[Unit] > +Description=NTPSec > +After=network.target > +Conflicts=systemd-timesyncd.service > + > +[Service] > +ExecStart=/usr/sbin/ntpd -d -g You probably need to turn off dnssec validation for hostname lookups to work prior to ntp syncing. You can do that like this: https://github.com/buildroot/buildroot/blob/2022.05.1/package/openntpd/ntpd.service#L8-L11 > + > +[Install] > +WantedBy=multi-user.target > diff --git a/package/ntpsec/ntpsec.mk b/package/ntpsec/ntpsec.mk > index a0d0662..a0fc49b 100644 > --- a/package/ntpsec/ntpsec.mk > +++ b/package/ntpsec/ntpsec.mk > @@ -62,6 +62,11 @@ define NTPSEC_INSTALL_INIT_SYSV > $(INSTALL) -D -m 755 package/ntpsec/S49ntpd $(TARGET_DIR)/etc/init.d/S49ntpd > endef > > +define NTPSEC_INSTALL_INIT_SYSTEMD > + $(INSTALL) -D -m 0644 package/ntpsec/ntpd.service \ > + $(TARGET_DIR)/usr/lib/systemd/system/ntpd.service > +endef > + > define NTPSEC_USERS > ntp -1 ntp -1 * - - - ntpd user > endef > -- > 1.8.3.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
diff --git a/package/ntpsec/ntpd.service b/package/ntpsec/ntpd.service new file mode 100644 index 0000000..3987085 --- /dev/null +++ b/package/ntpsec/ntpd.service @@ -0,0 +1,10 @@ +[Unit] +Description=NTPSec +After=network.target +Conflicts=systemd-timesyncd.service + +[Service] +ExecStart=/usr/sbin/ntpd -d -g + +[Install] +WantedBy=multi-user.target diff --git a/package/ntpsec/ntpsec.mk b/package/ntpsec/ntpsec.mk index a0d0662..a0fc49b 100644 --- a/package/ntpsec/ntpsec.mk +++ b/package/ntpsec/ntpsec.mk @@ -62,6 +62,11 @@ define NTPSEC_INSTALL_INIT_SYSV $(INSTALL) -D -m 755 package/ntpsec/S49ntpd $(TARGET_DIR)/etc/init.d/S49ntpd endef +define NTPSEC_INSTALL_INIT_SYSTEMD + $(INSTALL) -D -m 0644 package/ntpsec/ntpd.service \ + $(TARGET_DIR)/usr/lib/systemd/system/ntpd.service +endef + define NTPSEC_USERS ntp -1 ntp -1 * - - - ntpd user endef
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com> --- Use -d to daemonize, use -g to allow a "big leap". Prior synchronization, system date is usually set to 01/01/1970. By default, ntpsec does not allow that big of a leap, and we never obtain synchronization without -g. Ideally, you want some sort of mechanism to only use -g on first (ever) boot. --- package/ntpsec/ntpd.service | 10 ++++++++++ package/ntpsec/ntpsec.mk | 5 +++++ 2 files changed, 15 insertions(+) create mode 100644 package/ntpsec/ntpd.service