From: Markus Roberts <[email protected]> Ensure that resources whose refs are included in the catalog are skipped to avoid duplication.
* Refactor to avoid early bailout on resources that cannot be ensured absent. * Remove check for managed? in generate Checking if a resource is managed is unnecessary when checking for its inclusion in the catalog. * Add test coverage for Puppet::Type::Resources#generate Signed-off-by: Rein Henrichs <[email protected]> --- lib/puppet/type/resources.rb | 46 +++++++++++++++--------------- spec/unit/type/resources.rb | 65 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb index ab564a1..e1de557 100644 --- a/lib/puppet/type/resources.rb +++ b/lib/puppet/type/resources.rb @@ -85,33 +85,33 @@ Puppet::Type.newtype(:resources) do end end + def able_to_ensure_absent?(resource) + begin + resource[:ensure] = :absent + rescue ArgumentError, Puppet::Error => detail + err "The 'ensure' attribute on #{self[:name]} resources does not accept 'absent' as a value" + false + end + end + # Generate any new resources we need to manage. This is pretty hackish # right now, because it only supports purging. def generate return [] unless self.purge? - hascheck = false - method = - resource_type.instances.find_all do |resource| - ! resource.managed? - end.find_all do |resource| - check(resource) - end.each do |resource| - begin - resource[:ensure] = :absent - rescue ArgumentError, Puppet::Error => detail - err "The 'ensure' attribute on %s resources does not accept 'absent' as a value" % - [self[:name]] - return [] - end - @parameters.each do |name, param| - next unless param.metaparam? - resource[name] = param.value - end - - # Mark that we're purging, so transactions can handle relationships - # correctly - resource.purging - end + resource_type.instances. + reject { |r| catalog.resources.include? r.ref }. + select { |r| check(r) }. + select { |r| r.class.validproperty?(:ensure) }. + select { |r| able_to_ensure_absent?(r) }. + each { |resource| + @parameters.each do |name, param| + resource[name] = param.value if param.metaparam? + end + + # Mark that we're purging, so transactions can handle relationships + # correctly + resource.purging + } end def resource_type diff --git a/spec/unit/type/resources.rb b/spec/unit/type/resources.rb index 147f21d..a3faede 100644 --- a/spec/unit/type/resources.rb +++ b/spec/unit/type/resources.rb @@ -21,4 +21,69 @@ describe resources do resources.new(:name => "file").resource_type.should == Puppet::Type.type(:file) end end + + describe "#generate" do + before do + @host1 = Puppet::Type.type(:host).new(:name => 'localhost', :ip => '127.0.0.1') + @catalog = Puppet::Resource::Catalog.new + @context = Puppet::Transaction.new(@catalog) + end + + describe "when dealing with non-purging resources" do + before do + @resources = Puppet::Type.type(:resources).new(:name => 'host') + end + + it "should not generate any resource" do + @resources.generate.should be_empty + end + end + + describe "when the catalog contains a purging resource" do + before do + @resources = Puppet::Type.type(:resources).new(:name => 'host', :purge => true) + @purgeable_resource = Puppet::Type.type(:host).new(:name => 'localhost', :ip => '127.0.0.1') + @catalog.add_resource @resources + end + + it "should not generate a duplicate of that resource" do + Puppet::Type.type(:host).stubs(:instances).returns [...@host1] + @catalog.add_resource @host1 + @resources.generate.collect { |r| r.ref }.should_not include(@host1.ref) + end + + + describe "when generating a purgeable resource" do + it "should be included in the generated resources" do + Puppet::Type.type(:host).stubs(:instances).returns [...@purgeable_resource] + @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref) + end + end + + describe "when the instance's do not have an ensure property" do + it "should not be included in the generated resources" do + @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo') + Puppet::Type.type(:host).stubs(:instances).returns [...@no_ensure_resource] + @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref) + end + end + + describe "when the instance's ensure property does not accept absent" do + it "should not be included in the generated resources" do + @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar') + Puppet::Type.type(:host).stubs(:instances).returns [...@no_absent_resource] + @resources.generate.collect { |r| r.ref }.should_not include(@no_absent_resource.ref) + end + end + + describe "when checking the instance fails" do + it "should not be included in the generated resources" do + @purgeable_resource = Puppet::Type.type(:host).new(:name => 'foobar') + Puppet::Type.type(:host).stubs(:instances).returns [...@purgeable_resource] + @resources.expects(:check).with(@purgeable_resource).returns(false) + @resources.generate.collect { |r| r.ref }.should_not include(@purgeable_resource.ref) + end + end + end + end end -- 1.6.4.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 -~----------~----~----~----~------~----~------~--~---
