Merge pull request #80 from apenney/single_template

Merge all the templates together, add a few new features to help with the merge.

19 files changed, 448 insertions, 689 deletions

diff --git a/README.markdown b/README.markdown
index 65dde32..3aedd47 100644
--- a/README.markdown
+++ b/README.markdown
@@ -120,6 +120,26 @@ This sets the file to write ntp configuration into.
 
 This determines which template puppet should use for the ntp configuration.
 
+####`driftfile`
+
+This sets the location of the driftfile for ntp.
+
+####`keys_controlkey`
+
+Which of the keys is used as the control key.
+
+####`keys_enable`
+
+Should the ntp keys functionality be enabled.
+
+####`keys_file`
+
+Location of the keys file.
+
+####`keys_requestkey`
+
+Which of the keys is used as the request key.
+
 ####`package_ensure`
 
 This can be set to 'present' or 'latest' or a specific version to choose the
@@ -135,6 +155,11 @@ This determines if ntp should 'panic' in the event of a very large clock skew.
 We set this to false if you're on a virtual machine by default as they don't
 do a great job with keeping time.
 
+####`preferred_servers`
+
+List of ntp servers to prefer.  Will append prefer for any server in this list
+that also appears in the servers list.
+
 ####`restrict`
 
 This sets the restrict options in the ntp configuration.
