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. Some new spec tests are added as well. Signed-off-by: Steven Jenkins <[email protected]> --- lib/puppet/type/file.rb | 126 +------------------------------------------- lib/puppet/util/backups.rb | 94 ++++++++++++++++++++++++++++++++ spec/unit/util/backups.rb | 27 +++++++++ 3 files changed, 123 insertions(+), 124 deletions(-) create mode 100644 lib/puppet/util/backups.rb create mode 100755 spec/unit/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..f76a52b --- /dev/null +++ b/lib/puppet/util/backups.rb @@ -0,0 +1,94 @@ +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(fileobj) + file = (fileobj.class == String) ? fileobj : fileobj.name + 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(fileobj, backup) + file = (fileobj.class == String) ? fileobj : fileobj.name + 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 diff --git a/spec/unit/util/backups.rb b/spec/unit/util/backups.rb new file mode 100755 index 0000000..2ec06e5 --- /dev/null +++ b/spec/unit/util/backups.rb @@ -0,0 +1,27 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +require 'puppet/type/file' +require 'puppet/util/backups' + +describe Puppet::Util::Backups do + describe "when backing up a file" do + it "should succeed silently if the file does not exist" do + Puppet::Type::File.new(:name => "/no/such/file").handlebackup + end + it "should succeed silently if self[:backup] is false" do + Puppet::Type::File.new(:name => "/my/file", :backup => false).handlebackup + end + it "should error if no bucket or backup suffix available" do + #lambda { Puppet::Type::File.new(:name => "/my/file", :backup => false, :bucket => false) }. should raise_error(Puppet::Error) + end + + # For the cases where there is an actual bucket or backup, + # cf spec/unit/type/file.rb section starting "when setting the backup" + # as those are the higher-level tests and cover the cases here. + # Also see test/ral/type/filebucket.rb and test/ral/type/file.rb, + # Testing #303, 304 are good examples + end +end + -- 1.6.3.3 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
