From fe228435b1c9219b7fe61e6faafe810b117fd5e4 Mon Sep 17 00:00:00 2001 From: Daniele Sluijters Date: Sat, 28 Feb 2015 16:48:48 +0100 Subject: [PATCH] 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. --- manifests/init.pp | 29 ++++++++++++++++++++++------- manifests/params.pp | 7 +++++++ manifests/update.pp | 8 ++++---- spec/classes/apt_spec.rb | 14 ++++++-------- spec/classes/apt_update_spec.rb | 32 ++++++++++++++++---------------- 5 files changed, 55 insertions(+), 35 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index cc4a330..ea546dc 100644 --- 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 51b6aea..2a29b6d 100644 --- 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 6f338f0..a19e90a 100644 --- 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 a021993..d3559e4 100644 --- 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 d0bfae2..6ae9e8f 100644 --- 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}) -- 2.32.3