On Jul 4, 2009, at 9:48 AM, Brice Figureau wrote: > > The issue is that when we convert Puppet::Parser::Resource catalog > to a Puppet::Resource catalog before storing it to the database, > we don't allow virtual resource to be converted. > Unfortunately exported resources are virtual by design, and as > such aren't converted, and we lose them, so it isn't possible > to store them in the database. > Unfortunately, the client will get the exported resources too. > So we filter out them when this catalog is converted to the RAL > catalog, client-side.
It feels kind of like we're peeling an onion here, like exported resources and the Indirector model are incompatible. It's even worse with the queueing thrown in, because we could otherwise filter out at serialization (although that would only work with client/server). In other words, this is a bit ugly, but no uglier than the system already was. The only way I can see better than this, and that's questionable, is to have the whole system support virtual resources, and have the transaction skip them. The client's already getting those exported/ virtual resources, so you might as well keep them through the whole chain, and then at least the model's clean and data's not getting destroyed at weird, arbitrary times. If we decide not to go that route, though, +1 to this patch. > > Signed-off-by: Brice Figureau <[email protected]> > --- > lib/puppet/resource/catalog.rb | 11 +++++++- > spec/unit/resource/catalog.rb | 57 +++++++++++++++++++++++++++++++ > +++++++++ > 2 files changed, 67 insertions(+), 1 deletions(-) > > diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/ > catalog.rb > index 42e92f4..9f3ecea 100644 > --- a/lib/puppet/resource/catalog.rb > +++ b/lib/puppet/resource/catalog.rb > @@ -534,7 +534,16 @@ class Puppet::Resource::Catalog < > Puppet::SimpleGraph > > map = {} > vertices.each do |resource| > - next if resource.respond_to?(:virtual?) and > resource.virtual? > + > + # when converting to resource we want to keep the > exported resource > + # to be able to save/cache them for storeconfigs > + if convert == :to_resource > + next if resource.respond_to?(:virtual?) and > resource.virtual? and (resource.respond_to?(:exported?) and not > resource.exported?) > + else > + # but in ral mode, there is no point keeping > exported resources > + next if resource.respond_to?(:virtual?) and > resource.virtual? > + next if resource.respond_to?(:exported?) and > resource.exported? > + end > > #This is hackity hack for 1094 > #Aliases aren't working in the ral catalog because the > current instance of the resource > diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/ > catalog.rb > index 2f4476a..ae9988b 100755 > --- a/spec/unit/resource/catalog.rb > +++ b/spec/unit/resource/catalog.rb > @@ -219,6 +219,53 @@ describe Puppet::Resource::Catalog, "when > compiling" do > end > end > > + describe " when converting a Puppet::Parser::Resource catalog > to a Puppet::Resource catalog" do > + before do > + @original = Puppet::Resource::Catalog.new("mynode") > + @original.tag(*%w{one two three}) > + @original.add_class *%w{four five six} > + > + @res1 = stub_everything 'res1', :ref => "File[/a]" > + @res1.stubs(:is_a?).with(Puppet::Resource).returns true > + @res2 = stub_everything 'res2', :ref => "File[/b]" > + @res2.stubs(:is_a?).with(Puppet::Resource).returns true > + > + @pres1 = stub_everything 'pres1', :ref => "File[/ > a]", :to_resource => @res1, :virtual? => false, :exported? => false > + > @pres1.stubs(:is_a?).with(Puppet::Parser::Resource).returns true > + @pres2 = stub_everything 'pres2', :ref => "File[/ > b]", :to_resource => @res2, :virtual? => false, :exported? => false > + > @pres2.stubs(:is_a?).with(Puppet::Parser::Resource).returns true > + > + @resources = [...@pres1, @pres2] > + > + @original.add_resource(*...@resources) > + > + end > + > + it "should add all resources as Puppet::Resource instances" > do > + @catalog = @original.to_resource > + @resources.each { |resource| > @catalog.resource(resource.ref).should_not be_nil } > + end > + > + it "should not add virtual resources as Puppet::Resource > instances" do > + @pres2.stubs(:virtual?).returns(true) > + @pres2.stubs(:exported?).returns(false) > + > + @catalog = @original.to_resource > + > + @catalog.resource(@pres2.ref).should be_nil > + end > + > + it "should add exported resources as Puppet::Resource > instances" do > + @pres2.stubs(:virtual?).returns(true) > + @pres2.stubs(:exported?).returns(true) > + > + @catalog = @original.to_resource > + > + @catalog.resource(@res2.ref).should_not be_nil > + end > + end > + > + > describe "when converting to a RAL catalog" do > before do > @original = Puppet::Resource::Catalog.new("mynode") > @@ -267,6 +314,16 @@ describe Puppet::Resource::Catalog, "when > compiling" do > @catalog.vertices.each { |v| v.catalog.object_id.should > equal(@catalog.object_id) } > end > > + it "should filter out exported resources" do > + @exported = Puppet::Resource.new :file, '/exported' > + @exported.exported = true > + > + @original.add_resource(@exported) > + > + catalog = @original.to_ral > + catalog.resource(@exported.ref).should be_nil > + end > + > # This tests #931. > it "should not lose track of resources whose names vary" do > changer = Puppet::TransObject.new 'changer', 'test' > -- > 1.6.0.2 > > > > -- The secret of being a bore is to tell everything. -- Voltaire --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
