Merge branch '2.6.x' into 2.7.x

* 2.6.x:
  (#7956) Porting cron tests
  (#7956) Port resource acceptance tests
  Readying for release of 2.6.9
  (#6854) Update Red Hat spec file
  Bumping release in lib/puppet.rb and updating CHANGELOG.
  Bumping RPM spec file to 2.6.9rc1.
  (#7506) Organize READMEs; specify supported Ruby versions in README.md
  (#6418) Make test 64118 more portable
  (#7127) Stop puppet if a prerun command fails
  Do not needlessly create multiple reports when creating a transaction
  (#4416) Ensure types are providified after reloading
  (#4416) Always remove old provider before recreating it
  Cleanup indentation, comment, and unused code

Conflicts:
	CHANGELOG
	README.md
	conf/redhat/puppet.spec
	lib/puppet.rb
	lib/puppet/transaction.rb
	spec/unit/configurer_spec.rb
	spec/unit/transaction_spec.rb

CHANGELOG

@@ -734,8 +734,52 @@ d532e6d Fixing #3185 Rakefile is loading puppet.rb twice
 5aa596c Fix #3150 - require function doesn't like ::class syntax
 3457b87 Added time module to tagmail report
 
-2.6.8rc1
+2.6.9
 ====
+db1a392 (#7506) Organize READMEs; specify supported Ruby versions in README.md
+381fa40 (#6418) Make test 64118 more portable
+98ba407 (#7127) Stop puppet if a prerun command fails
+6996e0b Do not needlessly create multiple reports when creating a transaction
+caca469 (#4416) Ensure types are providified after reloading
+413b136 (#4416) Always remove old provider before recreating it
+d866ce1 Cleanup indentation, comment, and unused code
+98f58ce (#2128) Add WARNING for node_name_{fact,value} descriptions
+1cd848c (#2128) Whitespace only reflow commit
+d9b5c1a (#2128) In-line docs for node_name_{fact,value}
+3f0dbb5 (#650) Allow symlinks for configuration directories
+c260cf1 Fix acceptance tests not managing their masters
+1c70f0c (#2128) Add support for setting node name based on a fact
+c629958 (#2128) Get facts before retrieving catalog
+cd4fe14 (#2128) Add the ability to specify a node name
+8ebec1e (#7193) Fix path issues with acceptance tests that call old shell tests
+16b2311 (#6885) puppet agent fingerprint requires --verbose to return a value.
+a00fd25 maint: Refactor specs in preparation for making node name more flexible
+75e2764 (#5318) Always notice changes to manifests when compiling.
+4a5e99d (#7681) Add an acceptance test for resource refs with array variables
+646919e (4123) Fix test for 4123/4 on old egrep in cent4
+8b76be3 (#3836) External nodes should only capture stdout
+8f907f2 adding test for ticket 7139
+90eb937 (#7139) Accept '/' as a valid path in filesets
+1b2a7d9 case seems needless here as there is only two opts, also the rest of the file seems to use if so this should make thin
+729336e (#6845) Mount writes incorrect vfstab entries
+16cf1ac (#6442) Be able to start agents --listen without namespaceauth.conf
+0352402 (#3420) Nagios "name" attribute does not output correctly
+f656818 (#4487) When setting environment on a host, ensure it is a string.
+2cce326 add test for ticket 7101
+c306db2 (#6487) Add some testing for OS X version support in DirectoryService provider
+0008b63 (#6487) Directoryservice provider will fail in future OS releases
+34f9f41 Maint: Fix a #4655 introduced log inconsistency
+6b18f8f Move acceptance tests from puppet-acceptance repo
+9a5bf6e Fixed #7166 - Replaced deprecated stomp "send" method with "publish"
+656eff8 (#4655) Allow stage to be set using a default class parameter
+7f658e6 vim: Initial ftplugin and indent support
+ccbe9f3 Fixed #6681 - Remove --force-yes option from aptitude is used
+
+2.6.8
+=====
+c1edcb2 add test for ticket 7101
+db26326 Move tests from puppet-acceptance repo
+bee1ef7 Updated CHANGELOG for 2.6.8rc1
 8b7444d (#2331) Remove darwinports pkg provider, replace with rewritten macports provider
 65c4e14 Fixed #7082 - Added system support for groups
 b7f4ff7 (#7018) Give more context on the service type's assumptions. Wording tweaks.

README.md

@@ -41,4 +41,3 @@ Support
 -------
 
 Please log tickets and issues at our [Projects site](http://projects.puppetlabs.com)
-

acceptance/tests/resource/cron/should_create_cron.rb

@@ -1,6 +1,6 @@
 test_name "should create cron"
 
-tmpuser = "cron-test-#{Time.new.to_i}"
+tmpuser = "pl#{rand(999999).to_i}"
 tmpfile = "/tmp/cron-test-#{Time.new.to_i}"
 
 create_user = "user { '#{tmpuser}': ensure => present, managehome => false }"
@@ -13,18 +13,16 @@ agents.each do |host|
     step "apply the resource on the host using puppet resource"
     on(host, puppet_resource("cron", "crontest", "user=#{tmpuser}",
                   "command=/bin/true", "ensure=present")) do
-        fail_test "didn't notice creation of the cron stuff" unless
-            stdout.include? 'created'
+      assert_match(/created/, stdout, "Did not create crontab for #{tmpuser} on #{host}")
     end
 
     step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
-        fail_test "didn't find the command as expected" unless
-            stdout.include? "* * * * * /bin/true"
+    run_cron_on(host, :list, tmpuser) do
+      assert_match(/\* \* \* \* \* \/bin\/true/, stdout, "Incorrect crontab for #{tmpuser} on #{host}")
     end
 
     step "remove the crontab file for that user"
-    on host, "crontab -r -u #{tmpuser}"
+    run_cron_on(host, :remove, tmpuser)
 
     step "remove the user from the system"
     apply_manifest_on host, delete_user

acceptance/tests/resource/cron/should_match_existing.rb

@@ -1,6 +1,6 @@
+test_name "puppet should match existing job"
 
-
-tmpuser = "cron-test-#{Time.new.to_i}"
+tmpuser = "pl#{rand(999999).to_i}"
 tmpfile = "/tmp/cron-test-#{Time.new.to_i}"
 
 create_user = "user { '#{tmpuser}': ensure => present, managehome => false }"
@@ -10,30 +10,22 @@ agents.each do |host|
     step "ensure the user exist via puppet"
     apply_manifest_on host, create_user
 
-    step "create the existing job by hand..."
-    on host, "echo '* * * * * /bin/true' | crontab -u #{tmpuser} -"
+    step "Create the existing cron job by hand..."
+    run_cron_on(host,:add,tmpuser,"* * * * * /bin/true")
 
-    step "apply the resource on the host using puppet resource"
+    step "Apply the resource on the host using puppet resource"
     on(host, puppet_resource("cron", "crontest", "user=#{tmpuser}",
                   "command=/bin/true", "ensure=present")) do
-        # REVISIT: This is ported from the original test, which seems to me a
-        # weak test, but I don't want to improve it now.  --daniel 2010-12-23
-        # 
-        # This is a weak/fragile test.  The output has changed
-        # causing this test to fail erronously.  Changed to the correct
-        # output to match, but this code should be re-feactored.
-        fail_test "didn't see the output we expected..." unless
-            stdout.include? 'present'
+      assert_match(/present/, stdout, "Failed creating crontab for #{tmpuser} on #{host}")
     end
 
-    step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
-        fail_test "didn't find the command as expected" unless
-            stdout.include? "* * * * * /bin/true"
+    step "Verify that crontab -l contains what you expected"
+    run_cron_on(host, :list, tmpuser) do
+      assert_match(/\* \* \* \* \* \/bin\/true/, stdout, "Did not find crontab for #{tmpuser} on #{host}")
     end
 
     step "remove the crontab file for that user"
-    on host, "crontab -r -u #{tmpuser}"
+    run_cron_on(host, :remove, tmpuser)
 
     step "remove the user from the system"
     apply_manifest_on host, delete_user

acceptance/tests/resource/cron/should_remove_cron.rb

@@ -1,6 +1,6 @@
 test_name "puppet should remove a crontab entry as expected"
 
-tmpuser = "cron-test-#{Time.new.to_i}"
+tmpuser = "pl#{rand(999999).to_i}"
 tmpfile = "/tmp/cron-test-#{Time.new.to_i}"
 
 create_user = "user { '#{tmpuser}': ensure => present, managehome => false }"
@@ -11,25 +11,21 @@ agents.each do |host|
     apply_manifest_on host, create_user
 
     step "create the existing job by hand..."
-    on host, "printf '# Puppet Name: crontest\n* * * * * /bin/true\n' | crontab -u #{tmpuser} -"
+    run_cron_on(host,:add,tmpuser,"* * * * * /bin/true")
 
     step "apply the resource on the host using puppet resource"
     on(host, puppet_resource("cron", "crontest", "user=#{tmpuser}",
                   "command=/bin/true", "ensure=absent")) do
-        # REVISIT: This is ported from the original test, which seems to me a
-        # weak test, but I don't want to improve it now.  --daniel 2010-12-23
-        fail_test "didn't see the output we expected..." unless
-            stdout.include? 'removed'
+      assert_match(/crontest\D+ensure:\s+removed/, stdout, "Didn't remove crobtab entry for #{tmpuser} on #{host}")
     end
 
     step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
-        fail_test "didn't found the command we tried to remove" if
-            stdout.include? "/bin/true"
+    run_cron_on(host, :list, tmpuser) do
+      assert_match(/\/bin\/true/, stdout, "Error: Found entry for #{tmpuser} on #{host}")
     end
 
     step "remove the crontab file for that user"
-    on host, "crontab -r -u #{tmpuser}"
+    run_cron_on(host, :remove, tmpuser)
 
     step "remove the user from the system"
     apply_manifest_on host, delete_user

acceptance/tests/resource/cron/should_remove_matching.rb

@@ -1,6 +1,6 @@
 test_name "puppet should remove a crontab entry based on command matching"
 
-tmpuser = "cron-test-#{Time.new.to_i}"
+tmpuser = "pl#{rand(999999).to_i}"
 tmpfile = "/tmp/cron-test-#{Time.new.to_i}"
 
 cron = '# Puppet Name: crontest\n* * * * * /bin/true\n1 1 1 1 1 /bin/true\n'
@@ -13,23 +13,22 @@ agents.each do |host|
     apply_manifest_on host, create_user
 
     step "create the existing job by hand..."
-    on host, "printf '#{cron}' | crontab -u #{tmpuser} -"
+    run_cron_on(host,:add,tmpuser,"* * * * * /bin/true")
 
-    step "apply the resource change on the host"
+    step "Remove cron resource"
     on(host, puppet_resource("cron", "bogus", "user=#{tmpuser}",
                   "command=/bin/true", "ensure=absent")) do
-        fail_test "didn't see the output we expected..." unless
-            stdout.include? 'removed'
+      assert_match(/bogus\D+ensure: removed/, stdout, "Removing cron entry failed for #{tmpuser} on #{host}")
     end
 
     step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
+    run_cron_on(host,:list,tmpuser) do
         count = stdout.scan("/bin/true").length
         fail_test "found /bin/true the wrong number of times (#{count})" unless count == 1
     end
 
     step "remove the crontab file for that user"
-    on host, "crontab -r -u #{tmpuser}"
+    run_cron_on(host,:remove,tmpuser)
 
     step "remove the user from the system"
     apply_manifest_on host, delete_user

acceptance/tests/resource/cron/should_update_existing.rb

@@ -1,10 +1,8 @@
 test_name "puppet should update existing crontab entry"
 
-tmpuser = "cron-test-#{Time.new.to_i}"
+tmpuser = "pl#{rand(999999).to_i}"
 tmpfile = "/tmp/cron-test-#{Time.new.to_i}"
 
-cron = '# Puppet Name: crontest\n* * * * * /bin/true\n'
-
 create_user = "user { '#{tmpuser}': ensure => present, managehome => false }"
 delete_user = "user { '#{tmpuser}': ensure => absent,  managehome => false }"
 
@@ -13,29 +11,26 @@ agents.each do |host|
     apply_manifest_on host, create_user
 
     step "create the existing job by hand..."
-    on host, "printf '#{cron}' | crontab -u #{tmpuser} -"
+    run_cron_on(host,:add,tmpuser,"* * * * * /bin/true")
 
     step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
-        fail_test "didn't find the content in the crontab" unless
-            stdout.include? '* * * * * /bin/true'
+    run_cron_on(host,:list,tmpuser) do
+      assert_match(/\* \* \* \* \* \/bin\/true/, stdout, "Didn't find correct crobtab entry for #{tmpuser} on #{host}")
     end
 
     step "apply the resource change on the host"
     on(host, puppet_resource("cron", "crontest", "user=#{tmpuser}",
-                  "command=/bin/true", "ensure=present", "hour='0-6'")) do
-        fail_test "didn't update the time as expected" unless
-            stdout.include? "defined 'hour' as '0-6'"
+      "command=/bin/true", "ensure=present", "hour='0-6'")) do
+        assert_match(/hour\s+=>\s+\['0-6'\]/, stdout, "Modifying cron entry failed for #{tmpuser} on #{host}")
     end
 
     step "verify that crontab -l contains what you expected"
-    on host, "crontab -l -u #{tmpuser}" do
-        fail_test "didn't find the content in the crontab" unless
-            stdout.include? '* 0-6 * * * /bin/true'
+    run_cron_on(host,:list,tmpuser) do
+      assert_match(/\* 0-6 \* \* \* \/bin\/true/, stdout, "Didn't find correctly modified time entry in crobtab entry for #{tmpuser} on #{host}")
     end
 
     step "remove the crontab file for that user"
-    on host, "crontab -r -u #{tmpuser}"
+    run_cron_on(host,:remove,tmpuser)
 
     step "remove the user from the system"
     apply_manifest_on host, delete_user

acceptance/tests/resource/file/content_attribute.rb

@@ -1,25 +1,33 @@
-test_name "The content attribute"
-pass_test "Pass forced pending test failure investigation"
+test_name "Content Attribute"
 
 step "Ensure the test environment is clean"
 on agents, 'rm -f /tmp/content_file_test.txt'
 
-step "When using raw content"
+step "Content Attribute: using raw content"
 
 manifest = "file { '/tmp/content_file_test.txt': content => 'This is the test file content', ensure => present }"
 apply_manifest_on agents, manifest
 
-on agents, 'test "$(cat /tmp/content_file_test.txt)" = "This is the test file content"'
+agents.each do |host|
+  on host, "cat /tmp/content_file_test.txt" do
+    assert_match(/This is the test file content/, stdout, "File content not matched on #{host}")
+  end
+end
 
 step "Ensure the test environment is clean"
 on agents, 'rm -f /tmp/content_file_test.txt'
 
-step "When using a filebucket checksum from filebucket"
-
+step "Content Attribute: using a checksum from filebucket"
 on agents, "echo 'This is the checksum file contents' > /tmp/checksum_test_file.txt"
-on agents, "puppet filebucket backup --local /tmp/checksum_test_file.txt"
+step "Backup file into the filebucket"
+on agents, puppet_filebucket("backup --local /tmp/checksum_test_file.txt")
+
+agents.each do |agent|
+  bucketdir="not set"
+  on agent, puppet_filebucket("--configprint bucketdir") do 
+    bucketdir = stdout.chomp
+  end
 
-get_remote_option(agents, 'filebucket', 'bucketdir') do |bucketdir|
   manifest = %Q|
     filebucket { 'local':
       path => '#{bucketdir}',
@@ -31,7 +39,14 @@ get_remote_option(agents, 'filebucket', 'bucketdir') do |bucketdir|
       backup => local,
     }
   |
-  apply_manifest_on agents, manifest
+
+  step "Applying Manifest on Agents"
+  apply_manifest_on agent, manifest
 end
 
-on agents, 'test "$(cat /tmp/content_file_test.txt)" = "This is the checksum file contents"'
+step "Validate filebucket checksum file contents"
+agents.each do |host|
+  on host, "cat /tmp/content_file_test.txt" do
+    assert_match(/This is the checksum file content/, stdout, "File content not matched on #{host}")
+  end
+end

acceptance/tests/resource/file/should_create_directory.rb

@@ -3,7 +3,7 @@ test_name "should create directory"
 target = "/tmp/test-#{Time.new.to_i}"
 
 step "clean up the system before we begin"
-on agents, "rm -vrf #{target}"
+on agents, "rm -rf #{target}"
 
 step "verify we can create a directory"
 on(agents, puppet_resource("file", target, 'ensure=directory'))
@@ -12,4 +12,4 @@ step "verify the directory was created"
 on agents, "test -d #{target}"
 
 step "clean up after the test run"
-on agents, "rm -vrf #{target}"
+on agents, "rm -rf #{target}"

acceptance/tests/resource/file/should_create_empty.rb

@@ -3,13 +3,13 @@ test_name "should create empty file for 'present'"
 target = "/tmp/test-#{Time.new.to_i}"
 
 step "clean up the system before we begin"
-on agents, "rm -vrf #{target}"
+on agents, "rm -rf #{target}"
 
 step "verify we can create an empty file"
 on(agents, puppet_resource("file", target, 'ensure=present'))
 
 step "verify the target was created"
-on agents, "test -f #{target} && ! test -s #{target}"
+on agents, "test -f #{target} && test ! -s #{target}"
 
 step "clean up after the test run"
-on agents, "rm -vrf #{target}"
+on agents, "rm -rf #{target}"

acceptance/tests/resource/file/should_create_symlink.rb

@@ -5,7 +5,7 @@ target  = "/tmp/test-#{Time.new.to_i}"
 source  = "/tmp/test-#{Time.new.to_i}-source"
 
 step "clean up the system before we begin"
-on agents, "rm -vrf #{target}"
+on agents, "rm -rf #{target}"
 on agents, "echo '#{message}' > #{source}"
 
 step "verify we can create a symlink"
@@ -13,6 +13,7 @@ on(agents, puppet_resource("file", target, "ensure=#{source}"))
 
 step "verify the symlink was created"
 on agents, "test -L #{target} && test -f #{target}"
+step "verify source file"
 on agents, "test -f #{source}"
 
 step "verify the content is identical on both sides"
@@ -24,4 +25,4 @@ on(agents, "cat #{target}") do
 end
 
 step "clean up after the test run"
-on agents, "rm -vrf #{target} #{source}"
+on agents, "rm -rf #{target} #{source}"

acceptance/tests/resource/file/should_remove_dir.rb

@@ -3,7 +3,7 @@ test_name "should remove directory, but force required"
 target = "/tmp/test-#{Time.new.to_i}"
 
 step "clean up the system before we begin"
-on agents, "test -e #{target} && rm -vrf #{target} ; mkdir -p #{target}"
+on agents, "rm -rf #{target} ; mkdir -p #{target}"
 
 step "verify we can't remove a directory without 'force'"
 on(agents, puppet_resource("file", target, 'ensure=absent')) do

acceptance/tests/resource/file/should_remove_file.rb

@@ -3,7 +3,7 @@ test_name "should remove file"
 target = "/tmp/test-#{Time.new.to_i}"
 
 step "clean up the system before we begin"
-on agents, "rm -vrf #{target} && touch #{target}"
+on agents, "rm -rf #{target} && touch #{target}"
 
 step "verify we can remove a file"
 on(agents, puppet_resource("file", target, 'ensure=absent'))

acceptance/tests/resource/file/source_attribtute.rb

@@ -4,35 +4,41 @@ step "Ensure the test environment is clean"
 on agents, 'rm -f /tmp/source_file_test.txt'
 
 # TODO: Add tests for puppet:// URIs with master/agent setups.
-step "when using a puppet:/// URI with a master/agent setup"
-step "when using a puppet://$server/ URI with a master/agent setup"
-
-step "when using a local file path"
+# step "when using a puppet:/// URI with a master/agent setup"
+# step "when using a puppet://$server/ URI with a master/agent setup"
 
+step "Using a local file path"
 on agents, "echo 'Yay, this is the local file.' > /tmp/local_source_file_test.txt"
-
 manifest = "file { '/tmp/source_file_test.txt': source => '/tmp/local_source_file_test.txt', ensure => present }"
 
 apply_manifest_on agents, manifest
-
-on agents, 'test "$(cat /tmp/source_file_test.txt)" = "Yay, this is the local file."'
+agents.each do |host|
+  on host, "cat /tmp/source_file_test.txt" do
+    assert_match(/Yay, this is the local file./, stdout, "FIRST: File contents not matched on #{host}")
+  end
+end
 
 step "Ensure the test environment is clean"
 on agents, 'rm -f /tmp/source_file_test.txt'
 
-step "when using a puppet:/// URI with puppet apply"
+step "Using a puppet:/// URI with puppet apply"
 
-on agents, 'puppet agent --configprint modulepath' do
+on agents, puppet_agent("--configprint modulepath") do
   modulepath = stdout.split(':')[0]
   modulepath = modulepath.chomp
   on agents, "mkdir -p #{modulepath}/test_module/files"
-  on agents, "echo 'Yay, this is the puppet:/// file.' > #{modulepath}/test_module/files/test_file.txt"
+  #on agents, "echo 'Yay, this is the puppet:/// file.' > #{modulepath}/test_module/files/test_file.txt"
+  on agents, "echo 'Yay, this is the puppetfile.' > #{modulepath}/test_module/files/test_file.txt"
 end
 
 on agents, %q{echo "file { '/tmp/source_file_test.txt': source => 'puppet:///modules/test_module/test_file.txt', ensure => present }" > /tmp/source_test_manifest.pp}
-on agents, "puppet apply /tmp/source_test_manifest.pp"
+on agents, puppet_apply("/tmp/source_test_manifest.pp") 
 
-on agents, 'test "$(cat /tmp/source_file_test.txt)" = "Yay, this is the puppet:/// file."'
+agents.each do |host|
+  on host, "cat /tmp/source_file_test.txt" do
+    assert_match(/Yay, this is the puppetfile./, stdout, "SECOND: File contents not matched on #{host}")
+  end
+end
 
 # Oops. We (Jesse & Jacob) ended up writing this before realizing that you
 # can't actually specify "source => 'http://...'".  However, we're leaving it

acceptance/tests/resource/group/should_create.rb

@@ -1,6 +1,6 @@
 test_name "should create group"
 
-name = "test-group-#{Time.new.to_i}"
+name = "pl#{rand(999999).to_i}"
 
 def cleanup(name)
     step "remove group #{name} if it exists"
@@ -14,7 +14,7 @@ on(agents, puppet_resource('group', name, 'ensure=present'))
 
 step "verify the group #{name} was created"
 on(agents, "getent group #{name}") do
-    fail_test "group information is not sensible" unless stdout =~ /^#{name}:x:[0-9]+:/
+    fail_test "group information is not sensible" unless stdout =~ /^#{name}:.*:[0-9]+:/
 end
 
 cleanup(name)

acceptance/tests/resource/group/should_modify_gid.rb

@@ -1,22 +1,23 @@
 test_name "should modify gid of existing group"
 
-name = "test-group-#{Time.new.to_i}"
-gid  = 12345
+name = "pl#{rand(999999).to_i}"
+gid1  = rand(999999).to_i
+gid2  = rand(999999).to_i
 
-step "ensure that the group exists with gid #{gid}"
-on(agents, puppet_resource('group', name, 'ensure=present', "gid=#{gid}")) do
-    fail_test "missing gid notice" unless stdout =~ /gid +=> +'#{gid}'/
+step "ensure that the group exists with gid #{gid1}"
+on(agents, puppet_resource('group', name, 'ensure=present', "gid=#{gid1}")) do
+    fail_test "missing gid notice" unless stdout =~ /gid +=> +'#{gid1}'/
 end
 
-step "ensure that we can modify the GID of the group to #{gid*2}"
-on(agents, puppet_resource('group', name, 'ensure=present', "gid=#{gid*2}")) do
-    fail_test "missing gid notice" unless stdout =~ /gid +=> +'#{gid*2}'/
+step "ensure that we can modify the GID of the group to #{gid2}"
+on(agents, puppet_resource('group', name, 'ensure=present', "gid=#{gid2}")) do
+    fail_test "missing gid notice" unless stdout =~ /gid +=> +'#{gid2}'/
 end
 
 step "verify that the GID changed"
 on(agents, "getent group #{name}") do
     fail_test "gid is wrong through getent output" unless
-        stdout =~ /^#{name}:x:#{gid*2}:/
+        stdout =~ /^#{name}:.*:#{gid2}:/
 end
 
 step "clean up the system after the test run"

acceptance/tests/resource/host/should_create.rb

@@ -3,7 +3,7 @@ test_name "host should create"
 target = "/tmp/host-#{Time.new.to_i}"
 
 step "clean up for the test"
-on agents, "rm -vf #{target}"
+on agents, "rm -f #{target}"
 
 step "create the host record"
 on(agents, puppet_resource("host", "test", "ensure=present",

acceptance/tests/resource/host/should_create_aliases.rb

@@ -3,7 +3,7 @@ test_name "host should create aliases"
 target  = "/tmp/host-#{Time.new.to_i}"
 
 step "clean up the system for testing"
-on agents, "rm -vf #{target}"
+on agents, "rm -f #{target}"
 
 step "create the record"
 on(agents, puppet_resource('host', 'test', "ensure=present",

acceptance/tests/resource/host/should_destroy.rb

@@ -16,4 +16,4 @@ on(agents, "cat #{file}; rm -f #{file}") do
 end
 
 step "clean up after the test"
-on agents, "rm -vf #{file}"
+on agents, "rm -f #{file}"

acceptance/tests/resource/host/should_modify_alias.rb

@@ -16,4 +16,4 @@ on(agents, "cat #{file}; rm -f #{file}") do
 end
 
 step "clean up after the test"
-on agents, "rm -vf #{file}"
+on agents, "rm -f #{file}"

acceptance/tests/resource/host/should_modify_ip.rb

@@ -16,4 +16,4 @@ on(agents, "cat #{file}; rm -f #{file}") do
 end
 
 step "clean up after the test"
-on agents, "rm -vf #{file}"
+on agents, "rm -f #{file}"

acceptance/tests/resource/host/should_not_create_existing.rb

@@ -13,5 +13,5 @@ on(agents, puppet_resource('host', 'test', "target=#{file}",
 end
 
 step "clean up after we created things"
-on agents, "rm -vf #{file}"
+on agents, "rm -f #{file}"
 

acceptance/tests/resource/host/should_query.rb

@@ -12,4 +12,4 @@ on(agents, puppet_resource('host', 'localhost', "target=#{file}")) do
 end
 
 step "clean up the system"
-on agents, "rm -vf #{file}"
+on agents, "rm -f #{file}"

acceptance/tests/resource/host/should_query_all.rb

@@ -23,4 +23,4 @@ on(agents, puppet_resource('host')) do
 end
 
 step "clean up the system afterwards"
-on agents, "mv -vf #{backup} /etc/hosts"
+on agents, "mv -f #{backup} /etc/hosts"

acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.rb

@@ -4,9 +4,10 @@ step "Validate services running agreement ralsh vs. OS service count"
 # ticket_4123_should_list_all_running_redhat.sh
 
 hosts.each do |host|
-  unless host['platform'].include? 'centos' or host['platform'].include? 'redhat'
-    skip_test "Test not supported on this plaform" 
-  else
+  if host['platform'].include?('centos') or host['platform'].include?('redhat')
+    puts "XXX #{host['platform']}"
     run_script_on(host,'acceptance-tests/tests/resource/service/ticket_4123_should_list_all_running_redhat.sh')
+  else
+    skip_test "Test not supported on this plaform" 
   end
 end

acceptance/tests/resource/user/should_create.rb

@@ -1,9 +1,6 @@
 test_name "should create a user, and the default matching group"
-# REVISIT: This is a direct port of the original test, but it contains a
-# non-portable assumption that "user private groups" are used by default by
-# everything that we target. --daniel 2010-12-24
 
-name = "test-user-#{Time.new.to_i}"
+name = "pl#{rand(999999).to_i}"
 
 step "ensure that the user and group #{name} do not exist"
 on agents, "if getent passwd #{name}; then userdel #{name}; fi"
@@ -13,7 +10,14 @@ step "ask puppet to create the user"
 on(agents, puppet_resource('user', name, 'ensure=present'))
 
 step "verify that the user and group now exist"
-on agents, "getent passwd #{name} && getent group #{name}"
+agents.each do |agent|
+  if agent['platform'].include? 'sles' or agent['platform'].include? 'solaris'  # no private user groups by default
+    on agent, "getent passwd #{name}"
+  else
+    on agent, "getent passwd #{name} && getent group #{name}"
+  end
+end
+
 
 step "ensure that the user and group #{name} do not exist"
 on agents, "if getent passwd #{name}; then userdel #{name}; fi"

acceptance/tests/resource/user/should_create_with_gid.rb

@@ -1,14 +1,14 @@
 test_name "verifies that puppet resource creates a user and assigns the correct group"
 
-user  = "test-user-#{Time.new.to_i}"
-group = "test-user-#{Time.new.to_i}-group"
+user = "pl#{rand(999999).to_i}"
+group = "gp#{rand(999999).to_i}"
 
 agents.each do |host|
     step "user should not exist"
     on host, "if getent passwd #{user}; then userdel #{user}; fi"
 
     step "group should exist"
-    on host, "if ! getent group #{group}; then groupadd #{group}; fi"
+    on host, "getent group #{group} || groupadd #{group}"
 
     step "create user with group"
     on(host, puppet_resource('user', user, 'ensure=present', "gid=#{group}"))

acceptance/tests/resource/user/should_destroy.rb

@@ -1,6 +1,6 @@
 test_name "verify that puppet resource correctly destroys users"
 
-user  = "test-user-#{Time.new.to_i}"
+user = "pl#{rand(999999).to_i}"
 group = user
 
 step "ensure that the user and associated group exist"

acceptance/tests/resource/user/should_modify_gid.rb

@@ -1,8 +1,8 @@
 test_name "verify that we can modify the gid"
 
-user   = "test-user-#{Time.new.to_i}"
-group1 = "#{user}-old"
-group2 = "#{user}-new"
+user = "pl#{rand(99999).to_i}"
+group1 = "#{user}old"
+group2 = "#{user}new"
 
 agents.each do |host|
     step "ensure that the groups both exist"

acceptance/tests/resource/user/should_not_destoy_unexisting.rb

@@ -1,6 +1,6 @@
 test_name "ensure that puppet does not report removing a user that does not exist"
 
-name = "test-user-#{Time.new.to_i}"
+name = "pl#{rand(999999).to_i}"
 
 step "verify that user #{name} does not exist"
 on agents, "getent passwd #{name}", :acceptable_exit_codes => [2]

acceptance/tests/resource/user/should_query.rb

@@ -1,6 +1,6 @@
 test_name "test that we can query and find a user that exists."
 
-name = "test-user-#{Time.new.to_i}"
+name = "pl#{rand(999999).to_i}"
 
 step "ensure that our test user exists"
 on(agents, puppet_resource('user', name, 'ensure=present'))

acceptance/tests/ticket_6418_file_recursion_and_audit.rb

@@ -2,11 +2,9 @@
 #
 # AffectedVersion: 2.6.0-2.6.5
 # FixedVersion:
-#
 
 test_name "#6418: file recursion and audit"
 
-on agents, "rm -f /var/lib/puppet/state/state.yaml "
 manifest = %q{
     file { "/tmp/6418": ensure => directory }
     file { "/tmp/6418/dir": ensure => directory}
@@ -17,6 +15,19 @@ manifest = %q{
     File["/tmp/6418"] -> File["/tmp/6418/dir"] -> File["/tmp/6418/dir/dir"] -> File["/tmp/6418/dir/dir/dir"] -> File["/tmp/6418-copy"]
 }
 
+step "Query agent for statefile"
+agent=agents.first
+on agent, puppet_agent('--configprint statefile')
+statefile=stdout.chomp
+
+step "Remove the statefile on all Agents"
+on agents, "rm -f #{statefile}"
+
 step "Apply the manifest"
 apply_manifest_on agents, manifest
-on agents, "! grep ensure.*directory /var/lib/puppet/state/state.yaml"
+
+
+step "Verify corecct file recursion and audit state"
+agents.each do |agent|
+  on(agent, "grep ensure.*directory #{statefile}", :acceptable_exit_codes => [ 1 ])
+end

conf/redhat/puppet.spec

@@ -5,13 +5,13 @@
 %global confdir conf/redhat
 
 Name:           puppet
-Version:        2.6.0
+Version:        2.6.9
 Release:        1%{?dist}
 Summary:        A network tool for managing many disparate systems
 License:        Apache 2.0
 URL:            http://puppetlabs.com
-Source0:        http://puppetlabs.com/downloads/%{name}/%{name}-%{version}.tar.gz
-Source1:        http://puppetlabs.com/downloads/%{name}/%{name}-%{version}.tar.gz.sign
+Source0:        http://puppetlabs.com/downloads/%{name}/%{name}-%{version}rc1.tar.gz
+#Source1:        http://puppetlabs.com/downloads/%{name}/%{name}-%{version}.tar.gz.asc
 
 Group:          System Environment/Base
 
@@ -27,11 +27,11 @@ Requires:       ruby-shadow
 %endif
 
 # Pull in ruby selinux bindings where available
-%if 0%{?fedora} >= 12 || 0%{?rhel} >= 6
-%{!?_without_selinux:Requires: ruby(selinux)}
+%if 0%{?fedora} || 0%{?rhel} >= 6
+%{!?_without_selinux:Requires: ruby(selinux), libselinux-utils}
 %else
-%if 0%{?fedora} || 0%{?rhel} >= 5
-%{!?_without_selinux:Requires: libselinux-ruby}
+%if 0%{?rhel} && 0%{?rhel} == 5
+%{!?_without_selinux:Requires: libselinux-ruby, libselinux-utils}
 %endif
 %endif
 
@@ -65,8 +65,8 @@ Provides the central puppet server daemon which provides manifests to clients.
 The server can also function as a certificate authority and file server.
 
 %prep
-%setup -q
-patch -p1 < conf/redhat/rundir-perms.patch
+%setup -q -n %{name}-%{version}rc1
+patch -s -p1 < conf/redhat/rundir-perms.patch
 
 
 %build
@@ -102,7 +102,6 @@ install -Dp -m0644 %{confdir}/server.sysconfig %{buildroot}%{_sysconfdir}/syscon
 install -Dp -m0755 %{confdir}/server.init %{buildroot}%{_initrddir}/puppetmaster
 install -Dp -m0644 %{confdir}/fileserver.conf %{buildroot}%{_sysconfdir}/puppet/fileserver.conf
 install -Dp -m0644 %{confdir}/puppet.conf %{buildroot}%{_sysconfdir}/puppet/puppet.conf
-install -Dp -m0644 conf/auth.conf %{buildroot}%{_sysconfdir}/puppet/auth.conf
 install -Dp -m0644 %{confdir}/logrotate %{buildroot}%{_sysconfdir}/logrotate.d/puppet
 
 # We need something for these ghosted files, otherwise rpmbuild
@@ -128,9 +127,16 @@ vimdir=%{buildroot}%{_datadir}/vim/vimfiles
 install -Dp -m0644 ext/vim/ftdetect/puppet.vim $vimdir/ftdetect/puppet.vim
 install -Dp -m0644 ext/vim/syntax/puppet.vim $vimdir/syntax/puppet.vim
 
+%if 0%{?fedora} >= 15
+# Setup tmpfiles.d config
+mkdir -p %{buildroot}%{_sysconfdir}/tmpfiles.d
+echo "D /var/run/%{name} 0755 %{name} %{name} -" > \
+    %{buildroot}%{_sysconfdir}/tmpfiles.d/%{name}.conf
+%endif
+
 %files
 %defattr(-, root, root, 0755)
-%doc CHANGELOG COPYING LICENSE README README.queueing examples
+%doc CHANGELOG COPYING LICENSE README.md README.queueing examples
 %{_bindir}/pi
 %{_bindir}/puppet
 %{_bindir}/ralsh
@@ -141,6 +147,9 @@ install -Dp -m0644 ext/vim/syntax/puppet.vim $vimdir/syntax/puppet.vim
 %{ruby_sitelibdir}/*
 %{_initrddir}/puppet
 %dir %{_sysconfdir}/puppet
+%if 0%{?fedora} >= 15
+%config(noreplace) %{_sysconfdir}/tmpfiles.d/%{name}.conf
+%endif
 %config(noreplace) %{_sysconfdir}/sysconfig/puppet
 %config(noreplace) %{_sysconfdir}/puppet/puppet.conf
 %config(noreplace) %{_sysconfdir}/puppet/auth.conf
@@ -185,27 +194,48 @@ install -Dp -m0644 ext/vim/syntax/puppet.vim $vimdir/syntax/puppet.vim
 getent group puppet &>/dev/null || groupadd -r puppet -g 52 &>/dev/null
 getent passwd puppet &>/dev/null || \
 useradd -r -u 52 -g puppet -d %{_localstatedir}/lib/puppet -s /sbin/nologin \
-    -c "Puppet" puppet &>/dev/null || :
+    -c "Puppet" puppet &>/dev/null
 # ensure that old setups have the right puppet home dir
 if [ $1 -gt 1 ] ; then
-  usermod -d %{_localstatedir}/lib/puppet puppet &>/dev/null || :
+  usermod -d %{_localstatedir}/lib/puppet puppet &>/dev/null
 fi
+exit 0
 
 %post
 /sbin/chkconfig --add puppet || :
+if [ "$1" -ge 1 ]; then
+  # The pidfile changed from 0.25.x to 2.6.x, handle upgrades without leaving
+  # the old process running.
+  oldpid="%{_localstatedir}/run/puppet/puppetd.pid"
+  newpid="%{_localstatedir}/run/puppet/agent.pid"
+  if [ -s "$oldpid" -a ! -s "$newpid" ]; then
+    (kill $(< "$oldpid") && rm -f "$oldpid" && \
+      /sbin/service puppet start) >/dev/null 2>&1 || :
+  fi
+fi
 
 %post server
 /sbin/chkconfig --add puppetmaster || :
+if [ "$1" -ge 1 ]; then
+  # The pidfile changed from 0.25.x to 2.6.x, handle upgrades without leaving
+  # the old process running.
+  oldpid="%{_localstatedir}/run/puppet/puppetmasterd.pid"
+  newpid="%{_localstatedir}/run/puppet/master.pid"
+  if [ -s "$oldpid" -a ! -s "$newpid" ]; then
+    (kill $(< "$oldpid") && rm -f "$oldpid" && \
+      /sbin/service puppetmaster start) >/dev/null 2>&1 || :
+  fi
+fi
 
 %preun
 if [ "$1" = 0 ] ; then
-  /sbin/service puppet stop > /dev/null 2>&1
+  /sbin/service puppet stop >/dev/null 2>&1
   /sbin/chkconfig --del puppet || :
 fi
 
 %preun server
 if [ "$1" = 0 ] ; then
-  /sbin/service puppetmaster stop > /dev/null 2>&1
+  /sbin/service puppetmaster stop >/dev/null 2>&1
   /sbin/chkconfig --del puppetmaster || :
 fi
 
@@ -216,27 +246,68 @@ fi
 
 %postun server
 if [ "$1" -ge 1 ]; then
-  /sbin/service puppetmaster condrestart > /dev/null 2>&1 || :
+  /sbin/service puppetmaster condrestart >/dev/null 2>&1 || :
 fi
 
 %clean
 rm -rf %{buildroot}
 
 %changelog
-* Tue Jul 20 2010 Todd Zullinger <tmz@pobox.com> - 2.6.0-1
-- Update to 2.6.0
-- Create and own /usr/share/puppet/modules (#615432)
+* Tue Jun 21 2011 Michael Stahnke <stahnma@puppetlabs.com> - 2.6.9-1
+- Release of 2.6.9 
 
-* Mon May 03 2010 Todd Zullinger <tmz@pobox.com> - 0.25.5-1
+* Wed Jun 15 2011 Todd Zullinger <tmz@pobox.com> - 2.6.9-0.1.rc1
+- Update rc versioning to ensure 2.6.9 final is newer to rpm
+- sync changes with Fedora/EPEL
+
+* Tue Jun 14 2011 Michael Stahnke <stahnma@puppetlabs.com> - 2.6.9rc1-1
+- Update to 2.6.9rc1
+
+* Thu Apr 14 2011 Todd Zullinger <tmz@pobox.com> - 2.6.8-1
+- Update to 2.6.8
+
+* Thu Mar 24 2011 Todd Zullinger <tmz@pobox.com> - 2.6.7-1
+- Update to 2.6.7
+
+* Wed Mar 16 2011 Todd Zullinger <tmz@pobox.com> - 2.6.6-1
+- Update to 2.6.6
+- Ensure %%pre exits cleanly
+- Fix License tag, puppet is now GPLv2 only
+- Create and own /usr/share/puppet/modules (#615432)
+- Properly restart puppet agent/master daemons on upgrades from 0.25.x
+- Require libselinux-utils when selinux support is enabled
+- Support tmpfiles.d for Fedora >= 15 (#656677)
+
+* Wed Feb 09 2011 Fedora Release Engineering <rel-eng@lists.fedoraproject.org> - 0.25.5-2
+- Rebuilt for https://fedoraproject.org/wiki/Fedora_15_Mass_Rebuild
+
+* Mon May 17 2010 Todd Zullinger <tmz@pobox.com> - 0.25.5-1
 - Update to 0.25.5
 - Adjust selinux conditional for EL-6
 - Apply rundir-perms patch from tarball rather than including it separately
+- Update URL's to reflect the new puppetlabs.com domain
 
-* Fri Jan 01 2010 Todd Zullinger <tmz@pobox.com> - 0.25.2-1
+* Fri Jan 29 2010 Todd Zullinger <tmz@pobox.com> - 0.25.4-1
+- Update to 0.25.4
+
+* Tue Jan 19 2010 Todd Zullinger <tmz@pobox.com> - 0.25.3-2
+- Apply upstream patch to fix cron resources (upstream #2845)
+
+* Mon Jan 11 2010 Todd Zullinger <tmz@pobox.com> - 0.25.3-1
+- Update to 0.25.3
+
+* Tue Jan 05 2010 Todd Zullinger <tmz@pobox.com> - 0.25.2-1.1
+- Replace %%define with %%global for macros
+
+* Tue Jan 05 2010 Todd Zullinger <tmz@pobox.com> - 0.25.2-1
 - Update to 0.25.2
+- Fixes CVE-2010-0156, tmpfile security issue (#502881)
 - Install auth.conf, puppetqd manpage, and queuing examples/docs
 
-* Tue Oct 20 2009 Todd Zullinger <tmz@pobox.com> - 0.25.1-1
+* Wed Nov 25 2009 Jeroen van Meeuwen <j.van.meeuwen@ogd.nl> - 0.25.1-1
+- New upstream version
+
+* Tue Oct 27 2009 Todd Zullinger <tmz@pobox.com> - 0.25.1-0.3
 - Update to 0.25.1
 - Include the pi program and man page (R.I.Pienaar)
 

lib/puppet/application/apply.rb

@@ -213,7 +213,13 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License
       configurer = Puppet::Configurer.new
       report = configurer.run(:skip_plugin_download => true, :catalog => catalog)
 
-      exit( options[:detailed_exitcodes] ? report.exit_status : 0 )
+      if not report
+        exit(1)
+      elsif options[:detailed_exitcodes] then
+        exit(report.exit_status)
+      else
+        exit(0)
+      end
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       $stderr.puts detail.message

lib/puppet/configurer.rb

@@ -5,8 +5,6 @@ require 'puppet/network/http_pool'
 require 'puppet/util'
 
 class Puppet::Configurer
-  class CommandHookError < RuntimeError; end
-
   require 'puppet/configurer/fact_handler'
   require 'puppet/configurer/plugin_handler'
 
@@ -79,8 +77,6 @@ class Puppet::Configurer
     download_plugins unless options[:skip_plugin_download]
 
     download_fact_plugins unless options[:skip_plugin_download]
-
-    execute_prerun_command
   end
 
   # Get the remote catalog, yo.  Returns nil if no catalog can be found.
@@ -109,67 +105,73 @@ class Puppet::Configurer
     catalog
   end
 
-  # The code that actually runs the catalog.
-  # This just passes any options on to the catalog,
-  # which accepts :tags and :ignoreschedules.
-  def run(options = {})
-    begin
-      prepare(options)
-    rescue SystemExit,NoMemoryError
-      raise
-    rescue Exception => detail
-      puts detail.backtrace if Puppet[:trace]
-      Puppet.err "Failed to prepare catalog: #{detail}"
-    end
-
-    if Puppet::Resource::Catalog.indirection.terminus_class == :rest
-      # This is a bit complicated.  We need the serialized and escaped facts,
-      # and we need to know which format they're encoded in.  Thus, we
-      # get a hash with both of these pieces of information.
-      fact_options = facts_for_uploading
-    end
-
-    options[:report] ||= Puppet::Transaction::Report.new("apply")
-    report = options[:report]
-    Puppet::Util::Log.newdestination(report)
-
-    if catalog = options[:catalog]
-      options.delete(:catalog)
-    elsif ! catalog = retrieve_catalog(fact_options)
+  # Retrieve (optionally) and apply a catalog. If a catalog is passed in
+  # the options, then apply that one, otherwise retrieve it.
+  def retrieve_and_apply_catalog(options, fact_options)
+    unless catalog = (options.delete(:catalog) || retrieve_catalog(fact_options))
       Puppet.err "Could not retrieve catalog; skipping run"
       return
     end
 
+    report = options[:report]
     report.configuration_version = catalog.version
 
-    transaction = nil
-
-    begin
-      benchmark(:notice, "Finished catalog run") do
-        transaction = catalog.apply(options)
-      end
-      report
-    rescue => detail
-      puts detail.backtrace if Puppet[:trace]
-      Puppet.err "Failed to apply catalog: #{detail}"
-      return
+    benchmark(:notice, "Finished catalog run") do
+      catalog.apply(options)
     end
-  ensure
-    # Make sure we forget the retained module_directories of any autoload
-    # we might have used.
-    Thread.current[:env_module_directories] = nil
 
-    # Now close all of our existing http connections, since there's no
-    # reason to leave them lying open.
-    Puppet::Network::HttpPool.clear_http_instances
-    execute_postrun_command
-
-    Puppet::Util::Log.close(report)
-    send_report(report, transaction)
+    report.finalize_report
+    report
   end
 
-  def send_report(report, trans)
-    report.finalize_report if trans
+  # The code that actually runs the catalog.
+  # This just passes any options on to the catalog,
+  # which accepts :tags and :ignoreschedules.
+  def run(options = {})
+    options[:report] ||= Puppet::Transaction::Report.new("apply")
+    report = options[:report]
+
+    Puppet::Util::Log.newdestination(report)
+    begin
+      prepare(options)
+
+      if Puppet::Resource::Catalog.indirection.terminus_class == :rest
+        # This is a bit complicated.  We need the serialized and escaped facts,
+        # and we need to know which format they're encoded in.  Thus, we
+        # get a hash with both of these pieces of information.
+        fact_options = facts_for_uploading
+      end
+
+      # set report host name now that we have the fact
+      report.host = Puppet[:node_name_value]
+
+      begin
+        execute_prerun_command or return nil
+        retrieve_and_apply_catalog(options, fact_options)
+      rescue SystemExit,NoMemoryError
+        raise
+      rescue => detail
+        puts detail.backtrace if Puppet[:trace]
+        Puppet.err "Failed to apply catalog: #{detail}"
+        return nil
+      ensure
+        execute_postrun_command or return nil
+      end
+    ensure
+      # Make sure we forget the retained module_directories of any autoload
+      # we might have used.
+      Thread.current[:env_module_directories] = nil
+
+      # Now close all of our existing http connections, since there's no
+      # reason to leave them lying open.
+      Puppet::Network::HttpPool.clear_http_instances
+    end
+  ensure
+    Puppet::Util::Log.close(report)
+    send_report(report)
+  end
+
+  def send_report(report)
     puts report.summary if Puppet[:summarize]
     save_last_run_summary(report)
     Puppet::Transaction::Report.indirection.save(report) if Puppet[:report]
@@ -207,12 +209,15 @@ class Puppet::Configurer
   end
 
   def execute_from_setting(setting)
-    return if (command = Puppet[setting]) == ""
+    return true if (command = Puppet[setting]) == ""
 
     begin
       Puppet::Util.execute([command])
+      true
     rescue => detail
-      raise CommandHookError, "Could not run command from #{setting}: #{detail}"
+      puts detail.backtrace if Puppet[:trace]
+      Puppet.err "Could not run command from #{setting}: #{detail}"
+      false
     end
   end
 

lib/puppet/metatype/manager.rb

@@ -61,10 +61,9 @@ module Manager
 
     # Then create the class.
 
-          klass = genclass(
-        name,
+    klass = genclass(
+      name,
       :parent => (parent || Puppet::Type),
-        
       :overwrite => true,
       :hash => @types,
       :attributes => options,
@@ -87,14 +86,11 @@ module Manager
 
     # Now set up autoload any providers that might exist for this type.
 
-          klass.providerloader = Puppet::Util::Autoload.new(
-        klass,
-        
-      "puppet/provider/#{klass.name.to_s}"
-    )
+    klass.providerloader = Puppet::Util::Autoload.new(klass, "puppet/provider/#{klass.name.to_s}")
 
-    # We have to load everything so that we can figure out the default type.
+    # We have to load everything so that we can figure out the default provider.
     klass.providerloader.loadall
+    klass.providify unless klass.providers.empty?
 
     klass
   end
@@ -103,11 +99,7 @@ module Manager
   def rmtype(name)
     # Then create the class.
 
-          klass = rmclass(
-        name,
-        
-      :hash => @types
-    )
+    klass = rmclass(name, :hash => @types)
 
     singleton_class.send(:remove_method, "new#{name}") if respond_to?("new#{name}")
   end
@@ -132,12 +124,7 @@ module Manager
   # Create a loader for Puppet types.
   def typeloader
     unless defined?(@typeloader)
-
-            @typeloader = Puppet::Util::Autoload.new(
-        self,
-        
-        "puppet/type", :wrap => false
-      )
+      @typeloader = Puppet::Util::Autoload.new(self, "puppet/type", :wrap => false)
     end
 
     @typeloader

lib/puppet/resource/catalog.rb

@@ -128,9 +128,10 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     expire
 
     Puppet::Util::Storage.load if host_config?
-    transaction = Puppet::Transaction.new(self)
 
-    transaction.report = options[:report] if options[:report]
+    transaction = Puppet::Transaction.new(self, options[:report])
+    register_report = options[:report].nil?
+
     transaction.tags = options[:tags] if options[:tags]
     transaction.ignoreschedules = true if options[:ignoreschedules]
     transaction.for_network_device = options[:network_device]
@@ -138,7 +139,12 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     transaction.add_times :config_retrieval => self.retrieval_duration || 0
 
     begin
-      transaction.evaluate
+      Puppet::Util::Log.newdestination(transaction.report) if register_report
+      begin
+        transaction.evaluate
+      ensure
+        Puppet::Util::Log.close(transaction.report) if register_report
+      end
     rescue Puppet::Error => detail
       puts detail.backtrace if Puppet[:trace]
       Puppet.err "Could not apply complete catalog: #{detail}"

lib/puppet/transaction.rb

@@ -16,7 +16,7 @@ class Puppet::Transaction
   attr_accessor :configurator
 
   # The report, once generated.
-  attr_accessor :report
+  attr_reader :report
 
   # Routes and stores any events and subscriptions.
   attr_reader :event_manager
@@ -92,25 +92,17 @@ class Puppet::Transaction
   # collects all of the changes, executes them, and responds to any
   # necessary events.
   def evaluate
-    # Start logging.
-    Puppet::Util::Log.newdestination(@report)
-
     prepare
 
     Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version
 
-    begin
-      relationship_graph.traverse do |resource|
-        if resource.is_a?(Puppet::Type::Component)
-          Puppet.warning "Somehow left a component in the relationship graph"
-        else
-          seconds = thinmark { eval_resource(resource) }
-          resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
-        end
+    relationship_graph.traverse do |resource|
+      if resource.is_a?(Puppet::Type::Component)
+        Puppet.warning "Somehow left a component in the relationship graph"
+      else
+        seconds = thinmark { eval_resource(resource) }
+        resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
       end
-    ensure
-      # And then close the transaction log.
-      Puppet::Util::Log.close(@report)
     end
 
     Puppet.debug "Finishing transaction #{object_id}"
@@ -221,10 +213,10 @@ class Puppet::Transaction
 
   # this should only be called by a Puppet::Type::Component resource now
   # and it should only receive an array
-  def initialize(catalog)
+  def initialize(catalog, report = nil)
     @catalog = catalog
 
-    @report = Puppet::Transaction::Report.new("apply")
+    @report = report || Puppet::Transaction::Report.new("apply", catalog.version)
 
     @event_manager = Puppet::Transaction::EventManager.new(self)
 

lib/puppet/transaction/report.rb

@@ -10,8 +10,8 @@ class Puppet::Transaction::Report
 
   indirects :report, :terminus_class => :processor
 
-  attr_accessor :configuration_version
-  attr_reader :resource_statuses, :logs, :metrics, :host, :time, :kind, :status
+  attr_accessor :configuration_version, :host
+  attr_reader :resource_statuses, :logs, :metrics, :time, :kind, :status
 
   # This is necessary since Marshall doesn't know how to
   # dump hash with default proc (see below @records)

lib/puppet/type.rb

@@ -1442,9 +1442,8 @@ class Type
   def self.provide(name, options = {}, &block)
     name = Puppet::Util.symbolize(name)
 
-    if obj = provider_hash[name]
+    if unprovide(name)
       Puppet.debug "Reloading #{name} #{self.name} provider"
-      unprovide(name)
     end
 
     parent = if pname = options[:parent]
@@ -1467,16 +1466,14 @@ class Type
 
     self.providify
 
-
-      provider = genclass(
-        name,
-      :parent => parent,
-      :hash => provider_hash,
-      :prefix => "Provider",
-      :block => block,
-      :include => feature_module,
-      :extend => feature_module,
-
+    provider = genclass(
+      name,
+      :parent     => parent,
+      :hash       => provider_hash,
+      :prefix     => "Provider",
+      :block      => block,
+      :include    => feature_module,
+      :extend     => feature_module,
       :attributes => options
     )
 
@@ -1536,18 +1533,11 @@ class Type
   end
 
   def self.unprovide(name)
-    if provider_hash.has_key? name
-
-      rmclass(
-        name,
-        :hash => provider_hash,
-
-        :prefix => "Provider"
-      )
-      if @defaultprovider and @defaultprovider.name == name
-        @defaultprovider = nil
-      end
+    if @defaultprovider and @defaultprovider.name == name
+      @defaultprovider = nil
     end
+
+    rmclass(name, :hash => provider_hash, :prefix => "Provider")
   end
 
   # Return an array of all of the suitable providers.
@@ -1607,7 +1597,6 @@ class Type
 
       # Collect the current prereqs
       list.each { |dep|
-        obj = nil
         # Support them passing objects directly, to save some effort.
         unless dep.is_a? Puppet::Type
           # Skip autorequires that we aren't managing

spec/integration/type_spec.rb

@@ -18,4 +18,15 @@ describe Puppet::Type do
 
     type.provider(:myprovider).should equal(provider)
   end
+
+  it "should not lose its provider parameter when it is reloaded" do
+    type = Puppet::Type.newtype(:reload_test_type)
+
+    provider = type.provide(:test_provider)
+
+    # reload it
+    type = Puppet::Type.newtype(:reload_test_type)
+
+    type.parameters.should include(:provider)
+  end
 end

spec/unit/application/apply_spec.rb

@@ -277,8 +277,8 @@ describe Puppet::Application::Apply do
       end
 
       it "should call the prerun and postrun commands on a Configurer instance" do
-        Puppet::Configurer.any_instance.expects(:execute_prerun_command)
-        Puppet::Configurer.any_instance.expects(:execute_postrun_command)
+        Puppet::Configurer.any_instance.expects(:execute_prerun_command).returns(true)
+        Puppet::Configurer.any_instance.expects(:execute_postrun_command).returns(true)
 
         expect { @apply.main }.to exit_with 0
       end

spec/unit/configurer_spec.rb

@@ -39,16 +39,16 @@ describe Puppet::Configurer do
 
     it "should execute any pre-run command provided via the 'prerun_command' setting" do
       Puppet.settings[:prerun_command] = "/my/command"
-      Puppet::Util.expects(:execute).with { |args| args[0] == "/my/command" }
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
 
       @agent.execute_prerun_command
     end
 
     it "should fail if the command fails" do
       Puppet.settings[:prerun_command] = "/my/command"
-      Puppet::Util.expects(:execute).raises Puppet::ExecutionFailure
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
 
-      lambda { @agent.execute_prerun_command }.should raise_error(Puppet::Configurer::CommandHookError)
+      @agent.execute_prerun_command.should be_false
     end
   end
 
@@ -62,16 +62,16 @@ describe Puppet::Configurer do
 
     it "should execute any post-run command provided via the 'postrun_command' setting" do
       Puppet.settings[:postrun_command] = "/my/command"
-      Puppet::Util.expects(:execute).with { |args| args[0] == "/my/command" }
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
 
       @agent.execute_postrun_command
     end
 
     it "should fail if the command fails" do
       Puppet.settings[:postrun_command] = "/my/command"
-      Puppet::Util.expects(:execute).raises Puppet::ExecutionFailure
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
 
-      lambda { @agent.execute_postrun_command }.should raise_error(Puppet::Configurer::CommandHookError)
+      @agent.execute_postrun_command.should be_false
     end
   end
 
@@ -88,6 +88,9 @@ describe Puppet::Configurer do
       Puppet::Resource::Catalog.indirection.terminus_class = :rest
       Puppet::Resource::Catalog.indirection.stubs(:find).returns(@catalog)
       @agent.stubs(:send_report)
+      @agent.stubs(:save_last_run_summary)
+
+      Puppet::Util::Log.stubs(:close_all)
     end
 
     it "should prepare for the run" do
@@ -98,7 +101,7 @@ describe Puppet::Configurer do
 
     it "should initialize a transaction report if one is not provided" do
       report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).at_least_once.returns report
+      Puppet::Transaction::Report.expects(:new).returns report
 
       @agent.run
     end
@@ -128,9 +131,10 @@ describe Puppet::Configurer do
 
     it "should set the report as a log destination" do
       report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).at_least_once.returns report
+      Puppet::Transaction::Report.expects(:new).returns report
 
-      Puppet::Util::Log.expects(:newdestination).with(report).at_least_once
+      Puppet::Util::Log.expects(:newdestination).with(report)
+      Puppet::Util::Log.expects(:close).with(report)
 
       @agent.run
     end
@@ -180,24 +184,12 @@ describe Puppet::Configurer do
 
     it "should send the report" do
       report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).at_least_once.returns(report)
-      @agent.expects(:send_report).with { |r, trans| r == report }
+      Puppet::Transaction::Report.expects(:new).returns(report)
+      @agent.expects(:send_report).with(report)
 
       @agent.run
     end
 
-    it "should send the transaction report with a reference to the transaction if a run was actually made" do
-      report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).returns(report)
-
-      trans = stub 'transaction'
-      @catalog.expects(:apply).returns trans
-
-      @agent.expects(:send_report).with { |r, t| t == trans }
-
-      @agent.run :catalog => @catalog
-    end
-
     it "should send the transaction report even if the catalog could not be retrieved" do
       @agent.expects(:retrieve_catalog).returns nil
 
@@ -215,12 +207,12 @@ describe Puppet::Configurer do
       Puppet::Transaction::Report.expects(:new).returns(report)
       @agent.expects(:send_report)
 
-      lambda { @agent.run }.should raise_error
+      @agent.run.should be_nil
     end
 
     it "should remove the report as a log destination when the run is finished" do
       report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).at_least_once.returns(report)
+      Puppet::Transaction::Report.expects(:new).returns(report)
 
       @agent.run
 
@@ -229,11 +221,100 @@ describe Puppet::Configurer do
 
     it "should return the report as the result of the run" do
       report = Puppet::Transaction::Report.new("apply")
-      Puppet::Transaction::Report.expects(:new).at_least_once.returns(report)
+      Puppet::Transaction::Report.expects(:new).returns(report)
 
       @agent.run.should equal(report)
     end
 
+    it "should send the transaction report even if the pre-run command fails" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:prerun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+      @agent.expects(:send_report)
+
+      @agent.run.should be_nil
+    end
+
+    it "should include the pre-run command failure in the report" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:prerun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+
+      report.expects(:<<).with { |log| log.message.start_with?("Could not run command from prerun_command") }
+
+      @agent.run.should be_nil
+    end
+
+    it "should send the transaction report even if the post-run command fails" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:postrun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+      @agent.expects(:send_report)
+
+      @agent.run.should be_nil
+    end
+
+    it "should include the post-run command failure in the report" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:postrun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+
+      report.expects(:<<).with { |log| log.message.start_with?("Could not run command from postrun_command") }
+
+      @agent.run.should be_nil
+    end
+
+    it "should execute post-run command even if the pre-run command fails" do
+      Puppet.settings[:prerun_command] = "/my/precommand"
+      Puppet.settings[:postrun_command] = "/my/postcommand"
+      Puppet::Util.expects(:execute).with(["/my/precommand"]).raises(Puppet::ExecutionFailure, "Failed")
+      Puppet::Util.expects(:execute).with(["/my/postcommand"])
+
+      @agent.run.should be_nil
+    end
+
+    it "should finalize the report" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      report.expects(:finalize_report)
+      @agent.run
+    end
+
+    it "should not apply the catalog if the pre-run command fails" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:prerun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+
+      @catalog.expects(:apply).never()
+      @agent.expects(:send_report)
+
+      @agent.run.should be_nil
+    end
+
+    it "should apply the catalog, send the report, and return nil if the post-run command fails" do
+      report = Puppet::Transaction::Report.new("apply")
+      Puppet::Transaction::Report.expects(:new).returns(report)
+
+      Puppet.settings[:postrun_command] = "/my/command"
+      Puppet::Util.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
+
+      @catalog.expects(:apply)
+      @agent.expects(:send_report)
+
+      @agent.run.should be_nil
+    end
+
     describe "when not using a REST terminus for catalogs" do
       it "should not pass any facts when retrieving the catalog" do
         Puppet::Resource::Catalog.indirection.terminus_class = :compiler
@@ -268,12 +349,6 @@ describe Puppet::Configurer do
       Puppet[:lastrunfile] = tmpfile('last_run_file')
 
       @report = Puppet::Transaction::Report.new("apply")
-      @trans = stub 'transaction'
-    end
-
-    it "should finalize the report" do
-      @report.expects(:finalize_report)
-      @configurer.send_report(@report, @trans)
     end
 
     it "should print a report summary if configured to do so" do
@@ -282,42 +357,42 @@ describe Puppet::Configurer do
       @report.expects(:summary).returns "stuff"
 
       @configurer.expects(:puts).with("stuff")
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should not print a report summary if not configured to do so" do
       Puppet.settings[:summarize] = false
 
       @configurer.expects(:puts).never
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should save the report if reporting is enabled" do
       Puppet.settings[:report] = true
 
       Puppet::Transaction::Report.indirection.expects(:save).with(@report)
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should not save the report if reporting is disabled" do
       Puppet.settings[:report] = false
 
       Puppet::Transaction::Report.indirection.expects(:save).with(@report).never
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should save the last run summary if reporting is enabled" do
       Puppet.settings[:report] = true
 
       @configurer.expects(:save_last_run_summary).with(@report)
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should save the last run summary if reporting is disabled" do
       Puppet.settings[:report] = false
 
       @configurer.expects(:save_last_run_summary).with(@report)
-      @configurer.send_report(@report, nil)
+      @configurer.send_report(@report)
     end
 
     it "should log but not fail if saving the report fails" do
@@ -326,7 +401,7 @@ describe Puppet::Configurer do
       Puppet::Transaction::Report.indirection.expects(:save).raises("whatever")
 
       Puppet.expects(:err)
-      lambda { @configurer.send_report(@report, nil) }.should_not raise_error
+      lambda { @configurer.send_report(@report) }.should_not raise_error
     end
   end
 
@@ -505,7 +580,6 @@ describe Puppet::Configurer do
       Puppet.settings.stubs(:use).returns(true)
       @agent.stubs(:download_fact_plugins)
       @agent.stubs(:download_plugins)
-      @agent.stubs(:execute_prerun_command)
       @facts = {"one" => "two", "three" => "four"}
     end
 
@@ -526,10 +600,5 @@ describe Puppet::Configurer do
 
       @agent.prepare({})
     end
-
-    it "should perform the pre-run commands" do
-      @agent.expects(:execute_prerun_command)
-      @agent.prepare({})
-    end
   end
 end

spec/unit/resource/catalog_spec.rb

@@ -588,7 +588,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
     before :each do
       @catalog = Puppet::Resource::Catalog.new("host")
 
-      @transaction = mock 'transaction'
+      @transaction = Puppet::Transaction.new(@catalog)
       Puppet::Transaction.stubs(:new).returns(@transaction)
       @transaction.stubs(:evaluate)
       @transaction.stubs(:add_times)

spec/unit/transaction_spec.rb

@@ -87,13 +87,19 @@ describe Puppet::Transaction do
     @transaction.should_not be_any_failed
   end
 
-  it "should be possible to replace the report object" do
+  it "should use the provided report object" do
     report = Puppet::Transaction::Report.new("apply")
-    @transaction.report = report
+    @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new, report)
 
     @transaction.report.should == report
   end
 
+  it "should create a report if none is provided" do
+    @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
+
+    @transaction.report.should be_kind_of Puppet::Transaction::Report
+  end
+
   describe "when initializing" do
     it "should create an event manager" do
       @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)

spec/unit/type_spec.rb

@@ -164,6 +164,72 @@ describe Puppet::Type do
     end
   end
 
+  describe "when creating a provider" do
+    before :each do
+      @type = Puppet::Type.newtype(:provider_test_type)
+    end
+
+    after :each do
+      @type.provider_hash.clear
+    end
+
+    it "should create a subclass of Puppet::Provider for the provider" do
+      provider = @type.provide(:test_provider)
+
+      provider.ancestors.should include(Puppet::Provider)
+    end
+
+    it "should use a parent class if specified" do
+      parent_provider = @type.provide(:parent_provider)
+      child_provider  = @type.provide(:child_provider, :parent => parent_provider)
+
+      child_provider.ancestors.should include(parent_provider)
+    end
+
+    it "should use a parent class if specified by name" do
+      parent_provider = @type.provide(:parent_provider)
+      child_provider  = @type.provide(:child_provider, :parent => :parent_provider)
+
+      child_provider.ancestors.should include(parent_provider)
+    end
+
+    it "should raise an error when the parent class can't be found" do
+      expect {
+        @type.provide(:child_provider, :parent => :parent_provider)
+      }.to raise_error(Puppet::DevError, /Could not find parent provider.+parent_provider/)
+    end
+
+    it "should ensure its type has a 'provider' parameter" do
+      @type.provide(:test_provider)
+
+      @type.parameters.should include(:provider)
+    end
+
+    it "should remove a previously registered provider with the same name" do
+      old_provider = @type.provide(:test_provider)
+      new_provider = @type.provide(:test_provider)
+
+      old_provider.should_not equal(new_provider)
+    end
+
+    it "should register itself as a provider for the type" do
+      provider = @type.provide(:test_provider)
+
+      provider.should == @type.provider(:test_provider)
+    end
+
+    it "should create a provider when a provider with the same name previously failed" do
+      @type.provide(:test_provider) do
+        raise "failed to create this provider"
+      end rescue nil
+
+      provider = @type.provide(:test_provider)
+
+      provider.ancestors.should include(Puppet::Provider)
+      provider.should == @type.provider(:test_provider)
+    end
+  end
+
   describe "when choosing a default provider" do
     it "should choose the provider with the highest specificity" do
       # Make a fake type