From 1020c04c11f2bde3522a78860ff9ae8a97933f35 Mon Sep 17 00:00:00 2001 From: luke Date: Mon, 16 Oct 2006 18:04:46 +0000 Subject: Making all test suites executable, adding some tests for handling changing files from one type to another, and fixing #304. The problem with #304 was only occurring when backing up to a filebucket (I can only think the example code was wrong) git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1786 980ebf18-57e1-0310-9a29-db15c13687c0 --- lib/puppet/type/pfile.rb | 336 ++++++++++++++++++++++------------------ lib/puppet/type/pfile/ensure.rb | 4 + lib/puppet/type/pfile/target.rb | 18 +-- test/client/client.rb | 0 test/client/master.rb | 0 test/client/pelement.rb | 0 test/language/lexer.rb | 0 test/language/node.rb | 0 test/language/parser.rb | 0 test/lib/puppettest.rb | 0 test/other/autoload.rb | 0 test/other/inifile.rb | 0 test/other/log.rb | 0 test/other/metrics.rb | 0 test/other/provider.rb | 0 test/other/transactions.rb | 0 test/providers/nameservice.rb | 0 test/providers/package.rb | 0 test/providers/provider.rb | 0 test/providers/user.rb | 0 test/puppet/tc_suidmanager.rb | 0 test/server/bucket.rb | 0 test/server/ca.rb | 0 test/server/logger.rb | 0 test/server/master.rb | 0 test/server/pelement.rb | 0 test/server/server.rb | 0 test/tagging/tagging.rb | 0 test/types/basic.rb | 0 test/types/file.rb | 121 +++++++++++++++ test/types/fileignoresource.rb | 0 test/types/package.rb | 0 test/types/parameter.rb | 0 test/types/query.rb | 0 test/types/service.rb | 0 test/types/state.rb | 0 test/types/type.rb | 0 test/types/yumrepo.rb | 0 38 files changed, 312 insertions(+), 167 deletions(-) mode change 100644 => 100755 test/client/client.rb mode change 100644 => 100755 test/client/master.rb mode change 100644 => 100755 test/client/pelement.rb mode change 100644 => 100755 test/language/lexer.rb mode change 100644 => 100755 test/language/node.rb mode change 100644 => 100755 test/language/parser.rb mode change 100644 => 100755 test/lib/puppettest.rb mode change 100644 => 100755 test/other/autoload.rb mode change 100644 => 100755 test/other/inifile.rb mode change 100644 => 100755 test/other/log.rb mode change 100644 => 100755 test/other/metrics.rb mode change 100644 => 100755 test/other/provider.rb mode change 100644 => 100755 test/other/transactions.rb mode change 100644 => 100755 test/providers/nameservice.rb mode change 100644 => 100755 test/providers/package.rb mode change 100644 => 100755 test/providers/provider.rb mode change 100644 => 100755 test/providers/user.rb mode change 100644 => 100755 test/puppet/tc_suidmanager.rb mode change 100644 => 100755 test/server/bucket.rb mode change 100644 => 100755 test/server/ca.rb mode change 100644 => 100755 test/server/logger.rb mode change 100644 => 100755 test/server/master.rb mode change 100644 => 100755 test/server/pelement.rb mode change 100644 => 100755 test/server/server.rb mode change 100644 => 100755 test/tagging/tagging.rb mode change 100644 => 100755 test/types/basic.rb mode change 100644 => 100755 test/types/file.rb mode change 100644 => 100755 test/types/fileignoresource.rb mode change 100644 => 100755 test/types/package.rb mode change 100644 => 100755 test/types/parameter.rb mode change 100644 => 100755 test/types/query.rb mode change 100644 => 100755 test/types/service.rb mode change 100644 => 100755 test/types/state.rb mode change 100644 => 100755 test/types/type.rb mode change 100644 => 100755 test/types/yumrepo.rb diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb index 3a07acac7..7f2ff798c 100644 --- a/lib/puppet/type/pfile.rb +++ b/lib/puppet/type/pfile.rb @@ -318,25 +318,12 @@ module Puppet end end - require 'fileutils' - FileUtils.rmtree(self[:path]) return true when String: newfile = file + backup # Just move it, since it's a directory. - if FileTest.directory?(newfile) - raise Puppet::Error, "Will not replace directory backup; use a filebucket" - elsif FileTest.exists?(newfile) - begin - File.unlink(newfile) - rescue => detail - if Puppet[:trace] - puts detail.backtrace - end - self.err "Could not remove old backup: %s" % - detail - return false - end + if FileTest.exists?(newfile) + remove_backup(newfile) end begin bfile = file + backup @@ -367,13 +354,7 @@ module Puppet when String: newfile = file + backup if FileTest.exists?(newfile) - begin - File.unlink(newfile) - rescue => detail - self.err "Could not remove old backup: %s" % - detail - return false - end + remove_backup(newfile) end begin # FIXME Shouldn't this just use a Puppet object with @@ -394,6 +375,7 @@ module Puppet self.err "Invalid backup type %s" % backup.inspect return false end + when "link": return true else self.notice "Cannot backup files of type %s" % File.stat(file).ftype @@ -440,6 +422,108 @@ module Puppet @stat = nil end + + # Build a recursive map of a link source + def linkrecurse(recurse) + target = @states[:target].should + + method = :lstat + if self[:links] == :follow + method = :stat + end + + targetstat = nil + unless FileTest.exist?(target) + #self.info "%s does not exist; not recursing" % + # target + return + end + # Now stat our target + targetstat = File.send(method, target) + unless targetstat.ftype == "directory" + #self.info "%s is not a directory; not recursing" % + # target + return + end + + # Now that we know our corresponding target is a directory, + # change our type + self[:ensure] = :directory + + unless FileTest.readable? target + self.notice "Cannot manage %s: permission denied" % self.name + return + end + + children = Dir.entries(target).reject { |d| d =~ /^\.+$/ } + + #Get rid of ignored children + if @parameters.include?(:ignore) + children = handleignore(children) + end + + added = [] + children.each do |file| + Dir.chdir(target) do + longname = File.join(target, file) + + # Files know to create directories when recursion + # is enabled and we're making links + args = { + :recurse => recurse, + :ensure => longname + } + + if child = self.newchild(file, true, args) + unless @children.include?(child) + self.push child + added.push file + end + end + end + end + end + + # Build up a recursive map of what's around right now + def localrecurse(recurse) + unless FileTest.exist?(self[:path]) and self.stat.directory? + #self.info "%s is not a directory; not recursing" % + # self[:path] + return + end + + unless FileTest.readable? self[:path] + self.notice "Cannot manage %s: permission denied" % self.name + return + end + + children = Dir.entries(self[:path]) + + #Get rid of ignored children + if @parameters.include?(:ignore) + children = handleignore(children) + end + + added = [] + children.each { |file| + file = File.basename(file) + next if file =~ /^\.\.?$/ # skip . and .. + options = {:recurse => recurse} + + if child = self.newchild(file, true, options) + # Mark any unmanaged files for removal if purge is set. + # Use the array rather than [] because tidy uses this method, too. + if @parameters.include?(:purge) and self[:purge] == :true and child.implicit? + child[:ensure] = :absent + end + + unless @children.include?(child) + self.push child + added.push file + end + end + } + end # Create a new file or directory object as a child to the current # object. @@ -624,104 +708,105 @@ module Puppet end end - # Build a recursive map of a link source - def linkrecurse(recurse) - target = @states[:target].should - - method = :lstat - if self[:links] == :follow + # Remove the old backup. + def remove_backup(newfile) + if self.class.name == :file and self[:links] != :follow + method = :lstat + else method = :stat end + old = File.send(method, newfile).ftype - targetstat = nil - unless FileTest.exist?(target) - #self.info "%s does not exist; not recursing" % - # target - return - end - # Now stat our target - targetstat = File.send(method, target) - unless targetstat.ftype == "directory" - #self.info "%s is not a directory; not recursing" % - # target - return + if old == "directory" + raise Puppet::Error, + "Will not remove directory backup %s; use a filebucket" % + newfile end - # Now that we know our corresponding target is a directory, - # change our type - self[:ensure] = :directory + info "Removing old backup of type %s" % + File.send(method, newfile).ftype - unless FileTest.readable? target - self.notice "Cannot manage %s: permission denied" % self.name - return + begin + File.unlink(newfile) + rescue => detail + if Puppet[:trace] + puts detail.backtrace + end + self.err "Could not remove old backup: %s" % + detail + return false end + end - children = Dir.entries(target).reject { |d| d =~ /^\.+$/ } - - #Get rid of ignored children - if @parameters.include?(:ignore) - children = handleignore(children) - end + # Remove any existing data. This is only used when dealing with + # links or directories. + def remove_existing(should) + return unless s = stat(true) - added = [] - children.each do |file| - Dir.chdir(target) do - longname = File.join(target, file) + unless handlebackup + self.fail "Could not back up; will not replace" + end - # Files know to create directories when recursion - # is enabled and we're making links - args = { - :recurse => recurse, - :ensure => longname - } + unless should.to_s == "link" + return if s.ftype.to_s == should.to_s + end - if child = self.newchild(file, true, args) - unless @children.include?(child) - self.push child - added.push file - end - end + case s.ftype + when "directory": + if self[:force] == :true + debug "Removing existing directory for replacement with %s" % + should + FileUtils.rmtree(self[:path]) + else + notice "Not replacing directory; use 'force' to override" end + when "link", "file": + debug "Removing existing %s for replacement with %s" % + [s.ftype, should] + File.unlink(self[:path]) + else + self.fail "Could not back up files of type %s" % s.ftype end end - # Build up a recursive map of what's around right now - def localrecurse(recurse) - unless FileTest.exist?(self[:path]) and self.stat.directory? - #self.info "%s is not a directory; not recursing" % - # self[:path] - return - end + # a wrapper method to make sure the file exists before doing anything + def retrieve + if @states.include?(:source) + # This probably isn't the best place for it, but we need + # to make sure that we have a corresponding checksum state. + unless @states.include?(:checksum) + self[:checksum] = "md5" + end - unless FileTest.readable? self[:path] - self.notice "Cannot manage %s: permission denied" % self.name - return + # We have to retrieve the source info before the recursion happens, + # although I'm not exactly clear on why. + @states[:source].retrieve end - children = Dir.entries(self[:path]) - - #Get rid of ignored children - if @parameters.include?(:ignore) - children = handleignore(children) - end + if @parameters.include?(:recurse) + self.recurse + end - added = [] - children.each { |file| - file = File.basename(file) - next if file =~ /^\.\.?$/ # skip . and .. - options = {:recurse => recurse} + unless stat = self.stat(true) + self.debug "File does not exist" + @states.each { |name,state| + # We've already retrieved the source, and we don't + # want to overwrite whatever it did. This is a bit + # of a hack, but oh well, source is definitely special. + next if name == :source + state.is = :absent + } - if child = self.newchild(file, true, options) - # Mark any unmanaged files for removal if purge is set. - # Use the array rather than [] because tidy uses this method, too. - if @parameters.include?(:purge) and self[:purge] == :true and child.implicit? - child[:ensure] = :absent - end + return + end - unless @children.include?(child) - self.push child - added.push file - end + states().each { |state| + # We don't want to call 'describe()' twice, so only do a local + # retrieve on the source. + if state.name == :source + state.retrieve(false) + else + state.retrieve end } end @@ -778,48 +863,6 @@ module Puppet } end - # a wrapper method to make sure the file exists before doing anything - def retrieve - if @states.include?(:source) - # This probably isn't the best place for it, but we need - # to make sure that we have a corresponding checksum state. - unless @states.include?(:checksum) - self[:checksum] = "md5" - end - - # We have to retrieve the source info before the recursion happens, - # although I'm not exactly clear on why. - @states[:source].retrieve - end - - if @parameters.include?(:recurse) - self.recurse - end - - unless stat = self.stat(true) - self.debug "File does not exist" - @states.each { |name,state| - # We've already retrieved the source, and we don't - # want to overwrite whatever it did. This is a bit - # of a hack, but oh well, source is definitely special. - next if name == :source - state.is = :absent - } - - return - end - - states().each { |state| - # We don't want to call 'describe()' twice, so only do a local - # retrieve on the source. - if state.name == :source - state.retrieve(false) - else - state.retrieve - end - } - end - # Set the checksum, from another state. There are multiple states that # modify the contents of a file, and they need the ability to make sure # that the checksum value is in sync. @@ -930,16 +973,7 @@ module Puppet def write(usetmp = true) mode = self.should(:mode) - #if FileTest.exists?(self[:path]) - if s = stat(false) - # this makes sure we have a copy for posterity - @backed = self.handlebackup - - if s.ftype == "link" and self[:links] != :follow - # Remove existing links, since we're writing out a file - File.unlink(self[:path]) - end - end + remove_existing(:file) # The temporary file path = nil diff --git a/lib/puppet/type/pfile/ensure.rb b/lib/puppet/type/pfile/ensure.rb index 2e48e0165..6f7b15d49 100755 --- a/lib/puppet/type/pfile/ensure.rb +++ b/lib/puppet/type/pfile/ensure.rb @@ -154,6 +154,10 @@ module Puppet end def sync + unless self.should == :absent + @parent.remove_existing(self.should) + end + event = super # There are some cases where all of the work does not get done on diff --git a/lib/puppet/type/pfile/target.rb b/lib/puppet/type/pfile/target.rb index a2d174c2e..38151bfe0 100644 --- a/lib/puppet/type/pfile/target.rb +++ b/lib/puppet/type/pfile/target.rb @@ -27,23 +27,9 @@ module Puppet def mklink target = self.should - if stat = @parent.stat - unless @parent.handlebackup - self.fail "Could not back up; will not replace with link" - end + # Clean up any existing objects. + @parent.remove_existing(target) - case stat.ftype - when "directory": - if @parent[:force] == :true - FileUtils.rmtree(@parent[:path]) - else - notice "Not replacing directory with link; use 'force' to override" - return :nochange - end - else - File.unlink(@parent[:path]) - end - end Dir.chdir(File.dirname(@parent[:path])) do Puppet::SUIDManager.asuser(@parent.asuser()) do mode = @parent.should(:mode) diff --git a/test/client/client.rb b/test/client/client.rb old mode 100644 new mode 100755 diff --git a/test/client/master.rb b/test/client/master.rb old mode 100644 new mode 100755 diff --git a/test/client/pelement.rb b/test/client/pelement.rb old mode 100644 new mode 100755 diff --git a/test/language/lexer.rb b/test/language/lexer.rb old mode 100644 new mode 100755 diff --git a/test/language/node.rb b/test/language/node.rb old mode 100644 new mode 100755 diff --git a/test/language/parser.rb b/test/language/parser.rb old mode 100644 new mode 100755 diff --git a/test/lib/puppettest.rb b/test/lib/puppettest.rb old mode 100644 new mode 100755 diff --git a/test/other/autoload.rb b/test/other/autoload.rb old mode 100644 new mode 100755 diff --git a/test/other/inifile.rb b/test/other/inifile.rb old mode 100644 new mode 100755 diff --git a/test/other/log.rb b/test/other/log.rb old mode 100644 new mode 100755 diff --git a/test/other/metrics.rb b/test/other/metrics.rb old mode 100644 new mode 100755 diff --git a/test/other/provider.rb b/test/other/provider.rb old mode 100644 new mode 100755 diff --git a/test/other/transactions.rb b/test/other/transactions.rb old mode 100644 new mode 100755 diff --git a/test/providers/nameservice.rb b/test/providers/nameservice.rb old mode 100644 new mode 100755 diff --git a/test/providers/package.rb b/test/providers/package.rb old mode 100644 new mode 100755 diff --git a/test/providers/provider.rb b/test/providers/provider.rb old mode 100644 new mode 100755 diff --git a/test/providers/user.rb b/test/providers/user.rb old mode 100644 new mode 100755 diff --git a/test/puppet/tc_suidmanager.rb b/test/puppet/tc_suidmanager.rb old mode 100644 new mode 100755 diff --git a/test/server/bucket.rb b/test/server/bucket.rb old mode 100644 new mode 100755 diff --git a/test/server/ca.rb b/test/server/ca.rb old mode 100644 new mode 100755 diff --git a/test/server/logger.rb b/test/server/logger.rb old mode 100644 new mode 100755 diff --git a/test/server/master.rb b/test/server/master.rb old mode 100644 new mode 100755 diff --git a/test/server/pelement.rb b/test/server/pelement.rb old mode 100644 new mode 100755 diff --git a/test/server/server.rb b/test/server/server.rb old mode 100644 new mode 100755 diff --git a/test/tagging/tagging.rb b/test/tagging/tagging.rb old mode 100644 new mode 100755 diff --git a/test/types/basic.rb b/test/types/basic.rb old mode 100644 new mode 100755 diff --git a/test/types/file.rb b/test/types/file.rb old mode 100644 new mode 100755 index fba13bd12..1b305dfee --- a/test/types/file.rb +++ b/test/types/file.rb @@ -1,3 +1,5 @@ +#!/usr/bin/env ruby -I../lib -I../../lib + require 'puppet' require 'fileutils' require 'puppettest' @@ -1466,6 +1468,125 @@ class TestFile < Test::Unit::TestCase Puppet::Type.type(:file).clear end end + + # Testing #304 + def test_links_to_directories + link = tempfile() + file = tempfile() + dir = tempfile() + Dir.mkdir(dir) + + bucket = Puppet::Type.newfilebucket :name => "main" + File.symlink(dir, link) + File.open(file, "w") { |f| f.puts "" } + assert_equal(dir, File.readlink(link)) + obj = Puppet::Type.newfile :path => link, :ensure => :link, + :target => file, :recurse => false, :backup => "main" + + assert_apply(obj) + + assert_equal(file, File.readlink(link)) + end + + # Testing #303 + def test_nobackups_with_links + link = tempfile() + new = tempfile() + + File.open(link, "w") { |f| f.puts "old" } + File.open(new, "w") { |f| f.puts "new" } + obj = Puppet::Type.newfile :path => link, :ensure => :link, + :target => new, :recurse => true, :backup => false + + assert_nothing_raised do + obj.handlebackup + end + + bfile = [link, "puppet-bak"].join(".") + + assert(! FileTest.exists?(bfile), "Backed up when told not to") + + assert_apply(obj) + + assert(! FileTest.exists?(bfile), "Backed up when told not to") + end + + def test_remove_existing + end + + # Make sure we consistently handle backups for all cases. + def test_ensure_with_backups + # We've got three file types, so make sure we can replace any type + # with the other type and that backups are done correctly. + types = [:file, :directory, :link] + + dir = tempfile() + path = File.join(dir, "test") + linkdest = tempfile() + creators = { + :file => proc { File.open(path, "w") { |f| f.puts "initial" } }, + :directory => proc { Dir.mkdir(path) }, + :link => proc { File.symlink(linkdest, path) } + } + + bucket = Puppet::Type.newfilebucket :name => "main", :path => tempfile() + + obj = Puppet::Type.newfile :path => path, :force => true, + :links => :manage + + Puppet[:trace] = true + ["main", false].each do |backup| + obj[:backup] = backup + obj.finish + types.each do |should| + types.each do |is| + # It makes no sense to replace a directory with a directory + # next if should == :directory and is == :directory + + Dir.mkdir(dir) + + # Make the thing + creators[is].call + + obj[:ensure] = should + + if should == :link + obj[:target] = linkdest + else + if obj.state(:target) + obj.delete(:target) + end + end + + # First try just removing the initial data + assert_nothing_raised do + obj.remove_existing(should) + end + + unless is == should + # Make sure the original is gone + assert(! FileTest.exists?(obj[:path]), + "remove_existing did not work: " + + "did not remove %s with %s" % [is, should]) + end + FileUtils.rmtree(obj[:path]) + + # Now make it again + creators[is].call + + state = obj.state(:ensure) + + state.retrieve + unless state.insync? + assert_nothing_raised do + state.sync + end + end + FileUtils.rmtree(dir) + end + end + end + end end # $Id$ diff --git a/test/types/fileignoresource.rb b/test/types/fileignoresource.rb old mode 100644 new mode 100755 diff --git a/test/types/package.rb b/test/types/package.rb old mode 100644 new mode 100755 diff --git a/test/types/parameter.rb b/test/types/parameter.rb old mode 100644 new mode 100755 diff --git a/test/types/query.rb b/test/types/query.rb old mode 100644 new mode 100755 diff --git a/test/types/service.rb b/test/types/service.rb old mode 100644 new mode 100755 diff --git a/test/types/state.rb b/test/types/state.rb old mode 100644 new mode 100755 diff --git a/test/types/type.rb b/test/types/type.rb old mode 100644 new mode 100755 diff --git a/test/types/yumrepo.rb b/test/types/yumrepo.rb old mode 100644 new mode 100755 -- cgit