Fix #1469 - Add an option to recurse only on remote side

When using recurse and a source, if the client side has many files
it can take a lot of CPU/memory to checksum the whole client
hierarchy. The idea is that it is not necessary to recurse on the
client side if all we want is to manage the files that are sourced
from the server.

This changeset adds the "remote" recurse value which prevents recursing
on the client side when a source is present. Since it also is necessary
to limit the remote side recursion a new File{} parameter has been
added called "recurselimit".

Moreover, the Filetset API is changing to allow the new recurselimit
parameter, and passing the recursion depth limit in the recurse parameter
as an integer is now deprecated and not supported anymore.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

2 files changed, 56 insertions, 22 deletions

diff --git a/spec/unit/file_serving/fileset.rb b/spec/unit/file_serving/fileset.rb
index dfba9c1a1..6269d345a 100755
--- a/spec/unit/file_serving/fileset.rb
+++ b/spec/unit/file_serving/fileset.rb
@@ -24,6 +24,12 @@ describe Puppet::FileServing::Fileset, " when initializing" do
         set.recurse.should be_true
     end
 
+    it "should accept a 'recurselimit' option" do
+        File.expects(:lstat).with("/some/file").returns stub("stat")
+        set = Puppet::FileServing::Fileset.new("/some/file", :recurselimit => 3)
+        set.recurselimit.should == 3
+    end
+
     it "should accept an 'ignore' option" do
         File.expects(:lstat).with("/some/file").returns stub("stat")
         set = Puppet::FileServing::Fileset.new("/some/file", :ignore => ".svn")
@@ -45,6 +51,11 @@ describe Puppet::FileServing::Fileset, " when initializing" do
         Puppet::FileServing::Fileset.new("/some/file").recurse.should == false
     end
 
+    it "should default to 0 (infinite) for recurselimit" do
+        File.expects(:lstat).with("/some/file").returns stub("stat")
+        Puppet::FileServing::Fileset.new("/some/file").recurselimit.should == 0
+    end
+
     it "should default to an empty ignore list" do
         File.expects(:lstat).with("/some/file").returns stub("stat")
         Puppet::FileServing::Fileset.new("/some/file").ignore.should == []
@@ -64,22 +75,27 @@ describe Puppet::FileServing::Fileset, " when initializing" do
     describe "using an indirector request" do
         before do
             File.stubs(:lstat).returns stub("stat")
-            @values = {:links => :manage, :ignore => %w{a b}, :recurse => true}
+            @values = {:links => :manage, :ignore => %w{a b}, :recurse => true, :recurselimit => 1234}
             @request = Puppet::Indirector::Request.new(:file_serving, :find, "foo")
         end
 
-        [:recurse, :ignore, :links].each do |option|
-            it "should pass :recurse, :ignore, and :links settings on to the fileset if present" do
+        [:recurse, :recurselimit, :ignore, :links].each do |option|
+            it "should pass :recurse, :recurselimit, :ignore, and :links settings on to the fileset if present" do
                 @request.stubs(:options).returns(option => @values[option])
                 Puppet::FileServing::Fileset.new("/my/file", @request).send(option).should == @values[option]
             end
 
-            it "should pass :recurse, :ignore, and :links settings on to the fileset if present with the keys stored as strings" do
+            it "should pass :recurse, :recurselimit, :ignore, and :links settings on to the fileset if present with the keys stored as strings" do
                 @request.stubs(:options).returns(option.to_s => @values[option])
                 Puppet::FileServing::Fileset.new("/my/file", @request).send(option).should == @values[option]
             end
         end
 
+        it "should convert the integer as a string to their integer counterpart when setting options" do
+            @request.stubs(:options).returns(:recurselimit => "1234")
+            Puppet::FileServing::Fileset.new("/my/file", @request).recurselimit.should == 1234
+        end
+
         it "should convert the string 'true' to the boolean true when setting options" do
             @request.stubs(:options).returns(:recurse => "true")
             Puppet::FileServing::Fileset.new("/my/file", @request).recurse.should == true
