Various refactoring to improve code readability

Hopefully some of the logic in (especially) `init.pp` is less complicated.
It should now also be possible to split `init.pp` into the
`package -> config ~> service` pattern if someone wants to take on that
work.

* Replace `if != and !=` with `unless == or ==`
* Replace `$snmptrapd_conf_notify` with `subscribes`
* Refactor `snmpv3_user` by replacing `require` with `before`
* Remove duplication of identical `Service['snmptrapd']` resources
* Remove some redundant relationships and reorder resources in init.pp
* Refactor client.pp
* Don't use absolute class names
* Use proper relationship matchers in tests
* Replace Exec with File resource for `/etc/init.d/snmptrapd` on Suse

README.md

@@ -112,7 +112,7 @@ class { 'snmp':
 If you just want to install the SNMP client:
 
 ```puppet
-include ::snmp::client
+include snmp::client
 ```
 
 To install the SNMP service and the client:

manifests/client.pp

@@ -64,10 +64,11 @@
     $file_ensure = 'absent'
   }
 
-  if $::osfamily != 'Suse' {
+  unless $::osfamily == 'Suse' {
     package { 'snmp-client':
       ensure => $package_ensure,
       name   => $package_name,
+      before => File['snmp.conf'],
     }
   }
 
@@ -77,19 +78,12 @@
     }
   }
 
-  $req = $::osfamily ? {
-    'RedHat' => [Package['snmp-client'], File['/etc/snmp']],
-    'Suse'   => undef,
-    default  => Package['snmp-client'],
-  }
-
   file { 'snmp.conf':
     ensure  => $file_ensure,
     mode    => '0644',
     owner   => 'root',
     group   => 'root',
     path    => $snmp::params::client_config,
     content => template('snmp/snmp.conf.erb'),
-    require => $req,
   }
 }

manifests/init.pp

@@ -399,12 +399,6 @@
     $trapdrun = 'no'
   }
 
-  if $::osfamily != 'Debian' {
-    $snmptrapd_conf_notify = Service['snmptrapd']
-  } else {
-    $snmptrapd_conf_notify = Service['snmpd']
-  }
-
   if $manage_client {
     class { 'snmp::client':
       ensure      => $ensure,
@@ -413,6 +407,7 @@
     }
   }
 
+  # Install
   package { 'snmpd':
     ensure => $package_ensure,
     name   => $package_name,
@@ -437,6 +432,18 @@
     }
   }
 
+  if $::osfamily == 'Suse' {
+    file { '/etc/init.d/snmptrapd':
+      source  => '/usr/share/doc/packages/net-snmp/rc.snmptrapd',
+      owner   => 'root',
+      group   => 'root',
+      mode    => '0755',
+      require => Package['snmpd'],
+      before  => Service['snmptrapd'],
+    }
+  }
+
+  # Config
   file { 'snmpd.conf':
     ensure  => $file_ensure,
     mode    => $service_config_perms,
@@ -445,10 +452,20 @@
     path    => $snmp::params::service_config,
     content => template($template_snmpd_conf),
     require => Package['snmpd'],
-    notify  => Service['snmpd'],
   }
 
