Bug 230561 - ntpd error: only one pidfile option allowed
Summary: ntpd error: only one pidfile option allowed
Status: Closed Not A Bug
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Dag-Erling Smørgrav
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2018-08-12 13:29 UTC by cyclaero
Modified: 2018-10-09 11:08 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description cyclaero 2018-08-12 13:29:48 UTC
FreeBSD 12.0-ALPHA1 (GENERIC) #0 r337557: Fri Aug 10 03:49:58 UTC 2018

During startup of the latest 12-CURRENT snapshot on a BeagleBone Black, I see:

ntpd error:  only one pidfile option allowed
ntpd - NTP daemon program - Ver. 4.2.8p11
Usage:  ntpd [ -<flag> [<val>] | --<name>[{=| }<val>] ]... \
		[ <server1> ... <serverN> ]
Try 'ntpd --help' for more information.
/etc/rc: WARNING: failed to start ntpd


As a workaround, I changed line 104 of /etc/rc.d/ntpd ...

from:

    command_args="-p ${pidfile} -c ${ntpd_config} ${driftopt}"

to:

    command_args="-c ${ntpd_config} ${driftopt}"

With this change, ntpd starts-up as usual.
Comment 1 Ian Lepore freebsd_committer 2018-08-12 13:50:55 UTC
The -p flag was removed from ntpd_flags in /etc/defaults/rc.conf, and must also be removed from your rc.conf if you have overridden ntpd_flags and included that option.
Comment 2 Rodney W. Grimes freebsd_committer 2018-08-12 15:11:41 UTC
This is a regression, in prior versions of FreeBSD you could specify and override the -p option that came from /etc/defaults/rc.conf.  This new /etc/rc.d/ntpd scripts does not allow that.  This is force set policy which FreeBSD should not be doing.

I am unclear why these user configurable defaults have been hard coded, but it is bad design.
Comment 3 Ian Lepore freebsd_committer 2018-08-12 15:50:08 UTC
The fact that the pidfile argument was included in the default ntpd_flags is a historical and accidental mistake dating back to when the ntpd rc stuff was imported from netbsd.  The freebsd rc system does not support changing the location of the pidfile to some arbitrary place it is unaware of; doing so removes the ability to properly restart, stop, query status, etc.

In other words, while the flag was historically in the default flags, it could never be usefully changed, and indeed if you do some searching you'll see that its accidental presence in the defaults has led to many errors and questions from users over the years as they tried to set ntpd_flags in rc.conf as you would for any other service/daemon and accidentally removed the -p flag in doing so.

So this change was NOT a regression, it was in fact the fix for PR 199127.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2018-10-05 09:47:41 UTC
Hi Ian,

I'm commenting here as I've only just realised my CURRENT system's ntpd was not running (for a while it seems!), discovering the reported error when trying to start the service manually. 

I'm not sure why/how I didn't pick the issue up earlier, perhaps it was missed in daily periodic logs. I'm a fairly seasoned FreeBSD user and I worry about the user experience of this issue for those less experienced.

There is a section (can_run_root() function) in the startup script which checks for user-supplied arguments (including pidfile), which when set, causes the scripts default behaviour (to use user:ntp and its own arguments/locations) to not be used, *but* apparently (?) only for the driftfile argument

  # Otherwise, figure out what to do about the driftfile option.  If set
  # by the admin, we don't add the option.
  <snip>
  driftopt="" # admin set the option, we don't need to add it.

Why can't the same, if not specific, then general behaviour, apply to !driftfile arguments as well?

Why is pidfile a/the special case? If its a permission issue, why isn't that a problem for driftfile, keyfile, etc?

Isn't the actual and only issue that the only argument the script explicitly sets is "-p ${pidfile}" causing the conflict/error reported here, but not for any other arguments?

If the pidfile argument absolutely and positively cannot be supported in any capacity, differently from other path/file-value arguments, pidfile should be stripped from arguments, given the *only* outcome of the user supplying it is this error.

In addition, and note I am not advocating that rc scripts should support any or all *particular values* for command arguments (see bug 231592), but not supporting overriding arguments via foo_flags via rc.conf is a pola violation and inconsistent (I believe) with; the expectations of our users generally, and almost all other rc scripts we ship (in base and ports).

@with bugmeister hat, assign to Ian as previous issue closer and committer of base r336547
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2018-10-05 09:55:16 UTC
My previous specific reference to bug 231592 is bug 231592 comment 4
Comment 6 Dag-Erling Smørgrav freebsd_committer 2018-10-09 10:15:24 UTC
The pidfile is a special case because it is used by the rc system itself.  This is how it has always worked, or at least since we adopted the NetBSD framework in the early 2000s.  If you disagree, then by all means open a new PR suggesting a general mechanism for overriding the pidfile location on a per-service basis and assign it to me; but forcibly reopening this bug and reassigning to Ian just because you happen to have made the same mistake as the originator is, in my opinion, an abuse of your position as bugmeister.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2018-10-09 11:08:45 UTC
I've responded privately to des@'s non-issue related remarks in comment 6.