@@ -99,8 +115,9 @@ describe Puppet::FileServing::Fileset, " when determining whether to recurse" do
         @fileset = Puppet::FileServing::Fileset.new(@path)
     end
 
-    it "should always recurse if :recurse is set to 'true'" do
+    it "should always recurse if :recurse is set to 'true' and with infinite recursion" do
         @fileset.recurse = true
+        @fileset.recurselimit = 0
         @fileset.recurse?(0).should be_true
     end
 
@@ -109,25 +126,23 @@ describe Puppet::FileServing::Fileset, " when determining whether to recurse" do
         @fileset.recurse?(-1).should be_false
     end
 
-    it "should recurse if :recurse is set to an integer and the current depth is less than that integer" do
-        @fileset.recurse = 1
+    it "should recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is less than that integer" do
+        @fileset.recurse = true
+        @fileset.recurselimit = 1
         @fileset.recurse?(0).should be_true
     end
 
-    it "should recurse if :recurse is set to an integer and the current depth is equal to that integer" do
-        @fileset.recurse = 1
+    it "should recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is equal to that integer" do
+        @fileset.recurse = true
+        @fileset.recurselimit = 1
         @fileset.recurse?(1).should be_true
     end
 
-    it "should not recurse if :recurse is set to an integer and the current depth is greater than that integer" do
-        @fileset.recurse = 1
+    it "should not recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is greater than that integer" do
+        @fileset.recurse = true
+        @fileset.recurselimit = 1
         @fileset.recurse?(2).should be_false
     end
-
-    it "should not recurse if :recurse is set to 0" do
-        @fileset.recurse = 0
-        @fileset.recurse?(-1).should be_false
-    end
 end
 
 describe Puppet::FileServing::Fileset, " when recursing" do
@@ -173,9 +188,10 @@ describe Puppet::FileServing::Fileset, " when recursing" do
 
     # It seems like I should stub :recurse? here, or that I shouldn't stub the
     # examples above, but...
-    it "should recurse to the level set if :recurse is set to an integer" do
+    it "should recurse to the level set if :recurselimit is set to an integer" do
         mock_dir_structure(@path)
-        @fileset.recurse = 1
+        @fileset.recurse = true
+        @fileset.recurselimit = 1
         @fileset.files.should == %w{. one two .svn CVS}
     end
 
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 2be388f8e..0187cc41d 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -71,7 +71,7 @@ describe Puppet::Type.type(:file) do
     end
 
     describe "when validating attributes" do
-        %w{path backup recurse source replace force ignore links purge sourceselect}.each do |attr|
+        %w{path backup recurse recurselimit source replace force ignore links purge sourceselect}.each do |attr|
             it "should have a '#{attr}' parameter" do
                 Puppet::Type.type(:file).attrtype(attr.intern).should == :param
             end
@@ -233,8 +233,20 @@ describe Puppet::Type.type(:file) do
         end
 
         it "should pass its recursion value to the search" do
-            @file[:recurse] = 10
-            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == 10 }
+            @file[:recurse] = true
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true }
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should pass true if recursion is remote" do
+            @file[:recurse] = :remote
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true }
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should pass its recursion limit value to the search" do
+            @file[:recurselimit] = 10
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurselimit] == 10 }
             @file.perform_recursion(@file[:path])
         end
 
@@ -575,11 +587,17 @@ describe Puppet::Type.type(:file) do
             end
         end
 
-        it "should use recurse_local" do
+        it "should use recurse_local if recurse is not remote" do
             @file.expects(:recurse_local).returns({})
             @file.recurse
         end
 
+        it "should not use recurse_local if recurse remote" do
+            @file[:recurse] = :remote
+            @file.expects(:recurse_local).never
+            @file.recurse
+        end
+
         it "should return the generated resources as an array sorted by file path" do
             one = stub 'one', :[] => "/one"
             two = stub 'two', :[] => "/one/two"