diff --git a/manifests/config.pp b/manifests/config.pp
index 0a4a710..c8a5625 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -1,10 +1,29 @@
-class ntp::config (
-  $config          = $ntp::config,
-  $config_template = $ntp::config_template,
-  $panic           = $ntp::panic,
-  $restrict        = $ntp::restrict,
-  $servers         = $ntp::servers,
-) inherits ntp {
+#
+class ntp::config {
+
+  $config            = $ntp::config
+  $config_template   = $ntp::config_template
+  $driftfile         = $ntp::driftfile
+  $keys_enable       = $ntp::keys_enable
+  $keys_file         = $ntp::keys_file
+  $keys_controlkey   = $ntp::keys_controlkey
+  $keys_requestkey   = $ntp::keys_requestkey
+  $keys_trusted      = $ntp::keys_trusted
+  $panic             = $ntp::panic
+  $preferred_servers = $ntp::preferred_servers
+  $restrict          = $ntp::restrict
+  $servers           = $ntp::servers
+
+  if $keys_enable {
+    $directory = dirname($keys_file)
+    file { $directory:
+      ensure  => directory,
+      owner   => 0,
+      group   => 0,
+      mode    => '0755',
+      recurse => true,
+    }
+  }
 
   file { $config:
     ensure  => file,
diff --git a/manifests/init.pp b/manifests/init.pp
index 61a5ad9..2c8b9e4 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,18 +1,43 @@
 class ntp (
-  $autoupdate      = $ntp::params::autoupdate,
-  $config          = $ntp::params::config,
-  $config_template = $ntp::params::config_template,
-  $package_ensure  = $ntp::params::package_ensure,
-  $package_name    = $ntp::params::package_name,
-  $panic           = $ntp::params::panic,
-  $restrict        = $ntp::params::restrict,
-  $servers         = $ntp::params::servers,
-  $service_enable  = $ntp::params::service_enable,
-  $service_ensure  = $ntp::params::service_ensure,
-  $service_manage  = $ntp::params::service_manage,
-  $service_name    = $ntp::params::service_name,
+  $autoupdate        = $ntp::params::autoupdate,
+  $config            = $ntp::params::config,
+  $config_template   = $ntp::params::config_template,
+  $driftfile         = $ntp::params::driftfile,
+  $keys_enable       = $ntp::params::keys_enable,
+  $keys_file         = $ntp::params::keys_file,
+  $keys_controlkey   = $ntp::params::keys_controlkey,
+  $keys_requestkey   = $ntp::params::keys_requestkey,
+  $keys_trusted      = $ntp::params::keys_trusted,
+  $package_ensure    = $ntp::params::package_ensure,
+  $package_name      = $ntp::params::package_name,
+  $panic             = $ntp::params::panic,
+  $preferred_servers = $ntp::params::preferred_servers,
+  $restrict          = $ntp::params::restrict,
+  $servers           = $ntp::params::servers,
+  $service_enable    = $ntp::params::service_enable,
+  $service_ensure    = $ntp::params::service_ensure,
+  $service_manage    = $ntp::params::service_manage,
+  $service_name      = $ntp::params::service_name,
 ) inherits ntp::params {
 
+  validate_absolute_path($config)
+  validate_string($config_template)
+  validate_absolute_path($driftfile)
+  validate_bool($keys_enable)
+  validate_re($keys_controlkey, ['^\d+$', ''])
+  validate_re($keys_requestkey, ['^\d+$', ''])
+  validate_array($keys_trusted)
+  validate_string($package_ensure)
+  validate_array($package_name)
+  validate_bool($panic)
+  validate_array($preferred_servers)
+  validate_bool($restrict)
+  validate_array($servers)
+  validate_bool($service_enable)
+  validate_string($service_ensure)
+  validate_bool($service_manage)
+  validate_string($service_name)
+
   if $autoupdate {
     notice('autoupdate parameter has been deprecated and replaced with package_ensure.  Set this to latest for the same behavior as autoupdate => true.')
   }
diff --git a/manifests/install.pp b/manifests/install.pp
index 94cb080..e175964 100644
--- a/manifests/install.pp
+++ b/manifests/install.pp
@@ -1,7 +1,8 @@
-class ntp::install (
-  $package_ensure = $ntp::package_ensure,
-  $package_name   = $ntp::package_name,
-) inherits ntp {
+#
+class ntp::install {
+
+  $package_ensure = $ntp::package_ensure
+  $package_name   = $ntp::package_name
 
   package { 'ntp':
     ensure => $package_ensure,
diff --git a/manifests/params.pp b/manifests/params.pp
index 45551a8..ef037fc 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -1,22 +1,29 @@
 class ntp::params {
 
-  $autoupdate     = false
-  $package_ensure = 'present'
-  $restrict       = true
-  $service_enable = true
-  $service_ensure = 'running'
-  $service_manage = true
+  $autoupdate        = false
+  $config_template   = 'ntp/ntp.conf.erb'
+  $keys_enable       = false
+  $keys_controlkey   = ''
+  $keys_requestkey   = ''
+  $keys_trusted      = []
+  $package_ensure    = 'present'
+  $preferred_servers = []
+  $restrict          = true
+  $service_enable    = true
+  $service_ensure    = 'running'
+  $service_manage    = true
 
   # On virtual machines allow large clock skews.
-  $panic = $::is_virtual ? {
-    'true'  => false,
+  $panic = str2bool($::is_virtual) ? {
+    true    => false,
     default => true,
   }
 
   case $::osfamily {
     'Debian': {
       $config          = '/etc/ntp.conf'
-      $config_template = 'ntp/ntp.conf.debian.erb'
+      $keysfile        = '/etc/ntp/keys'
+      $driftfile       = '/var/lib/ntp/drift'
       $package_name    = [ 'ntp' ]
       $service_name    = 'ntp'
       $servers         = [
@@ -28,7 +35,8 @@ class ntp::params {
     }
     'RedHat': {
       $config          = '/etc/ntp.conf'
-      $config_template = 'ntp/ntp.conf.el.erb'
+      $driftfile       = '/var/lib/ntp/drift'
+      $keysfile        = '/etc/ntp/keys'
       $package_name    = [ 'ntp' ]
       $service_name    = 'ntpd'
       $servers         = [
@@ -39,7 +47,8 @@ class ntp::params {
     }
     'SuSE': {
       $config          = '/etc/ntp.conf'
-      $config_template = 'ntp/ntp.conf.suse.erb'
+      $driftfile       = '/var/lib/ntp/drift/ntp.drift'
+      $keysfile        = '/etc/ntp/keys'
       $package_name    = [ 'ntp' ]
       $service_name    = 'ntp'
       $servers         = [
@@ -51,7 +60,8 @@ class ntp::params {
     }
     'FreeBSD': {
       $config          = '/etc/ntp.conf'
-      $config_template = 'ntp/ntp.conf.freebsd.erb'
+      $driftfile       = '/var/db/ntpd.drift'
+      $keysfile        = '/etc/ntp/keys'
       $package_name    = ['net/ntp']
       $service_name    = 'ntpd'
       $servers         = [
@@ -63,8 +73,9 @@ class ntp::params {
     }
     'Archlinux': {
       $config          = '/etc/ntp.conf'
-      $config_template = 'ntp/ntp.conf.archlinux.erb'
-      $package_name    = ['ntp']
+      $driftfile       = '/var/lib/ntp/drift'
+      $keysfile        = '/etc/ntp/keys'
+      $package_name    = [ 'ntp' ]
       $service_name    = 'ntpd'
       $servers         = [
         '0.pool.ntp.org',
@@ -77,7 +88,8 @@ class ntp::params {
       case $::operatingsystem {
         'Gentoo': {
           $config          = '/etc/ntp.conf'
-          $config_template = 'ntp/ntp.conf.gentoo.erb'
+          $driftfile       = '/var/lib/ntp/drift'
+          $keysfile        = '/etc/ntp/keys'
           $package_name    = ['net-misc/ntp']
           $service_name    = 'ntpd'
           $servers         = [
diff --git a/manifests/service.pp b/manifests/service.pp
index 741e65b..2b1eee0 100644
--- a/manifests/service.pp
+++ b/manifests/service.pp
@@ -1,9 +1,10 @@
-class ntp::service (
-  $service_enable = $ntp::service_enable,
-  $service_ensure = $ntp::service_ensure,
-  $service_manage = $ntp::service_manage,
-  $service_name   = $ntp::service_name,
-) inherits ntp {
+#
+class ntp::service {
+
+  $service_enable = $ntp::service_enable
+  $service_ensure = $ntp::service_ensure
+  $service_manage = $ntp::service_manage
+  $service_name   = $ntp::service_name
 
   if ! ($service_ensure in [ 'running', 'stopped' ]) {
     fail('service_ensure parameter must be running or stopped')
diff --git a/spec/classes/ntp_config_spec.rb b/spec/classes/ntp_config_spec.rb
deleted file mode 100644
index 940eee0..0000000
--- a/spec/classes/ntp_config_spec.rb
+++ /dev/null
@@ -1,179 +0,0 @@
-require 'spec_helper'
-
-describe 'ntp::config' do
-
-  def param_value(subject, type, title, param)
-    catalogue.resource(type, title).send(:parameters)[param.to_sym]
-  end
-
-  let(:params) {{:servers => 'fake.pool.ntp.org'}  }
-
-  describe 'test platform specific resources' do
-
-    describe "for operating system family Debian" do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily => 'debian' }}
-
-      it 'should use the debian ntp servers by default' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = ['server 0.debian.pool.ntp.org iburst',
-         'server 1.debian.pool.ntp.org iburst',
-         'server 2.debian.pool.ntp.org iburst',
-         'server 3.debian.pool.ntp.org iburst']
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-
-      it 'should use different restrict settings if set' do
-        params[:restrict] == '127.0.0.1'
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = ['restrict 127.0.0.1']
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-    end
-
-    describe "for operating system family RedHat" do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily => 'redhat' }}
-
-      it 'should use the redhat ntp servers by default' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [
-         'server 0.centos.pool.ntp.org',
-         'server 1.centos.pool.ntp.org',
-         'server 2.centos.pool.ntp.org']
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-    end
-
-    describe "for operating system family SuSE" do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily => 'suse' }}
-
-      it 'should use the opensuse ntp servers by default' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [
-         'server 0.opensuse.pool.ntp.org',
-         'server 1.opensuse.pool.ntp.org',
-         'server 2.opensuse.pool.ntp.org',
-         'server 3.opensuse.pool.ntp.org']
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-    end
-
-    describe "for operating system family FreeBSD" do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily => 'freebsd' }}
-
-      it 'should use the freebsd ntp servers by default' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [
-          "server 0.freebsd.pool.ntp.org iburst maxpoll 9",
-          "server 1.freebsd.pool.ntp.org iburst maxpoll 9",
-          "server 2.freebsd.pool.ntp.org iburst maxpoll 9",
-          "server 3.freebsd.pool.ntp.org iburst maxpoll 9"]
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-
-      describe "for operating system family Archlinux" do
-  
-        let(:params) {{}}
-        let(:facts) {{ :osfamily => 'Archlinux' }}
-  
-        it 'should use the NTP pool servers by default' do
-          content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-          expected_lines = [
-            "server 0.pool.ntp.org",
-            "server 1.pool.ntp.org",
-            "server 2.pool.ntp.org"]
-          (content.split("\n") & expected_lines).should == expected_lines
-        end
-      end
-
-      describe "for operating system family unsupported" do
-        let(:facts) {{
-          :osfamily  => 'unsupported',
-        }}
-
-        it { expect{ subject }.to raise_error(
-          /^The ntp module is not supported on an unsupported based system./
-        )}
-      end
-
-    end
-
-    describe 'for virtual machines' do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily        => 'Archlinux',
-                     :is_virtual      => 'true' }}
-
-      it 'should not use local clock as a time source' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [
-          'server  127.127.1.0  # local clock',
-          'fudge  127.127.1.0 stratum 10' ]
-        (content.split("\n") & expected_lines).should_not == expected_lines
-      end
-
-      it 'allows large clock skews' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [ 'tinker panic 0' ]
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-
-    end
-
-    describe 'for physical machines' do
-
-      let(:params) {{}}
-      let(:facts) {{ :osfamily        => 'Archlinux',
-                     :is_virtual      => 'false' }}
-
-      it 'disallows large clock skews' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [ 'tinker panic 0' ]
-        (content.split("\n") & expected_lines).should_not == expected_lines
-      end
-
-    end
-
-    describe "for operating system Gentoo" do
-
-      let(:params) {{}}
-      let(:facts) {{ :operatingsystem => 'Gentoo',
-                      :osfamily        => 'Linux' }}
-
-
-      it 'should use the NTP pool servers by default' do
-        content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-        expected_lines = [
-          "server 0.gentoo.pool.ntp.org",
-          "server 1.gentoo.pool.ntp.org",
-          "server 2.gentoo.pool.ntp.org",
-          "server 3.gentoo.pool.ntp.org"]
-        (content.split("\n") & expected_lines).should == expected_lines
-      end
-    end
-
-    ['Debian', 'RedHat','SuSE', 'FreeBSD', 'Archlinux'].each do |osfamily|
-      describe "for operating system family #{osfamily}" do
-
-        let(:facts) {{ :osfamily => osfamily }}
-
-        it { should contain_file('/etc/ntp.conf').with_owner('0') }
-        it { should contain_file('/etc/ntp.conf').with_group('0') }
-        it { should contain_file('/etc/ntp.conf').with_mode('0644') }
-        it 'should allow template to be overridden' do
-          params[:config_template] = 'my_ntp/ntp.conf.erb'
-          content = param_value(subject, 'file', '/etc/ntp.conf', 'content')
-          expected_lines = ['server foobar']
-          (content.split("\n") & expected_lines).should == expected_lines
-        end
-      end
-    end
-  end
-end
diff --git a/spec/classes/ntp_install_spec.rb b/spec/classes/ntp_install_spec.rb
deleted file mode 100644
index 4ed263e..0000000
--- a/spec/classes/ntp_install_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe 'ntp::install' do
-
-  ['Debian', 'RedHat', 'SuSE', 'FreeBSD', 'Archlinux'].each do |osfamily|
-    describe "for osfamily #{osfamily}" do
-
-      let(:facts) {{ :osfamily => osfamily }}
-      let(:params) {{
-        :package_ensure => 'present',
-        :package_name   => 'ntp',
-      }}
-
-      it { should contain_package('ntp').with(
-        :ensure => 'present',
-        :name   => 'ntp'
-      )}
-
-      it 'should allow package ensure to be overridden' do
-        params[:package_ensure] = 'latest'
-        subject.should contain_package('ntp').with_ensure('latest')
-      end
-
-      it 'should allow the package name to be overridden' do
-        params[:package_name] = 'hambaby'
-        subject.should contain_package('ntp').with_name('hambaby')
-      end
-
-    end
-  end
-
-  describe "for distribution gentoo" do
-
-    let(:facts) {{ :osfamily => 'Linux', :operatingsystem => 'Gentoo' }}
-    let(:params) {{
-      :package_ensure => 'present',
-      :package_name   => 'net-misc/ntp',
-    }}
-
-    it { should contain_package('ntp').with(
-      :ensure => 'present',
-      :name   => 'net-misc/ntp'
-    )}
-
-    it 'should allow package ensure to be overridden' do
-      params[:package_ensure] = 'latest'
-      subject.should contain_package('ntp').with_ensure('latest')
-    end
-
-  end
-
-end
diff --git a/spec/classes/ntp_service_spec.rb b/spec/classes/ntp_service_spec.rb
deleted file mode 100644
index bbf9dd1..0000000
--- a/spec/classes/ntp_service_spec.rb
+++ /dev/null
@@ -1,73 +0,0 @@
-require 'spec_helper'
-
-describe 'ntp::service' do
-
-  ['Debian', 'RedHat', 'SuSE', 'FreeBSD', 'Archlinux'].each do |osfamily|
-    describe "for osfamily #{osfamily}" do
-
-      let(:facts) {{ :osfamily => osfamily }}
-      let(:params) {{
-        :service_manage => true,
-        :service_enable => true,
-        :service_ensure => 'running',
-        :service_name   => 'ntp'
-      }}
-
-      it { should contain_service('ntp').with(
-        :enable => true,
-        :ensure => 'running',
-        :name   => 'ntp'
-      )}
-
-      it 'should allow service ensure to be overridden' do
-        params[:service_ensure] = 'stopped'
-        subject.should contain_service('ntp').with_ensure('stopped')
-      end
-    end
-  end
-
-  ['Gentoo'].each do |operatingsystem|
-    describe "for distribution #{operatingsystem}" do
-
-      let(:facts) {{ :osfamily => 'Linux', :operatingsystem => operatingsystem }}
-      let(:params) {{
-        :service_manage => true,
-        :service_enable => true,
-        :service_ensure => 'running',
-        :service_name   => 'ntpd' }
-      }
-
-      it 'should contain service' do
-        should contain_service('ntp').with(
-          :enable => true,
-          :ensure => 'running',
-          :name   => 'ntpd')
-      end
-
-      it 'should allow service ensure to be overridden' do
-        params[:service_ensure] = 'stopped'
-        subject.should contain_service('ntp').with_ensure('stopped')
-      end
-
-    end
-  end
-
-  describe "isn't managed if service_manage is false" do
-
-    let(:facts) {{ :osfamily => 'Debian' }}
-
-    let(:params) {{
-      :service_manage => false,
-      :service_enable => true,
-      :service_ensure => 'running',
-      :service_name   => 'ntpd',
-    }}
-
-    it { should_not contain_service('ntp').with(
-      :enable => true,
-      :ensure => 'running',
-      :name   => 'ntpd'
-    )}
-  end
-
-end
diff --git a/spec/classes/ntp_spec.rb b/spec/classes/ntp_spec.rb
index 4ffd817..6c636f4 100644
--- a/spec/classes/ntp_spec.rb
+++ b/spec/classes/ntp_spec.rb
@@ -2,14 +2,260 @@ require 'spec_helper'
 
 describe 'ntp' do
 
-  let(:facts) {{ :osfamily => 'Debian' }}
+  ['Debian', 'RedHat','SuSE', 'FreeBSD', 'Archlinux', 'Gentoo'].each do |system|
+    if system == 'Gentoo'
+      let(:facts) {{ :osfamily => 'Linux', :operatingsystem => system }}
+    else
+      let(:facts) {{ :osfamily => system }}
+    end
 
-  it { should include_class('ntp::install') }
-  it { should include_class('ntp::config') }
-  it { should include_class('ntp::service') }
+    it { should include_class('ntp::install') }
+    it { should include_class('ntp::config') }
+    it { should include_class('ntp::service') }
 
-  # These are currently breaking for me.
-  #it { should have_class_count(3) }
-  #it { should have_resource_count(0) }
+    describe 'ntp::config on #{system}' do
+      it { should contain_file('/etc/ntp.conf').with_owner('0') }
+      it { should contain_file('/etc/ntp.conf').with_group('0') }
+      it { should contain_file('/etc/ntp.conf').with_mode('0644') }
+
+      describe 'allows template to be overridden' do
+        let(:params) {{ :config_template => 'my_ntp/ntp.conf.erb' }}
+        it { should contain_file('/etc/ntp.conf').with({
+          'content' => /server foobar/})
+        }
+      end
+
+      describe "keys for osfamily #{system}" do
+        context "when enabled" do
+          let(:params) {{
+            :keys_enable     => true,
+            :keys_file       => '/etc/ntp/ntp.keys',
+            :keys_trusted    => ['1', '2', '3'],
+            :keys_controlkey => '2',
+            :keys_requestkey => '3',
+          }}
+
+          it { should contain_file('/etc/ntp').with({
+            'ensure'  => 'directory'})
+          }
+          it { should contain_file('/etc/ntp.conf').with({
+            'content' => /trustedkey 1 2 3/})
+          }
+          it { should contain_file('/etc/ntp.conf').with({
+            'content' => /controlkey 2/})
+          }
+          it { should contain_file('/etc/ntp.conf').with({
+            'content' => /requestkey 3/})
+          }
+        end
+      end
+
+      context "when disabled" do
+        let(:params) {{
+          :keys_enable     => false,
+          :keys_file       => '/etc/ntp/ntp.keys',
+          :keys_trusted    => ['1', '2', '3'],
+          :keys_controlkey => '2',
+          :keys_requestkey => '3',
+        }}
+
+        it { should_not contain_file('/etc/ntp').with({
+          'ensure'  => 'directory'})
+        }
+        it { should_not contain_file('/etc/ntp.conf').with({
+          'content' => /trustedkey 1 2 3/})
+        }
+        it { should_not contain_file('/etc/ntp.conf').with({
+          'content' => /controlkey 2/})
+        }
+        it { should_not contain_file('/etc/ntp.conf').with({
+          'content' => /requestkey 3/})
+        }
+      end
+
+      describe 'preferred servers' do
+        context "when set" do
+          let(:params) {{
+            :servers           => ['a', 'b', 'c', 'd'],
+            :preferred_servers => ['a', 'b']
+          }}
+
+          it { should contain_file('/etc/ntp.conf').with({
+            'content' => /server a prefer\nserver b prefer\nserver c\nserver d/})
+          }
+        end
+        context "when not set" do
+          let(:params) {{
+            :servers           => ['a', 'b', 'c', 'd'],
+            :preferred_servers => []
+          }}
+
+          it { should_not contain_file('/etc/ntp.conf').with({
+            'content' => /server a prefer/})
+          }
+        end
+      end
+
+      describe 'ntp::install on #{system}' do
+        let(:params) {{ :package_ensure => 'present', :package_name => ['ntp'], }}
+
+        it { should contain_package('ntp').with(
+          :ensure => 'present',
+          :name   => 'ntp'
+        )}
+
+        describe 'should allow package ensure to be overridden' do
+          let(:params) {{ :package_ensure => 'latest', :package_name => ['ntp'] }}
+          it { should contain_package('ntp').with_ensure('latest') }
+        end
+
+        describe 'should allow the package name to be overridden' do
+          let(:params) {{ :package_ensure => 'present', :package_name => ['hambaby'] }}
+          it { should contain_package('ntp').with_name('hambaby') }
+        end
+      end
+
+      describe 'ntp::service' do
+        let(:params) {{
+          :service_manage => true,
+          :service_enable => true,
+          :service_ensure => 'running',
+          :service_name   => 'ntp'
+        }}
+
+        describe 'with defaults' do
+          it { should contain_service('ntp').with(
+            :enable => true,
+            :ensure => 'running',
+            :name   => 'ntp'
+          )}
+        end
+
+        describe 'service_ensure' do
+          describe 'when overridden' do
+            let(:params) {{ :service_name => 'ntp', :service_ensure => 'stopped' }}
+            it { should contain_service('ntp').with_ensure('stopped') }
+          end
+        end
+
+        describe 'service_manage' do
+          let(:params) {{
+            :service_manage => false,
+            :service_enable => true,
+            :service_ensure => 'running',
+            :service_name   => 'ntpd',
+          }}
+
+          it 'when set to false' do
+            should_not contain_service('ntp').with({
+              'enable' => true,
+              'ensure' => 'running',
+              'name'   => 'ntpd'
+            })
+          end
+        end
+      end
+    end
+
+    context 'ntp::config' do
+      describe "for operating system Gentoo" do
+        let(:facts) {{ :operatingsystem => 'Gentoo',
+                       :osfamily        => 'Linux' }}
+
+        it 'uses the NTP pool servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.gentoo.pool.ntp.org/,
+          })
+        end
+      end
+      describe "on osfamily Debian" do
+        let(:facts) {{ :osfamily => 'debian' }}
+
+        it 'uses the debian ntp servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.debian.pool.ntp.org iburst/,
+          })
+        end
+      end
+
+      describe "on osfamily RedHat" do
+        let(:facts) {{ :osfamily => 'RedHat' }}
+
+        it 'uses the redhat ntp servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.centos.pool.ntp.org/,
+          })
+        end
+      end
+
+      describe "on osfamily SuSE" do
+        let(:facts) {{ :osfamily => 'SuSE' }}
+
+        it 'uses the opensuse ntp servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.opensuse.pool.ntp.org/,
+          })
+        end
+      end
+
+      describe "on osfamily FreeBSD" do
+        let(:facts) {{ :osfamily => 'FreeBSD' }}
+
+        it 'uses the freebsd ntp servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.freebsd.pool.ntp.org iburst maxpoll 9/,
+          })
+        end
+      end
+
+      describe "on osfamily ArchLinux" do
+        let(:facts) {{ :osfamily => 'ArchLinux' }}
+
+        it 'uses the NTP pool servers by default' do
+          should contain_file('/etc/ntp.conf').with({
+            'content' => /server \d.pool.ntp.org/,
+          })
+        end
+      end
+
+      describe "for operating system family unsupported" do
+        let(:facts) {{
+          :osfamily  => 'unsupported',
+        }}
+
+        it { expect{ subject }.to raise_error(
+          /^The ntp module is not supported on an unsupported based system./
+        )}
+      end
+    end
+
+    describe 'for virtual machines' do
+      let(:facts) {{ :osfamily        => 'Archlinux',
+                     :is_virtual      => 'true' }}
+
+      it 'should not use local clock as a time source' do
+        should_not contain_file('/etc/ntp.conf').with({
+          'content' => /server.*127.127.1.0.*fudge.*127.127.1.0 stratum 10/,
+        })
+      end
+
+      it 'allows large clock skews' do
+        should contain_file('/etc/ntp.conf').with({
+          'content' => /tinker panic 0/,
+        })
+      end
+    end
+
+    describe 'for physical machines' do
+      let(:facts) {{ :osfamily        => 'Archlinux',
+                     :is_virtual      => 'false' }}
+
+      it 'disallows large clock skews' do
+        should_not contain_file('/etc/ntp.conf').with({
+          'content' => /tinker panic 0/,
+        })
+      end
+    end
+  end
 
 end
diff --git a/spec/system/ntp_config_spec.rb b/spec/system/ntp_config_spec.rb
index 263bc9d..194cdf1 100644
--- a/spec/system/ntp_config_spec.rb
+++ b/spec/system/ntp_config_spec.rb
@@ -27,9 +27,9 @@ describe 'ntp::config class' do
     end
   end
 
-    describe file('/etc/ntp.conf') do
-      it { should be_file }
-      it { should contain line }
-    end
+  describe file('/etc/ntp.conf') do
+    it { should be_file }
+    it { should contain line }
+  end
 
 end
diff --git a/spec/system/preferred_servers_spec.rb b/spec/system/preferred_servers_spec.rb
new file mode 100644
index 0000000..686861b
--- /dev/null
+++ b/spec/system/preferred_servers_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper_system'
+
+describe 'preferred servers' do
+  it 'applies cleanly' do
+    puppet_apply(%{
+      class { '::ntp':
+        servers           => ['a', 'b', 'c', 'd'],
+        preferred_servers => ['c', 'd'],
+      }
+    })
+  end
+
+  describe file('/etc/ntp.conf') do
+    it { should be_file }
+    it { should contain 'server a' }
+    it { should contain 'server b' }
+    it { should contain 'server c prefer' }
+    it { should contain 'server d prefer' }
+  end
+end
diff --git a/templates/ntp.conf.archlinux.erb b/templates/ntp.conf.archlinux.erb
deleted file mode 100644
index ef3b725..0000000
--- a/templates/ntp.conf.archlinux.erb
+++ /dev/null
@@ -1,30 +0,0 @@
-# With the default settings below, ntpd will only synchronize your clock.
-#
-# For details, see:
-# - the ntp.conf man page
-# - http://support.ntp.org/bin/view/Support/GettingStarted
-# - https://wiki.archlinux.org/index.php/Network_Time_Protocol_daemon
-#
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# # when a VM guest is suspended and resumed.
-tinker panic 0
-<% end -%>
-
-# Associate to public NTP pool servers; see http://www.pool.ntp.org/
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
-
-<% if @restrict -%>
-# Only allow read-only access from localhost
-restrict default noquery nopeer
-restrict 127.0.0.1
-restrict ::1
-<% end -%>
-
-# Location of drift file
-driftfile /var/lib/ntp/ntp.drift
-
-# NOTE: If you run dhcpcd and have lines like 'restrict' and 'fudge' appearing
-# here, be sure to add '-Y -N' to the dhcpcd_ethX variables in /etc/conf.d/net
diff --git a/templates/ntp.conf.debian.erb b/templates/ntp.conf.debian.erb
deleted file mode 100644
index 1ca6b20..0000000
--- a/templates/ntp.conf.debian.erb
+++ /dev/null
@@ -1,63 +0,0 @@
-# /etc/ntp.conf, configuration for ntpd; see ntp.conf(5) for help
-
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# when a VM guest is suspended and resumed.
-tinker panic 0
-
-<% end -%>
-driftfile /var/lib/ntp/ntp.drift
-
-
-# Enable this if you want statistics to be logged.
-#statsdir /var/log/ntpstats/
-
-statistics loopstats peerstats clockstats
-filegen loopstats file loopstats type day enable
-filegen peerstats file peerstats type day enable
-filegen clockstats file clockstats type day enable
-
-
-# You do need to talk to an NTP server or two (or three).
-#server ntp.your-provider.example
-
-# pool.ntp.org maps to about 1000 low-stratum NTP servers.  Your server will
-# pick a different set every time it starts up.  Please consider joining the
-# pool: <http://www.pool.ntp.org/join.html>
-
-# Managed by puppet class { 'ntp': servers => [ ... ] }
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
-
-<% if @restrict -%>
-# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for
-# details.  The web page <http://support.ntp.org/bin/view/Support/AccessRestrictions>
-# might also be helpful.
-#
-# Note that "restrict" applies to both servers and clients, so a configuration
-# that might be intended to block requests from certain clients could also end
-# up blocking replies from your own upstream servers.
-
-# By default, exchange time with everybody, but don't allow configuration.
-restrict -4 default kod notrap nomodify nopeer noquery
-restrict -6 default kod notrap nomodify nopeer noquery
-
-# Local users may interrogate the ntp server more closely.
-restrict 127.0.0.1
-restrict ::1
-
-# Clients from this (example!) subnet have unlimited access, but only if
-# cryptographically authenticated.
-#restrict 192.168.123.0 mask 255.255.255.0 notrust
-
-<% end -%>
-
-# If you want to provide time to your local subnet, change the next line.
-# (Again, the address is an example only.)
-#broadcast 192.168.123.255
-
-# If you want to listen to time broadcasts on your local subnet, de-comment the
-# next lines.  Please do this only if you trust everybody on the network!
-#disable auth
-#broadcastclient
diff --git a/templates/ntp.conf.el.erb b/templates/ntp.conf.el.erb
deleted file mode 100644
index 9a5d51b..0000000
--- a/templates/ntp.conf.el.erb
+++ /dev/null
@@ -1,62 +0,0 @@
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# when a VM guest is suspended and resumed.
-tinker panic 0
-
-<% end -%>
-<% if @restrict -%>
-# Permit time synchronization with our time source, but do not
-# permit the source to query or modify the service on this system.
-restrict default kod nomodify notrap nopeer noquery
-restrict -6 default kod nomodify notrap nopeer noquery
-
-# Permit all access over the loopback interface.  This could
-# be tightened as well, but to do so would effect some of
-# the administrative functions.
-restrict 127.0.0.1 
-restrict -6 ::1
-
-# Hosts on local network are less restricted.
-#restrict 192.168.1.0 mask 255.255.255.0 nomodify notrap
-
-<% end -%>
-# Use public servers from the pool.ntp.org project.
-# Please consider joining the pool (http://www.pool.ntp.org/join.html).
-
-# Managed by puppet class { 'ntp': servers => [ ... ] }
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
-
-#broadcast 192.168.1.255 key 42		# broadcast server
-#broadcastclient			# broadcast client
-#broadcast 224.0.1.1 key 42		# multicast server
-#multicastclient 224.0.1.1		# multicast client
-#manycastserver 239.255.254.254		# manycast server
-#manycastclient 239.255.254.254 key 42	# manycast client
-
-<% if scope.lookupvar('::is_virtual') == "false" -%>
-# Undisciplined Local Clock. This is a fake driver intended for backup
-# and when no outside source of synchronized time is available. 
-server	127.127.1.0	# local clock
-fudge	127.127.1.0 stratum 10	
-<% end -%>
-
-# Drift file.  Put this in a directory which the daemon can write to.
-# No symbolic links allowed, either, since the daemon updates the file
-# by creating a temporary in the same directory and then rename()'ing
-# it to the file.
-driftfile /var/lib/ntp/drift
-
-# Key file containing the keys and key identifiers used when operating
-# with symmetric key cryptography. 
-keys /etc/ntp/keys
-
-# Specify the key identifiers which are trusted.
-#trustedkey 4 8 42
-
-# Specify the key identifier to use with the ntpdc utility.
-#requestkey 8
-
-# Specify the key identifier to use with the ntpq utility.
-#controlkey 8
diff --git a/templates/ntp.conf.erb b/templates/ntp.conf.erb
new file mode 100644
index 0000000..c0a821b
--- /dev/null
+++ b/templates/ntp.conf.erb
@@ -0,0 +1,44 @@
+# ntp.conf: Managed by puppet.
+#
+<% if @panic == false -%>
+# Keep ntpd from panicking in the event of a large clock skew
+# when a VM guest is suspended and resumed.
+tinker panic 0
+<% end -%>
+
+<% if @restrict -%>
+# Permit time synchronization with our time source, but do not
+# permit the source to query or modify the service on this system.
+restrict default kod nomodify notrap nopeer noquery
+restrict -6 default kod nomodify notrap nopeer noquery
+restrict 127.0.0.1 
+restrict -6 ::1
+<% end -%>
+
+# Servers
+<% [@servers].flatten.each do |server| -%>
+server <%= server %><% if @preferred_servers.include?(server) -%> prefer<% end %>
+<% end -%>
+
+<% if scope.lookupvar('::is_virtual') == "false" -%>
+# Undisciplined Local Clock. This is a fake driver intended for backup
+# and when no outside source of synchronized time is available. 
+server	127.127.1.0	# local clock
+fudge	127.127.1.0 stratum 10	
+<% end -%>
+
+# Driftfile.
+driftfile <%= @driftfile %>
+
+<% if @keys_enable -%>
+keys <%= @keys_file %>
+<% unless @keys_trusted.empty? -%>
+trustedkey <%= @keys_trusted.join(' ') %>
+<% end -%>
+<% if @keys_requestkey != '' -%>
+requestkey <%= @keys_requestkey %>
+<% end -%>
+<% if @keys_controlkey != '' -%>
+controlkey <%= @keys_controlkey %>
+<% end -%>
+<% end -%>
diff --git a/templates/ntp.conf.freebsd.erb b/templates/ntp.conf.freebsd.erb
deleted file mode 100644
index 12be09c..0000000
--- a/templates/ntp.conf.freebsd.erb
+++ /dev/null
@@ -1,29 +0,0 @@
-#
-# $FreeBSD: release/9.0.0/etc/ntp.conf 195652 2009-07-13 05:51:33Z dwmalone $
-#
-# Default NTP servers for the FreeBSD operating system.
-#
-# Don't forget to enable ntpd in /etc/rc.conf with:
-# ntpd_enable="YES"
-#
-# The driftfile is by default /var/db/ntpd.drift, check
-# /etc/defaults/rc.conf on how to change the location.
-#
-#
-# See http://www.pool.ntp.org/ for details. Note, the pool encourages
-# users with a static IP and good upstream NTP servers to add a server
-# to the pool. See http://www.pool.ntp.org/join.html if you are interested.
-#
-# The option `iburst' is used for faster initial synchronisation.
-# The option `maxpoll 9' is used to prevent PLL/FLL flipping on FreeBSD.
-#
-# Managed by puppet class { 'ntp': servers => [ ... ] }
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# when a VM guest is suspended and resumed.
-tinker panic 0
-<% end -%>
-
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
diff --git a/templates/ntp.conf.gentoo.erb b/templates/ntp.conf.gentoo.erb
deleted file mode 100644
index c981c7d..0000000
--- a/templates/ntp.conf.gentoo.erb
+++ /dev/null
@@ -1,59 +0,0 @@
-# NOTES:
-# DHCP clients can append or replace NTP configuration files.
-# You should consult your DHCP client documentation about its
-# default behaviour and how to change it.
-
-# Name of the servers ntpd should sync with
-# Please respect the access policy as stated by the responsible person.
-#server   ntp.example.tld   iburst
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# # when a VM guest is suspended and resumed.
-tinker panic 0
-<% end -%>
-
-# Managed by puppet class { 'ntp': servers => [ ... ] }
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
-
-# Common pool for random people
-#server pool.ntp.org
-
-##
-# A list of available servers can be found here:
-# http://www.pool.ntp.org/
-# http://www.pool.ntp.org/#use
-# A good way to get servers for your machine is:
-# netselect -s 3 pool.ntp.org
-##
-
-# you should not need to modify the following paths
-driftfile /var/lib/ntp/ntp.drift
-
-#server ntplocal.example.com prefer 
-#server timeserver.example.org 
-
-# Warning: Using default NTP settings will leave your NTP
-# server accessible to all hosts on the Internet.
-
-# If you want to deny all machines (including your own)
-# from accessing the NTP server, uncomment:
-#restrict default ignore
-
-
-<% if @restrict -%>
-# To deny other machines from changing the
-# configuration but allow localhost:
-restrict default nomodify nopeer
-restrict 127.0.0.1
-restrict ::1
-<% end -%>
-
-
-# To allow machines within your network to synchronize
-# their clocks with your server, but ensure they are
-# not allowed to configure the server or used as peers
-# to synchronize against, uncomment this line.
-#
-#restrict 192.168.0.0 mask 255.255.255.0 nomodify nopeer notrap
diff --git a/templates/ntp.conf.suse.erb b/templates/ntp.conf.suse.erb
deleted file mode 100644
index 1a4361e..0000000
--- a/templates/ntp.conf.suse.erb
+++ /dev/null
@@ -1,87 +0,0 @@
-################################################################################
-## /etc/ntp.conf
-##
-## Sample NTP configuration file.
-## See package 'ntp-doc' for documentation, Mini-HOWTO and FAQ.
-## Copyright (c) 1998 S.u.S.E. GmbH Fuerth, Germany.
-##
-## Author: Michael Andres,  <ma@suse.de>
-##         Michael Skibbe,  <mskibbe@suse.de>
-##
-################################################################################
-
-##
-## Radio and modem clocks by convention have addresses in the 
-## form 127.127.t.u, where t is the clock type and u is a unit 
-## number in the range 0-3. 
-##
-## Most of these clocks require support in the form of a 
-## serial port or special bus peripheral. The particular  
-## device is normally specified by adding a soft link 
-## /dev/device-u to the particular hardware device involved, 
-## where u correspond to the unit number above. 
-## 
-## Generic DCF77 clock on serial port (Conrad DCF77)
-## Address:     127.127.8.u
-## Serial Port: /dev/refclock-u
-##  
-## (create soft link /dev/refclock-0 to the particular ttyS?)
-##
-# server 127.127.8.0 mode 5 prefer
-
-<% if @panic == true -%>
-##
-## Undisciplined Local Clock. This is a fake driver intended for backup
-## and when no outside source of synchronized time is available.
-##
-server 127.127.1.0		# local clock (LCL)
-fudge  127.127.1.0 stratum 10	# LCL is unsynchronized
-<% end -%>
-
-# Managed by puppet class { 'ntp': servers => [ ... ] }
-<% [@servers].flatten.each do |server| -%>
-server <%= server %>
-<% end -%>
-
-<% if @panic == false -%>
-# Keep ntpd from panicking in the event of a large clock skew
-# when a VM guest is suspended and resumed.
-tinker panic 0
-
-<% end -%>
-##
-## Miscellaneous stuff
-##
-
-driftfile /var/lib/ntp/drift/ntp.drift # path for drift file
-
-logfile   /var/log/ntp		# alternate log file
-# logconfig =syncstatus + sysevents
-# logconfig =all
-
-# statsdir /tmp/		# directory for statistics files
-# filegen peerstats  file peerstats  type day enable
-# filegen loopstats  file loopstats  type day enable
-# filegen clockstats file clockstats type day enable
-
-<% if @restrict -%>
-# Permit time synchronization with our time source, but do not
-# permit the source to query or modify the service on this system.
-restrict -4 default kod nomodify notrap nopeer noquery
-restrict -6 default kod nomodify notrap nopeer noquery
-
-# Permit all access over the loopback interface. This could
-# be tightened as well, but to do so would effect some of
-# the administrative functions.
-restrict 127.0.0.1
-restrict -6 ::1
-
-<% end -%>
-
-#
-# Authentication stuff
-#
-keys /etc/ntp.keys		# path for keys file
-trustedkey 1			# define trusted keys
-requestkey 1			# key (7) for accessing server variables
-# controlkey 15			# key (6) for accessing server variables