Closed
Bug 1376804
Opened 8 years ago
Closed 8 years ago
Fix ntp::atboot for osx
Categories
(Infrastructure & Operations :: RelOps: Puppet, task)
Infrastructure & Operations
RelOps: Puppet
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dividehex, Assigned: dragrom)
Details
Attachments
(1 file)
2.98 KB,
patch
|
dividehex
:
review+
dragrom
:
checked-in+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•8 years ago
|
||
Fix ntp::atboot for OSX, to call ntpdate instead to use ntp service
Attachment #8882023 -
Flags: review?(jwatkins)
Reporter | ||
Comment 2•8 years ago
|
||
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
Attachment #8882023 -
Flags: review?(jwatkins) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8882023 [details] [diff] [review]
Bug_1376804_Fix_ntp_atboot_for_OSX.patch
https://hg.mozilla.org/build/puppet/rev/3e2b8e2cf6a4699a2bd39881d0ddd545413e84c1
Attachment #8882023 -
Flags: checked-in+
Assignee | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 4•8 years ago
|
||
I backed the change out because it landed without including the review changes:
https://hg.mozilla.org/build/puppet/rev/41834c9630af
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•8 years ago
|
Attachment #8882023 -
Flags: checked-in+ → checked-in-
Assignee | ||
Comment 5•8 years ago
|
||
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.
Flags: needinfo?(nthomas)
Comment 6•8 years ago
|
||
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.
Flags: needinfo?(nthomas)
Assignee | ||
Comment 7•8 years ago
|
||
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
Attachment #8882023 -
Flags: checked-in- → checked-in+
Assignee | ||
Updated•8 years ago
|
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•