Brice,
I think the "real" fix for the underlying problem is fixing the
regression bug introduced in the file serving refactor. That said,
if this hot patch works around the 100% CPU usage on recursive
chowns bug in the user list thread, I think it's desirable to get
this into the 0.25.x branch ASAP.
+1
Rein Henrichs
http://reductivelabs.com
On Sat, Mar 13, 2010 at 5:59 AM, Brice Figureau <[email protected]
> wrote:
File checksum is "md5" by default. When managing local files (not
sourced
or content) it might be desirable to not checksum files, especially
when managing deep hierarchies containing many files.
This patch allows to write such manifests:
file {
"/path/to/deep/hierarchy":
owner => brice, recurse => true, checksum => none
}
Then puppet(d) won't checksum those files, just manage their
ownership.
Signed-off-by: Brice Figureau <[email protected]>
---
lib/puppet/file_serving/fileset.rb | 4 ++--
lib/puppet/file_serving/terminus_helper.rb | 1 +
lib/puppet/type/file.rb | 9 ++++++++-
lib/puppet/type/file/checksum.rb | 3 ++-
lib/puppet/util/checksums.rb | 5 +++++
spec/unit/file_serving/fileset.rb | 6 ++++++
spec/unit/file_serving/terminus_helper.rb | 10 ++++++++++
spec/unit/type/file.rb | 7 +++++++
spec/unit/type/file/checksum.rb | 28 ++++++++++++++++++
++++++++++
spec/unit/util/checksums.rb | 8 +++++++-
10 files changed, 76 insertions(+), 5 deletions(-)
create mode 100644 spec/unit/type/file/checksum.rb
diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/
file_serving/fileset.rb
index 50e4e1e..a66d935 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, :recurselimit
+ attr_accessor :recurse, :recurselimit, :checksum_type
# Produce a hash of files, with merged so that earlier files
# with the same postfix win. E.g., /dir1/subfile beats /dir2/
subfile.
@@ -105,7 +105,7 @@ class Puppet::FileServing::Fileset
end
def initialize_from_request(request)
- [:links, :ignore, :recurse, :recurselimit].each do |param|
+
[:links, :ignore, :recurse, :recurselimit, :checksum_type].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)
diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/
file_serving/terminus_helper.rb
index c88bacc..6f5d52b 100644
--- a/lib/puppet/file_serving/terminus_helper.rb
+++ b/lib/puppet/file_serving/terminus_helper.rb
@@ -16,6 +16,7 @@ module Puppet::FileServing::TerminusHelper
Puppet::FileServing::Fileset.merge(*filesets).collect do |
file, base_path|
inst = model.new(base_path, :relative_path => file)
+ inst.checksum_type = request.options[:checksum_type] if
request.options[:checksum_type]
inst.links = request.options[:links] if
request.options[:links]
inst.collect
inst
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 2f5b5df..fdb5a35 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -592,7 +592,14 @@ module Puppet
end
def perform_recursion(path)
- Puppet::FileServing::Metadata.search(path, :links =>
self[:links], :recurse => (self[:recurse] == :remote ? true :
self[:recurse]), :recurselimit => self[:recurselimit], :ignore =>
self[:ignore])
+ params = {
+ :links => self[:links],
+ :recurse => (self[:recurse] == :remote ? true :
self[:recurse]),
+ :recurselimit => self[:recurselimit],
+ :ignore => self[:ignore]
+ }
+ params[:checksum_type] = self[:checksum] if
self[:checksum] == :none
+ Puppet::FileServing::Metadata.search(path, params)
end
# Remove any existing data. This is only used when dealing
with
diff --git a/lib/puppet/type/file/checksum.rb b/lib/puppet/type/file/
checksum.rb
index 23a3e5a..0c45aad 100755
--- a/lib/puppet/type/file/checksum.rb
+++ b/lib/puppet/type/file/checksum.rb
@@ -23,7 +23,7 @@ Puppet::Type.type(:file).newproperty(:checksum) do
@unmanaged = true
- @validtypes = %w{md5 md5lite timestamp mtime time}
+ @validtypes = %w{md5 md5lite timestamp mtime time none}
def self.validtype?(type)
@validtypes.include?(type)
@@ -52,6 +52,7 @@ Puppet::Type.type(:file).newproperty(:checksum) do
cache(type, sum)
return type
else
+ return :none if value.nil? or value.to_s == "" or
value.to_s == "none"
if FileTest.directory?(@resource[:path])
return :time
elsif @resource[:source] and value.to_s != "md5"
diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/
checksums.rb
index 98bf5de..39477ee 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -68,6 +68,11 @@ module Puppet::Util::Checksums
File.stat(filename).send(:ctime)
end
+ # Return a "no checksum"
+ def none_file(filename)
+ ""
+ end
+
private
# Perform an incremental checksum on a file.
diff --git a/spec/unit/file_serving/fileset.rb b/spec/unit/
file_serving/fileset.rb
index 55cc2a9..c03522d 100755
--- a/spec/unit/file_serving/fileset.rb
+++ b/spec/unit/file_serving/fileset.rb
@@ -42,6 +42,12 @@ describe Puppet::FileServing::Fileset, " when
initializing" do
set.links.should == :manage
end
+ it "should accept a 'checksum_type' option" do
+ File.expects(:lstat).with("/some/file").returns stub("stat")
+ set = Puppet::FileServing::Fileset.new("/some/
file", :checksum_type => :test)
+ set.checksum_type.should == :test
+ end
+
it "should fail if 'links' is set to anything other than :manage
or :follow" do
proc { Puppet::FileServing::Fileset.new("/some/file", :links
=> :whatever) }.should raise_error(ArgumentError)
end
diff --git a/spec/unit/file_serving/terminus_helper.rb b/spec/unit/
file_serving/terminus_helper.rb
index d0c06f1..98c64b7 100755
--- a/spec/unit/file_serving/terminus_helper.rb
+++ b/spec/unit/file_serving/terminus_helper.rb
@@ -76,6 +76,16 @@ describe Puppet::FileServing::TerminusHelper do
@helper.path2instances(@request, "/my/file")
end
+ it "should set the request checksum_type if one is
provided" do
+ @one.expects(:checksum_type=).with :test
+ @two.expects(:checksum_type=).with :test
+ @model.expects(:new).returns(@one)
+ @model.expects(:new).returns(@two)
+
+ @request.options[:checksum_type] = :test
+ @helper.path2instances(@request, "/my/file")
+ end
+
it "should collect the instance's attributes" do
@one.expects(:collect)
@two.expects(:collect)
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 1b3fe6a..b5963a6 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -320,6 +320,13 @@ describe Puppet::Type.type(:file) do
@file.expects(:newchild).with("my/file").returns "fiebar"
@file.recurse_local.should == {"my/file" => "fiebar"}
end
+
+ it "should set checksum_type to none if this file checksum
is none" do
+ @file[:checksum] = :none
+ Puppet::FileServing::Metadata.expects(:search).with { |
path,params| params[:checksum_type] == :none }.returns [...@metadata]
+ @file.expects(:newchild).with("my/file").returns "fiebar"
+ @file.recurse_local
+ end
end
it "should have a method for performing link recursion" do
diff --git a/spec/unit/type/file/checksum.rb b/spec/unit/type/file/
checksum.rb
new file mode 100644
index 0000000..5d715d1
--- /dev/null
+++ b/spec/unit/type/file/checksum.rb
@@ -0,0 +1,28 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?
(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/
spec_helper.rb") }
+
+checksum = Puppet::Type.type(:file).attrclass(:checksum)
+describe checksum do
+ before do
+ # Wow that's a messy interface to the resource.
+ @resource = stub 'resource', :[] => nil, :[]= =>
nil, :property => nil, :newattr => nil, :parameter => nil
+ end
+
+ it "should be a subclass of Property" do
+ checksum.superclass.must == Puppet::Property
+ end
+
+ it "should have default checksum of :md5" do
+ @checksum = checksum.new(:resource => @resource)
+ @checksum.checktype.should == :md5
+ end
+
+ [:none, nil, ""].each do |ck|
+ it "should use a none checksum for #{ck.inspect}" do
+ @checksum = checksum.new(:resource => @resource)
+ @checksum.should = "none"
+ @checksum.checktype.should == :none
+ end
+ end
+end
diff --git a/spec/unit/util/checksums.rb b/spec/unit/util/checksums.rb
index d31d7a0..615ed90 100755
--- a/spec/unit/util/checksums.rb
+++ b/spec/unit/util/checksums.rb
@@ -14,7 +14,7 @@ describe Puppet::Util::Checksums do
end
content_sums = [:md5, :md5lite, :sha1, :sha1lite]
- file_only = [:ctime, :mtime]
+ file_only = [:ctime, :mtime, :none]
content_sums.each do |sumtype|
it "should be able to calculate %s sums from strings" %
sumtype do
@@ -104,4 +104,10 @@ describe Puppet::Util::Checksums do
end
end
end
+
+ describe "when using the none checksum" do
+ it "should return an empty string" do
+ @summer.none_file("/my/file").should == ""
+ end
+ end
end
--
1.6.6.1
--
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
.
--
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
.
