Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ntp stepout, minpoll and maxpoll options #255

Merged
merged 1 commit into from
Apr 16, 2015

Conversation

sorrowless
Copy link
Contributor

(MODULES-1925) ntp stepout, minpoll and maxpoll options should be added

Change options:

  • panic - panic is not boolean, it is integer, actually

Add options:

  • tinker - disable or enable tinker
  • minpoll - to change ntpd send packages frequency
  • maxpoll - to change ntpd send packages frequency
  • stepout - to change stepout interval

Add tests, change readme.

@sorrowless
Copy link
Contributor Author

Actually, I don't understand, why Travis CI cannot build this on 3.0 with future parser. I tested it on 3.4.2 with standard parser and with future parser w/o any warning.

@underscorgan
Copy link
Contributor

@sorrowless the issue with newer versions of the future parser is it doesn't automatically cast ints to strings, and validate_re only works on strings.

@@ -42,10 +46,12 @@
validate_re($keys_controlkey, ['^\d+$', ''])
validate_re($keys_requestkey, ['^\d+$', ''])
validate_array($keys_trusted)
validate_re($minpoll, '^([3-9]|1[0-6])$')
validate_re($minpoll, '^([3-9]|1[0-6])$')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using validate_re here you should try validate_numeric with min/max values set (docs). This should also resolve the future parser issues.

@underscorgan
Copy link
Contributor

Hey @sorrowless , thanks for the contribution. In general this looks pretty good, but I had some inline comments. If you could address those this should be good to merge I think. Thanks!

@sorrowless
Copy link
Contributor Author

Hi, @mhaskel . Your request done. Would you kind to see it again and tell whether it can be merged?

should contain_file('/etc/ntp.conf').with({
'content' => /tinker panic 0/,
})
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you get rid of the trailing whitespace on L608-612?

@underscorgan
Copy link
Contributor

@sorrowless other than the whitespace issue the code looks good. Can you please squash this down to a single commit though? THanks!

Change options:
* panic - panic is not boolean, it is integer, actually

Add options:
* tinker - disable or enable tinker
* minpoll - to change ntpd send packages frequency
* maxpoll - to change ntpd send packages frequency
* stepout - to change stepout interval

Also Readme file changed accordingly.
@sorrowless
Copy link
Contributor Author

@mhaskel, done.

underscorgan pushed a commit that referenced this pull request Apr 16, 2015
Add ntp stepout, minpoll and maxpoll options
@underscorgan underscorgan merged commit da5d844 into puppetlabs:master Apr 16, 2015
@underscorgan
Copy link
Contributor

thanks @sorrowless!

@underscorgan
Copy link
Contributor

I did just notice that this makes the module require the latest version of
stdlib. I'll update the dependency in the metadata, but updating stdlib
should resolve this for you

On Thu, Apr 16, 2015 at 11:19 AM, jasondowns notifications@github.com
wrote:

You pretty much just broke everyone in the world using HEAD...

Error 400 on SERVER: Unknown function validate_numeric at
/etc/puppet/environments/production/modules/ntp/manifests/init.pp:55


Reply to this email directly or view it on GitHub
#255 (comment)
.

Morgan Haskel
morgan@puppetlabs.com
Module Engineer

PuppetConf 2015 http://2015.puppetconf.com/ is coming to Portland,
Oregon! Join us October 5-9.

_Register now to take advantage of the Early Adopter discount
https://www.eventbrite.com/e/puppetconf-2015-october-5-9-tickets-13115894995?discount=EarlyAdopter
*
*—_save $349!

underscorgan pushed a commit to underscorgan/puppetlabs-ntp that referenced this pull request Apr 16, 2015
bmjen added a commit that referenced this pull request Apr 16, 2015
PR #255 added a dependency on stdlib >= 4.6.0
colewhite pushed a commit to colewhite/puppetlabs-ntp that referenced this pull request Apr 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants