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.
Some new RSpec tests which use mock and stubs are added as well, including removing the old test/ral/type/filebucket.rb tests since they are already covered by RSpec tests. Signed-off-by: Steven Jenkins <[email protected]> --- lib/puppet/type/file.rb | 128 +------------------------------------------ lib/puppet/util/backups.rb | 86 +++++++++++++++++++++++++++++ spec/unit/util/backups.rb | 68 +++++++++++++++++++++++ test/ral/type/filebucket.rb | 110 ------------------------------------- 4 files changed, 157 insertions(+), 235 deletions(-) create mode 100644 lib/puppet/util/backups.rb create mode 100755 spec/unit/util/backups.rb delete mode 100755 test/ral/type/filebucket.rb diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 55d4ec7..ed5147e 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,39 +573,12 @@ 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) return unless s = stat - self.fail "Could not back up; will not replace" unless handlebackup + self.fail "Could not back up; will not replace" unless perform_backup unless should.to_s == "link" return if s.ftype.to_s == should.to_s diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb new file mode 100644 index 0000000..b4bfbd2 --- /dev/null +++ b/lib/puppet/util/backups.rb @@ -0,0 +1,86 @@ +require 'find' +module Puppet::Util::Backups + + # Deal with backups. + def perform_backup(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] + + return perform_backup_with_bucket(file) if self.bucket + return perform_backup_with_backuplocal(file, self[:backup]) + end + + private + + def perform_backup_with_bucket(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" + Find.find(self[:path]) { |f| backup_file_with_filebucket(f) if + File.file?(f) } + when "file"; backup_file_with_filebucket(file) + when "link"; return true + end + end + + def perform_backup_with_backuplocal(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 + + def backup_file_with_filebucket(f) + sum = self.bucket.backup(f) + self.info "Filebucketed %s to %s with sum %s" % [f, self.bucket.name, sum] + return sum + end +end diff --git a/spec/unit/util/backups.rb b/spec/unit/util/backups.rb new file mode 100755 index 0000000..2ddab5e --- /dev/null +++ b/spec/unit/util/backups.rb @@ -0,0 +1,68 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +require 'puppet/type/file' +require 'puppet/util/backups' +include PuppetTest + +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').perform_backup.should be_true + end + it "should succeed silently if self[:backup] is false" do + FileTest.stubs(:exists?).returns true + Puppet::Type::File.new(:name => '/some/file', :backup => false).perform_backup.should be_true + end + it "a bucket should work when provided" do + path = '/my/file' + + FileTest.stubs(:exists?).with(path).returns true + File.stubs(:stat).with(path).returns(mock('stat', :ftype => 'file')) + + bucket = mock('bucket', 'name' => 'foo') + bucket.expects(:backup).with(path) + + file = Puppet::Type::File.new(:name => path, :backup => 'foo') + file.stubs(:bucket).returns bucket + + file.perform_backup.should be_nil + end + it "a local backup should work" do + path = '/my/file' + FileTest.stubs(:exists?).with(path).returns true + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + file.stubs(:perform_backup_with_backuplocal).returns true + file.perform_backup.should be_true + end + end + describe "when backing up a directory" do + it "a bucket should work when provided" do + path = '/my/dir' + + FileTest.stubs(:exists?).with(path).returns true + File.stubs(:stat).with(path).returns(mock('stat', :ftype => 'directory')) + Find.stubs(:find).returns('') + + #bucket = mock('bucket', 'name' => 'foo') + bucket = mock('bucket') + bucket.stubs(:backup).with(path).returns true + + file = Puppet::Type::File.new(:name => path, :backup => 'foo') + file.stubs(:bucket).returns bucket + + file.perform_backup.should be_true + end + it "a local backup should work" do + path = '/my/dir' + FileTest.stubs(:exists?).with(path).returns true + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + file.stubs(:perform_backup_with_backuplocal).returns true + file.perform_backup.should be_true + end + end +end + diff --git a/test/ral/type/filebucket.rb b/test/ral/type/filebucket.rb deleted file mode 100755 index b18a66f..0000000 --- a/test/ral/type/filebucket.rb +++ /dev/null @@ -1,110 +0,0 @@ -#!/usr/bin/env ruby - -require File.dirname(__FILE__) + '/../../lib/puppettest' - -require 'puppettest' -require 'puppettest/support/utils' -require 'fileutils' - -class TestFileBucket < Test::Unit::TestCase - include PuppetTest::Support::Utils - include PuppetTest::FileTesting - # hmmm - # this is complicated, because we store references to the created - # objects in a central store - def mkfile(hash) - file = nil - assert_nothing_raised { - file = Puppet::Type.type(:file).new(hash) - } - return file - end - - def mkbucket(name,path) - bucket = nil - assert_nothing_raised { - bucket = Puppet::Type.type(:filebucket).new( - :name => name, - :path => path - ) - } - - @@tmpfiles.push path - - return bucket - end - - def mktestfile - # 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 - mkfile(:name => tmpfile) - end - - def setup - super - begin - initstorage - rescue - system("rm -rf %s" % Puppet[:statefile]) - end - end - - def initstorage - Puppet::Util::Storage.init - Puppet::Util::Storage.load - end - - def clearstorage - Puppet::Util::Storage.store - Puppet::Util::Storage.clear - end - - def test_simplebucket - name = "yayness" - bucketpath = tempfile() - resource = mkbucket(name, bucketpath) - - bucket = resource.bucket - - assert_instance_of(Puppet::Network::Client.dipper, bucket) - - md5 = nil - newpath = tempfile() - @@tmpfiles << newpath - system("cp /etc/passwd %s" % newpath) - assert_nothing_raised { - md5 = bucket.backup(newpath) - } - - assert(md5) - - dir, file, pathfile = Puppet::Network::Handler.filebucket.paths(bucketpath, md5) - - assert(FileTest.directory?(dir), - "MD5 directory does not exist") - - newmd5 = nil - - # Just in case the file isn't writable - File.chmod(0644, newpath) - File.open(newpath, "w") { |f| f.puts ";lkjasdf;lkjasdflkjwerlkj134lkj" } - - assert_nothing_raised { - newmd5 = bucket.backup(newpath) - } - - assert(md5 != newmd5) - - assert_nothing_raised { - bucket.restore(newpath, md5) - } - - File.open(newpath) { |f| newmd5 = Digest::MD5.hexdigest(f.read) } - - assert_equal(md5, newmd5) - 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 -~----------~----~----~----~------~----~------~--~---
