From: Morgan Haskel <morgan@puppetlabs.com>
Date: Fri, 26 Sep 2014 19:01:16 +0000 (-0400)
Subject: Merge pull request #349 from wolfspyre/apt_update_tooling
X-Git-Tag: 1.7.0~2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=43b894b12e53cb6943dfb936c5bdc700ee4997b2;hp=761a4d26c7b05f91d914033236d14a3557e6eab6;p=puppet-modules%2Fpuppetlabs-apt.git

Merge pull request #349 from wolfspyre/apt_update_tooling

Apt update tooling
---

diff --git a/README.md b/README.md
index c7938a6..b0574a7 100644
--- a/README.md
+++ b/README.md
@@ -48,6 +48,7 @@ The parameters for `apt` are not generally required and are predominantly for de
 
     class { 'apt':
       always_apt_update    => false,
+      apt_update_frequency => undef,
       disable_keys         => undef,
       proxy_host           => false,
       proxy_port           => '8080',
@@ -153,9 +154,9 @@ Moreover, when Puppet is told to hold a package, it holds it at the current vers
 
 At first glance, it seems this issue could also be solved by passing the version required to the ensure attribute---but that only means that Puppet will install that
 version after it processes the package. It does not inform apt that we want
-this package to be held; that is, should another package want to upgrade this one (because of a version requirement in a dependency, for example), we want apt to refuse. 
+this package to be held; that is, should another package want to upgrade this one (because of a version requirement in a dependency, for example), we want apt to refuse.
 
-To solve this issue, use apt::hold. Implement this by creating a preferences file with a priority of 1001. Under normal circumstances, this preference will always win. Because the priority is > 1000, apt will maintain the required version, downgrading the current package if necessary. 
+To solve this issue, use apt::hold. Implement this by creating a preferences file with a priority of 1001. Under normal circumstances, this preference will always win. Because the priority is > 1000, apt will maintain the required version, downgrading the current package if necessary.
 
 With this, you can now set a package's ensure attribute to 'latest' but get the version specified by apt::hold:
 
@@ -211,7 +212,7 @@ If you would like to configure your system so the source is the Puppet Labs APT
 
 ### apt::update
 
-Runs `apt-get update`, updating the list of available packages and their versions without installing or upgrading any packages. 
+Runs `apt-get update`, updating the list of available packages and their versions without installing or upgrading any packages.
 
 The update runs on the first Puppet run after you include the class, then whenever `notify  => Exec['apt_update']` occurs---this should happen when config files get updated or other relevant changes occur. If you set the `always_apt_update` parameter, the update will run on every Puppet run.
 
@@ -222,6 +223,7 @@ There are a few facts included in the apt module describing the state of the apt
 * `apt_updates` --- the number of updates available on the system
 * `apt_security_updates` --- the number of updates which are security updates
 * `apt_package_updates` --- the package names that are available for update. In Facter 2.0 and later, this will be a list type; in earlier versions, it is a comma-delimited string.
+* `apt_update_last_success` --- The date in epochtime that `apt-get update` last ran successfully. This is determined by reading the mtime of the file `/var/lib/apt/periodic/update-success-stamp`. That file is generated by the `/etc/apt/apt.conf.d/15update-stamp` file.
 
 #### Hiera example
 <pre>
@@ -332,3 +334,4 @@ A lot of great people have contributed to this module. A somewhat current list f
 * Zach Leslie <zach@puppetlabs.com>
 * Daniele Sluijters <github@daenney.net>
 * Daniel Paulus <daniel@inuits.eu>
+* Wolf Noble <wolf@wolfspyre.com>
diff --git a/lib/facter/apt_update_last_success.rb b/lib/facter/apt_update_last_success.rb
new file mode 100644
index 0000000..21c33d5
--- /dev/null
+++ b/lib/facter/apt_update_last_success.rb
@@ -0,0 +1,18 @@
+require 'facter'
+
+#This is derived from the file /var/lib/apt/periodic/update-success-stamp
+# This is generated upon a successful apt-get update run natively in ubuntu.
+# the Puppetlabs-apt module deploys this same functionality for other debian-ish OSes
+Facter.add('apt_update_last_success') do
+  confine :osfamily => 'Debian'
+  setcode do
+    if File.exists?('/var/lib/apt/periodic/update-success-stamp')
+      #get epoch time
+      lastsuccess = File.mtime('/var/lib/apt/periodic/update-success-stamp').to_i
+      lastsuccess
+    else
+      lastsuccess = -1
+      lastsuccess
+    end
+  end
+end
diff --git a/manifests/init.pp b/manifests/init.pp
index eb8c709..89fb684 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -5,15 +5,32 @@
 # Parameters:
 #   The parameters listed here are not required in general and were
 #     added for use cases related to development environments.
+#
 #   disable_keys - disables the requirement for all packages to be signed
+#
 #   always_apt_update - rather apt should be updated on every run (intended
 #     for development environments where package updates are frequent)
+#
+#   apt_update_frequency - *string* Supported values:
+#     **always**: Will fire `apt-get update` at every puppet run. Intended to
+#       deprecate the `always_apt_update` parameter.
+#     **daily**: Trigger `apt-get update` if the value of the fact
+#       `apt_update_last_success` is less than current epoch time - 86400.
+#        *notifying the apt_update exec will trigger apt-get update regardless*
+#     **weekly**: Trigger `apt-get update` if the value of the fact
+#       `apt_update_last_success` is less than current epoch time - 604800.
+#        *notifying the apt_update exec will trigger apt-get update regardless*
+#     **reluctantly**: *Default* only run apt-get update if the exec resource `apt_update` is notified.
+#
 #   purge_sources_list - Accepts true or false. Defaults to false If set to
 #     true, Puppet will purge all unmanaged entries from sources.list
+#
 #   purge_sources_list_d - Accepts true or false. Defaults to false. If set
 #     to true, Puppet will purge all unmanaged entries from sources.list.d
+#
 #   update_timeout - Overrides the exec timeout in seconds for apt-get update.
 #     If not set defaults to Exec's default (300)
+#
 #   update_tries - Number of times that `apt-get update` will be tried. Use this
 #     to work around transient DNS and HTTP errors. By default, the command
 #     will only be run once.
@@ -27,6 +44,7 @@
 
 class apt(
   $always_apt_update    = false,
+  $apt_update_frequency = 'reluctantly',
   $disable_keys         = undef,
   $proxy_host           = undef,
   $proxy_port           = '8080',
@@ -44,6 +62,8 @@ class apt(
     fail('This module only works on Debian or derivatives like Ubuntu')
   }
 
+  $frequency_options = ['always','daily','weekly','reluctantly']
+  validate_re($apt_update_frequency, $frequency_options)
   include apt::params
   include apt::update
 
@@ -61,6 +81,14 @@ class apt(
     }
   }
 
+  file { '/etc/apt/apt.conf.d/15update-stamp':
+    ensure  => 'file',
+    content => 'APT::Update::Post-Invoke-Success {"touch /var/lib/apt/periodic/update-success-stamp 2>/dev/null || true";};',
+    group   => 'root',
+    mode    => '0644',
+    owner   => 'root',
+  }
+
   $root           = $apt::params::root
   $apt_conf_d     = $apt::params::apt_conf_d
   $sources_list_d = $apt::params::sources_list_d
diff --git a/manifests/update.pp b/manifests/update.pp
index 725bb3b..9112c9b 100644
--- a/manifests/update.pp
+++ b/manifests/update.pp
@@ -1,10 +1,58 @@
 class apt::update {
   include apt::params
+  #TODO: to catch if $::apt_update_last_success has the value of -1 here. If we
+  #opt to do this, a info/warn would likely be all you'd need likely to happen
+  #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 always_apt_update is true there's no point in parsing this logic.
+
+    case $apt::apt_update_frequency {
+      'always': {
+        $_kick_apt = true
+      }
+      'daily': {
+        #compare current date with the apt_update_last_success fact to determine
+        #if we should kick apt_update.
+        $daily_threshold = (strftime('%s') - 86400)
+        if $::apt_update_last_success {
+          if $::apt_update_last_success < $daily_threshold {
+            $_kick_apt = true
+          }
+        } else {
+          #if apt-get update has not successfully run, we should kick apt_update
+          $_kick_apt = true
+        }
+      }
+      'weekly':{
+        #compare current date with the apt_update_last_success fact to determine
+        #if we should kick apt_update.
+        $weekly_threshold = (strftime('%s') - 604800)
+        if $::apt_update_last_success {
+          if ( $::apt_update_last_success < $weekly_threshold ) {
+            $_kick_apt = true
+          }
+        } else {
+          #if apt-get update has not successfully run, we should kick apt_update
+          $_kick_apt = true
+        }
+      }
+      default: {
+        #catches 'recluctantly', and any other value (which should not occur).
+        #do nothing.
+      }
+    }
+  }
+  if $_kick_apt {
+    $_refresh = false
+  } else {
+    $_refresh = true
+  }
   exec { 'apt_update':
     command     => "${apt::params::provider} update",
     logoutput   => 'on_failure',
-    refreshonly => true,
+    refreshonly => $_refresh,
     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 2c70e3e..0a1c0a1 100644
--- a/spec/classes/apt_spec.rb
+++ b/spec/classes/apt_spec.rb
@@ -37,6 +37,14 @@ describe 'apt', :type => :class do
       'notify' => 'Exec[apt_update]',
     })}
 
