From: TP Honey Date: Tue, 21 Jul 2015 17:59:19 +0000 (+0100) Subject: Merge pull request #547 from mhaskel/MODULES-2190 X-Git-Tag: 2.1.1~2^2^2 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fa86d8ed5052407f9279c308c2a9079599380bf8;hp=428c044be5a6d1b3b5a73247e961eef15c3bd7c3;p=puppet-modules%2Fpuppetlabs-apt.git Merge pull request #547 from mhaskel/MODULES-2190 Fix anchor issues --- diff --git a/README.md b/README.md index c5f4340..a40a0eb 100644 --- a/README.md +++ b/README.md @@ -456,6 +456,14 @@ Manages the GPG keys that Apt uses to authenticate packages. This module is tested and officially supported on Debian 6 and 7 and Ubuntu 10.04, 12.04, and 14.04. Testing on other platforms has been light and cannot be guaranteed. +### Adding new sources or PPAs + +If you are adding a new source or PPA and trying to install packages from the new source or PPA on the same puppet run, in addition to depending on the `Apt::Source` or the `Apt::Ppa`, your `package` resource should depend on `Class['apt::update']`. You can also add [collectors](https://docs.puppetlabs.com/puppet/latest/reference/lang_collectors.html) to ensure all packages happen after `apt::update`, but this can lead to dependency cycles and has implications for [virtual resources](https://docs.puppetlabs.com/puppet/latest/reference/lang_collectors.html#behavior) + +~~~puppet +Class['apt::update'] -> Package<| |> +~~~ + ## Development Puppet Labs modules on the Puppet Forge are open projects, and community contributions are essential for keeping them great. We can't access the huge number of platforms and myriad hardware, software, and deployment configurations that Puppet is intended to serve. We want to keep it as easy as possible to contribute changes so that our modules work in your environment. There are a few guidelines that we need contributors to follow so that we can have a chance of keeping on top of things. diff --git a/manifests/init.pp b/manifests/init.pp index 48c4f53..578d733 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -26,7 +26,7 @@ class apt( } $_update = merge($::apt::update_defaults, $update) - include apt::update + include ::apt::update validate_hash($purge) if $purge['sources.list'] { @@ -99,7 +99,7 @@ class apt( group => root, mode => '0644', content => $sources_list_content, - notify => Exec['apt_update'], + notify => Class['apt::update'], } file { 'sources.list.d': @@ -110,7 +110,7 @@ class apt( mode => '0644', purge => $_purge['sources.list.d'], recurse => $_purge['sources.list.d'], - notify => Exec['apt_update'], + notify => Class['apt::update'], } file { 'preferences': @@ -119,7 +119,7 @@ class apt( owner => root, group => root, mode => '0644', - notify => Exec['apt_update'], + notify => Class['apt::update'], } file { 'preferences.d': @@ -130,11 +130,9 @@ class apt( mode => '0644', purge => $_purge['preferences.d'], recurse => $_purge['preferences.d'], - notify => Exec['apt_update'], + notify => Class['apt::update'], } - anchor { 'apt_first': } -> Class['apt::update'] -> anchor { 'apt_last': } - # manage sources if present if $sources { create_resources('apt::source', $sources) diff --git a/manifests/ppa.pp b/manifests/ppa.pp index f3e2bfd..6352352 100644 --- a/manifests/ppa.pp +++ b/manifests/ppa.pp @@ -45,7 +45,7 @@ define apt::ppa( unless => "/usr/bin/test -s ${::apt::sources_list_d}/${sources_list_d_filename}", user => 'root', logoutput => 'on_failure', - notify => Exec['apt_update'], + notify => Class['apt::update'], require => $_require, } @@ -57,7 +57,7 @@ define apt::ppa( else { file { "${::apt::sources_list_d}/${sources_list_d_filename}": ensure => 'absent', - notify => Exec['apt_update'], + notify => Class['apt::update'], } } } diff --git a/manifests/setting.pp b/manifests/setting.pp index 59d0dd4..d723eb2 100644 --- a/manifests/setting.pp +++ b/manifests/setting.pp @@ -47,7 +47,7 @@ define apt::setting ( $_ext = $::apt::params::config_files[$setting_type]['ext'] if $notify_update { - $_notify = Exec['apt_update'] + $_notify = Class['apt::update'] } else { $_notify = undef } diff --git a/manifests/source.pp b/manifests/source.pp index 734f375..1307a3a 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -23,7 +23,8 @@ define apt::source( validate_bool($allow_unsigned) validate_hash($include) - include 'apt::params' + # This is needed for compat with 1.8.x + include ::apt $_before = Apt::Setting["list-${title}"] diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb index 1c8cac7..5a71fb5 100644 --- a/spec/classes/apt_spec.rb +++ b/spec/classes/apt_spec.rb @@ -3,16 +3,16 @@ describe 'apt' do let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :puppetversion => Puppet.version} } context 'defaults' do - it { is_expected.to contain_file('sources.list').that_notifies('Exec[apt_update]').only_with({ + it { is_expected.to contain_file('sources.list').that_notifies('Class[Apt::Update]').only_with({ :ensure => 'file', :path => '/etc/apt/sources.list', :owner => 'root', :group => 'root', :mode => '0644', - :notify => 'Exec[apt_update]', + :notify => 'Class[Apt::Update]', })} - it { is_expected.to contain_file('sources.list.d').that_notifies('Exec[apt_update]').only_with({ + it { is_expected.to contain_file('sources.list.d').that_notifies('Class[Apt::Update]').only_with({ :ensure => 'directory', :path => '/etc/apt/sources.list.d', :owner => 'root', @@ -20,19 +20,19 @@ describe 'apt' do :mode => '0644', :purge => false, :recurse => false, - :notify => 'Exec[apt_update]', + :notify => 'Class[Apt::Update]', })} - it { is_expected.to contain_file('preferences').that_notifies('Exec[apt_update]').only_with({ + it { is_expected.to contain_file('preferences').that_notifies('Class[Apt::Update]').only_with({ :ensure => 'file', :path => '/etc/apt/preferences', :owner => 'root', :group => 'root', :mode => '0644', - :notify => 'Exec[apt_update]', + :notify => 'Class[Apt::Update]', })} - it { is_expected.to contain_file('preferences.d').that_notifies('Exec[apt_update]').only_with({ + it { is_expected.to contain_file('preferences.d').that_notifies('Class[Apt::Update]').only_with({ :ensure => 'directory', :path => '/etc/apt/preferences.d', :owner => 'root', @@ -40,7 +40,7 @@ describe 'apt' do :mode => '0644', :purge => false, :recurse => false, - :notify => 'Exec[apt_update]', + :notify => 'Class[Apt::Update]', })} it 'should lay down /etc/apt/apt.conf.d/15update-stamp' do diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index c110e50..b7a2f6c 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -18,7 +18,7 @@ describe 'apt::ppa' do let(:title) { 'ppa:needs/such.substitution/wow' } it { is_expected.to_not contain_package('python-software-properties') } - it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Class[Apt::Update]').with({ :environment => [], :command => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow', :unless => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', @@ -57,7 +57,7 @@ describe 'apt::ppa' do let(:title) { 'ppa:needs/such.substitution/wow' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Class[Apt::Update]').with({ 'environment' => [], 'command' => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow', 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', @@ -94,7 +94,7 @@ describe 'apt::ppa' do let(:title) { 'ppa:needs/such.substitution/wow' } it { is_expected.to_not contain_package('python-software-properties') } - it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Class[Apt::Update]').with({ 'environment' => [], 'command' => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow', 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', @@ -135,7 +135,7 @@ describe 'apt::ppa' do let(:title) { 'ppa:foo' } it { is_expected.to compile.with_all_deps } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ :environment => [], :command => '/usr/bin/add-apt-repository ppa:foo', :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', @@ -169,7 +169,7 @@ describe 'apt::ppa' do end let(:title) { 'ppa:foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8080'], :command => '/usr/bin/add-apt-repository ppa:foo', :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', @@ -203,7 +203,7 @@ describe 'apt::ppa' do end let(:title) { 'ppa:foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180'], :command => '/usr/bin/add-apt-repository ppa:foo', :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', @@ -237,7 +237,7 @@ describe 'apt::ppa' do end let(:title) { 'ppa:foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180', 'https_proxy=https://localhost:8180'], :command => '/usr/bin/add-apt-repository ppa:foo', :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', @@ -267,7 +267,7 @@ describe 'apt::ppa' do :ensure => 'absent' } end - it { is_expected.to contain_file('/etc/apt/sources.list.d/foo-trusty.list').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_file('/etc/apt/sources.list.d/foo-trusty.list').that_notifies('Class[Apt::Update]').with({ :ensure => 'absent', }) } diff --git a/spec/defines/setting_spec.rb b/spec/defines/setting_spec.rb index b109ea6..07d94ef 100644 --- a/spec/defines/setting_spec.rb +++ b/spec/defines/setting_spec.rb @@ -16,25 +16,25 @@ describe 'apt::setting' do context 'with title=conf-teddybear ' do let(:params) { default_params } - it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Exec[apt_update]') } + it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Class[Apt::Update]') } end context 'with title=pref-teddybear' do let(:title) { 'pref-teddybear' } let(:params) { default_params } - it { is_expected.to contain_file('/etc/apt/preferences.d/50teddybear').that_notifies('Exec[apt_update]') } + it { is_expected.to contain_file('/etc/apt/preferences.d/50teddybear').that_notifies('Class[Apt::Update]') } end context 'with title=list-teddybear' do let(:title) { 'list-teddybear' } let(:params) { default_params } - it { is_expected.to contain_file('/etc/apt/sources.list.d/teddybear.list').that_notifies('Exec[apt_update]') } + it { is_expected.to contain_file('/etc/apt/sources.list.d/teddybear.list').that_notifies('Class[Apt::Update]') } end context 'with source' do let(:params) { { :source => 'puppet:///la/die/dah' } } it { - is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Exec[apt_update]').with({ + is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Class[Apt::Update]').with({ :ensure => 'file', :owner => 'root', :group => 'root', @@ -45,7 +45,7 @@ describe 'apt::setting' do context 'with content' do let(:params) { default_params } - it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Class[Apt::Update]').with({ :ensure => 'file', :owner => 'root', :group => 'root', @@ -103,12 +103,12 @@ describe 'apt::setting' do describe 'with priority=100' do let(:params) { default_params.merge({ :priority => 100 }) } - it { is_expected.to contain_file('/etc/apt/apt.conf.d/100teddybear').that_notifies('Exec[apt_update]') } + it { is_expected.to contain_file('/etc/apt/apt.conf.d/100teddybear').that_notifies('Class[Apt::Update]') } end describe 'with ensure=absent' do let(:params) { default_params.merge({ :ensure => 'absent' }) } - it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Exec[apt_update]').with({ + it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').that_notifies('Class[Apt::Update]').with({ :ensure => 'absent', })} end diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb index c9863dd..8a2cfcc 100644 --- a/spec/defines/source_spec.rb +++ b/spec/defines/source_spec.rb @@ -70,6 +70,8 @@ describe 'apt::source' do }).with_content(/hello.there wheezy main\n/) } + it { is_expected.to contain_file('/etc/apt/sources.list.d/my_source.list').that_notifies('Class[Apt::Update]')} + it { is_expected.to contain_apt__pin('my_source').that_comes_before('Apt::Setting[list-my_source]').with({ :ensure => 'present', :priority => 1001,