From 33d3624c91b236e237fe194d9df3d9fa324111c3 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Tue, 24 Feb 2009 18:48:52 +0100 Subject: 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 --- lib/puppet/file_serving/fileset.rb | 20 +++++---------- lib/puppet/type/file.rb | 52 +++++++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 22 deletions(-) (limited to 'lib') 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. -- cgit