+    it 'should lay down /etc/apt/apt.conf.d/15update-stamp' do
+      should contain_file('/etc/apt/apt.conf.d/15update-stamp').with({
+        'group' => 'root',
+        'mode'  => '0644',
+        'owner' => 'root',
+      }).with_content('APT::Update::Post-Invoke-Success {"touch /var/lib/apt/periodic/update-success-stamp 2>/dev/null || true";};')
+    end
+
     it { should contain_file('old-proxy-file').that_notifies('Exec[apt_update]').only_with({
       'ensure' => 'absent',
       'path'   => '/etc/apt/apt.conf.d/proxy',
diff --git a/spec/classes/apt_update_spec.rb b/spec/classes/apt_update_spec.rb
new file mode 100644
index 0000000..d70efd3
--- /dev/null
+++ b/spec/classes/apt_update_spec.rb
@@ -0,0 +1,109 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+describe 'apt::update', :type => :class do
+  context 'when apt::always_apt_update is 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}" }
+    it 'should trigger an apt-get update run' do
+      #set the apt_update exec's refreshonly attribute to false
+      should 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
+        { '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}' }" }
+            it 'should trigger an apt-get update run' do
+              # set the apt_update exec's refreshonly attribute to false
+              should contain_exec('apt_update').with({'refreshonly' => false})
+            end
+          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}' }" }
+            it 'should trigger an apt-get update run' do
+              #set the apt_update exec\'s refreshonly attribute to false
+              should contain_exec('apt_update').with({'refreshonly' => false})
+            end
+          end
+        end
+      end
+    end
+  end
+
+  context 'when apt::always_apt_update is false' do
+    context "and apt::apt_update_frequency has the value of 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' }" }
+          it 'should trigger an apt-get update run' do
+            #set the apt_update exec's refreshonly attribute to false
+            should contain_exec('apt_update').with({'refreshonly' => false})
+          end
+        end
+      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' }" }
+        it 'should trigger an apt-get update run' do
+          #set the apt_update exec\'s refreshonly attribute to false
+          should contain_exec('apt_update').with({'refreshonly' => false})
+        end
+      end
+    end
+    context "and apt::apt_update_frequency has the value of 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' }" }
+          it 'should not trigger an apt-get update run' do
+            #don't change the apt_update exec's refreshonly attribute. (it should be true)
+            should contain_exec('apt_update').with({'refreshonly' => true})
+          end
+        end
+      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' }" }
+        it 'should not trigger an apt-get update run' do
+          #don't change the apt_update exec's refreshonly attribute. (it should be true)
+          should contain_exec('apt_update').with({'refreshonly' => true})
+        end
+      end
+    end
+    ['daily','weekly'].each do |update_frequency|
+      context "and apt::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}' }" }
+            it 'should trigger an apt-get update run' do
+              #set the apt_update exec\'s refreshonly attribute to false
+              should contain_exec('apt_update').with({'refreshonly' => false})
+            end
+          end
+        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}' }" }
+          it 'should not trigger an apt-get update run' do
+            #don't change the apt_update exec\'s refreshonly attribute. (it should be true)
+            should contain_exec('apt_update').with({'refreshonly' => true})
+          end
+        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}' }" }
+          it 'should trigger an apt-get update run' do
+            #set the apt_update exec\'s refreshonly attribute to false
+            should contain_exec('apt_update').with({'refreshonly' => false})
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/classes/backports_spec.rb b/spec/classes/backports_spec.rb
index 17c86eb..18f6337 100644
--- a/spec/classes/backports_spec.rb
+++ b/spec/classes/backports_spec.rb
@@ -5,7 +5,8 @@ describe 'apt::backports', :type => :class do
     let :facts do
       {
         'lsbdistcodename' => 'Karmic',
-        'lsbdistid'       => 'Ubuntu'
+        'lsbdistid'       => 'Ubuntu',
+        'osfamily'        => 'Debian'
       }
     end
 
