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

fix is_virtual detection issue #235

Merged
merged 1 commit into from
Feb 25, 2015
Merged

fix is_virtual detection issue #235

merged 1 commit into from
Feb 25, 2015

Conversation

guessi
Copy link
Contributor

@guessi guessi commented Jan 22, 2015

adjust the expression to ensure 127.127.1.0 is set only if target is running
under virtual environment or udlc is set.

if server pointed to 127.127.1.0 without preferred_servers settings, it
would try to sync with itself but not to the server.

@@ -31,7 +31,7 @@ interface listen <%= interface %>
server <%= server %><% if @iburst_enable == true -%> iburst<% end %><% if @preferred_servers.include?(server) -%> prefer<% end %>
<% end -%>

<% if scope.lookupvar('::is_virtual') == "false" or @udlc -%>
<% if scope.lookupvar('::is_virtual') == "true" or @udlc -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this use scope.lookupvar anyway?
are facts not exposed as @fact, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can be look up via @fact, just follow previous change : )

adjust the expression to ensure `127.127.1.0` is set only if target is running
under virtual environment or `udlc` is set.

if `server` pointed to `127.127.1.0` without `preferred_servers` settings, it
would try to sync with itself but not to the `server`.
@igalic
Copy link
Contributor

igalic commented Feb 6, 2015

+1

hunner added a commit that referenced this pull request Feb 25, 2015
@hunner hunner merged commit 4acfa3b into puppetlabs:master Feb 25, 2015
@hunner
Copy link
Contributor

hunner commented Feb 25, 2015

Actually, I don't agree with this. UDLC should not be used on VMs, and this is a backwards incompatible change (as the udlc will now be ENABLED on all vms instead of disabled).

Perhaps the udlc argument for the module should just be able to override that. I'll send a PR.

@hunner
Copy link
Contributor

hunner commented Feb 25, 2015

I also think I disagree with "if server pointed to 127.127.1.0 without preferred_servers settings, it
would try to sync with itself but not to the server." It appears that the prefer keyword would have to be used for a 127.127.x.y server to be used over other servers (according to the manpage).

hunner added a commit to hunner/puppetlabs-ntp that referenced this pull request Feb 25, 2015
The UDLC should really only be used on machines with accurate clocks, and
this logic was reversed incorrectly.

This PR also allows the udlc setting to be overridden, whereas before it
was not.
underscorgan pushed a commit that referenced this pull request Feb 25, 2015
Fix PR #235 and make udlc configurable
@guessi guessi deleted the fix_virtual_detection branch February 27, 2015 02:38
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.

3 participants