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".
Signed-off-by: Brice Figureau <brice-pup...@daysofwonder.com>
---
lib/puppet/file_serving/fileset.rb | 9 ++++--
lib/puppet/type/file.rb | 38 ++++++++++++++++++++++---
spec/unit/file_serving/fileset.rb | 52 ++++++++++++++++++++++++++++++++---
spec/unit/type/file.rb | 22 ++++++++++++++-
test/ral/type/file.rb | 2 +-
5 files changed, 107 insertions(+), 16 deletions(-)
diff --git a/lib/puppet/file_serving/fileset.rb
b/lib/puppet/file_serving/fileset.rb
index 61ca897..6fd2f5e 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
if options.is_a?(Puppet::Indirector::Request)
initialize_from_request(options)
@@ -88,12 +89,13 @@ class Puppet::FileServing::Fileset
# 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 true if self.recurse == true and self.recurselimit == 0
# 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.recurselimit and depth <= self.recurselimit
return true if self.recurse.is_a?(Fixnum) and depth <= self.recurse
# Else, return false.
@@ -112,7 +114,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 +123,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 7b2de7a..18c7d69 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.
@@ -125,6 +125,7 @@ module Puppet
case newval
when :true, :inf: true
when :false: false
+ when :remote: :remote
when Integer, Fixnum, Bignum: value
when /^\d+$/: Integer(value)
else
@@ -133,6 +134,22 @@ module Puppet
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 +264,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[:recurse].is_a?(Fixnum) and self[:recurselimit]
+ self.fail "You cannot specify a number in recurse and
recurselimit at the same time"
+ end
+
+ if !self[:source] and self[:recurse] == :remote
+ self.fail "You cannot specify a remote recurse without a
source"
+ end
end
def self.[](path)
@@ -479,7 +504,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].each do |param|
+ [:parent, :ensure, :recurse, :recurselimit, :target].each do
|param|
options.delete(param) if options.include?(param)
end
@@ -517,7 +542,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 +562,7 @@ module Puppet
val = @parameters[:recurse].value
- if val and (val == true or val > 0)
+ if val and (val == true or val == :remote or (val.is_a?(Fixnum)
and val > 0))
return true
else
return false
@@ -624,7 +652,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.
diff --git a/spec/unit/file_serving/fileset.rb
b/spec/unit/file_serving/fileset.rb
index dfba9c1..ea0cda9 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 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 recurselimit 0"
do
@fileset.recurse = true
+ @fileset.recurselimit = 0
@fileset.recurse?(0).should be_true
end
@@ -124,6 +141,24 @@ describe Puppet::FileServing::Fileset, " when determining
whether to recurse" do
@fileset.recurse?(2).should be_false
end
+ 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 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 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
@@ -179,6 +214,13 @@ describe Puppet::FileServing::Fileset, " when recursing" do
@fileset.files.should == %w{. one two .svn CVS}
end
+ it "should recurse to the level set if :recurselimit is set to an integer"
do
+ mock_dir_structure(@path)
+ @fileset.recurse = true
+ @fileset.recurselimit = 1
+ @fileset.files.should == %w{. one two .svn CVS}
+ end
+
it "should ignore the '.' and '..' directories in subdirectories" do
mock_dir_structure(@path)
@fileset.recurse = true
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 1bd049b..8e1366f 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
@@ -238,6 +238,18 @@ describe Puppet::Type.type(:file) do
@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
+
it "should configure the search to ignore or manage links" do
@file[:links] = :manage
Puppet::FileServing::Metadata.expects(:search).with { |key,
options| options[:links] == :manage }
@@ -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"
diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb
index ec81c61..fd91ca9 100755
--- a/test/ral/type/file.rb
+++ b/test/ral/type/file.rb
@@ -392,7 +392,7 @@ class TestFile < Test::Unit::TestCase
# Make sure we default to false
assert(! file.recurse?, "Recurse defaulted to true")
- [true, "true", 10, "inf"].each do |value|
+ [true, "true", 10, "inf", "remote"].each do |value|
file[:recurse] = value
assert(file.recurse?, "%s did not cause recursion" % value)
end
--
1.6.0.2
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
