Merge pull request #547 from mhaskel/MODULES-2190

Fix anchor issues

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.
 

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)

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'],
     }
   }
 }

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
   }

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}"]
 

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

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',
     })
     }

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

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,