[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, master, updated. debian/0.24.5-2-109-g5a96442

Micah Anderson micah at riseup.net
Sun Nov 30 00:48:21 UTC 2008


The following commit has been merged in the master branch:
commit fd83fbd99033889b2e3cf197770a600b51162b19
Author: Luke Kanies <luke at madstop.com>
Date:   Sat Nov 15 02:16:57 2008 -0600

    Completely refactoring the tidy type.
    
    This was necessary because of how I changed file recursion.
    
    The type works much more intelligently now -- files to
    be removed have a file resource generated for them, and that
    resource handles deletion.
    
    Also fixes #1717; neither age nor size is required now.
    
    Signed-off-by: Luke Kanies <luke at madstop.com>

diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index 2ceaf4d..f0425d2 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -1,12 +1,17 @@
-# We need to keep the :file type as the parent type until
-# we refactor backups.
-Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
+Puppet::Type.newtype(:tidy) do
+    require 'puppet/file_serving/fileset'
+
     @doc = "Remove unwanted files based on specific criteria.  Multiple
         criteria are OR'd together, so a file that is too large but is not
         old enough will still get tidied.
-        
-        You must specify either the size or age of the file (or both) for
-        files to be tidied."
+
+        If you don't specify either 'age' or 'size', then all files will
+        be removed.
+
+        This resource type works by generating a file resource for every file
+        that should be deleted and then letting that resource perform the
+        actual deletion.
+        "
 
     newparam(:path) do
         desc "The path to the file or directory to manage.  Must be fully
@@ -18,119 +23,36 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
         desc "One or more file glob patterns, which restrict the list of
             files to be tidied to those whose basenames match at least one
             of the patterns specified.  Multiple patterns can be specified
-            using an array."
-    end
-
-    copyparam(Puppet::Type.type(:file), :backup)
-    
-    newproperty(:ensure) do
-        desc "An internal attribute used to determine which files should be removed."
-
-        @nodoc = true
-        
-        TATTRS = [:age, :size]
-        
-        defaultto :anything # just so we always get this property
-
-        def change_to_s(currentvalue, newvalue)
-            start = "Tidying"
-            if @out.include?(:age)
-                start += ", older than %s seconds" % @resource.should(:age)
-            end
-            if @out.include?(:size)
-                start += ", larger than %s bytes" % @resource.should(:size)
-            end
-
-            start
-        end
-
-        def insync?(is)
-            if is.is_a?(Symbol)
-                if [:absent, :notidy].include?(is)
-                    return true
-                else
-                    return false
-                end
-            else
-                @out = []
-                if @resource[:matches]
-                    basename = File.basename(@resource[:path])
-                    flags = File::FNM_DOTMATCH | File::FNM_PATHNAME
-                    unless @resource[:matches].any? {|pattern| File.fnmatch(pattern, basename, flags) }
-                        self.debug "No patterns specified match basename, skipping"
-                        return true
-                    end
-                end
-                TATTRS.each do |param|
-                    if property = @resource.property(param)
-                        self.debug "No is value for %s", [param] if is[property].nil?
-                        unless property.insync?(is[property])
-                            @out << param
-                        end
-                    end
-                end
-                
-                if @out.length > 0
-                    return false
-                else
-                    return true
-                end
-            end
-        end
-        
-        def retrieve
-            stat = nil
-            unless stat = @resource.stat
-                return { self => :absent}
-            end
+            using an array.
             
-            if stat.ftype == "directory" and ! @resource[:rmdirs]
-                return {self => :notidy}
-            end
+            Note that the patterns are matched against the
+            basename of each file -- that is, your glob patterns should not
+            have any '/' characters in them, since you're only specifying
+            against the last bit of the file."
 
-            allprops = TATTRS.inject({}) { |prophash, param|
-                if property = @resource.property(param)
-                    prophash[property] = property.assess(stat)
-                end
-                prophash
-            }
-            return { self => allprops } 
+        # Make sure we convert to an array.
+        munge do |value|
+            value = [value] unless value.is_a?(Array)
+            value
         end
 
-        def sync
-            file = @resource[:path]
-            case File.lstat(file).ftype
-            when "directory":
-                if @resource[:rmdirs]
-                    subs = Dir.entries(@resource[:path]).reject { |d|
-                        d == "." or d == ".."
-                    }.length
-                    if subs > 0
-                        self.info "%s has %s children; not tidying" %
-                            [@resource[:path], subs]
-                        self.info Dir.entries(@resource[:path]).inspect
-                    else
-                        Dir.rmdir(@resource[:path])
-                    end
-                else
-                    self.debug "Not tidying directories"
-                    return nil
-                end
-            when "file":
-                @resource.handlebackup(file)
-                File.unlink(file)
-            when "link":
-                File.unlink(file)
-            else
-                self.fail "Cannot tidy files of type %s" %
-                    File.lstat(file).ftype
-            end
-
-            return :file_tidied
+        # Does a given path match our glob patterns, if any?  Return true
+        # if no patterns have been provided.
+        def tidy?(path, stat)
+            basename = File.basename(path)
+            flags = File::FNM_DOTMATCH | File::FNM_PATHNAME
+            return true if value.find {|pattern| File.fnmatch(pattern, basename, flags) }
+            return false
         end
     end
 
-    newproperty(:age) do
+    newparam(:backup) do
+        desc "Whether tidied files should be backed up.  Any values are passed
+            directly to the file resources used for actual file deletion, so use
+            its backup documentation to determine valid values."
+    end
+
+    newparam(:age) do
         desc "Tidy files whose age is equal to or greater than
             the specified time.  You can choose seconds, minutes,
             hours, days, or weeks by specifying the first letter of any
@@ -145,17 +67,6 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
         @@ageconvertors[:d] = @@ageconvertors[:h] * 24
         @@ageconvertors[:w] = @@ageconvertors[:d] * 7
 
-        def assess(stat)
-            type = nil
-            if stat.ftype == "directory"
-                type = :mtime
-            else
-                type = @resource[:type] || :atime
-            end
-            
-            return stat.send(type).to_i
-        end
-
         def convert(unit, multi)
             if num = @@ageconvertors[unit]
                 return num * multi
@@ -164,12 +75,13 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
             end
         end
 
-        def insync?(is)
-            if (Time.now.to_i - is) > self.should
+        def tidy?(path, stat)
+            # If the file's older than we allow, we should get rid of it.
+            if (Time.now.to_i - stat.send(resource[:type]).to_i) > value
+                return true 
+            else
                 return false
             end
-
-            true
         end
 
         munge do |age|
@@ -189,7 +101,7 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
         end
     end
 
-    newproperty(:size) do
+    newparam(:size) do
         desc "Tidy files whose size is equal to or greater than
             the specified size.  Unqualified values are in kilobytes, but
             *b*, *k*, and *m* can be appended to specify *bytes*, *kilobytes*,
@@ -203,11 +115,6 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
             :g => 3
         }
 
