[PATCH/puppet 1/1] (#6353) Restore the ability to store paths in the filebucket
Paul Berry
usage for the case where a file is already in the filebucket.
However, it took away the ability to store paths.
This change restores the ability to store paths while maintaining
optimal network usage for the case where the file is already in the
filebucket with the given path. This is expected to be the most
common case.
Paired-with: Jesse Wolfe <je...@puppetlabs.com>
Signed-off-by: Paul Berry <pa...@puppetlabs.com>
---
lib/puppet/file_bucket/dipper.rb | 5 +-
lib/puppet/indirector/file_bucket_file/file.rb | 55 ++++++++---
spec/unit/indirector/file_bucket_file/file_spec.rb | 109 ++++++++++++++++++--
3 files changed, 144 insertions(+), 25 deletions(-)
diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb
index f4bef28..de4c01b 100644
--- a/lib/puppet/file_bucket/dipper.rb
+++ b/lib/puppet/file_bucket/dipper.rb
@@ -34,11 +34,12 @@ class Puppet::FileBucket::Dipper
contents = ::File.read(file)
begin
file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path)
- dest_path = "#{@rest_path}#{file_bucket_file.name}"
+ files_original_path = absolutize_path(file)
+ dest_path = "#{@rest_path}#{file_bucket_file.name}#{files_original_path}"
# Make a HEAD request for the file so that we don't waste time
# uploading it if it already exists in the bucket.
- unless Puppet::FileBucket::File.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}")
+ unless Puppet::FileBucket::File.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}#{files_original_path}")
file_bucket_file.save(dest_path)
end
diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb
index 8bea2d7..0fd8a91 100644
--- a/lib/puppet/indirector/file_bucket_file/file.rb
+++ b/lib/puppet/indirector/file_bucket_file/file.rb
@@ -14,10 +14,12 @@ module Puppet::FileBucketFile
end
def find( request )
- checksum = request_to_checksum( request )
- file_path = path_for(request.options[:bucket_path], checksum, 'contents')
+ checksum, files_original_path = request_to_checksum_and_path( request )
+ dir_path = path_for(request.options[:bucket_path], checksum)
+ file_path = ::File.join(dir_path, 'contents')
return nil unless ::File.exists?(file_path)
+ return nil unless path_match(dir_path, files_original_path)
if request.options[:diff_with]
hash_protocol = sumtype(checksum)
@@ -32,32 +34,47 @@ module Puppet::FileBucketFile
end
def head(request)
- checksum = request_to_checksum(request)
- file_path = path_for(request.options[:bucket_path], checksum, 'contents')
- ::File.exists?(file_path)
+ checksum, files_original_path = request_to_checksum_and_path(request)
+ dir_path = path_for(request.options[:bucket_path], checksum)
+
+ ::File.exists?(::File.join(dir_path, 'contents')) and path_match(dir_path, files_original_path)
end
def save( request )
instance = request.instance
+ checksum, files_original_path = request_to_checksum_and_path(request)
- save_to_disk(instance)
+ save_to_disk(instance, files_original_path)
instance.to_s
end
private
- def save_to_disk( bucket_file )
+ def path_match(dir_path, files_original_path)
+ return true unless files_original_path # if no path was provided, it's a match
+ paths_path = ::File.join(dir_path, 'paths')
+ return false unless ::File.exists?(paths_path)
+ ::File.open(paths_path) do |f|
+ f.each do |line|
+ return true if line.chomp == files_original_path
+ end
+ end
+ return false
+ end
+
+ def save_to_disk( bucket_file, files_original_path )
filename = path_for(bucket_file.bucket_path, bucket_file.checksum_data, 'contents')
- dirname = path_for(bucket_file.bucket_path, bucket_file.checksum_data)
+ dir_path = path_for(bucket_file.bucket_path, bucket_file.checksum_data)
+ paths_path = ::File.join(dir_path, 'paths')
# If the file already exists, do nothing.
if ::File.exist?(filename)
verify_identical_file!(bucket_file)
else
# Make the directories if necessary.
- unless ::File.directory?(dirname)
+ unless ::File.directory?(dir_path)
Puppet::Util.withumask(0007) do
- ::FileUtils.mkdir_p(dirname)
+ ::FileUtils.mkdir_p(dir_path)
end
end
@@ -68,15 +85,27 @@ module Puppet::FileBucketFile
::File.open(filename, ::File::WRONLY|::File::CREAT, 0440) do |of|
of.print bucket_file.contents
end
+ ::File.open(paths_path, ::File::WRONLY|::File::CREAT, 0640) do |of|
+ # path will be written below
+ end
+ end
+ end
+
+ unless path_match(dir_path, files_original_path)
+ ::File.open(paths_path, 'a') do |f|
+ f.puts(files_original_path)
end
end
end
- def request_to_checksum( request )
- checksum_type, checksum, path = request.key.split(/\//, 3) # Note: we ignore path if present.
+ def request_to_checksum_and_path( request )
+ checksum_type, checksum, path = request.key.split(/\//, 3)
+ if path == '' # Treat "md5/<checksum>/" like "md5/<checksum>"
+ path = nil
+ end
raise "Unsupported checksum type #{checksum_type.inspect}" if checksum_type != 'md5'
raise "Invalid checksum #{checksum.inspect}" if checksum !~ /^[0-9a-f]{32}$/
- checksum
+ [checksum, path]
end
def path_for(bucket_path, digest, subfile = nil)
diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb
index 9187f4d..0c33593 100755
--- a/spec/unit/indirector/file_bucket_file/file_spec.rb
+++ b/spec/unit/indirector/file_bucket_file/file_spec.rb
@@ -22,13 +22,97 @@ describe Puppet::FileBucketFile::File do
Puppet[:bucketdir] = tmpdir('bucketdir')
end
- describe "when diffing files" do
- def save_bucket_file(contents)
- bucket_file = Puppet::FileBucket::File.new(contents)
- bucket_file.save
- bucket_file.checksum_data
+ def save_bucket_file(contents, path = "/who_cares")
+ bucket_file = Puppet::FileBucket::File.new(contents)
+ bucket_file.save("md5/#{Digest::MD5.hexdigest(contents)}#{path}")
+ bucket_file.checksum_data
+ end
+
+ describe "when servicing a save request" do
+ describe "when supplying a path" do
+ it "should store the path if not already stored" do
+ checksum = save_bucket_file("stuff", "/foo/bar")
+ dir_path = "#{Puppet[:bucketdir]}/c/1/3/d/8/8/c/b/c13d88cb4cb02003daedb8a84e5d272a"
+ File.read("#{dir_path}/contents").should == "stuff"
+ File.read("#{dir_path}/paths").should == "foo/bar\n"
+ end
+
+ it "should leave the paths file alone if the path is already stored" do
+ checksum = save_bucket_file("stuff", "/foo/bar")
+ checksum = save_bucket_file("stuff", "/foo/bar")
+ dir_path = "#{Puppet[:bucketdir]}/c/1/3/d/8/8/c/b/c13d88cb4cb02003daedb8a84e5d272a"
+ File.read("#{dir_path}/contents").should == "stuff"
+ File.read("#{dir_path}/paths").should == "foo/bar\n"
+ end
+
+ it "should store an additional path if the new path differs from those already stored" do
+ checksum = save_bucket_file("stuff", "/foo/bar")
+ checksum = save_bucket_file("stuff", "/foo/baz")
+ dir_path = "#{Puppet[:bucketdir]}/c/1/3/d/8/8/c/b/c13d88cb4cb02003daedb8a84e5d272a"
+ File.read("#{dir_path}/contents").should == "stuff"
+ File.read("#{dir_path}/paths").should == "foo/bar\nfoo/baz\n"
+ end
+ end
+
+ describe "when not supplying a path" do
+ it "should save the file and create an empty paths file" do
+ checksum = save_bucket_file("stuff", "")
+ dir_path = "#{Puppet[:bucketdir]}/c/1/3/d/8/8/c/b/c13d88cb4cb02003daedb8a84e5d272a"
+ File.read("#{dir_path}/contents").should == "stuff"
+ File.read("#{dir_path}/paths").should == ""
+ end
+ end
+ end
+
+ describe "when servicing a head/find request" do
+ describe "when supplying a path" do
+ it "should return false/nil if the file isn't bucketed" do
+ Puppet::FileBucket::File.head("md5/0ae2ec1980410229885fe72f7b44fe55/foo/bar").should == false
+ Puppet::FileBucket::File.find("md5/0ae2ec1980410229885fe72f7b44fe55/foo/bar").should == nil
+ end
+
+ it "should return false/nil if the file is bucketed but with a different path" do
+ checksum = save_bucket_file("I'm the contents of a file", '/foo/bar')
+ Puppet::FileBucket::File.head("md5/#{checksum}/foo/baz").should == false
+ Puppet::FileBucket::File.find("md5/#{checksum}/foo/baz").should == nil
+ end
+
+ it "should return true/file if the file is already bucketed with the given path" do
+ contents = "I'm the contents of a file"
+ checksum = save_bucket_file(contents, '/foo/bar')
+ Puppet::FileBucket::File.head("md5/#{checksum}/foo/bar").should == true
+ find_result = Puppet::FileBucket::File.find("md5/#{checksum}/foo/bar")
+ find_result.should be_a(Puppet::FileBucket::File)
+ find_result.checksum.should == "{md5}#{checksum}"
+ find_result.to_s.should == contents
+ end
+ end
+
+ describe "when not supplying a path" do
+ [false, true].each do |trailing_slash|
+ describe "#{trailing_slash ? 'with' : 'without'} a trailing slash" do
+ trailing_string = trailing_slash ? '/' : ''
+
+ it "should return false/nil if the file isn't bucketed" do
+ Puppet::FileBucket::File.head("md5/0ae2ec1980410229885fe72f7b44fe55#{trailing_string}").should == false
+ Puppet::FileBucket::File.find("md5/0ae2ec1980410229885fe72f7b44fe55#{trailing_string}").should == nil
+ end
+
+ it "should return true/file if the file is already bucketed" do
+ contents = "I'm the contents of a file"
+ checksum = save_bucket_file(contents, '/foo/bar')
+ Puppet::FileBucket::File.head("md5/#{checksum}#{trailing_string}").should == true
+ find_result = Puppet::FileBucket::File.find("md5/#{checksum}#{trailing_string}")
+ find_result.should be_a(Puppet::FileBucket::File)
+ find_result.checksum.should == "{md5}#{checksum}"
+ find_result.to_s.should == contents
+ end
+ end
+ end
end
+ end
+ describe "when diffing files" do
it "should generate an empty string if there is no diff" do
checksum = save_bucket_file("I'm the contents of a file")
Puppet::FileBucket::File.find("md5/#{checksum}", :diff_with => checksum).should == ''
@@ -102,7 +186,7 @@ HERE
key = "md5/#{@digest}"
if supply_path
- key += "//path/to/file"
+ key += "/path/to/file"
end
@request = Puppet::Indirector::Request.new(:indirection_name, :find, key, request_options)
@@ -116,10 +200,15 @@ HERE
it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do
make_bucketed_file
- bucketfile = @store.find(@request)
- bucketfile.should be_a(Puppet::FileBucket::File)
- bucketfile.contents.should == @contents
- @store.head(@request).should == true
+ if supply_path
+ @store.find(@request).should == nil
+ @store.head(@request).should == false # because path didn't match
+ else
+ bucketfile = @store.find(@request)
+ bucketfile.should be_a(Puppet::FileBucket::File)
+ bucketfile.contents.should == @contents
+ @store.head(@request).should == true
+ end
end
it "should return nil if no file is found" do
--
1.7.2
Paul Berry
--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppe...@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.