Merge pull request #56 from mhaskel/modernize

Modernize Module

.gitignore

@@ -1 +1,9 @@
 pkg/
+Gemfile.lock
+vendor/
+spec/fixtures/
+.vagrant/
+.bundle/
+coverage/
+.idea/
+*.iml

.travis.yml

@@ -0,0 +1,15 @@
+---
+language: ruby
+bundler_args: --without system_tests
+script: "bundle exec rake validate && bundle exec rake lint && bundle exec rake spec SPEC_OPTS='--format documentation'"
+matrix:
+  fast_finish: true
+  include:
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.0"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.0"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.0"
+notifications:
+  email: false

Gemfile

@@ -0,0 +1,31 @@
+source ENV['GEM_SOURCE'] || "https://rubygems.org"
+
+group :development, :unit_tests do
+  gem 'rake',                    :require => false
+  gem 'rspec-core', '3.1.7',     :require => false
+  gem 'rspec-puppet', '~> 1.0',  :require => false
+  gem 'puppetlabs_spec_helper',  :require => false
+  gem 'puppet-lint',             :require => false
+  gem 'simplecov',               :require => false
+  gem 'puppet_facts',            :require => false
+  gem 'json',                    :require => false
+end
+
+group :system_tests do
+  gem 'beaker-rspec',  :require => false
+  gem 'serverspec',    :require => false
+end
+
+if facterversion = ENV['FACTER_GEM_VERSION']
+  gem 'facter', facterversion, :require => false
+else
+  gem 'facter', :require => false
+end
+
+if puppetversion = ENV['PUPPET_GEM_VERSION']
+  gem 'puppet', puppetversion, :require => false
+else
+  gem 'puppet', :require => false
+end
+
+# vim:ft=ruby

README.md

@@ -15,14 +15,14 @@ Define the server and the zones it will be responsible for.
       dnsdomain      => [
         'dc1.example.net',
         '1.0.10.in-addr.arpa',
-        ],
-      nameservers    => ['10.0.1.20'],
-      ntpservers     => ['us.pool.ntp.org'],
-      interfaces     => ['eth0'],
-      dnsupdatekey   => "/etc/bind/keys.d/$ddnskeyname",
-      require        => Bind::Key[ $ddnskeyname ],
-      pxeserver      => '10.0.1.50',
-      pxefilename    => 'pxelinux.0',
+      ],
+      nameservers  => ['10.0.1.20'],
+      ntpservers   => ['us.pool.ntp.org'],
+      interfaces   => ['eth0'],
+      dnsupdatekey => "/etc/bind/keys.d/$ddnskeyname",
+      require      => Bind::Key[ $ddnskeyname ],
+      pxeserver    => '10.0.1.50',
+      pxefilename  => 'pxelinux.0',
     }
 
 ### dhcp::pool
@@ -48,14 +48,13 @@ requests forwarded by switches etc.
 ### dhcp::host
 Create host reservations.
 
-    dhcp::host {
-      'server1': mac => "00:50:56:00:00:01", ip => "10.0.1.51";
-      'server2': mac => "00:50:56:00:00:02", ip => "10.0.1.52";
-      'server3': mac => "00:50:56:00:00:03", ip => "10.0.1.53";
+    dhcp::host { 'server1':
+      mac => '00:50:56:00:00:01',
+      ip  => '10.0.1.51',
     }
 
 ## Contributors
 Zach Leslie <zach.leslie@gmail.com>
 Ben Hughes <git@mumble.org.uk>
 Sam Dunster <sdunster@uow.edu.au>
-
+Garrett Honeycutt <gh@learnpuppet.com>

Rakefile

@@ -1,2 +1,10 @@
-require 'rake'
 require 'puppetlabs_spec_helper/rake_tasks'
+require 'puppet-lint/tasks/puppet-lint'
+
+PuppetLint.configuration.fail_on_warnings = true
+PuppetLint.configuration.send('relative')
+PuppetLint.configuration.send('disable_80chars')
+PuppetLint.configuration.send('disable_class_inherits_from_params_class')
+PuppetLint.configuration.send('disable_documentation')
+PuppetLint.configuration.send('disable_single_quote_string_with_variables')
+PuppetLint.configuration.ignore_paths = ["spec/**/*.pp", "pkg/**/*.pp"]

manifests/failover.pp

@@ -1,22 +1,24 @@
+# == Class: dhcp::failover
+#
 class dhcp::failover (
-  $role                = 'primary',
-  $address             = $ipaddress,
   $peer_address,
+  $role                = 'primary',
+  $address             = $::ipaddress,
   $port                = '519',
   $max_response_delay  = '30',
   $max_unacked_updates = '10',
   $mclt                = '300',
   $load_split          = '128',
   $load_balance        = '3',
-  $omapi_key           = ''
+  $omapi_key           = '',
 ) {
 
   include dhcp::params
+
   $dhcp_dir = $dhcp::params::dhcp_dir
 
   concat::fragment { 'dhcp-conf-failover':
     target  => "${dhcp_dir}/dhcpd.conf",
     content => template('dhcp/dhcpd.conf.failover.erb'),
   }
-
 }

manifests/host.pp

@@ -1,10 +1,13 @@
+# == Define: dhcp::host
+#
 define dhcp::host (
   $ip,
   $mac,
   $comment=''
 ) {
 
   $host = $name
+
   include dhcp::params
 
   $dhcp_dir = $dhcp::params::dhcp_dir
@@ -15,4 +18,3 @@
     order   => '10',
   }
 }
