diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2016-08-15 11:38:21 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-15 11:38:21 -0700 |
commit | 27235a41389b5f419b14a9dda48208c546d7e684 (patch) | |
tree | b23f6795510495a6d78f9113bec092dfe4f9d0cf | |
parent | ba819110a6643d1f65e6b20650e5c8905b083aa8 (diff) | |
parent | 09d36b317feb91b53092601e7010dbbbf00fab5d (diff) | |
download | chef-27235a41389b5f419b14a9dda48208c546d7e684.tar.gz |
Merge pull request #5151 from chef/lcg/keepalives-again-again
HTTP 1.1 keepalives re-re-redux
-rw-r--r-- | lib/chef/cookbook/synchronizer.rb | 2 | ||||
-rw-r--r-- | lib/chef/http.rb | 27 | ||||
-rw-r--r-- | lib/chef/http/basic_client.rb | 13 | ||||
-rw-r--r-- | spec/unit/cookbook/synchronizer_spec.rb | 7 | ||||
-rw-r--r-- | spec/unit/http/basic_client_spec.rb | 20 | ||||
-rw-r--r-- | spec/unit/http_spec.rb | 14 |
6 files changed, 77 insertions, 6 deletions
diff --git a/lib/chef/cookbook/synchronizer.rb b/lib/chef/cookbook/synchronizer.rb index 1ee30bacc7..01f6155bf3 100644 --- a/lib/chef/cookbook/synchronizer.rb +++ b/lib/chef/cookbook/synchronizer.rb @@ -291,7 +291,7 @@ class Chef end def server_api - Chef::ServerAPI.new(Chef::Config[:chef_server_url]) + Thread.current[:server_api] ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], keepalives: true) end end diff --git a/lib/chef/http.rb b/lib/chef/http.rb index 3e69f58383..ec6665f5c0 100644 --- a/lib/chef/http.rb +++ b/lib/chef/http.rb @@ -77,6 +77,9 @@ class Chef attr_reader :middlewares + # [Boolean] if we're doing keepalives or not + attr_reader :keepalives + # Create a HTTP client object. The supplied +url+ is used as the base for # all subsequent requests. For example, when initialized with a base url # http://localhost:4000, a call to +get+ with 'nodes' will make an @@ -87,6 +90,7 @@ class Chef @sign_on_redirect = true @redirects_followed = 0 @redirect_limit = 10 + @keepalives = options[:keepalives] || false @options = options @middlewares = [] @@ -228,6 +232,25 @@ class Chef def http_client(base_url = nil) base_url ||= url + if keepalives && !base_url.nil? + # only reuse the http_client if we want keepalives and have a base_url + @http_client ||= {} + # the per-host per-port cache here gets peristent connections correct when + # redirecting to different servers + if base_url.is_a?(String) # sigh, this kind of abuse can't happen with strongly typed languages + @http_client[base_url] ||= build_http_client(base_url) + else + @http_client[base_url.host] ||= {} + @http_client[base_url.host][base_url.port] ||= build_http_client(base_url) + end + else + build_http_client(base_url) + end + end + + private + + def build_http_client(base_url) if chef_zero_uri?(base_url) # PERFORMANCE CRITICAL: *MUST* lazy require here otherwise we load up webrick # via chef-zero and that hits DNS (at *require* time) which may timeout, @@ -239,12 +262,10 @@ class Chef SocketlessChefZeroClient.new(base_url) else - BasicClient.new(base_url, :ssl_policy => Chef::HTTP::APISSLPolicy) + BasicClient.new(base_url, ssl_policy: Chef::HTTP::APISSLPolicy, keepalives: keepalives) end end - protected - def create_url(path) return path if path.is_a?(URI) if path =~ /^(http|https|chefzero):\/\//i diff --git a/lib/chef/http/basic_client.rb b/lib/chef/http/basic_client.rb index 3a87fe85e4..460744ea2a 100644 --- a/lib/chef/http/basic_client.rb +++ b/lib/chef/http/basic_client.rb @@ -34,6 +34,7 @@ class Chef attr_reader :url attr_reader :http_client attr_reader :ssl_policy + attr_reader :keepalives # Instantiate a BasicClient. # === Arguments: @@ -43,7 +44,11 @@ class Chef def initialize(url, opts = {}) @url = url @ssl_policy = opts[:ssl_policy] || DefaultSSLPolicy - @http_client = build_http_client + @keepalives = opts[:keepalives] || false + end + + def http_client + @http_client ||= build_http_client end def host @@ -114,7 +119,11 @@ class Chef http_client.read_timeout = config[:rest_timeout] http_client.open_timeout = config[:rest_timeout] - http_client + if keepalives + http_client.start + else + http_client + end end def config diff --git a/spec/unit/cookbook/synchronizer_spec.rb b/spec/unit/cookbook/synchronizer_spec.rb index 3f5624f3b0..82876273e7 100644 --- a/spec/unit/cookbook/synchronizer_spec.rb +++ b/spec/unit/cookbook/synchronizer_spec.rb @@ -414,6 +414,13 @@ describe Chef::CookbookSynchronizer do and_return("/file-cache/cookbooks/cookbook_a/templates/default/apache2.conf.erb") end + describe "#server_api" do + it "sets keepalive to true" do + expect(Chef::ServerAPI).to receive(:new).with(Chef::Config[:chef_server_url], keepalives: true) + synchronizer.server_api + end + end + describe "when syncing cookbooks with the server" do let(:server_api) { double("Chef::ServerAPI (mock)") } diff --git a/spec/unit/http/basic_client_spec.rb b/spec/unit/http/basic_client_spec.rb index 8cf63d4441..0def00a220 100644 --- a/spec/unit/http/basic_client_spec.rb +++ b/spec/unit/http/basic_client_spec.rb @@ -29,6 +29,26 @@ describe "HTTP Connection" do end end + describe "#initialize" do + it "calls .start when doing keepalives" do + basic_client = Chef::HTTP::BasicClient.new(uri, keepalives: true) + expect(basic_client).to receive(:configure_ssl) + net_http_mock = instance_double(Net::HTTP, proxy_address: nil, "proxy_port=" => nil, "read_timeout=" => nil, "open_timeout=" => nil) + expect(net_http_mock).to receive(:start).and_return(net_http_mock) + expect(Net::HTTP).to receive(:new).and_return(net_http_mock) + expect(basic_client.http_client).to eql(net_http_mock) + end + + it "does not call .start when not doing keepalives" do + basic_client = Chef::HTTP::BasicClient.new(uri) + expect(basic_client).to receive(:configure_ssl) + net_http_mock = instance_double(Net::HTTP, proxy_address: nil, "proxy_port=" => nil, "read_timeout=" => nil, "open_timeout=" => nil) + expect(net_http_mock).not_to receive(:start) + expect(Net::HTTP).to receive(:new).and_return(net_http_mock) + expect(basic_client.http_client).to eql(net_http_mock) + end + end + describe "#build_http_client" do it "should build an http client" do subject.build_http_client diff --git a/spec/unit/http_spec.rb b/spec/unit/http_spec.rb index 69e59ecaf8..d99f03aac9 100644 --- a/spec/unit/http_spec.rb +++ b/spec/unit/http_spec.rb @@ -43,6 +43,20 @@ describe Chef::HTTP do end + describe "#intialize" do + it "accepts a keepalive option and passes it to the http_client" do + http = Chef::HTTP.new(uri, keepalives: true) + expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, keepalives: true).and_call_original + expect(http.http_client).to be_a_kind_of(Chef::HTTP::BasicClient) + end + + it "the default is not to use keepalives" do + http = Chef::HTTP.new(uri) + expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, keepalives: false).and_call_original + expect(http.http_client).to be_a_kind_of(Chef::HTTP::BasicClient) + end + end + describe "create_url" do it "should return a correctly formatted url 1/3 CHEF-5261" do |