Merge branch '0.24.x'

4 files changed, 58 insertions, 12 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 507805014..275e52f08 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+    Closing existing http connections when opening a new one,
+    and closing all connections after each run. (#961)
+
     Removed warning about deprecated explicit plugins mounts.
 
 0.24.0 (misspiggy)
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 54b1dcaa4..341192740 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -271,6 +271,10 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
                         @catalog.apply(options)
                     end
                 end
+
+                # Now close all of our existing http connections, since there's no
+                # reason to leave them lying open.
+                Puppet::Network::XMLRPCClient.clear_http_instances
             end
             
             lockfile.unlock
diff --git a/lib/puppet/network/xmlrpc/client.rb b/lib/puppet/network/xmlrpc/client.rb
index 5283daf5f..5048a040a 100644
--- a/lib/puppet/network/xmlrpc/client.rb
+++ b/lib/puppet/network/xmlrpc/client.rb
@@ -19,8 +19,11 @@ module Puppet::Network
             include Puppet::Util::ClassGen
         end
 
-        # Clear our http cache.
+        # Clear our http cache, closing all connections.
         def self.clear_http_instances
+            @@http_cache.each do |name, connection|
+                connection.finish if connection.started?
+            end
             @@http_cache.clear
         end
 
@@ -34,31 +37,37 @@ module Puppet::Network
             # a cache, as long as we're not resetting the instance.
             return @@http_cache[key] if ! reset and Puppet[:http_keepalive] and @@http_cache[key]
 
+            # Clean up old connections if we have them.
+            if http = @@http_cache[key]
+                @@http_cache.delete(key)
+                http.finish if http.started?
+            end
+
             args = [host, port]
             if Puppet[:http_proxy_host] == "none"
                 args << nil << nil
             else
                 args << Puppet[:http_proxy_host] << Puppet[:http_proxy_port]
             end
-            @http = Net::HTTP.new(*args)
+            http = Net::HTTP.new(*args)
 
-            # Pop open @http a little; older versions of Net::HTTP(s) didn't
+            # Pop open the http client a little; older versions of Net::HTTP(s) didn't
             # give us a reader for ca_file... Grr...
-            class << @http; attr_accessor :ca_file; end
+            class << http; attr_accessor :ca_file; end
 
-            @http.use_ssl = true
-            @http.read_timeout = 120
-            @http.open_timeout = 120
+            http.use_ssl = true
+            http.read_timeout = 120
+            http.open_timeout = 120
             # JJM Configurable fix for #896.
             if Puppet[:http_enable_post_connection_check]
-                @http.enable_post_connection_check = true
+                http.enable_post_connection_check = true
             else
-                @http.enable_post_connection_check = false
+                http.enable_post_connection_check = false
             end
 
-            @@http_cache[key] = @http if Puppet[:http_keepalive]
+            @@http_cache[key] = http if Puppet[:http_keepalive]
 
-            return @http
+            return http
         end
 
         # Create a netclient for each handler
diff --git a/spec/unit/network/xmlrpc/client.rb b/spec/unit/network/xmlrpc/client.rb
index 2e7b566d5..b40486e13 100755
--- a/spec/unit/network/xmlrpc/client.rb
+++ b/spec/unit/network/xmlrpc/client.rb
@@ -14,7 +14,7 @@ describe Puppet::Network::XMLRPCClient, " when managing http instances" do
     end
 
     it "should return an http instance created with the passed host and port" do
-        http = stub 'http', :use_ssl= => nil, :read_timeout= => nil, :open_timeout= => nil, :enable_post_connection_check= => nil
+        http = stub 'http', :use_ssl= => nil, :read_timeout= => nil, :open_timeout= => nil, :enable_post_connection_check= => nil, :started? => false
         Net::HTTP.expects(:new).with("me", 54321, nil, nil).returns(http)
         Puppet::Network::XMLRPCClient.http_instance("me", 54321).should equal(http)
     end
@@ -62,6 +62,20 @@ describe Puppet::Network::XMLRPCClient, " when managing http instances" do
         Puppet::Network::XMLRPCClient.http_instance("me", 54321).should_not equal(old)
     end
 
+    it "should have a mechanism for getting a new http instance instead of the cached instance" do
+        stub_settings :http_keepalive => true, :http_proxy_host => "myhost", :http_proxy_port => 432, :http_enable_post_connection_check => true
+        old = Puppet::Network::XMLRPCClient.http_instance("me", 54321)
+        Puppet::Network::XMLRPCClient.http_instance("me", 54321, true).should_not equal(old)
+    end
+
+    it "should close existing, open connections when requesting a new connection" do
+        stub_settings :http_keepalive => true, :http_proxy_host => "myhost", :http_proxy_port => 432, :http_enable_post_connection_check => true
+        old = Puppet::Network::XMLRPCClient.http_instance("me", 54321)
+        old.expects(:started?).returns(true)
+        old.expects(:finish)
+        Puppet::Network::XMLRPCClient.http_instance("me", 54321, true)
+    end
+
     it "should have a mechanism for clearing the http cache" do
         stub_settings :http_keepalive => true, :http_proxy_host => "myhost", :http_proxy_port => 432, :http_enable_post_connection_check => true
         old = Puppet::Network::XMLRPCClient.http_instance("me", 54321)
@@ -71,6 +85,22 @@ describe Puppet::Network::XMLRPCClient, " when managing http instances" do
         Puppet::Network::XMLRPCClient.http_instance("me", 54321).should_not equal(old)
     end
 
+    it "should close open http connections when clearing the cache" do
+        stub_settings :http_keepalive => true, :http_proxy_host => "myhost", :http_proxy_port => 432, :http_enable_post_connection_check => true
+        one = Puppet::Network::XMLRPCClient.http_instance("me", 54321)
+        one.expects(:started?).returns(true)
+        one.expects(:finish).returns(true)
+        Puppet::Network::XMLRPCClient.clear_http_instances
+    end
+
+    it "should not close unopened http connections when clearing the cache" do
+        stub_settings :http_keepalive => true, :http_proxy_host => "myhost", :http_proxy_port => 432, :http_enable_post_connection_check => true
+        one = Puppet::Network::XMLRPCClient.http_instance("me", 54321)
+        one.expects(:started?).returns(false)
+        one.expects(:finish).never
+        Puppet::Network::XMLRPCClient.clear_http_instances
+    end
+
     after do
         Puppet::Network::XMLRPCClient.clear_http_instances
     end