From 553b2ad8add20cd629fcd90b512d97d4edd7e481 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 19 Dec 2007 11:42:22 -0600 Subject: Entirely refactoring http keep-alive. There's now a central module responsible for managing the http pool (Puppet::Network::HttpPool), and it also handles setting certificate information. This gets rid of what were otherwise long chains of method calls, and it makes the code paths much clearer. --- lib/puppet/network/client.rb | 13 ++--- lib/puppet/network/client/master.rb | 3 +- lib/puppet/network/http_pool.rb | 92 +++++++++++++++++++++++++++++++++++ lib/puppet/network/xmlrpc/client.rb | 97 +++++-------------------------------- 4 files changed, 109 insertions(+), 96 deletions(-) create mode 100644 lib/puppet/network/http_pool.rb (limited to 'lib/puppet/network') diff --git a/lib/puppet/network/client.rb b/lib/puppet/network/client.rb index 52431e227..9a861fdf2 100644 --- a/lib/puppet/network/client.rb +++ b/lib/puppet/network/client.rb @@ -122,13 +122,8 @@ class Puppet::Network::Client end # Make sure we set the driver up when we read the cert in. - def read_cert - if super - @driver.recycle_connection(self) if @driver.respond_to?(:recycle_connection) - return true - else - return false - end + def recycle_connection + @driver.recycle_connection if @driver.respond_to?(:recycle_connection) end # A wrapper method to run and then store the last run time @@ -141,9 +136,7 @@ class Puppet::Network::Client self.run self.lastrun = Time.now.to_i rescue => detail - if Puppet[:trace] - puts detail.backtrace - end + puts detail.backtrace if Puppet[:trace] Puppet.err "Could not run %s: %s" % [self.class, detail] end end diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb index 341192740..8d768018f 100644 --- a/lib/puppet/network/client/master.rb +++ b/lib/puppet/network/client/master.rb @@ -1,6 +1,7 @@ # The client for interacting with the puppetmaster config server. require 'sync' require 'timeout' +require 'puppet/network/http_pool' class Puppet::Network::Client::Master < Puppet::Network::Client unless defined? @@sync @@ -274,7 +275,7 @@ class Puppet::Network::Client::Master < Puppet::Network::Client # Now close all of our existing http connections, since there's no # reason to leave them lying open. - Puppet::Network::XMLRPCClient.clear_http_instances + Puppet::Network::HttpPool.clear_http_instances end lockfile.unlock diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb new file mode 100644 index 000000000..f6038f189 --- /dev/null +++ b/lib/puppet/network/http_pool.rb @@ -0,0 +1,92 @@ +require 'puppet/sslcertificates/support' +require 'net/https' + +# Manage Net::HTTP instances for keep-alive. +module Puppet::Network::HttpPool + # This handles reading in the key and such-like. + extend Puppet::SSLCertificates::Support + @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 + @cert = nil + @key = nil + end + + # Make sure we set the driver up when we read the cert in. + def self.read_cert + if val = super # This calls read_cert from the Puppet::SSLCertificates::Support module. + # Clear out all of our connections, since they previously had no cert and now they + # should have them. + clear_http_instances + return val + else + return false + end + end + + # Use cert information from a Puppet client to set up the http object. + def self.cert_setup(http) + # Just no-op if we don't have certs. + return false unless (defined?(@cert) and @cert) or self.read_cert + + store = OpenSSL::X509::Store.new + store.add_file Puppet[:localcacert] + store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT + + http.cert_store = store + http.ca_file = Puppet[:localcacert] + http.cert = self.cert + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + http.key = self.key + end + + # Retrieve a cached http instance of caching is enabled, else return + # a new one. + def self.http_instance(host, port, reset = false) + # We overwrite the uninitialized @http here with a cached one. + key = "%s:%s" % [host, port] + + # Return our cached instance if keepalive is enabled and we've got + # 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) + + # 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 + + 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 + else + http.enable_post_connection_check = false + end + + cert_setup(http) + + @http_cache[key] = http if Puppet[:http_keepalive] + + return http + end +end diff --git a/lib/puppet/network/xmlrpc/client.rb b/lib/puppet/network/xmlrpc/client.rb index 5048a040a..27bb3dc5e 100644 --- a/lib/puppet/network/xmlrpc/client.rb +++ b/lib/puppet/network/xmlrpc/client.rb @@ -1,4 +1,5 @@ require 'puppet/sslcertificates' +require 'puppet/network/http_pool' require 'openssl' require 'puppet/external/base64' @@ -10,66 +11,15 @@ module Puppet::Network class ClientError < Puppet::Error; end class XMLRPCClientError < Puppet::Error; end class XMLRPCClient < ::XMLRPC::Client + attr_accessor :puppet_server, :puppet_port @clients = {} - @@http_cache = {} class << self include Puppet::Util include Puppet::Util::ClassGen end - # 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 - - # Retrieve a cached http instance of caching is enabled, else return - # a new one. - def self.http_instance(host, port, reset = false) - # We overwrite the uninitialized @http here with a cached one. - key = "%s:%s" % [host, port] - - # Return our cached instance if keepalive is enabled and we've got - # 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) - - # 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 - - 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 - else - http.enable_post_connection_check = false - end - - @@http_cache[key] = http if Puppet[:http_keepalive] - - return http - end - # Create a netclient for each handler def self.mkclient(handler) interface = handler.interface @@ -81,8 +31,7 @@ module Puppet::Network # they want. constant = handler.name.to_s.capitalize name = namespace.downcase - newclient = genclass(name, :hash => @clients, - :constant => constant) + newclient = genclass(name, :hash => @clients, :constant => constant) interface.methods.each { |ary| method = ary[0] @@ -97,7 +46,7 @@ module Puppet::Network rescue OpenSSL::SSL::SSLError => detail if detail.message =~ /bad write retry/ Puppet.warning "Transient SSL write error; restarting connection and retrying" - self.recycle_connection(@cert_client) + self.recycle_connection retry end raise XMLRPCClientError, @@ -118,7 +67,7 @@ module Puppet::Network raise error rescue Errno::EPIPE, EOFError Puppet.warning "Other end went away; restarting connection and retrying" - self.recycle_connection(@cert_client) + self.recycle_connection retry rescue => detail if detail.message =~ /^Wrong size\. Was \d+, should be \d+$/ @@ -141,30 +90,6 @@ module Puppet::Network @clients[handler] || self.mkclient(handler) end - # Use cert information from a Puppet client to set up the http object. - def cert_setup(client) - # Cache it for next time - @cert_client = client - - unless FileTest.exist?(Puppet[:localcacert]) - raise Puppet::SSLCertificates::Support::MissingCertificate, - "Could not find ca certificate %s" % Puppet[:localcacert] - end - - # We can't overwrite certificates, @http will freeze itself - # once started. - unless @http.ca_file - @http.ca_file = Puppet[:localcacert] - store = OpenSSL::X509::Store.new - store.add_file Puppet[:localcacert] - store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT - @http.cert_store = store - @http.cert = client.cert - @http.verify_mode = OpenSSL::SSL::VERIFY_PEER - @http.key = client.key - end - end - def initialize(hash = {}) hash[:Path] ||= "/RPC2" hash[:Server] ||= Puppet[:server] @@ -188,13 +113,15 @@ module Puppet::Network true, # use_ssl 120 # a two minute timeout, instead of 30 seconds ) - @http = self.class.http_instance(@host, @port) + @http = Puppet::Network::HttpPool.http_instance(@host, @port) end - def recycle_connection(client) - @http = self.class.http_instance(@host, @port, true) # reset the instance - - cert_setup(client) + # Get rid of our existing connection, replacing it with a new one. + # This should only happen if we lose our connection somehow (e.g., an EPIPE) + # or we've just downloaded certs and we need to create new http instances + # with the certs added. + def recycle_connection + @http = Puppet::Network::HttpPool.http_instance(@host, @port, true) # reset the instance end def start -- cgit