(#12809) $release should use $lsbdistcodename and fall back to manual input

This commit changes $release to default to Facter's $lsbdistcodename
and fall back to a Parse Error if $release is not set and $lsbdistcodename
does not exist. Previously $release was hardcoded to karmic.

This commit also modifies apt::ppa to use $release and sets the
files to be ensured so that they are not purged when purge_sources_list_d
is set to true.

diff --git a/manifests/params.pp b/manifests/params.pp
index 3c3928803920f7caec69b39a2e4cfe70c7a05e53..ca5d65521d22b915c96e09430b33a2c06ffb8ef2 100644 (file)
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -1,4 +1,5 @@
 class apt::params {
-  $root = '/etc/apt'
-  $provider = '/usr/bin/apt-get'
+  $root           = '/etc/apt'
+  $provider       = '/usr/bin/apt-get'
+  $sources_list_d = "${root}/sources.list.d"
 }

diff --git a/manifests/ppa.pp b/manifests/ppa.pp
index a41c814f296e4a4f4c8f4462208ec3242e345a69..712f425f49b5b50c3de6f74fb639b8088af41ef3 100644 (file)
--- a/manifests/ppa.pp
+++ b/manifests/ppa.pp
@@ -1,21 +1,36 @@
 # ppa.pp
 
-define apt::ppa() {
+define apt::ppa(
+  $release = $lsbdistcodename
+) {
 
   Class['apt'] -> Apt::Ppa[$title]
 
+  include apt::params
+
+  if ! $release {
+    fail("lsbdistcodename fact not available: release parameter required")
+  }
+
   exec { "apt-update-${name}":
     command     => "/usr/bin/aptitude update",
     refreshonly => true,
   }
 
+  $filename_without_slashes = regsubst($name,'/','-','G')
+  $filename_without_ppa     = regsubst($filename_without_slashes, '^ppa:','','G')
+  $sources_list_d_filename   = "${filename_without_ppa}-${release}.list"
+
   exec { "add-apt-repository-${name}":
     command => "/usr/bin/add-apt-repository ${name}",
     notify  => Exec["apt-update-${name}"],
-    unless => $name? {
-      /ppa:(.*)/ => "/bin/cat /etc/apt/sources.list /etc/apt/sources.list.d/* | /bin/egrep '^[^#].*ppa.*$1.*$'",
-      default    => "/bin/cat /etc/apt/sources.list /etc/apt/sources.list.d/* | /bin/egrep '^[^#].*${title}.*$'",
-    }
+    creates => "${apt::params::sources_list_d}/${sources_list_d_filename}",
   }
+
+  file { "${apt::params::sources_list_d}/${sources_list_d_filename}":
+    ensure  => file,
+    require => Exec["add-apt-repository-${name}"];
+  }
+
 }
 

diff --git a/manifests/source.pp b/manifests/source.pp
index 9f31fe91308bbb911b5013a21227b17e4f0e11db..abc77e222bad886291a36dcc87d353fbe815512f 100644 (file)
--- a/manifests/source.pp
+++ b/manifests/source.pp
@@ -3,7 +3,7 @@
 
 define apt::source(
   $location = '',
-  $release = 'karmic',
+  $release = $lsbdistcodename,
   $repos = 'main',
   $include_src = true,
   $required_packages = false,
@@ -15,6 +15,10 @@ define apt::source(
 
   include apt::params
 
+  if ! $release {
+    fail("lsbdistcodename fact not available: release parameter required")
+  }
+
   file { "${name}.list":
     path => "${apt::params::root}/sources.list.d/${name}.list",
     ensure => file,

diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb
index c2ce26411ce33c40786db625614ca5cdef9b97f2..9d4750b737a2fdca34714a1334cb1e680c763039 100644 (file)
--- a/spec/defines/ppa_spec.rb
+++ b/spec/defines/ppa_spec.rb
@@ -1,37 +1,53 @@
 require 'spec_helper'
 describe 'apt::ppa', :type => :define do
-  ['ppa:dans_ppa', 'dans_ppa'].each do |t|
+  ['ppa:dans_ppa', 'dans_ppa','ppa:dans-daily/ubuntu'].each do |t|
     describe "with title #{t}" do
       let :pre_condition do
         'class { "apt": }'
       end
+      let :facts do
+        {:lsbdistcodename => 'natty'}
+      end
       let :title do
         t
       end
-      let :unless_statement do
-        if t =~ /ppa:(.*)/
-          /^[^#].*ppa.*#{$1}.*$/
-        else
-          /^[^#].*#{t}.*$/
-        end
+      let :release do
+        "natty"
+      end
+      let :filename do
+        t.sub(/^ppa:/,'').gsub('/','-') << "-" << "#{release}.list"
       end
+
+      it { should contain_exec("apt-update-#{t}").with(
+        'command'     => '/usr/bin/aptitude update',
+        'refreshonly' => true
+        )
+      }
+
       it { should contain_exec("add-apt-repository-#{t}").with(
         'command' => "/usr/bin/add-apt-repository #{t}",
-        'notify'  => "Exec[apt-update-#{t}]"
+        'notify'  => "Exec[apt-update-#{t}]",
+        'creates' => "/etc/apt/sources.list.d/#{filename}"
         )
       }
-      it { should contain_exec("add-apt-repository-#{t}").with({"unless" => unless_statement}) }
-      it { should contain_exec("apt-update-#{t}").with(
-        'command'     => '/usr/bin/aptitude update',
-        'refreshonly' => true
+
+      it { should create_file("/etc/apt/sources.list.d/#{filename}").with(
+        'ensure'  => 'file',
+        'require' => "Exec[add-apt-repository-#{t}]"
         )
       }
-      it { should contain_exec("apt-update-#{t}").without_unless }
     end
   end
 
   describe "without Class[apt] should raise a Puppet::Error" do
+    let(:release) { "natty" }
     let(:title) { "ppa" }
     it { expect { should contain_apt__ppa(title) }.to raise_error(Puppet::Error) }
   end
+
+  describe "without release should raise a Puppet::Error" do
+    let(:title) { "ppa:" }
+    it { expect { should contain_apt__ppa(:release) }.to raise_error(Puppet::Error) }
+  end
+
 end

diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb
index 041175c1cee8db98d196ae4248e5e71b177955d0..3306ab1ee001880c6a6a6292cb3ee0088b71f4b0 100644 (file)
--- a/spec/defines/source_spec.rb
+++ b/spec/defines/source_spec.rb
@@ -41,6 +41,10 @@ describe 'apt::source', :type => :define do
         default_params.merge(param_set)
       end
 
+      let :facts do
+        {:lsbdistcodename => 'karmic'}
+      end
+
       let :params do
         param_set
       end
@@ -157,5 +161,8 @@ describe 'apt::source', :type => :define do
       }
     end
   end
+    describe "without release should raise a Puppet::Error" do
+      it { expect { should contain_apt__source(:release) }.to raise_error(Puppet::Error) }
+    end
 end