-        # Retrieve the size from a File::Stat object
-        def assess(stat)
-            return stat.size
-        end
-
         def convert(unit, multi)
             if num = @@sizeconvertors[unit]
                 result = multi
@@ -218,12 +125,12 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
             end
         end
         
-        def insync?(is)
-            if is > self.should
+        def tidy?(path, stat)
+            if stat.size > value
+                return true
+            else
                 return false
             end
-
-            true
         end
         
         munge do |size|
@@ -272,11 +179,13 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
         end
     end
 
-    newparam(:rmdirs) do
+    newparam(:rmdirs, :boolean => true) do
         desc "Tidy directories in addition to files; that is, remove
             directories whose age is older than the specified criteria.
             This will only remove empty directories, so all contained
             files must also be tidied before a directory gets removed."
+
+        newvalues :true, :false
     end
     
     # Erase PFile's validate method
@@ -292,20 +201,18 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
     def initialize(hash)
         super
 
-        unless  @parameters.include?(:age) or
-                @parameters.include?(:size)
-            unless FileTest.directory?(self[:path])
-                # don't do size comparisons for directories
-                self.fail "Tidy must specify size, age, or both"
-            end
-        end
-
         # only allow backing up into filebuckets
         unless self[:backup].is_a? Puppet::Network::Client.dipper
             self[:backup] = false
         end
     end
     
