[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35

test branch puppet-dev at googlegroups.com
Wed Jul 14 10:33:25 UTC 2010


The following commit has been merged in the upstream branch:
commit ee5d7f196fa62046f8fc3d3d723da608b17ce531
Author: Brice Figureau <brice-puppet at daysofwonder.com>
Date:   Sat Apr 10 16:30:03 2010 +0200

    Add master side file content streaming
    
    This patch allows the puppetmaster to serve file chunks by chunks without
    ever reading the file content in RAM.
    This allows serving large files directly with the master without impacting
    the master memory footprint.
    
    Signed-off-by: Brice Figureau <brice-puppet at daysofwonder.com>

diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb
index 9fc1d08..87ef4fb 100644
--- a/lib/puppet/file_serving/content.rb
+++ b/lib/puppet/file_serving/content.rb
@@ -26,10 +26,11 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base
         instance
     end
 
-    # Collect our data.
+    # BF: we used to fetch the file content here, but this is counter-productive
+    # for puppetmaster streaming of file content. So collect just returns itself
     def collect
         return if stat.ftype == "directory"
-        content
+        self
     end
 
     # Read the content of our file in.
@@ -44,6 +45,6 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base
     end
 
     def to_raw
-        content
+        File.new(full_path(), "r")
     end
 end
diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb
index 7b28d88..8668bf8 100644
--- a/lib/puppet/network/http/mongrel/rest.rb
+++ b/lib/puppet/network/http/mongrel/rest.rb
@@ -54,12 +54,19 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler
         # we have a failure, unless we're on a version of mongrel that doesn't
         # support this.
         if status < 300
-            response.start(status) { |head, body| body.write(result) }
+            unless result.is_a?(File)
+                response.start(status) { |head, body| body.write(result) }
+            else
+                response.start(status) { |head, body| }
+                response.send_status(result.stat.size)
+                response.send_header
+                response.send_file(result.path)
+            end
         else
             begin
                 response.start(status,false,result) { |head, body| body.write(result) }
             rescue ArgumentError
-	        response.start(status)              { |head, body| body.write(result) }
+                response.start(status)              { |head, body| body.write(result) }
             end
         end
     end
diff --git a/lib/puppet/network/http/rack/rest.rb b/lib/puppet/network/http/rack/rest.rb
index 1047512..5245275 100644
--- a/lib/puppet/network/http/rack/rest.rb
+++ b/lib/puppet/network/http/rack/rest.rb
@@ -8,6 +8,24 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler
     HEADER_ACCEPT = 'HTTP_ACCEPT'.freeze
     ContentType = 'Content-Type'.freeze
 
+    CHUNK_SIZE = 8192
+
+    class RackFile
+        def initialize(file)
+            @file = file
+        end
+
+        def each
+            while chunk = @file.read(CHUNK_SIZE)
+                yield chunk
+            end
+        end
+
+        def close
+            @file.close
+        end
+    end
+
     def initialize(args={})
         super()
         initialize_for_puppet(args)
@@ -20,7 +38,12 @@ class Puppet::Network::HTTP::RackREST < Puppet::Network::HTTP::RackHttpHandler
     # produce the body of the response
     def set_response(response, result, status = 200)
         response.status = status
-        response.write result
+        unless result.is_a?(File)
+            response.write result
+        else
+            response["Content-Length"] = result.stat.size
+            response.body = RackFile.new(result)
+        end
     end
 
     # Retrieve the accept header from the http request.
diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb
index 274665d..9bfd0ce 100644
--- a/lib/puppet/network/http/webrick/rest.rb
+++ b/lib/puppet/network/http/webrick/rest.rb
@@ -50,7 +50,12 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet
 
     def set_response(response, result, status = 200)
         response.status = status
-        response.body          = result if status >= 200 and status != 304
+        if status >= 200 and status != 304
+            response.body = result
+            if result.is_a?(File)
+                response["content-length"] = result.stat.size
+            end
+        end
         response.reason_phrase = result if status < 200 or status >= 300
     end
 
diff --git a/spec/unit/file_serving/content.rb b/spec/unit/file_serving/content.rb
index eacaff8..cba7039 100755
--- a/spec/unit/file_serving/content.rb
+++ b/spec/unit/file_serving/content.rb
@@ -25,15 +25,15 @@ describe Puppet::FileServing::Content do
         Puppet::FileServing::Content.new("/path").should respond_to(:collect)
     end
 
-    it "should retrieve and store its contents when its attributes are collected if the file is a normal file" do
+    it "should not retrieve and store its contents when its attributes are collected if the file is a normal file" do
         content = Puppet::FileServing::Content.new("/path")
 
         result = "foo"
         File.stubs(:lstat).returns(stub("stat", :ftype => "file"))
-        File.expects(:read).with("/path").returns result
+        File.expects(:read).with("/path").never
         content.collect
 
-        content.instance_variable_get("@content").should_not be_nil
+        content.instance_variable_get("@content").should be_nil
     end
 
     it "should not attempt to retrieve its contents if the file is a directory" do
@@ -70,6 +70,14 @@ describe Puppet::FileServing::Content do
 
         Puppet::FileServing::Content.from_raw("foo/bar").should equal(instance)
     end
+
+    it "should return an opened File when converted to raw" do
+        content = Puppet::FileServing::Content.new("/path")
+
+        File.expects(:new).with("/path","r").returns :file
+
+        content.to_raw.should == :file
+    end
 end
 
 describe Puppet::FileServing::Content, "when returning the contents" do
diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb
index fb7e7e5..55b6172 100755
--- a/spec/unit/network/http/mongrel/rest.rb
+++ b/spec/unit/network/http/mongrel/rest.rb
@@ -81,6 +81,23 @@ describe "Puppet::Network::HTTP::MongrelREST" do
                 @handler.set_response(@response, "mybody", 200)
             end
 
+            describe "when the result is a File" do
+                it "should use response send_file" do
+                    head = mock 'head'
+                    body = mock 'body'
+                    stat = stub 'stat', :size => 100
+                    file = stub 'file', :stat => stat, :path => "/tmp/path"
+                    file.stubs(:is_a?).with(File).returns(true)
+
+                    @response.expects(:start).with(200).yields(head, body)
+                    @response.expects(:send_status).with(100)
+                    @response.expects(:send_header)
+                    @response.expects(:send_file).with("/tmp/path")
+
+                    @handler.set_response(@response, file, 200)
+                end
+            end
+
             it "should set the status and reason and write the body when setting the response for a successful request" do
                 head = mock 'head'
                 body = mock 'body'
diff --git a/spec/unit/network/http/rack/rest.rb b/spec/unit/network/http/rack/rest.rb
index e916712..75642f9 100755
--- a/spec/unit/network/http/rack/rest.rb
+++ b/spec/unit/network/http/rack/rest.rb
@@ -74,6 +74,26 @@ describe "Puppet::Network::HTTP::RackREST" do
 
                 @handler.set_response(@response, "mybody", 400)
             end
+
+            describe "when result is a File" do
+                before :each do
+                    stat = stub 'stat', :size => 100
+                    @file = stub 'file', :stat => stat, :path => "/tmp/path"
+                    @file.stubs(:is_a?).with(File).returns(true)
+                end
+
+                it "should set the Content-Length header" do
+                    @response.expects(:[]=).with("Content-Length", 100)
+
+                    @handler.set_response(@response, @file, 200)
+                end
+
+                it "should return a RackFile adapter as body" do
+                    @response.expects(:body=).with { |val| val.is_a?(Puppet::Network::HTTP::RackREST::RackFile) }
+
+                    @handler.set_response(@response, @file, 200)
+                end
+            end
         end
 
         describe "and determining the request parameters" do
@@ -197,3 +217,33 @@ describe "Puppet::Network::HTTP::RackREST" do
         end
     end
 end
+
+describe Puppet::Network::HTTP::RackREST::RackFile do
+    before(:each) do
+        stat = stub 'stat', :size => 100
+        @file = stub 'file', :stat => stat, :path => "/tmp/path"
+        @rackfile = Puppet::Network::HTTP::RackREST::RackFile.new(@file)
+    end
+
+    it "should have an each method" do
+        @rackfile.should be_respond_to(:each)
+    end
+
+    it "should yield file chunks by chunks" do
+        @file.expects(:read).times(3).with(8192).returns("1", "2", nil)
+        i = 1
+        @rackfile.each do |chunk|
+            chunk.to_i.should == i
+            i += 1
+        end
+    end
+
+    it "should have a close method" do
+        @rackfile.should be_respond_to(:close)
+    end
+
+    it "should delegate close to File close" do
+        @file.expects(:close)
+        @rackfile.close
+    end
+end
\ No newline at end of file
diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb
index f5c563e..f726fd9 100755
--- a/spec/unit/network/http/webrick/rest.rb
+++ b/spec/unit/network/http/webrick/rest.rb
@@ -69,6 +69,29 @@ describe Puppet::Network::HTTP::WEBrickREST do
                 @handler.set_response(@response, "mybody", 200)
             end
 
+            describe "when the result is a File" do
+                before(:each) do
+                    stat = stub 'stat', :size => 100
+                    @file = stub 'file', :stat => stat, :path => "/tmp/path"
+                    @file.stubs(:is_a?).with(File).returns(true)
+                end
+
+                it "should serve it" do
+                    @response.stubs(:[]=)
+
+                    @response.expects(:status=).with 200
+                    @response.expects(:body=).with @file
+
+                    @handler.set_response(@response, @file, 200)
+                end
+
+                it "should set the Content-Length header" do
+                    @response.expects(:[]=).with('content-length', 100)
+
+                    @handler.set_response(@response, @file, 200)
+                end
+            end
+
             it "should set the status and message on the response when setting the response for a failed query" do
                 @response.expects(:status=).with 400
                 @response.expects(:reason_phrase=).with "mybody"

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list