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]<puppet-dev%[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.
