+1 On Oct 13, 2009, at 10:02 AM, [email protected] wrote:
> > 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 > > > > -- It isn't necessary to have relatives in Kansas City in order to be unhappy. -- Groucho Marx --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
