Version 2 attached, with all the changes discussed, minus additional tests. If there are no further comments, I'll post version 3 (i.e., the additional tests) shortly and wrap this ticket up.
It would be very helpful if those experiencing 2371 would test this patch (assuming they are in a position to test patches..). Thanks, Steven Jenkins End Point Corporation --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
>From 3be81cd74e5e0014cc7883e50f0c422bc8aa3f48 Mon Sep 17 00:00:00 2001 From: Steven Jenkins <[email protected]> Date: Fri, 24 Jul 2009 12:31:36 -0400 Subject: [PATCH] Draft fixes for Redmine 2371. This changes the condition checking of handlebucket, as well as moves it (and remove_backup) into a separate module. It additionally refactors common code out of handlebucket into separate private methods. All spec tests and unit tests pass. --- lib/puppet/type/file.rb | 126 +------------------------------------------- lib/puppet/util/backups.rb | 92 ++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 124 deletions(-) create mode 100644 lib/puppet/util/backups.rb diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 55d4ec7..657c7ab 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -7,11 +7,13 @@ require 'puppet/network/handler' require 'puppet/util/diff' require 'puppet/util/checksums' require 'puppet/network/client' +require 'puppet/util/backups' module Puppet newtype(:file) do include Puppet::Util::MethodHelper include Puppet::Util::Checksums + include Puppet::Util::Backups @doc = "Manages local files, including setting ownership and permissions, creation of both files and directories, and retrieving entire files from remote servers. As Puppet matures, it @@ -391,103 +393,6 @@ module Puppet @stat = nil end - # Deal with backups. - def handlebackup(file = nil) - # let the path be specified - file ||= self[:path] - # if they specifically don't want a backup, then just say - # we're good - unless FileTest.exists?(file) - return true - end - - unless self[:backup] - return true - end - - case File.stat(file).ftype - when "directory" - if self[:recurse] - # we don't need to backup directories when recurse is on - return true - else - backup = self.bucket || self[:backup] - case backup - when Puppet::Network::Client.client(:Dipper) - notice "Recursively backing up to filebucket" - require 'find' - Find.find(self[:path]) do |f| - if File.file?(f) - sum = backup.backup(f) - self.notice "Filebucketed %s to %s with sum %s" % - [f, backup.name, sum] - end - end - - return true - when String - newfile = file + backup - # Just move it, since it's a directory. - if FileTest.exists?(newfile) - remove_backup(newfile) - end - begin - bfile = file + backup - - # Ruby 1.8.1 requires the 'preserve' addition, but - # later versions do not appear to require it. - FileUtils.cp_r(file, bfile, :preserve => true) - return true - rescue => detail - # since they said they want a backup, let's error out - # if we couldn't make one - self.fail "Could not back %s up: %s" % - [file, detail.message] - end - else - self.err "Invalid backup type %s" % backup.inspect - return false - end - end - when "file" - backup = self.bucket || self[:backup] - case backup - when Puppet::Network::Client.client(:Dipper) - sum = backup.backup(file) - self.notice "Filebucketed to %s with sum %s" % - [backup.name, sum] - return true - when String - newfile = file + backup - if FileTest.exists?(newfile) - remove_backup(newfile) - end - begin - # FIXME Shouldn't this just use a Puppet object with - # 'source' specified? - bfile = file + backup - - # Ruby 1.8.1 requires the 'preserve' addition, but - # later versions do not appear to require it. - FileUtils.cp(file, bfile, :preserve => true) - return true - rescue => detail - # since they said they want a backup, let's error out - # if we couldn't make one - self.fail "Could not back %s up: %s" % - [file, detail.message] - end - else - 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 - return false - end - end - def initialize(hash) # Used for caching clients @clients = {} @@ -668,33 +573,6 @@ module Puppet 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. - 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 - - if old == "directory" - raise Puppet::Error, - "Will not remove directory backup %s; use a filebucket" % - newfile - end - - info "Removing old backup of type %s" % - File.send(method, newfile).ftype - - begin - File.unlink(newfile) - rescue => detail - puts detail.backtrace if Puppet[:trace] - self.err "Could not remove old backup: %s" % detail - return false - end - end - # Remove any existing data. This is only used when dealing with # links or directories. def remove_existing(should) diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb new file mode 100644 index 0000000..9f87e7c --- /dev/null +++ b/lib/puppet/util/backups.rb @@ -0,0 +1,92 @@ +module Puppet::Util::Backups + + # Deal with backups. + def handlebackup(file = nil) + # let the path be specified + file ||= self[:path] + return true unless FileTest.exists?(file) + # if they specifically don't want a backup, then just say + # we're good + return true unless self[:backup] + unless self.bucket || self[:backup] + self.err "Need a bucket or a backup setting for backups" + return false + end + + return handlebucket(file) if self.bucket + return handlebackuplocal(file, self[:backup]) + end + + private + + def handlebucket(file) + case File.stat(file).ftype + when "directory" + # we don't need to backup directories when recurse is on + return true if self[:recurse] + info "Recursively backing up to filebucket" + require 'find' + Find.find(file) do |f| + if File.file?(f) + sum = self.bucket.backup(f) + self.info "Filebucketed %s to %s with sum %s" % + [f, self.bucket.name, sum] + end + end + return true + when "file" + sum = self.bucket.backup(file) + self.info "Filebucketed %s to %s with sum %s" % + [file, self.bucket.name, sum] + return true + when "link"; return true + end + end + + def handlebackuplocal(file, backup) + newfile = file + backup + if FileTest.exists?(newfile) + remove_backup(newfile) + end + begin + bfile = file + backup + + # Ruby 1.8.1 requires the 'preserve' addition, but + # later versions do not appear to require it. + # N.B. cp_r works on both files and directories + FileUtils.cp_r(file, bfile, :preserve => true) + return true + rescue => detail + # since they said they want a backup, let's error out + # if we couldn't make one + self.fail "Could not back %s up: %s" % + [file, detail.message] + end + end + + 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 + + if old == "directory" + raise Puppet::Error, + "Will not remove directory backup %s; use a filebucket" % + newfile + end + + info "Removing old backup of type %s" % + File.send(method, newfile).ftype + + begin + File.unlink(newfile) + rescue => detail + puts detail.backtrace if Puppet[:trace] + self.err "Could not remove old backup: %s" % detail + return false + end + end +end -- 1.6.1.3
