#2507 contains two issues: * a crash when we filters-out an unwanted resource which had edges pointing to it.
* resources are losing their virtuality when they are transformed from Puppet::Parser::Resource to Puppet::Resource. This means we weren't able to distinguish anymore between an exported resource collected in the same node as it was exported and an exported resource collected in another node. The net result is that we can't apply exported resources that are collected in the same node because they are filtered out by the catalog filter (see the commits for #2391 for more information). The fix is to keep the virtuality of the resources so that we can differentiate those two types of exported resources. We keep this until the catalog is ready to be sent, where we filter out the virtual resouces only, the other still exported ones needs to be sent to the client. To be real sure, the transaction also skips virtual resources. Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/indirector/catalog/compiler.rb | 2 +- lib/puppet/parser/resource.rb | 1 + lib/puppet/resource.rb | 8 +++++--- lib/puppet/resource/catalog.rb | 7 +++++-- lib/puppet/transaction.rb | 4 ++-- lib/puppet/type.rb | 11 ++++++++--- spec/integration/indirector/catalog/compiler.rb | 2 +- spec/unit/indirector/catalog/compiler.rb | 4 ++-- spec/unit/parser/resource.rb | 5 +++++ spec/unit/resource/catalog.rb | 11 +++++++++-- spec/unit/transaction.rb | 4 ++-- spec/unit/type.rb | 6 +++++- 12 files changed, 46 insertions(+), 19 deletions(-) diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb index 12da4de..4f6b060 100644 --- a/lib/puppet/indirector/catalog/compiler.rb +++ b/lib/puppet/indirector/catalog/compiler.rb @@ -43,7 +43,7 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code # filter-out a catalog to remove exported resources def filter(catalog) - return catalog.filter { |r| r.exported? } if catalog.respond_to?(:filter) + return catalog.filter { |r| r.virtual? } if catalog.respond_to?(:filter) catalog end diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index 6632d2b..7218ac0 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -268,6 +268,7 @@ class Puppet::Parser::Resource result.file = self.file result.line = self.line result.exported = self.exported + result.virtual = self.virtual result.tag(*self.tags) return result diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb index 0de089c..1340765 100644 --- a/lib/puppet/resource.rb +++ b/lib/puppet/resource.rb @@ -9,7 +9,7 @@ class Puppet::Resource include Puppet::Util::Tagging extend Puppet::Util::Json include Enumerable - attr_accessor :file, :line, :catalog, :exported + attr_accessor :file, :line, :catalog, :exported, :virtual attr_writer :type, :title ATTRIBUTES = [:file, :line, :exported] @@ -106,8 +106,10 @@ class Puppet::Resource @parameters.each { |p,v| yield p, v } end - def exported? - exported + %w{exported virtual}.each do |m| + define_method(m+"?") do + self.send(m) + end end # Create our resource. diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index bb82906..6ccfe73 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -576,8 +576,11 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph message = convert.to_s.gsub "_", " " edges.each do |edge| # Skip edges between virtual resources. - next if edge.source.respond_to?(:virtual?) and edge.source.virtual? - next if edge.target.respond_to?(:virtual?) and edge.target.virtual? + next if virtual_not_exported?(edge.source) + next if block_given? and yield edge.source + + next if virtual_not_exported?(edge.target) + next if block_given? and yield edge.target unless source = map[edge.source.ref] raise Puppet::DevError, "Could not find resource %s when converting %s resources" % [edge.source.ref, message] diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index f57eda6..d04856d 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -590,8 +590,8 @@ class Transaction resource.debug "Not scheduled" elsif failed_dependencies?(resource) resource.warning "Skipping because of failed dependencies" - elsif resource.exported? - resource.debug "Skipping because exported" + elsif resource.virtual? + resource.debug "Skipping because virtual" else return false end diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 33b4e92..ee87c26 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -1855,6 +1855,9 @@ class Type # is the resource exported attr_accessor :exported + # is the resource virtual (it should not :-)) + attr_accessor :virtual + # create a log at specified level def log(msg) Puppet::Util::Log.create( @@ -1888,7 +1891,7 @@ class Type self.title = resource.ref end - [:file, :line, :catalog, :exported].each do |getter| + [:file, :line, :catalog, :exported, :virtual].each do |getter| setter = getter.to_s + "=" if val = resource.send(getter) self.send(setter, val) @@ -2069,8 +2072,10 @@ class Type return trans end - def exported? - exported + %w{exported virtual}.each do |m| + define_method(m+"?") do + self.send(m) + end end end # Puppet::Type diff --git a/spec/integration/indirector/catalog/compiler.rb b/spec/integration/indirector/catalog/compiler.rb index f3ace8d..211b7c2 100755 --- a/spec/integration/indirector/catalog/compiler.rb +++ b/spec/integration/indirector/catalog/compiler.rb @@ -11,7 +11,7 @@ describe Puppet::Resource::Catalog::Compiler do @catalog = Puppet::Resource::Catalog.new @one = Puppet::Resource.new(:file, "/one") - @one.exported = true + @one.virtual = true @two = Puppet::Resource.new(:file, "/two") @catalog.add_resource(@one, @two) diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb index 78a8028..7f09422 100755 --- a/spec/unit/indirector/catalog/compiler.rb +++ b/spec/unit/indirector/catalog/compiler.rb @@ -236,8 +236,8 @@ describe Puppet::Resource::Catalog::Compiler do @compiler.filter(@catalog) end - it "should filter out exported resources" do - resource = mock 'resource', :exported? => true + it "should filter out virtual resources" do + resource = mock 'resource', :virtual? => true @catalog.stubs(:filter).yields(resource) @compiler.filter(@catalog) diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb index 8005e20..9eb8b13 100755 --- a/spec/unit/parser/resource.rb +++ b/spec/unit/parser/resource.rb @@ -419,6 +419,11 @@ describe Puppet::Parser::Resource do @parser_resource.to_resource.exported.should be_true end + it "should copy over the 'virtual' value" do + @parser_resource.virtual = true + @parser_resource.to_resource.virtual.should be_true + end + it "should convert any parser resource references to Puppet::Resource::Reference instances" do ref = Puppet::Parser::Resource::Reference.new(:title => "/my/file", :type => "file") @parser_resource = mkresource :source => @source, :params => {:foo => "bar", :fee => ref} diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/catalog.rb index 97b6ad7..af399aa 100755 --- a/spec/unit/resource/catalog.rb +++ b/spec/unit/resource/catalog.rb @@ -323,9 +323,9 @@ describe Puppet::Resource::Catalog, "when compiling" do end it "should scan each catalog resource in turn and apply filtering block" do - @resources.each { |r| r.expects(:exported?) } + @resources.each { |r| r.expects(:test?) } @original.filter do |r| - r.exported? + r.test? end end @@ -334,6 +334,13 @@ describe Puppet::Resource::Catalog, "when compiling" do r == @r1 end.resource("File[/a]").should be_nil end + + it "should not consider edges against resources that were filtered out" do + @original.add_edge(@r1,@r2) + @original.filter do |r| + r == @r1 + end.edge(@r1,@r2).should be_empty + end end describe "when functioning as a resource container" do diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb index 0e36747..7966c7a 100755 --- a/spec/unit/transaction.rb +++ b/spec/unit/transaction.rb @@ -75,8 +75,8 @@ describe Puppet::Transaction do @transaction.skip?(@resource).should be_true end - it "should skip exported resource" do - @resource.stubs(:exported?).returns true + it "should skip virtual resource" do + @resource.stubs(:virtual?).returns true @transaction.skip?(@resource).should be_true end end diff --git a/spec/unit/type.rb b/spec/unit/type.rb index b179677..fe2788e 100755 --- a/spec/unit/type.rb +++ b/spec/unit/type.rb @@ -72,6 +72,10 @@ describe Puppet::Type do Puppet::Type.type(:mount).new(:name => "foo").should respond_to(:exported?) end + it "should have a method to know if the resource is virtual" do + Puppet::Type.type(:mount).new(:name => "foo").should respond_to(:virtual?) + end + it "should consider its version to be its catalog version" do resource = Puppet::Type.type(:mount).new(:name => "foo") catalog = Puppet::Resource::Catalog.new @@ -121,7 +125,7 @@ describe Puppet::Type do Puppet::Type.type(:mount).new(resource).title.should == "User[foo]" end - [:line, :file, :catalog, :exported].each do |param| + [:line, :file, :catalog, :exported, :virtual].each do |param| it "should copy '#{param}' from the resource if present" do resource = Puppet::Resource.new(:mount, "/foo") resource.send(param.to_s + "=", "foo") -- 1.6.0.2 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