-

manifests/init.pp

@@ -1,6 +1,8 @@
+# == Class: dhcp
+#
 class dhcp (
-  $dnsdomain,
-  $nameservers,
+  $dnsdomain           = undef,
+  $nameservers         = [ '8.8.8.8', '8.8.4.4' ],
   $ntpservers          = undef,
   $dhcp_conf_header    = 'INTERNAL_TEMPLATE',
   $dhcp_conf_ddns      = 'INTERNAL_TEMPLATE',
@@ -16,19 +18,30 @@
   $logfacility         = 'daemon',
   $default_lease_time  = 3600,
   $max_lease_time      = 86400,
-  $service_ensure      = running
+  $service_ensure      = running,
 ) {
-  #input validation
-  validate_array($dnsdomain)
+
+  if $dnsdomain == undef {
+    if $::domain {
+      $dnsdomain_real = [ $::domain ]
+    } else {
+      fail('dhcp::dnsdomain must be set and domain fact is missing to use as a default value.')
+    }
+  } else {
+    $dnsdomain_real = $dnsdomain
+  }
+  validate_array($dnsdomain_real)
+
   validate_array($nameservers)
   validate_array($ntpservers)
 
   include dhcp::params
+  include dhcp::monitor
 
-  $dhcp_dir    = $dhcp::params::dhcp_dir
-  $packagename = $dhcp::params::packagename
-  $packageprov = $dhcp::params::packageprov
-  $servicename = $dhcp::params::servicename
+  $dhcp_dir         = $dhcp::params::dhcp_dir
+  $packagename      = $dhcp::params::packagename
+  $servicename      = $dhcp::params::servicename
+  $package_provider = $dhcp::params::package_provider
 
   # Incase people set interface instead of interfaces work around
   # that. If they set both, use interfaces and the user is a unwise
@@ -70,7 +83,7 @@
 
   package { $packagename:
     ensure   => installed,
-    provider => $packageprov,
+    provider => $package_provider,
   }
 
   file { $dhcp_dir:
@@ -80,7 +93,7 @@
 
   # Only debian and ubuntu have this style of defaults for startup.
   case $::osfamily {
-    debian: {
+    'debian': {
       file{ '/etc/default/isc-dhcp-server':
         ensure  => present,
         owner   => 'root',
@@ -91,7 +104,7 @@
         content => template('dhcp/debian/default_isc-dhcp-server'),
       }
     }
-    redhat: {
+    'redhat': {
       file{ '/etc/sysconfig/dhcpd':
         ensure  => present,
         owner   => 'root',
@@ -179,7 +192,4 @@
     subscribe => [Concat["${dhcp_dir}/dhcpd.pools"], Concat["${dhcp_dir}/dhcpd.hosts"], File["${dhcp_dir}/dhcpd.conf"]],
     require   => Package[$packagename],
   }
-
-  include dhcp::monitor
-
 }

manifests/params.pp

@@ -1,38 +1,42 @@
+# == Class: dhcp::params
+#
 class dhcp::params {
 
   case $::osfamily {
-    default: {
-      fail("${::osfamily} not spupprted in ${module_name}")
-    }
-    debian: {
+    'debian': {
       if ( $::operatingsystem == 'ubuntu' ) {
-        if versioncmp($::operatingsystemrelease, '12.04') >= 0 {
+        if (versioncmp($::operatingsystemrelease, '12.04') >= 0) {
           $dhcp_dir    = '/etc/dhcp'
         } else {
           $dhcp_dir    = '/etc/dhcp3'
         }
       } else {
         $dhcp_dir    = '/etc/dhcp'
       }
-      $packagename = 'isc-dhcp-server'
-      $servicename = 'isc-dhcp-server'
+      $packagename      = 'isc-dhcp-server'
+      $servicename      = 'isc-dhcp-server'
+      $package_provider = undef
     }
-    darwin: {
-      $dhcp_dir    = '/opt/local/etc/dhcp'
-      $packagename = 'dhcp'
-      $packageprov = 'macports'
-      $servicename = 'org.macports.dhcpd'
+    'darwin': {
+      $dhcp_dir         = '/opt/local/etc/dhcp'
+      $packagename      = 'dhcp'
+      $servicename      = 'org.macports.dhcpd'
+      $package_provider = 'macports'
     }
-    freebsd: {
-      $dhcp_dir    = '/usr/local/etc'
-      $packagename = 'net/isc-dhcp42-server'
-      $servicename = 'isc-dhcpd'
+    'freebsd': {
+      $dhcp_dir         = '/usr/local/etc'
+      $packagename      = 'net/isc-dhcp42-server'
+      $servicename      = 'isc-dhcpd'
+      $package_provider = undef
     }
-    redhat: {
-      $dhcp_dir    = '/etc/dhcp'
-      $packagename = 'dhcp'
-      $servicename = 'dhcpd'
+    'redhat': {
+      $dhcp_dir         = '/etc/dhcp'
+      $packagename      = 'dhcp'
+      $servicename      = 'dhcpd'
+      $package_provider = undef
+    }
+    default: {
+      fail('dhcp is supported on the following OS\'s: Debian, Ubuntu, Darwin, FreeBSD, RedHat, Fedora, and CentOS.')
     }
   }
-
 }

manifests/pool.pp

@@ -1,3 +1,5 @@
+# == Define: dhcp::pool
+#
 define dhcp::pool (
   $network,
   $mask,
@@ -19,6 +21,4 @@
     target  => "${dhcp_dir}/dhcpd.pools",
     content => template('dhcp/dhcpd.pool.erb'),
   }
-
 }
-

spec/classes/dhcp_spec.rb

@@ -1,13 +1,22 @@
 require 'spec_helper'
 require 'puppetlabs_spec_helper/module_spec_helper'
 describe 'dhcp', :type => :class do
-  let :default_params do
+  let(:default_params) do
     {
-      'dnsdomain'   => ['sampledomain.com','1.1.1.in-addr.arpa'],
-      'nameservers' => ['1.1.1.1'],
-      'ntpservers'  => ['time.sample.com'],
+      'dnsdomain'           => ['sampledomain.com','1.1.1.in-addr.arpa'],
+      'nameservers'         => ['1.1.1.1'],
+      'ntpservers'          => ['time.sample.com'],
+      'dhcp_conf_header'    => 'eth0',
+      'dhcp_conf_ddns'      => 'INTERNAL_TEMPLATE',
+      'dhcp_conf_pxe'       => 'INTERNAL_TEMPLATE',
+      'dhcp_conf_extra'     => 'INTERNAL_TEMPLATE',
+      'dhcp_conf_fragments' => {},
+      'logfacility'         => 'daemon',
+      'default_lease_time'  => '3600',
+      'max_lease_time'      => '86400',
     }
   end
+
   context 'on a RedHat OS' do
     let :facts do
       {
@@ -17,20 +26,15 @@
         :concat_basedir         => '/dne',
       }
     end
-    context 'called with defaults and mandatory params' do
-      let :params do
-        default_params
-      end
-      it 'should fail to compile' do
-        expect { should compile }.to raise_error()
-      end
-      context 'input validation' do
-        ['dnsdomain','nameservers','ntpservers'].each do |arrays|
-          context "when #{arrays} is not an array" do
-            it 'should fail' do
-              params.merge!({ arrays => 'BOGON'})
-              expect { subject }.to raise_error(Puppet::Error, /"BOGON" is not an Array.  It looks to be a String/)
-            end
+    let :params do
+       default_params
+    end
+    context 'input validation' do
+      ['dnsdomain','nameservers','ntpservers'].each do |arrays|
+        context "when #{arrays} is not an array" do
+          it 'should fail' do
+            params.merge!({ arrays => 'BOGON'})
+            expect { subject }.to raise_error(Puppet::Error, /"BOGON" is not an Array.  It looks to be a String/)
           end
         end
       end

spec/defines/host_spec.rb

@@ -4,7 +4,7 @@
   let :title do
     'test_host'
   end
-  let :facts do
+  let(:facts) do
     {
       :concat_basedir => '/dne',
       :osfamily       => 'RedHat',
@@ -18,7 +18,5 @@
     }
   end
 
-  it {
-    should contain_concat__fragment("dhcp_host_#{title}")
-  }
+  it { should contain_concat__fragment("dhcp_host_#{title}") }
 end