-  if $::osfamily != 'FreeBSD' and $::osfamily != 'OpenBSD' {
+  file { 'snmptrapd.conf':
+    ensure  => $file_ensure,
+    mode    => $service_config_perms,
+    owner   => 'root',
+    group   => $service_config_dir_group,
+    path    => $snmp::params::trap_service_config,
+    content => template($template_snmptrapd),
+    require => Package['snmpd'],
+  }
+
+
+  unless $::osfamily == 'FreeBSD' or $::osfamily == 'OpenBSD' {
     file { 'snmpd.sysconfig':
       ensure  => $file_ensure,
       mode    => '0644',
@@ -461,17 +478,6 @@
     }
   }
 
-  file { 'snmptrapd.conf':
-    ensure  => $file_ensure,
-    mode    => $service_config_perms,
-    owner   => 'root',
-    group   => $service_config_dir_group,
-    path    => $snmp::params::trap_service_config,
-    content => template($template_snmptrapd),
-    require => Package['snmpd'],
-    notify  => $snmptrapd_conf_notify,
-  }
-
   if $::osfamily == 'RedHat' {
     file { 'snmptrapd.sysconfig':
       ensure  => $file_ensure,
@@ -483,45 +489,18 @@
       require => Package['snmpd'],
       notify  => Service['snmptrapd'],
     }
+  }
 
+  # Services
+  unless $::osfamily == 'Debian' {
     service { 'snmptrapd':
       ensure     => $trap_service_ensure_real,
       name       => $trap_service_name,
       enable     => $trap_service_enable_real,
       hasstatus  => $trap_service_hasstatus,
       hasrestart => $trap_service_hasrestart,
-      require    => [ Package['snmpd'], File['var-net-snmp'], ],
-    }
-  } elsif $::osfamily == 'Suse' {
-    exec { 'install /etc/init.d/snmptrapd':
-      command => '/usr/bin/install -o 0 -g 0 -m0755 -p /usr/share/doc/packages/net-snmp/rc.snmptrapd /etc/init.d/snmptrapd',
-      creates => '/etc/init.d/snmptrapd',
-      require => Package['snmpd'],
-    }
-
-    service { 'snmptrapd':
-      ensure     => $trap_service_ensure_real,
-      name       => $trap_service_name,
-      enable     => $trap_service_enable_real,
-      hasstatus  => $trap_service_hasstatus,
-      hasrestart => $trap_service_hasrestart,
-      require    => [
-        Package['snmpd'],
-        File['var-net-snmp'],
-        Exec['install /etc/init.d/snmptrapd'],
-      ],
-    }
-  } elsif $::osfamily == 'FreeBSD'  or $::osfamily == 'OpenBSD' {
-    service { 'snmptrapd':
-      ensure     => $trap_service_ensure_real,
-      name       => $trap_service_name,
-      enable     => $trap_service_enable_real,
-      hasstatus  => $trap_service_hasstatus,
-      hasrestart => $trap_service_hasrestart,
-      require    => [
-        Package['snmpd'],
-        File['var-net-snmp'],
-      ],
+      require    => File['var-net-snmp'],
+      subscribe  => File['snmptrapd.conf'],
     }
   }
 
@@ -531,6 +510,11 @@
     enable     => $service_enable_real,
     hasstatus  => $service_hasstatus,
     hasrestart => $service_hasrestart,
-    require    => [ Package['snmpd'], File['var-net-snmp'], ],
+    require    => File['var-net-snmp'],
+    subscribe  => File['snmpd.conf'],
+  }
+
+  if $::osfamily == 'Debian' {
+    File['snmptrapd.conf'] ~> Service['snmpd']
   }
 }

manifests/snmpv3_user.pp

@@ -61,7 +61,7 @@
 
   Enum['snmpd','snmptrapd'] $daemon = 'snmpd'
 ) {
-  include ::snmp
+  include snmp
 
   if ($daemon == 'snmptrapd') and ($::osfamily != 'Debian') {
     $service_name   = 'snmptrapd'

spec/classes/snmp_client_spec.rb

@@ -51,9 +51,8 @@
             mode: '0644',
             owner: 'root',
             group: 'root',
-            path: '/etc/snmp/snmp.conf',
-            require: ['Package[snmp-client]', 'File[/etc/snmp]']
-          )
+            path: '/etc/snmp/snmp.conf'
+          ).that_requires(['Package[snmp-client]', 'File[/etc/snmp]'])
         }
       end
     end
@@ -83,9 +82,8 @@
             mode: '0644',
             owner: 'root',
             group: 'root',
-            path: '/etc/snmp/snmp.conf',
-            require: 'Package[snmp-client]'
-          )
+            path: '/etc/snmp/snmp.conf'
+          ).that_requires('Package[snmp-client]')
         }
       end
     end
@@ -141,8 +139,7 @@
             mode: '0755',
             owner: 'root',
             group: 'wheel',
-            path: '/usr/local/etc/snmp/snmp.conf',
-            require: nil
+            path: '/usr/local/etc/snmp/snmp.conf'
           )
         }
       end