Only install directories if tftp is managed (#267)

theforeman · Sep 15, 2016 · 7e24c97 · 7e24c97
1 parent 5f6393d
commit 7e24c97
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 24 deletions.
diff --git a/manifests/config.pp b/manifests/config.pp
@@ -14,7 +14,7 @@
     Class['puppet::server::install'] -> Class['foreman_proxy::config']
   }
 
-  if $foreman_proxy::tftp     { include ::foreman_proxy::tftp }
+  if $foreman_proxy::tftp and $foreman_proxy::tftp_managed { include ::foreman_proxy::tftp }
 
   # Somehow, calling these DHCP and DNS seems to conflict. So, they get a prefix...
   if $foreman_proxy::dhcp and $foreman_proxy::dhcp_managed { include ::foreman_proxy::proxydhcp }

diff --git a/manifests/tftp.pp b/manifests/tftp.pp
@@ -1,35 +1,26 @@
 # Set up the tftp component
-class foreman_proxy::tftp (
-  $tftp_managed = $foreman_proxy::tftp_managed,
-) {
-  $required_tftp_classes = $tftp_managed ? {
-    true    => Class['foreman_proxy::install', 'tftp::install'],
-    default => Class['foreman_proxy::install'],
+class foreman_proxy::tftp {
+  class { '::tftp':
+    root => $foreman_proxy::tftp_root,
   }
 
-  file{ $foreman_proxy::tftp_dirs:
+  file { $foreman_proxy::tftp_dirs:
     ensure  => directory,
     owner   => $foreman_proxy::user,
     mode    => '0644',
-    require => $required_tftp_classes,
-    recurse => true;
+    require => Class['foreman_proxy::install', 'tftp::install'],
+    recurse => true,
   }
 
-  foreman_proxy::tftp::copy_file{$foreman_proxy::tftp_syslinux_filenames:
+  foreman_proxy::tftp::copy_file { $foreman_proxy::tftp_syslinux_filenames:
     target_path => $foreman_proxy::tftp_root,
-    require     => $required_tftp_classes;
+    require     => Class['foreman_proxy::install', 'tftp::install'],
   }
 
   if $foreman_proxy::tftp_manage_wget {
     ensure_packages(['wget'], { ensure => $foreman_proxy::ensure_packages_version, })
   }
 
-  if $tftp_managed {
-    class { '::tftp':
-      root => $foreman_proxy::tftp_root,
-    }
-  }
-
   case $::osfamily {
     'RedHat': {
       $osreleasemajor = regsubst($::operatingsystemrelease, '^(\d+)\..*$', '\1') # workaround for the possibly missing operatingsystemmajrelease

diff --git a/spec/classes/foreman_proxy__config__spec.rb b/spec/classes/foreman_proxy__config__spec.rb
@@ -39,7 +39,7 @@
 
         it { should compile.with_all_deps }
 
-        it 'should include tftp' do
+        it 'should include managed tftp' do
           should contain_class('foreman_proxy::tftp')
           should contain_class('tftp')
         end
@@ -472,8 +472,8 @@
           }'
         end
 
-        it 'should include the foreman-proxy tftp class' do
-          should contain_class('foreman_proxy::tftp')
+        it 'should not include the foreman-proxy tftp class' do
+          should_not contain_class('foreman_proxy::tftp')
         end
 
         it 'should not include the ::tftp class' do
@@ -497,10 +497,10 @@
         end
       end
 
-      context 'with TFTP enabled and tftp_syslinux_filenames set' do
+      context 'with tftp_managed enabled and tftp_syslinux_filenames set' do
         let :pre_condition do
           'class {"foreman_proxy":
-            tftp => true,
+            tftp_managed            => true,
             tftp_syslinux_filenames => [ "/my/file", "/my/anotherfile" ],
           }'
         end
@@ -511,7 +511,7 @@
         end
       end
 
-      context 'with tftp_manage_wget disabled' do
+      context 'with tftp_managed enabled and tftp_manage_wget disabled' do
         let :pre_condition do
           'class {"foreman_proxy":
             tftp_manage_wget => false,