+    # Make a file resource to remove a given file.
+    def mkfile(path)
+        # Force deletion, so directories actually get deleted.
+        Puppet::Type.type(:file).create :path => path, :backup => self[:backup], :ensure => :absent, :force => true
+    end
+
     def retrieve
         # Our ensure property knows how to retrieve everything for us.
         if obj = @parameters[:ensure] 
@@ -319,4 +226,90 @@ Puppet::Type.newtype(:tidy, :parent => Puppet.type(:file)) do
     def properties
         []
     end
+
+    def eval_generate
+        []
+    end
+
+    def generate
+        return [] unless stat(self[:path])
+
+        if self[:recurse]
+            files = Puppet::FileServing::Fileset.new(self[:path], :recurse => self[:recurse]).files.collect do |f|
+                f == "." ? self[:path] : File.join(self[:path], f)
+            end
+        else
+            files = [self[:path]]
+        end
+        result = files.find_all { |path| tidy?(path) }.collect { |path| mkfile(path) }.each { |file| notice "Tidying %s" % file.ref }.sort { |a,b| b[:path] <=> a[:path] }
+
+        # No need to worry about relationships if we don't have rmdirs; there won't be
+        # any directories.
+        return result unless rmdirs?
+
+        # Now make sure that all directories require the files they contain, if all are available,
+        # so that a directory is emptied before we try to remove it.
+        files_by_name = result.inject({}) { |hash, file| hash[file[:path]] = file; hash }
+
+        files_by_name.keys.sort { |a,b| b <=> b }.each do |path|
+            dir = File.dirname(path)
+            next unless resource = files_by_name[dir]
+            if resource[:require] 
+                resource[:require] << [:file, path]
+            else
+                resource[:require] = [[:file, path]]
+            end
+        end
+
+        return result
+    end
+
+    # Does a given path match our glob patterns, if any?  Return true
+    # if no patterns have been provided.
+    def matches?(path)
+        return true unless self[:matches]
+
+        basename = File.basename(path)
+        flags = File::FNM_DOTMATCH | File::FNM_PATHNAME
+        if self[:matches].find {|pattern| File.fnmatch(pattern, basename, flags) }
+            return true
+        else
+            debug "No specified patterns match %s, not tidying" % path
+            return false
+        end
+    end
+
+    # Should we remove the specified file?
+    def tidy?(path)
+        return false unless stat = self.stat(path)
+
+        return false if stat.ftype == "directory" and ! rmdirs?
+
+        # The 'matches' parameter isn't OR'ed with the other tests --
+        # it's just used to reduce the list of files we can match.
+        return false if param = parameter(:matches) and ! param.tidy?(path, stat)
+
+        tested = false
+        [:age, :size].each do |name|
+            next unless param = parameter(name)
+            tested = true
+            return true if param.tidy?(path, stat)
+        end
+
+        # If they don't specify either, then the file should always be removed.
+        return true unless tested
+        return false
+    end
+
+    def stat(path)
+        begin
+            File.lstat(path)
+        rescue Errno::ENOENT => error
+            info "File does not exist"
+            return nil
+        rescue Errno::EACCES => error
+            warning "Could not stat; permission denied"
+            return nil
+        end
+    end
 end
