diff options
| -rw-r--r-- | lib/puppet/resource/catalog.rb | 42 | ||||
| -rw-r--r-- | lib/puppet/transaction.rb | 107 | ||||
| -rw-r--r-- | lib/puppet/type/file.rb | 12 | ||||
| -rwxr-xr-x | lib/puppet/type/tidy.rb | 4 | ||||
| -rwxr-xr-x | spec/integration/indirector/catalog/compiler_spec.rb | 7 | ||||
| -rwxr-xr-x | spec/integration/type/file_spec.rb | 13 | ||||
| -rwxr-xr-x | spec/unit/resource/catalog_spec.rb | 6 | ||||
| -rwxr-xr-x | spec/unit/transaction_spec.rb | 4 |
8 files changed, 89 insertions, 106 deletions
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index a8668d844..fed0f46da 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -61,36 +61,32 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph [$1, $2] end - # Add one or more resources to our graph and to our resource table. + # Add a resource to our graph and to our resource table. # This is actually a relatively complicated method, because it handles multiple # aspects of Catalog behaviour: # * Add the resource to the resource table # * Add the resource to the resource graph # * Add the resource to the relationship graph # * Add any aliases that make sense for the resource (e.g., name != title) - def add_resource(*resources) - resources.each do |resource| - raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref) - end.each { |resource| fail_on_duplicate_type_and_title(resource) }.each do |resource| - title_key = title_key_for_ref(resource.ref) - - @transient_resources << resource if applying? - @resource_table[title_key] = resource - - # If the name and title differ, set up an alias - - if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title - self.alias(resource, resource.uniqueness_key) if resource.isomorphic? - end - - resource.catalog = self if resource.respond_to?(:catalog=) - - add_vertex(resource) - - @relationship_graph.add_vertex(resource) if @relationship_graph - - yield(resource) if block_given? + def add_resource(*resource) + add_resource(*resource[0..-2]) if resource.length > 1 + resource = resource.pop + raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref) + fail_on_duplicate_type_and_title(resource) + title_key = title_key_for_ref(resource.ref) + + @transient_resources << resource if applying? + @resource_table[title_key] = resource + + # If the name and title differ, set up an alias + + if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title + self.alias(resource, resource.uniqueness_key) if resource.isomorphic? end + + resource.catalog = self if resource.respond_to?(:catalog=) + add_vertex(resource) + @relationship_graph.add_vertex(resource) if @relationship_graph end # Create an alias for a resource. diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index eba601cfe..aa0eec3ee 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -12,7 +12,7 @@ class Puppet::Transaction require 'puppet/resource/status' attr_accessor :component, :catalog, :ignoreschedules - attr_accessor :sorted_resources, :configurator + attr_accessor :configurator # The report, once generated. attr_accessor :report @@ -57,32 +57,23 @@ class Puppet::Transaction report.resource_statuses.values.find_all { |status| status.changed }.collect { |status| catalog.resource(status.resource) } end + # Find all of the applied resources (including failed attempts). + def applied_resources + report.resource_statuses.values.collect { |status| catalog.resource(status.resource) } + end + # Copy an important relationships from the parent to the newly-generated # child resource. - def make_parent_child_relationship(resource, children) - depthfirst = resource.depthfirst? - - children.each do |gen_child| - if depthfirst - edge = [gen_child, resource] - else - edge = [resource, gen_child] - end - relationship_graph.add_vertex(gen_child) - - unless relationship_graph.edge?(edge[1], edge[0]) - relationship_graph.add_edge(*edge) - else - resource.debug "Skipping automatic relationship to #{gen_child}" - end + def make_parent_child_relationship(parent, child) + relationship_graph.add_vertex(child) + edge = parent.depthfirst? ? [child, parent] : [parent, child] + if relationship_graph.edge?(*edge.reverse) + parent.debug "Skipping automatic relationship to #{child}" + else + relationship_graph.add_edge(*edge) end end - # See if the resource generates new resources at evaluation time. - def eval_generate(resource) - generate_additional_resources(resource, :eval_generate) - end - # Evaluate a single resource. def eval_resource(resource, ancestor = nil) if skip?(resource) @@ -97,26 +88,7 @@ class Puppet::Transaction def eval_children_and_apply_resource(resource, ancestor = nil) resource_status(resource).scheduled = true - - # We need to generate first regardless, because the recursive - # actions sometimes change how the top resource is applied. - children = eval_generate(resource) - - if ! children.empty? and resource.depthfirst? - children.each do |child| - # The child will never be skipped when the parent isn't - eval_resource(child, ancestor || resource) - end - end - - # Perform the actual changes apply(resource, ancestor) - - if ! children.empty? and ! resource.depthfirst? - children.each do |child| - eval_resource(child, ancestor || resource) - end - end end # This method does all the actual work of running a transaction. It @@ -131,7 +103,7 @@ class Puppet::Transaction Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version begin - @sorted_resources.each do |resource| + visit_resources do |resource| next if stop_processing? if resource.is_a?(Puppet::Type::Component) Puppet.warning "Somehow left a component in the relationship graph" @@ -177,9 +149,31 @@ class Puppet::Transaction found_failed end + def eval_generate(resource) + return [] unless resource.respond_to?(:eval_generate) + begin + made = resource.eval_generate + rescue => detail + puts detail.backtrace if Puppet[:trace] + resource.err "Failed to generate additional resources using 'eval_generate: #{detail}" + end + parents = [resource] + [made].flatten.compact.uniq.each do |res| + begin + res.tag(*resource.tags) + @catalog.add_resource(res) + res.finish + make_parent_child_relationship(parents.reverse.find { |r| r.name == res.name[0,r.name.length]}, res) + parents << res + rescue Puppet::Resource::Catalog::DuplicateResourceError + res.info "Duplicate generated resource; skipping" + end + end + end + # A general method for recursively generating new resources from a # resource. - def generate_additional_resources(resource, method) + def generate_additional_resources(resource, method, presume_prefix_dependencies=false) return [] unless resource.respond_to?(method) begin made = resource.send(method) @@ -192,13 +186,10 @@ class Puppet::Transaction made.uniq.find_all do |res| begin res.tag(*resource.tags) - @catalog.add_resource(res) do |r| - r.finish - make_parent_child_relationship(resource, [r]) - - # Call 'generate' recursively - generate_additional_resources(r, method) - end + @catalog.add_resource(res) + res.finish + make_parent_child_relationship(resource, res) + generate_additional_resources(res, method, presume_prefix_dependencies) true rescue Puppet::Resource::Catalog::DuplicateResourceError res.info "Duplicate generated resource; skipping" @@ -269,9 +260,21 @@ class Puppet::Transaction # Then prefetch. It's important that we generate and then prefetch, # so that any generated resources also get prefetched. prefetch + end - # This will throw an error if there are cycles in the graph. - @sorted_resources = relationship_graph.topsort + def visit_resources(&block) + eval_generated = {} + while r = relationship_graph.topsort.find { |r| !applied_resources.include?(r) } + #p relationship_graph.topsort.collect { |r0| r0.title[/(-0.*)/,1] } + if !eval_generated[r] + #p [:generate,r.title] + eval_generate(r) + eval_generated[r] = true + else + #p [:apply,r.title] + yield r + end + end end def relationship_graph diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 2c5a95bbc..cbc1f77da 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -462,8 +462,7 @@ Puppet::Type.newtype(:file) do # be used to copy remote files, manage local files, and/or make links # to map to another directory. def recurse - children = {} - children = recurse_local if self[:recurse] != :remote + children = (self[:recurse] == :remote) ? {} : recurse_local if self[:target] recurse_link(children) @@ -500,11 +499,7 @@ Puppet::Type.newtype(:file) do # A simple method for determining whether we should be recursing. def recurse? - return false unless @parameters.include?(:recurse) - - val = @parameters[:recurse].value - - !!(val and (val == true or val == :remote)) + self[:recurse] == true or self[:recurse] == :remote end # Recurse the target of the link. @@ -576,13 +571,10 @@ Puppet::Type.newtype(:file) do end def perform_recursion(path) - Puppet::FileServing::Metadata.indirection.search( - path, :links => self[:links], :recurse => (self[:recurse] == :remote ? true : self[:recurse]), - :recurselimit => self[:recurselimit], :ignore => self[:ignore], :checksum_type => (self[:source] || self[:content]) ? self[:checksum] : :none diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb index d2e9ebe22..1a308e17d 100755 --- a/lib/puppet/type/tidy.rb +++ b/lib/puppet/type/tidy.rb @@ -240,10 +240,6 @@ Puppet::Type.newtype(:tidy) do [] end - def eval_generate - [] - end - def generate return [] unless stat(self[:path]) diff --git a/spec/integration/indirector/catalog/compiler_spec.rb b/spec/integration/indirector/catalog/compiler_spec.rb index 1146c20b0..dafa1af7c 100755 --- a/spec/integration/indirector/catalog/compiler_spec.rb +++ b/spec/integration/indirector/catalog/compiler_spec.rb @@ -10,11 +10,8 @@ describe Puppet::Resource::Catalog::Compiler do before do Facter.stubs(:value).returns "something" @catalog = Puppet::Resource::Catalog.new - - @one = Puppet::Resource.new(:file, "/one") - - @two = Puppet::Resource.new(:file, "/two") - @catalog.add_resource(@one, @two) + @catalog.add_resource(@one = Puppet::Resource.new(:file, "/one")) + @catalog.add_resource(@two = Puppet::Resource.new(:file, "/two")) end after { Puppet.settings.clear } diff --git a/spec/integration/type/file_spec.rb b/spec/integration/type/file_spec.rb index 46900a0f1..513b96e41 100755 --- a/spec/integration/type/file_spec.rb +++ b/spec/integration/type/file_spec.rb @@ -28,7 +28,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo" catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket File.open(file[:path], "w") { |f| f.puts "bar" } @@ -80,7 +81,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => link, :target => dest2, :ensure => :link, :backup => "mybucket" catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket File.open(dest1, "w") { |f| f.puts "whatever" } File.symlink(dest1, link) @@ -113,7 +115,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo", :force => true catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket Dir.mkdir(file[:path]) foofile = File.join(file[:path], "foo") @@ -337,10 +340,10 @@ describe Puppet::Type.type(:file) do it "should have an edge to each resource in the relationship graph" do @catalog.apply do |trans| one = @catalog.resource(:file, File.join(@dest, "one")) - @catalog.relationship_graph.should be_edge(@file, one) + @catalog.relationship_graph.edge?(@file, one).should be two = @catalog.resource(:file, File.join(@dest, "two")) - @catalog.relationship_graph.should be_edge(@file, two) + @catalog.relationship_graph.edge?(@file, two).should be end end end diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb index 42850c23b..411d10b32 100755 --- a/spec/unit/resource/catalog_spec.rb +++ b/spec/unit/resource/catalog_spec.rb @@ -398,12 +398,6 @@ describe Puppet::Resource::Catalog, "when compiling" do relgraph.should be_vertex(@one) end - it "should yield added resources if a block is provided" do - yielded = [] - @catalog.add_resource(@one, @two) { |r| yielded << r } - yielded.length.should == 2 - end - it "should set itself as the resource's catalog if it is not a relationship graph" do @one.expects(:catalog=).with(@catalog) @catalog.add_resource @one diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb index 3677bfd87..5867f2e83 100755 --- a/spec/unit/transaction_spec.rb +++ b/spec/unit/transaction_spec.rb @@ -244,6 +244,8 @@ describe Puppet::Transaction do @catalog.stubs(:add_resource) child.expects(:tag).with("one", "two") + child.expects(:finish) + generator.expects(:depthfirst?) @transaction.generate_additional_resources(generator, :generate) end @@ -390,7 +392,7 @@ describe Puppet::Transaction do @resource = stub 'resource', :ref => 'some_ref' @catalog.add_resource @resource @transaction.stubs(:prepare) - @transaction.sorted_resources = [@resource] + @transaction.stubs(:visit_resources).yields(@resource) end it 'should stop processing if :stop_processing? is true' do |