apt: Change how update is managed.

* Instead of having 4 options controlling purging we now have a single
  hash with four possible keys.
* Include `apt::update` only _after_ we've assembled the `$_update`
  hash.

diff --git a/manifests/init.pp b/manifests/init.pp
index cc4a3308aba82c26959a93329f14f26fe5f3fdc1..ea546dc55ef8badde4921cf4b5a77715d8571186 100644 (file)
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,18 +1,33 @@
 #
 class apt(
+  $update               = {},
   $purge                = {},
   $proxy                = {},
-  $always_apt_update    = false,
-  $apt_update_frequency = 'reluctantly',
-  $update_timeout       = undef,
-  $update_tries         = undef,
   $sources              = undef,
 ) inherits ::apt::params {
 
-  include apt::update
 
   $frequency_options = ['always','daily','weekly','reluctantly']
-  validate_re($apt_update_frequency, $frequency_options)
+  validate_hash($update)
+  if $update['frequency'] {
+    validate_re($update['frequency'], $frequency_options)
+  }
+  if $update['always'] {
+    validate_bool($update['always'])
+  }
+  if $update['timeout'] {
+    unless is_integer($update['timeout']) {
+      fail('timeout value for update must be an integer')
+    }
+  }
+  if $update['tries'] {
+    unless is_integer($update['tries']) {
+      fail('tries value for update must be an integer')
+    }
+  }
+
+  $_update = merge($::apt::update_defaults, $update)
+  include apt::update
 
   validate_hash($purge)
   if $purge['sources.list'] {
@@ -62,7 +77,7 @@ class apt(
     true  => absent,
   }
 
-  if $always_apt_update == true {
+  if $_update['always'] {
     Exec <| title=='apt_update' |> {
       refreshonly => false,
     }

diff --git a/manifests/params.pp b/manifests/params.pp
index 51b6aea579ba055378b551967e43ff61f2607e1d..2a29b6d1be053a8a33d5fe7f6928c90e8e3ea4b8 100644 (file)
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -31,6 +31,13 @@ class apt::params {
     }
   }
 
+  $update_defaults = {
+    'always'    => false,
+    'frequency' => 'reluctantly',
+    'timeout'   => undef,
+    'tries'     => undef,
+  }
+
   $proxy_defaults = {
     'host'  => undef,
     'port'  => 8080,

diff --git a/manifests/update.pp b/manifests/update.pp
index 6f338f043c692525c71668291d4d8c97a24a2403..a19e90aacd091c423da07e2776a11c0eec70cfe1 100644 (file)
--- a/manifests/update.pp
+++ b/manifests/update.pp
@@ -4,10 +4,10 @@ class apt::update {
   #on the first run, but if it's not run in awhile something is likely borked
   #with apt and we'd want to know about it.
 
-  if $::apt::always_apt_update == false {
+  if $::apt::_update['always'] == false {
     #if always_apt_update is true there's no point in parsing this logic.
 
-    case $apt::apt_update_frequency {
+    case $::apt::_update['frequency'] {
       'always': {
         $_kick_apt = true
       }
@@ -60,8 +60,8 @@ class apt::update {
     command     => "${::apt::provider} update",
     logoutput   => 'on_failure',
     refreshonly => $_refresh,
-    timeout     => $apt::update_timeout,
-    tries       => $apt::update_tries,
+    timeout     => $::apt::_update['timeout'],
+    tries       => $::apt::_update['tries'],
     try_sleep   => 1
   }
 }

diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb
index a0219933946e168b88810622c4190c80ff4a9caa..d3559e4cef9453e6f944a88515275960571c13ec 100644 (file)
--- a/spec/classes/apt_spec.rb
+++ b/spec/classes/apt_spec.rb
@@ -96,11 +96,9 @@ describe 'apt' do
   context 'lots of non-defaults' do
     let :params do
       {
-        :always_apt_update    => true,
-        :purge                => { 'sources.list' => false, 'sources.list.d' => false,
-                                   'preferences' => false, 'preferences.d' => false, },
-        :update_timeout       => '1',
-        :update_tries         => '3',
+        :update => { 'always' => true, 'timeout' => 1, 'tries' => 3 },
+        :purge  => { 'sources.list' => false, 'sources.list.d' => false,
+                     'preferences' => false, 'preferences.d' => false, },
       }
     end
 
@@ -123,9 +121,9 @@ describe 'apt' do
     })}
 
     it { is_expected.to contain_exec('apt_update').with({
-      :refreshonly => 'false',
-      :timeout     => '1',
-      :tries       => '3',
+      :refreshonly => false,
+      :timeout     => 1,
+      :tries       => 3,
     })}
 
   end

diff --git a/spec/classes/apt_update_spec.rb b/spec/classes/apt_update_spec.rb
index d0bfae26759d0c4e7203c396c7781f8b987a1473..6ae9e8f9a58bfbddb1893117d47e83a41bf2a0a1 100644 (file)
--- a/spec/classes/apt_update_spec.rb
+++ b/spec/classes/apt_update_spec.rb
@@ -2,20 +2,20 @@
 require 'spec_helper'
 
 describe 'apt::update', :type => :class do
-  context 'when apt::always_apt_update is true' do
+  context "when update['always']=true" do
     #This should completely disable all of this logic. These tests are to guarantee that we don't somehow magically change the behavior.
     let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
-    let (:pre_condition) { "class{'::apt': always_apt_update => true}" }
+    let (:pre_condition) { "class{'::apt': update => {'always' => true},}" }
     it 'should trigger an apt-get update run' do
       #set the apt_update exec's refreshonly attribute to false
       is_expected.to contain_exec('apt_update').with({'refreshonly' => false })
     end
     ['always','daily','weekly','reluctantly'].each do |update_frequency|
-      context "when apt::apt_update_frequency has the value of #{update_frequency}" do
+      context "when apt::update['frequency'] has the value of #{update_frequency}" do
         { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
           context "and $::apt_update_last_success indicates #{desc}" do
             let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } }
-            let (:pre_condition) { "class{'::apt': always_apt_update => true, apt_update_frequency => '#{update_frequency}' }" }
+            let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
             it 'should trigger an apt-get update run' do
               # set the apt_update exec's refreshonly attribute to false
               is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
@@ -23,7 +23,7 @@ describe 'apt::update', :type => :class do
           end
           context 'when $::apt_update_last_success is nil' do
             let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
-            let (:pre_condition) { "class{'::apt': always_apt_update => true, apt_update_frequency => '#{update_frequency}' }" }
+            let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
             it 'should trigger an apt-get update run' do
               #set the apt_update exec\'s refreshonly attribute to false
               is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
@@ -34,12 +34,12 @@ describe 'apt::update', :type => :class do
     end
   end
 
-  context 'when apt::always_apt_update is false' do
-    context "and apt::apt_update_frequency has the value of always" do
+  context "when apt::update['always']=false" do
+    context "and apt::update['frequency']='always'" do
       { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
         context "and $::apt_update_last_success indicates #{desc}" do
           let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } }
-          let (:pre_condition) { "class{'::apt': always_apt_update => false, apt_update_frequency => 'always' }" }
+          let (:pre_condition) { "class{'::apt': update => {'always' => false, 'frequency' => 'always' },}" }
           it 'should trigger an apt-get update run' do
             #set the apt_update exec's refreshonly attribute to false
             is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
@@ -48,18 +48,18 @@ describe 'apt::update', :type => :class do
       end
       context 'when $::apt_update_last_success is nil' do
         let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
-        let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => 'always' }" }
+        let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'always' },}" }
         it 'should trigger an apt-get update run' do
           #set the apt_update exec\'s refreshonly attribute to false
           is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
         end
       end
     end
