From bd51a53b0870ee2355c046af255232ec868d8f1d Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 21 May 2008 00:46:25 -0500 Subject: Fixing transaction support for prefetching generated resources. Previously, we prefetched then generated, which caused generated resources that needed prefetching not to work. This just reorders the calls, so generated resources now get prefetched. --- CHANGELOG | 2 ++ lib/puppet/transaction.rb | 6 +++-- test/other/transactions.rb | 57 ++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4fd0a3297..7edb7bd3d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ + Fixing transaction support for prefetching generated resources. + Adding support for settings within the existing Facter provider confines. Moving all confine code out of the Provider class, and fixing #1197. diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index 14b2037f4..b191f8219 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -499,10 +499,12 @@ class Transaction # Prepare to evaluate the resources in a transaction. def prepare - prefetch() - # Now add any dynamically generated resources generate() + + # Then prefetch. It's important that we generate and then prefetch, + # so that any generated resources also get prefetched. + prefetch() # This will throw an error if there are cycles in the graph. @sorted_resources = relationship_graph.topsort diff --git a/test/other/transactions.rb b/test/other/transactions.rb index ce2d0d52b..a517ac731 100755 --- a/test/other/transactions.rb +++ b/test/other/transactions.rb @@ -72,11 +72,11 @@ class TestTransactions < Test::Unit::TestCase path1 = tempfile() path2 = tempfile() objects = [] - objects << Puppet::Type.newfile( + objects << Puppet::Type.type(:file).create( :path => path1, :content => "yayness" ) - objects << Puppet::Type.newfile( + objects << Puppet::Type.type(:file).create( :path => path2, :content => "booness" ) @@ -155,7 +155,7 @@ class TestTransactions < Test::Unit::TestCase path = tempfile() firstpath = tempfile() secondpath = tempfile() - file = Puppet::Type.newfile(:title => "file", :path => path, :content => "yayness") + file = Puppet::Type.type(:file).create(:title => "file", :path => path, :content => "yayness") first = Puppet::Type.newexec(:title => "first", :command => "/bin/echo first > #{firstpath}", :subscribe => [:file, path], @@ -716,6 +716,45 @@ class TestTransactions < Test::Unit::TestCase "Not all resources were evaluated or not in the right order") end + # We need to generate resources before we prefetch them, else generated + # resources that require prefetching don't work. + def test_generate_before_prefetch + config = mk_catalog() + trans = Puppet::Transaction.new(config) + + generate = nil + prefetch = nil + trans.expects(:generate).with { |*args| generate = Time.now; true } + trans.expects(:prefetch).with { |*args| ! generate.nil? } + trans.prepare + return + + resource = Puppet::Type.type(:file).create :ensure => :present, :path => tempfile() + other_resource = mock 'generated' + def resource.generate + [other_resource] + end + + + config = mk_catalog(yay, rah) + trans = Puppet::Transaction.new(config) + + assert_nothing_raised do + trans.generate + end + + %w{ya ra y r}.each do |name| + assert(trans.catalog.vertex?(Puppet::Type.type(:generator)[name]), + "Generated %s was not a vertex" % name) + assert($finished.include?(name), "%s was not finished" % name) + end + + # Now make sure that cleanup gets rid of those generated types. + assert_nothing_raised do + trans.cleanup + end + end + def test_ignore_tags? config = Puppet::Node::Catalog.new config.host_config = true @@ -876,7 +915,7 @@ class TestTransactions < Test::Unit::TestCase end def test_set_target - file = Puppet::Type.newfile(:path => tempfile(), :content => "yay") + file = Puppet::Type.type(:file).create(:path => tempfile(), :content => "yay") exec1 = Puppet::Type.type(:exec).create :command => "/bin/echo exec1" exec2 = Puppet::Type.type(:exec).create :command => "/bin/echo exec2" trans = Puppet::Transaction.new(mk_catalog(file, exec1, exec2)) @@ -911,7 +950,7 @@ class TestTransactions < Test::Unit::TestCase Puppet::Type.rmtype(:norefresh) end - file = Puppet::Type.newfile :path => tempfile(), :content => "yay" + file = Puppet::Type.type(:file).create :path => tempfile(), :content => "yay" one = klass.create :name => "one", :subscribe => file assert_apply(file, one) @@ -989,8 +1028,8 @@ class TestTransactions < Test::Unit::TestCase # Now files d = tempfile() f = File.join(d, "file") - file = Puppet::Type.newfile(:path => f, :content => "yay") - dir = Puppet::Type.newfile(:path => d, :ensure => :directory, :require => file) + file = Puppet::Type.type(:file).create(:path => f, :content => "yay") + dir = Puppet::Type.type(:file).create(:path => d, :ensure => :directory, :require => file) rels[dir] = file rels.each do |after, before| @@ -1014,7 +1053,7 @@ class TestTransactions < Test::Unit::TestCase path = tempfile epath = tempfile spath = tempfile - file = Puppet::Type.newfile(:path => path, :ensure => :file, + file = Puppet::Type.type(:file).create(:path => path, :ensure => :file, :title => "file") exec = Puppet::Type.type(:exec).create(:command => "touch %s" % epath, :path => ENV["PATH"], :subscribe => file, :refreshonly => true, @@ -1052,7 +1091,7 @@ class TestTransactions < Test::Unit::TestCase 3.times do |i| path = tempfile paths << path - file = Puppet::Type.newfile(:path => path, :ensure => :absent, + file = Puppet::Type.type(:file).create(:path => path, :ensure => :absent, :backup => false, :title => "file%s" % i) File.open(path, "w") { |f| f.puts "" } files << file -- cgit