Please convert init script to upstart

Bug #604717 reported by Chuck Short
44
This bug affects 7 people
Affects Status Importance Assigned to Milestone
ntp (Ubuntu)
Opinion
Wishlist
Ante Karamatić

Bug Description

Binary package hint: ntp

As per the server-maverick-upstart-conversion specification, please review the debdiff to convert the init script to upstart.

Tags: patch
Revision history for this message
Chuck Short (zulcss) wrote :
Thierry Carrez (ttx)
Changed in ntp (Ubuntu):
importance: Undecided → Wishlist
status: New → Confirmed
Revision history for this message
David Sugar (dyfet-deactivatedaccount) wrote :

Should the dh_installinit really be marked "upstart only"? I am thinking more about merging back to Debian or even backporting where upstart may not be in use. Otherwise I think this patch looks fine, and I may try a test build to verify it. I happen to agree ntp should migrate to upstart.

tags: added: patch
Daniel Hahler (blueyed)
Changed in ntp (Ubuntu):
status: Confirmed → Triaged
Revision history for this message
Colin Watson (cjwatson) wrote :

I agree with David's comment.

-invoke-rc.d --quiet ntp start >/dev/null 2>&1 || true
+start stop >/dev/null 2>&1 || true

This is wrong - should be 'start ntp'.

(Incomplete review; I still need to look at the job itself.)

Revision history for this message
Colin Watson (cjwatson) wrote :

+ test -f /etc/default/ntp && ./etc/default/ntp || true

Missing a space between '.' and '/etc/default/ntp'. This is probably why you had to add the '|| true' - remove that. Also, to match the init script this should be test -r not test -f.

+ test -e /var/lib/ntp.conf.dhcp && NTPD_OPTS="$NTPD_OPTS -c /var/lib/ntp/ntp.conf.dhcp"

The first filename is typoed - should be /var/lib/ntp/ntp.conf.dhcp.

You only seem to have ported part of the locking code over. You need the backgrounded lockfile-touch and the kill as well (otherwise the lock could time out on a very slow start), although quite how you're going to get the pid of the lockfile-touch process across to the post-start script I'm not quite sure.

Please try to avoid the use of a pidfile. Upstart's process tracking should be sufficient.

The apparmor code in the Upstart job appears to be new relative to the init script. Why?

Revision history for this message
Colin Watson (cjwatson) wrote :

The last point may be skew between what I was reviewing and current Maverick - a number of packages have been converted to dh_apparmor. I haven't looked into exactly what's involved here.

Revision history for this message
Chuck Short (zulcss) wrote :

Ill fix up all of these mistakes and resubmit the debdiff, I did test
them but I guess I didnt hit these problems while testng them.

chuck

Revision history for this message
Oxy (alessio-boidi) wrote : Excellent windows house

HAL sup LOWEEN SA ly LES!
EVERYTHING YOU NEED for a great and exciting day you can buy HERE!
Only TODAY ANY GOODS you can get with a next code 5486 which gives you a 40% dis pw count.
Be prepared for a scary day and make a show!

Get the great di fe scou rtg nts on popular so em ft loo wa jbs re today at www.digitalriverconsultingone.com.ua
All s rf of hiz wa qup re is instantly available to do hpq wnl jsf oad - No Need Wait!
ALL OUR SO tw FTW kn ARE hl S ON ALL EUROPEAN LANGUAGES -
USA, English, France, Italy, Spanish, German and more!!!

SO je FTW wus ARE:Windows 7 Ultimate 32 bit99.95Windows 7 Ultimate 64 bit99.95Windows XP Professional with Service Pack 369.95Office Professional Plus 2010 32-bit89.95Office Professional Plus 2010 64-bit89.95Adobe Photoshop CS5.1 Extended99.95Office Professional 200769.95Adobe Acrobat 9 Pro Extended59.95Office Home and Student 200749.95
Also we have so mu up ch s yrf of tkd t for MA gh CIN fi TO bwj SH!!!Adobe Creative Suite 5.5 Master Collection for MAC269.95Adobe Creative Suite 5.5 Design Premium for MAC219.95Microsoft Office 2008 Standart Edition for MAC119.95Aperture 3 for MAC79.95Adobe Photoshop CS5.1 Extended for MAC89.95
To re rny vi zw ew full list of the offers, v vfl is jx it www.digitalriverconsultingone.com.ua

Revision history for this message
isoma (isoma) wrote :

The ntp package provides /usr/sbin/ntp-wait

NTP can use this, or a similar technique, to emit an Upstart event when the time is synchronised. It would be nice to be able to react to this (although maybe not to block daemon startup indefinitely).

Revision history for this message
Ante Karamatić (ivoks) wrote :

This debdiff adds an upstart job for ntp. Upstart job does the same thing SysV script does. In addition, it adds two features that are common requirement in production:

1) does one off sync before starting ntpd in foreground - off by default
2) syncs hardware clock after the one off sync - off by default

Many services, OpenStack's nova comes to mind, require time to be synced, rather ntpd started. By doing one off sync before starting ntpd, system will have correct time before nova-* services start. Nova service then needs to depend on started ntpd.

Hardware clock sync might be an overkill, but, IMHO, doing it right after we synced the OS time seems the best place to do it.

Revision history for this message
Ante Karamatić (ivoks) wrote :

Found few typos. Fixed.

Changed in ntp (Ubuntu):
assignee: nobody → Ante Karamatić (ivoks)
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.

These days handled via systemd init script wrapper.
IMHO I don't think it still makes sense to add/convert upstart jobs for ntp.

If anything a native systemd job, but then systemd takes care of most basic time sync tasks on its own anyway.
That said - closing the bug by setting Opinion (which it is - an "old" opinion - in todays point of view)

Changed in ntp (Ubuntu):
status: Triaged → Opinion
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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