-    context "and apt::apt_update_frequency has the value of reluctantly" do
+    context "and apt::update['frequency']='reluctantly'" do
       {'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
         context "and $::apt_update_last_success indicates #{desc}" do
           let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval} }
-          let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => 'reluctantly' }" }
+          let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
           it 'should not trigger an apt-get update run' do
             #don't change the apt_update exec's refreshonly attribute. (it should be true)
             is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
@@ -68,7 +68,7 @@ describe 'apt::update', :type => :class do
       end
       context 'when $::apt_update_last_success is nil' do
         let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
-        let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => 'reluctantly' }" }
+        let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
         it 'should not trigger an apt-get update run' do
           #don't change the apt_update exec's refreshonly attribute. (it should be true)
           is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
@@ -76,11 +76,11 @@ describe 'apt::update', :type => :class do
       end
     end
     ['daily','weekly'].each do |update_frequency|
-      context "and apt::apt_update_frequency has the value of #{update_frequency}" do
+      context "and apt::update['frequency'] has the value of #{update_frequency}" do
         { 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
           context "and $::apt_update_last_success indicates #{desc}" do
             let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } }
-            let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => '#{update_frequency}' }" }
+            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
             it 'should trigger an apt-get update run' do
               #set the apt_update exec\'s refreshonly attribute to false
               is_expected.to contain_exec('apt_update').with({'refreshonly' => false})
@@ -89,7 +89,7 @@ describe 'apt::update', :type => :class do
         end
         context 'when the $::apt_update_last_success fact has a recent value' do
           let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => Time.now.to_i } }
-          let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => '#{update_frequency}' }" }
+            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
           it 'should not trigger an apt-get update run' do
             #don't change the apt_update exec\'s refreshonly attribute. (it should be true)
             is_expected.to contain_exec('apt_update').with({'refreshonly' => true})
@@ -97,7 +97,7 @@ describe 'apt::update', :type => :class do
         end
         context 'when $::apt_update_last_success is nil' do
           let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
-          let (:pre_condition) { "class{ '::apt': always_apt_update => false, apt_update_frequency => '#{update_frequency}' }" }
+            let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
           it 'should trigger an apt-get update run' do
             #set the apt_update exec\'s refreshonly attribute to false
             is_expected.to contain_exec('apt_update').with({'refreshonly' => false})