/etc/network/if-up.d/ntpdate does not detach correctly

Bug #1206164 reported by Scott Moser
64
This bug affects 9 people
Affects Status Importance Assigned to Milestone
ntp (Debian)
Fix Released
Unknown
ntp (Ubuntu)
Won't Fix
Medium
Christian Ehrhardt 

Bug Description

It seems that the intent of /etc/network/if-up.d/ntpdate is to
background itself, as it creates a subshell with &.

As reported in bug 1202758, this doesn't seem to work correctly. That doesn't close the file descriptors. As a result, if something is waiting on the output of 'ifup', it will sit and wait until ntpdate is finished.

Example:
$ time sudo sh -c 'o=$(sh -c "ifdown eth0 ; ifup eth0" 2>&1) ; echo $o'

If we change the way it detaches itself to close stdout, stdderr, and
stdin, then we're fine.

I'm reporting this, but I'm not sure how much of a real problem it is. As if th
e command sends stdin and stdout to /dev/null, then debugging its output would b
e more difficult.

Example diff:
$ diff -u /etc/network/if-up.d/ntpdate.dist /etc/network/if-up.d/ntpdate
--- /etc/network/if-up.d/ntpdate.dist 2013-07-29 15:47:54.242781947 +0000
+++ /etc/network/if-up.d/ntpdate 2013-07-29 15:48:06.946781947 +0000
@@ -56,4 +56,4 @@
   lockfile-remove $LOCKFILE
 fi

-) &
+) </dev/null >/dev/null 2>&1 &

ProblemType: Bug
DistroRelease: Ubuntu 13.10
Package: ntpdate 1:4.2.6.p5+dfsg-2ubuntu2
ProcVersionSignature: User Name 3.10.0-5.15-generic 3.10.2
Uname: Linux 3.10.0-5-generic x86_64
ApportVersion: 2.11-0ubuntu1
Architecture: amd64
Date: Mon Jul 29 15:32:30 2013
MarkForUpload: True
ProcEnviron:
 TERM=xterm
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: ntp
UpgradeStatus: No upgrade log present (probably fresh install)
mtime.conffile..etc.network.if.up.d.ntpdate: 2013-07-29T15:32:03.567775

Revision history for this message
Scott Moser (smoser) wrote :
Revision history for this message
Scott Moser (smoser) wrote :

for my usecase, it would probably be nicest if I could just disable this via environment variable.
perhaps:
  DISABLE_NTPDATE=1 ifup eth0

and then just a simple check for that variable in the script:
[ "${DISABLE_NTPDATE:-0}" != "0" ] && exit 0

Robie Basak (racb)
Changed in ntp (Ubuntu):
importance: Undecided → Medium
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I know it's a long time, but I'm cleaning up old NTP bugs atm.

Your latter suggestion is actually only a small fix and we have all we need - so setting triaged.

Probably not worth to work on it on its own - but it could be nicely done along a potential upload to bug 75347.
Therefore subscribing the Team to take a deeper look at it.

Changed in ntp (Ubuntu):
status: New → Triaged
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Since no more installed by default - lowering prio thou

Changed in ntp (Ubuntu):
importance: Medium → Low
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I found another bgu I marked as dup, this had another two dup.
So I think it is more of a request than I thought.

Raisng prio again.
IMHO we should even consider disabling it by default.

Changed in ntp (Ubuntu):
importance: Low → Medium
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Passing more bugs that I closed as dup, the general argument seems to be that these days in no way a server should:
1. not step time after boot (adressed in 75347)
2. not ntpupdate (by default) on every ifup especially with so much virtual interfaces

I'll try to suggest Scotts patch in a default-disabled version to Debian and see what the discussion turns up.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

While these days the systemd based timesync* things are doing most of the work there is still a lot of buzz around the automated ntpdate on ifup. I'll try to to summarize the outcome of multiple discussions and bugs around this and propose a solution.

- I've seen various reports of ntpdate syncing too much or time changes due to that annoying people.
- but then there are different user scenarios so some want and others won't "autoupdate" feature on ifup
- the ones that want it disabled are usually only "inconvenience issues" like too much calls, but also cases like "know it will hang", so could I disable it in advance
- the ones require the updates are mostly having more severe issues (like breaking authentication due to time being off afterwards)

We should try to create a solution for both parties to be able to config the system to their way.
So lets keep the default to sync (as it seems the more critical way), but provide a way to disable it via the config files.

Doing it via an environment variable also allows to "overwrite" it in ifup calls like
  DISABLE_NTPDATE=1 ifup eth0

The config file has to be read in ntpdate-debian if the variable is not set.

Then just a simple check for that variable in the script to exit if disabled:
[ "${DISABLE_NTPDATE:-0}" != "0" ] && exit 0

So the behavior would be like:
1. default it is running on ifup
2. one could set a different default in the config file
3. one can overwrite whatever was set via an environment variable

... creating a patch for that and providing to Debian for a latter sync.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

added the Debian bug I reported the patch

Changed in ntp:
status: Unknown → New
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Testing has shown that an environment var there isn't working well as call -> ifup -> hook isn't transferring env.
But checking for alternatives this got even better.

ifup makes all options in e/n/i or ifup -o available (prepended and uppercased) to called scripts.
That way one can not only disable per call, but also per device in e/n/i.

So the behavior will be like:
 1. default it is running on ifup (old default)
 2. one can set a global disable in the config file
 3. one disable per device in the devices e/n/i stanza via disable_ntpdate=1
 4. one can disable it per call via -o option setting -o disable_ntpdate=1

Changed in ntp (Ubuntu):
assignee: nobody → ChristianEhrhardt (paelzer)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

In reference to [1] the TL;DR summary is that:
1. ntpdate is deprecated
2. the patches got polished, tested and provided to Debian but not yet picked up
3. we are unwilling to add more delta to Debian for a deprecated binary
#. so if they are merged in Debian we will pick it up in the next Release, but unlikely do an SRU for it.

Great and more detailed summary including potential workarounds for old issues can be read at:
[1]: https://lists.ubuntu.com/archives/ubuntu-devel/2016-August/039484.html

Changed in ntp (Ubuntu):
status: Triaged → Won't Fix
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in ubuntu:
status: New → Confirmed
Mathew Hodson (mhodson)
affects: ntp → ubuntu
Changed in ubuntu:
importance: Unknown → Undecided
no longer affects: ubuntu
Changed in ntp (Debian):
status: Unknown → New
Changed in ntp (Debian):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.