diff --git a/spec/integration/type/tidy.rb b/spec/integration/type/tidy.rb
new file mode 100755
index 0000000..2c83e7a
--- /dev/null
+++ b/spec/integration/type/tidy.rb
@@ -0,0 +1,29 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+describe Puppet::Type.type(:tidy) do
+    def tmpfile(name)
+        source = Tempfile.new(name)
+        source.close!
+        source.path
+    end
+
+    # Testing #355.
+    it "should be able to remove dead links" do
+        dir = tmpfile("tidy_link_testing")
+        link = File.join(dir, "link")
+        target = tmpfile("no_such_file_tidy_link_testing")
+        Dir.mkdir(dir)
+        File.symlink(target, link)
+        
+        tidy = Puppet::Type.type(:tidy).create :path => dir, :recurse => true
+
+        catalog = Puppet::Node::Catalog.new
+        catalog.add_resource(tidy)
+
+        catalog.apply
+
+        FileTest.should_not be_symlink(link)
+    end
+end
diff --git a/spec/unit/type/tidy.rb b/spec/unit/type/tidy.rb
deleted file mode 100755
index ee820d4..0000000
--- a/spec/unit/type/tidy.rb
+++ /dev/null
@@ -1,60 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../spec_helper'
-
-tidy = Puppet::Type.type(:tidy)
-
-describe tidy do
-    after { tidy.clear }
-    [:ensure, :age, :size].each do |property|
-        it "should have a %s property" % property do
-            tidy.attrclass(property).ancestors.should be_include(Puppet::Property)
-        end
-
-        it "should have documentation for its %s property" % property do
-            tidy.attrclass(property).doc.should be_instance_of(String)
-        end
-    end
-
-    [:path, :matches, :type, :recurse, :rmdirs].each do |param|
-        it "should have a %s parameter" % param do
-            tidy.attrclass(param).ancestors.should be_include(Puppet::Parameter)
-        end
-
-        it "should have documentation for its %s param" % param do
-            tidy.attrclass(param).doc.should be_instance_of(String)
-        end
-    end
-
-    describe "when validating parameter values" do
-        describe "for 'recurse'" do
-            before do
-                @tidy = tidy.create :path => "/tmp", :age => "100d"
-            end
-
-            it "should allow 'true'" do
-                lambda { @tidy[:recurse] = true }.should_not raise_error
-            end
-
-            it "should allow 'false'" do
-                lambda { @tidy[:recurse] = false }.should_not raise_error
-            end
-
-            it "should allow integers" do
-                lambda { @tidy[:recurse] = 10 }.should_not raise_error
-            end
-
-            it "should allow string representations of integers" do
-                lambda { @tidy[:recurse] = "10" }.should_not raise_error
-            end
-
-            it "should allow 'inf'" do
-                lambda { @tidy[:recurse] = "inf" }.should_not raise_error
-            end
-
-            it "should not allow arbitrary values" do
-                lambda { @tidy[:recurse] = "whatever" }.should raise_error
-            end
-        end
-    end
-end
diff --git a/test/ral/type/tidy.rb b/test/ral/type/tidy.rb
deleted file mode 100755
index 657ca6e..0000000
--- a/test/ral/type/tidy.rb
+++ /dev/null
@@ -1,291 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../lib/puppettest'
-
-require 'puppettest'
-require 'puppettest/support/utils'
-
-class TestTidy < Test::Unit::TestCase
-    include PuppetTest::Support::Utils
-    include PuppetTest::FileTesting
-    def mktmpfile
-        # because luke's home directory is on nfs, it can't be used for testing
-        # as root
-        tmpfile = tempfile()
-        File.open(tmpfile, "w") { |f| f.puts rand(100) }
-        @@tmpfiles.push tmpfile
-        return tmpfile
-    end
-
-    def mktmpdir
-        dir = File.join(tmpdir(), "puppetlinkdir")
-        unless FileTest.exists?(dir)
-            Dir.mkdir(dir)
-        end
-        @@tmpfiles.push dir
-        return dir
-    end
-
-    def test_tidydirs
-        dir = mktmpdir
-        file = File.join(dir, "file")
-        File.open(file, "w") { |f|
-            f.puts "some stuff"
-        }
-
-        tidy = Puppet.type(:tidy).create(
-            :name => dir,
-            :size => "1b",
-            :rmdirs => true,
-            :recurse => true
-        )
-
-        assert_events([:file_tidied, :file_tidied], tidy)
-
-        assert(!FileTest.exists?(file), "Tidied %s still exists" % file)
-        assert(!FileTest.exists?(dir), "Tidied %s still exists" % dir)
-
-    end
-
-    def disabled_test_recursion
-        source = mktmpdir()
-        FileUtils.cd(source) {
-            mkranddirsandfiles()
-        }
-
-        link = nil
-        assert_nothing_raised {
-            link = newlink(:target => source, :recurse => true)
-        }
-        comp = mk_catalog("linktest",link)
-        cycle(comp)
-
-        path = link.name
-        list = file_list(path)
-        FileUtils.cd(path) {
-            list.each { |file|
-                unless FileTest.directory?(file)
-                    assert(FileTest.symlink?(file))
-                    target = File.readlink(file)
-                    assert_equal(target,File.join(source,file.sub(/^\.\//,'')))
-                end
-            }
-        }
-    end
-
-    # Test the different age iterations.
-    def test_age_conversions
-        tidy = Puppet::Type.type(:tidy).create :path => tempfile(), :age => "1m"
-
-        convertors = {
-            :second => 1,
-            :minute => 60
-        }
-
-        convertors[:hour] = convertors[:minute] * 60
-        convertors[:day] = convertors[:hour] * 24
-        convertors[:week] = convertors[:day] * 7
-
-        # First make sure we default to days
-        assert_nothing_raised do
-            tidy[:age] = "2"
-        end
-
-        assert_equal(2 * convertors[:day], tidy.should(:age),
-            "Converted 2 wrong")
-
-        convertors.each do |name, number|
-            init = name.to_s[0..0] # The first letter
-            [0, 1, 5].each do |multi|
-                [init, init.upcase].each do |letter|
-                    age = multi.to_s + letter.to_s
-                    assert_nothing_raised do
-                        tidy[:age] = age
-                    end
-
-                    assert_equal(multi * convertors[name], tidy.should(:age),
-                        "Converted %s wrong" % age)
-                end
-            end
-        end
-    end
-
-    def test_size_conversions
-        convertors = {
-            :b => 0,
-            :kb => 1,
-            :mb => 2,
-            :gb => 3
-        }
-
-        tidy = Puppet::Type.type(:tidy).create :path => tempfile(), :age => "1m"
-
-        # First make sure we default to kb
-        assert_nothing_raised do
-            tidy[:size] = "2"
-        end
-
-        assert_equal(2048, tidy.should(:size),
-            "Converted 2 wrong")
-
-        convertors.each do |name, number|
-            init = name.to_s[0..0] # The first letter
-            [0, 1, 5].each do |multi|
-                [init, init.upcase].each do |letter|
-                    size = multi.to_s + letter.to_s
-                    assert_nothing_raised do
-                        tidy[:size] = size
-                    end
-
-                    total = multi
-                    number.times do total *= 1024 end
-
-                    assert_equal(total, tidy.should(:size),
-                        "Converted %s wrong" % size)
-                end
-            end
-        end
-    end
-
-    def test_agetest
-        tidy = Puppet::Type.type(:tidy).create :path => tempfile(), :age => "1m"
-
-        age = tidy.property(:age)
-
-        # Set it to something that should be fine
-        assert(age.insync?(Time.now.to_i - 5), "Tried to tidy a low age")
-
-        # Now to something that should fail
-        assert(! age.insync?(Time.now.to_i - 120), "Incorrectly skipped tidy")
-    end
-
-    def test_sizetest
-        tidy = Puppet::Type.type(:tidy).create :path => tempfile(), :size => "1k"
-
-        size = tidy.property(:size)
-
-        # Set it to something that should be fine
-        assert(size.insync?(50), "Tried to tidy a low size")
-
-        # Now to something that should fail
-        assert(! size.insync?(2048), "Incorrectly skipped tidy")
-    end
-
-    # Make sure we can remove different types of files
-    def test_tidytypes
-        path = tempfile()
-        tidy = Puppet::Type.type(:tidy).create :path => path, :size => "1b", :age => "1s"
-
-        # Start with a file
-        File.open(path, "w") { |f| f.puts "this is a test" }
-        assert_events([:file_tidied], tidy)
-        assert(! FileTest.exists?(path), "File was not removed")
-
-        # Then a link
-        dest = tempfile
-        File.open(dest, "w") { |f| f.puts "this is a test" }
-        File.symlink(dest, path)
-        assert_events([:file_tidied], tidy)
-        assert(! FileTest.exists?(path), "Link was not removed")
-        assert(FileTest.exists?(dest), "Destination was removed")
-
-        # And a directory
-        Dir.mkdir(path)
-        tidy[:rmdirs] = true
-        assert_events([:file_tidied], tidy)
-        assert(! FileTest.exists?(path), "File was not removed")
-    end
-    
-    # Make sure we can specify either attribute and get appropriate behaviour.
-    # I found that the original implementation of this did not work unless both
-    # size and age were specified.
-    def test_one_attribute
-        path = tempfile()
-        File.open(path, "w") { |f| 10.times { f.puts "yayness " } }
-        tidy = Puppet::Type.type(:tidy).create :path => path, :size => "1b"
-        
-        assert_apply(tidy)
-        assert(! FileTest.exists?(path), "file did not get tidied")
-        
-        tidy.class.clear
-
-        # Now try one with just an age attribute.
-        time = Time.now - 10
-        stat = stub 'stat', :mtime => time, :atime => time, :ftype => "file"
-        File.stubs(:lstat)
-        File.stubs(:lstat).with(path).returns(stat)
-
-        File.open(path, "w") { |f| 10.times { f.puts "yayness " } }
-        tidy = Puppet::Type.type(:tidy).create :path => path, :age => "5s"
-        
-
-        assert_apply(tidy)
-        assert(! FileTest.exists?(path), "file did not get tidied")
-    end
-    
-    # Testing #355.
-    def test_remove_dead_links
-        dir = tempfile()
-        link = File.join(dir, "link")
-        target = tempfile()
-        Dir.mkdir(dir)
-        File.symlink(target, link)
-        
-        tidy = Puppet::Type.type(:tidy).create :path => dir, :size => "1b", :recurse => true
-        assert_apply(tidy)
-        assert(! FileTest.symlink?(link), "link was not tidied")
-    end
-
-    def test_glob_matches_single
-        dir = mktmpdir
-        files = {
-          :remove => File.join(dir, "01-foo"),
-          :keep   => File.join(dir, "default")
-        }
-        files.each do |tag, file|
-          File.open(file, "w") { |f|
-              f.puts "some stuff"
-          }
-        end
-
-        tidy = Puppet.type(:tidy).create(
-            :name => dir,
-            :size => "1b",
-            :rmdirs => true,
-            :recurse => true,
-            :matches => "01-*"
-        )
-        assert_apply(tidy)
-
-        assert(FileTest.exists?(files[:keep]), "%s was tidied" % files[:keep])
-        assert(!FileTest.exists?(files[:remove]), "Tidied %s still exists" % files[:remove])
-    end
-
-    def test_glob_matches_multiple
-        dir = mktmpdir
-        files = {
-          :remove1 => File.join(dir, "01-foo"),
-          :remove2 => File.join(dir, "02-bar"),
-          :keep1   => File.join(dir, "default")
-        }
-        files.each do |tag, file|
-          File.open(file, "w") { |f|
-              f.puts "some stuff"
-          }
-        end
-
-        tidy = Puppet.type(:tidy).create(
-            :name => dir,
-            :size => "1b",
-            :rmdirs => true,
-            :recurse => true,
-            :matches => ["01-*", "02-*"]
-        )
-        assert_apply(tidy)
-
-        assert(FileTest.exists?(files[:keep1]), "%s was tidied" % files[:keep1])
-        assert(!FileTest.exists?(files[:remove1]), "Tidied %s still exists" % files[:remove1])
-        assert(!FileTest.exists?(files[:remove2]), "Tidied %s still exists" % files[:remove2])
-    end
-end
-

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list