1376804 - Fix ntp::atboot for osx

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task)

Description

[Jake Watkins [:dividehex]]

During the firewall meeting, it was discovered that osx hosts in the test vlan are connecting to time.apple.com.  ntp should not be running on these hosts and definitely should not be contacting apple for ntp.

Comment 1

[Dragos Crisan [:dragrom]]

Attachment: Bug_1376804_Fix_ntp_atboot_for_OSX.patch

Fix ntp::atboot for OSX, to call ntpdate instead to use ntp service

Comment 2

[Jake Watkins [:dividehex]]

Comment on attachment 8882023 [details] [diff] [review]
Bug_1376804_Fix_ntp_atboot_for_OSX.patch

Review of attachment 8882023 [details] [diff] [review]:
-----------------------------------------------------------------

r+ on the changes mentioned below.  Make sure you don't change daemon.pp.  All this code is still valid.

::: modules/ntp/manifests/atboot.pp
@@ +17,5 @@
>          }
> +        Darwin: {
> +            file {
> +                '/Library/LaunchDaemons/org.mozilla.ntpdate.plist':
> +                    content => template('ntp/ntp.plist.erb');

this template should be named ntpdate.plist.erb.  Ntp(ntpd) is not the same as ntpdate

::: modules/ntp/manifests/daemon.pp
@@ -53,5 @@
> -            cron {
> -                'whack-apple-ntpd':
> -                    command => '/usr/bin/killall ntpd',
> -                    minute  => 0;
> -            }

None of this code should be removed in daemon.pp; it is still all valid.  We are only fixing ntp::atboot

::: modules/ntp/templates/ntp.plist.erb
@@ +9,5 @@
> +	<key>Program</key>
> +	<string>/usr/sbin/ntpdate</string>
> +	<key>ProgramArguments</key>
> +	<array>
> +	    <string>/usr/sbin/ntpdate</string>    

Trailing whitespace

Comment 3

[Dragos Crisan [:dragrom]]

Comment on attachment 8882023 [details] [diff] [review]
Bug_1376804_Fix_ntp_atboot_for_OSX.patch

https://hg.mozilla.org/build/puppet/rev/3e2b8e2cf6a4699a2bd39881d0ddd545413e84c1

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

I backed the change out because it landed without including the review changes:
https://hg.mozilla.org/build/puppet/rev/41834c9630af

Comment 5

[Dragos Crisan [:dragrom]]

I don't see what review changes I didn't applied:

 - modules/ntp/manifests/atboot.pp - this template should be named ntpdate.plist.erb.  Ntp(ntpd) is not the same as ntpdate - was applied
 - modules/ntp/templates/ntp.plist.erb - Trailing whitespace - renamed ntp.plist.erb to ntpdate.plist.erb and removed Trailing whitespace from the end of <string>/usr/sbin/ntpdate</string> line
 - modules/ntp/manifests/daemon.pp - None of this code should be removed in daemon.pp; it is still all valid.  We are only fixing ntp::atboot - so I reverted the changes from daemon.pp and kept the default code. From this reason daemon.pp bot appear in the commit, because there are no changes.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

I'm really sorry, somehow I completely misread the patch that actually landed. Please go ahead and reland it. Would be great if we could stick to our rule about merging to production as soon as travis-ci is green on pushes to default.

Comment 7

[Dragos Crisan [:dragrom]]

Comment on attachment 8882023 [details] [diff] [review]
Bug_1376804_Fix_ntp_atboot_for_OSX.patch

Re-landed:  https://hg.mozilla.org/build/puppet/rev/4461fe877dcc9c3cbb020b52caa9faf5061034c8