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, 50 insertions, 22 deletions

diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/file_serving/fileset.rb
index 61ca897e1..2076d0c4d 100644
--- a/lib/puppet/file_serving/fileset.rb
+++ b/lib/puppet/file_serving/fileset.rb
@@ -9,7 +9,7 @@ require 'puppet/file_serving/metadata'
 # Operate recursively on a path, returning a set of file paths.
 class Puppet::FileServing::Fileset
     attr_reader :path, :ignore, :links
-    attr_accessor :recurse
+    attr_accessor :recurse, :recurselimit
 
     # Produce a hash of files, with merged so that earlier files
     # with the same postfix win.  E.g., /dir1/subfile beats /dir2/subfile.
@@ -67,6 +67,7 @@ class Puppet::FileServing::Fileset
         @ignore = []
         @links = :manage
         @recurse = false
+        @recurselimit = 0 # infinite recursion
 
         if options.is_a?(Puppet::Indirector::Request)
             initialize_from_request(options)
@@ -75,6 +76,7 @@ class Puppet::FileServing::Fileset
         end
 
         raise ArgumentError.new("Fileset paths must exist") unless stat = stat(path)
+        raise ArgumentError.new("Fileset recurse parameter must not be a number anymore, please use recurselimit") if @recurse.is_a?(Integer)
     end
 
     def links=(links)
@@ -87,17 +89,8 @@ class Puppet::FileServing::Fileset
     # Should we recurse further?  This is basically a single
     # place for all of the logic around recursion.
     def recurse?(depth)
-        # If recurse is true, just return true
-        return true if self.recurse == true
-
-        # Return false if the value is false or zero.
-        return false if [false, 0].include?(self.recurse)
-
-        # Return true if our current depth is less than the allowed recursion depth.
-        return true if self.recurse.is_a?(Fixnum) and depth <= self.recurse
-
-        # Else, return false.
-        return false
+        # recurse if told to, and infinite recursion or current depth not at the limit
+        self.recurse and (self.recurselimit == 0 or depth <= self.recurselimit)
     end
 
     def initialize_from_hash(options)
@@ -112,7 +105,7 @@ class Puppet::FileServing::Fileset
     end
 
     def initialize_from_request(request)
-        [:links, :ignore, :recurse].each do |param|
+        [:links, :ignore, :recurse, :recurselimit].each do |param|
             if request.options.include?(param) # use 'include?' so the values can be false
                 value = request.options[param]
             elsif request.options.include?(param.to_s)
@@ -121,6 +114,7 @@ class Puppet::FileServing::Fileset
             next if value.nil? 
             value = true if value == "true"
             value = false if value == "false"
+            value = Integer(value) if value.is_a?(String) and value =~ /^\d+$/
             send(param.to_s + "=", value)
         end
     end
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 7d37fe865..05a4b37f9 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -115,7 +115,7 @@ module Puppet
             desc "Whether and how deeply to do recursive
                 management."
 
-            newvalues(:true, :false, :inf, /^[0-9]+$/)
+            newvalues(:true, :false, :inf, :remote, /^[0-9]+$/)
 
             # Replace the validation so that we allow numbers in
             # addition to string representations of them.
@@ -123,16 +123,39 @@ module Puppet
             munge do |value|
                 newval = super(value)
                 case newval
-                when :true, :inf; true
-                when :false; false
-                when Integer, Fixnum, Bignum; value
-                when /^\d+$/; Integer(value)
+                when :true, :inf: true
+                when :false: false
+                when :remote: :remote
+                when Integer, Fixnum, Bignum:
+                    self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit"
+                    resource[:recurselimit] = value
+                    true
+                when /^\d+$/:
+                    self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit"
+                    resource[:recurselimit] = Integer(value)
+                    true
                 else
                     raise ArgumentError, "Invalid recurse value %s" % value.inspect
                 end
             end
         end
 
+        newparam(:recurselimit) do
+            desc "How deeply to do recursive management."
+
+            newvalues(/^[0-9]+$/)
+
+            munge do |value|
+                newval = super(value)
+                case newval
+                when Integer, Fixnum, Bignum: value
+                when /^\d+$/: Integer(value)
+                else
+                    raise ArgumentError, "Invalid recurselimit value %s" % value.inspect
+                end
+            end
+        end
+
         newparam(:replace, :boolean => true) do
             desc "Whether or not to replace a file that is
                 sourced but exists.  This is useful for using file sources
@@ -247,6 +270,14 @@ module Puppet
             if count > 1
                 self.fail "You cannot specify more than one of %s" % CREATORS.collect { |p| p.to_s}.join(", ")
             end
+
+            if !self[:source] and self[:recurse] == :remote
+                self.fail "You cannot specify a remote recursion without a source"
+            end
+
+            if !self[:recurse] and self[:recurselimit]
+                self.warning "Possible error: recurselimit is set but not recurse, no recursion will happen"
+            end
         end
         
         def self.[](path)
@@ -479,7 +510,7 @@ module Puppet
             options = @original_parameters.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? }
 
             # These should never be passed to our children.
-            [:parent, :ensure, :recurse, :target, :alias, :source].each do |param|
+            [:parent, :ensure, :recurse, :recurselimit, :target, :alias, :source].each do |param|
                 options.delete(param) if options.include?(param)
             end
 
@@ -517,7 +548,10 @@ module Puppet
         # be used to copy remote files, manage local files, and/or make links
         # to map to another directory.
         def recurse
-            children = recurse_local
+            children = {}
+            if self[:recurse] != :remote
+                children = recurse_local
+            end
 
             if self[:target]
                 recurse_link(children)
@@ -534,7 +568,7 @@ module Puppet
 
             val = @parameters[:recurse].value
 
-            if val and (val == true or val > 0)
+            if val and (val == true or val == :remote)
                 return true
             else
                 return false
@@ -624,7 +658,7 @@ module Puppet
         end
 
         def perform_recursion(path)
-            Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore])
+            Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => (self[:recurse] == :remote ? true : self[:recurse]), :recurselimit => self[:recurselimit], :ignore => self[:ignore])
         end
 
         # Remove the old backup.