@@ -36,7 +37,8 @@ describe 'apt::backports', :type => :class do
     let :facts do
       {
         'lsbdistcodename' => 'Karmic',
-        'lsbdistid'       => 'Ubuntu'
+        'lsbdistid'       => 'Ubuntu',
+        'osfamily'        => 'Debian'
       }
     end
 
@@ -57,6 +59,7 @@ describe 'apt::backports', :type => :class do
       {
         'lsbdistcodename' => 'Squeeze',
         'lsbdistid'       => 'Debian',
+        'osfamily'        => 'Debian'
       }
     end
 
@@ -77,6 +80,7 @@ describe 'apt::backports', :type => :class do
       {
         'lsbdistcodename' => 'debian',
         'lsbdistid'       => 'LinuxMint',
+        'osfamily'        => 'Debian'
       }
     end
 
@@ -97,6 +101,7 @@ describe 'apt::backports', :type => :class do
       {
         'lsbdistcodename' => 'qiana',
         'lsbdistid'       => 'LinuxMint',
+        'osfamily'        => 'Debian'
       }
     end
 
@@ -116,7 +121,8 @@ describe 'apt::backports', :type => :class do
     let :facts do
       {
         'lsbdistcodename' => 'Squeeze',
-        'lsbdistid'       => 'Debian'
+        'lsbdistid'       => 'Debian',
+        'osfamily'        => 'Debian'
       }
     end
 
