-
Notifications
You must be signed in to change notification settings - Fork 328
add tos_orphan parameter #452
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
Conversation
templates/ntp.conf.epp
Outdated
@@ -101,6 +101,10 @@ ntpsigndsocket <%= $ntp::ntpsigndsocket %> | |||
<% $ntp::peers.each |$peer| {-%> | |||
peer <%= $peer %> | |||
<% } -%> | |||
# Enable peer group orphan mode? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment should be inside conditional block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it more practical to keep a comment in front of a block so it is still visible in case the block is collapsed in an IDE?
I don't mind moving it into the block, just my thoughts on comment placement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If attribute is not set, you will end up with hanging comment. Also it will cause existing installations to restart named for no reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'ntpd'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'ntpd'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Makes sense with templates. The situation could be avoided by encapulating the comment, but that's ugly. It is also redundant if you want to keep the comment in the generated file.
Moved comment into conditional statement.
data/common.yaml
Outdated
@@ -59,6 +59,7 @@ ntp::tos_floor: 1 | |||
ntp::tos_maxclock: 6 | |||
ntp::tos_minclock: 3 | |||
ntp::tos_minsane: 1 | |||
ntp::tos_orphan: 6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is having this enabled by default at a value of 6
for all users and OS's a reasonable default? This would affect all users as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't. This option was added in ntpd 4.2.2, I think, and might be not available everywhere, so it can be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ntpd 4.2.2 was released in October 2005. An older version shouldn't be around anymore. The default should have been fine for all users as you shouldn't have an upstream ntp with a stratum worse than 4 as source for your pool.
It would have clashed with users of the undisciplined local clock which shouldn't be used with 4.2.2 and later (ntpd documentation).
removed default orphan stratum for peer groups
Add 'tos orphan' parameter which replaces udlc for peer groups (ntp >= 4.2.2).