voxpupuli / puppet-vmwaretools

Puppet module to manage VMware Operating System Specific Packages for VMware tools installation.
http://forge.puppetlabs.com/razorsedge/vmwaretools
Apache License 2.0
34 stars 44 forks source link

Refactor ntp class for proper timesync behavior #7

Closed ryanycoleman closed 12 years ago

ryanycoleman commented 12 years ago

Prior to this commit, the vmwaretools::ntp class was a complete noop because the exec was marked as refreshonly with no refresh event (notify or subscribe) affecting it.

Additionally, the refresh event was reliant on package installation or upgrades which isn't very idempotent. Ideally, each time Puppet runs, the timesync status should be evaluated and corrected if needed.

Unfortunately, after a few hours of experimentation, I was unable to create a proper way of determining timesync status using the vmware-guestd tooling. As a result, I had to split the class into two exec resources for the different desired behaviors.

If the service_pattern is vmtoolsd, timesync will disable if the status exec returns 0, which happens when timesync is enabled. However, if the service_pattern is vmware-guestd, the previous exec is maintained but the package require is replaced by a subscribe. If the service_pattern isn't matched, the class does nothing as it did previously.

ryanycoleman commented 12 years ago

I regret having to split into two exec resources but it was the best outcome given desires of compatibility with vmware-guestd and wanting to be idempotent when possible with vmtoolsd. Let me know if you'd like to discuss this patch further.

razorsedge commented 12 years ago

Prior to this commit, the vmwaretools::ntp class was a complete noop because the exec was marked as refreshonly with no refresh event (notify or subscribe) affecting it.

This is intentional. vmwaretools::ntp should only be used in conjunction with a third-party ntp class. As per the sample usage in the ntp.pp file, it is up to the third-party ntp class to notify vmwaretools::ntp.

Ideally, each time Puppet runs, the timesync status should be evaluated and corrected if needed.

This is finally possible with vmware-toolbox-cmd.

Unfortunately, after a few hours of experimentation...

Heh. I could have saved you a few hours. :-) I've been tracking this VMware KB for 6+ years.

I like the split of the execs. It allows for the constant checking by vmware-toolbox-cmd as to whether or not to disable timesync. However, I still think that include vmwaretools::ntp should not do anything unless include ntp is also in use and the correct ntp.conf settings (as per the KB article) are present.

ryanycoleman commented 12 years ago

Ok, thanks for the response! Closing this pull.