diff --git a/spec/defines/builddep_spec.rb b/spec/defines/builddep_spec.rb
index 8836693..41152d5 100644
--- a/spec/defines/builddep_spec.rb
+++ b/spec/defines/builddep_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 describe 'apt::builddep', :type => :define do
 
-  let(:facts) { { :lsbdistid => 'Debian' } }
+  let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
   let(:title) { 'my_package' }
 
   describe "defaults" do
diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb
index 960bb62..8327ed2 100644
--- a/spec/defines/source_spec.rb
+++ b/spec/defines/source_spec.rb
@@ -12,6 +12,7 @@ describe 'apt::source', :type => :define do
       {
         :lsbdistid       => 'Debian',
         :lsbdistcodename => 'wheezy',
+        :osfamily        => 'Debian'
       }
     end
 
@@ -36,6 +37,7 @@ describe 'apt::source', :type => :define do
       {
         :lsbdistid       => 'Debian',
         :lsbdistcodename => 'wheezy',
+        :osfamily        => 'Debian'
       }
     end
     let :params do
@@ -95,6 +97,7 @@ describe 'apt::source', :type => :define do
       {
         :lsbdistid       => 'Debian',
         :lsbdistcodename => 'wheezy',
+        :osfamily        => 'Debian'
       }
     end
     let :params do
@@ -114,6 +117,7 @@ describe 'apt::source', :type => :define do
       let :facts do
         {
           :lsbdistid       => 'Debian',
+          :osfamily        => 'Debian'
         }
       end
 
diff --git a/spec/unit/facter/apt_update_last_success_spec.rb b/spec/unit/facter/apt_update_last_success_spec.rb
new file mode 100644
index 0000000..08774cd
--- /dev/null
+++ b/spec/unit/facter/apt_update_last_success_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe 'apt_update_last_success fact' do
+  subject { Facter.fact(:apt_update_last_success).value }
+  after(:each) { Facter.clear }
+
+  describe 'on Debian based distro which has not yet created the update-success-stamp file' do
+    before {
+      Facter.fact(:osfamily).stubs(:value).returns 'Debian'
+      File.stubs(:exists?).returns false
+    }
+    it 'should have a value of -1' do
+      should == -1
+    end
+  end
+
+  describe 'on Debian based distro which has created the update-success-stamp' do
+    before {
+      Facter.fact(:osfamily).stubs(:value).returns 'Debian'
+      File.stubs(:exists?).returns true
+      File.stubs(:mtime).returns 1407660561
+    }
+    it 'should have the value of the mtime of the file' do
+      should == 1407660561
+    end
+  end
+
+end