Merge pull request #244 from cben/timeout

Add requests timeouts option

.rubocop.yml

@@ -9,14 +9,17 @@ Metrics/AbcSize:
 Metrics/LineLength:
   Max: 100
 Metrics/ParameterLists:
-  Max: 8
+  Max: 5
+  CountKeywordArgs: false
 Metrics/CyclomaticComplexity:
   Max: 8
 Metrics/ModuleLength:
   Enabled: false
 Style/MethodCallWithArgsParentheses:
   Enabled: true
   IgnoredMethods: [require, raise, include, attr_reader, refute, assert]
-  Exclude: [Gemfile, Rakefile, kubeclient.gemspec]
+  Exclude: [Gemfile*, Rakefile, kubeclient.gemspec]
+Style/FileName:
+  Exclude: [Gemfile*]
 Security/MarshalLoad:
   Exclude: [test/**/*]

.travis.yml

@@ -4,9 +4,17 @@ rvm:
   - "2.1"
   - "2.2"
   - "2.3.0"
+gemfile:
+  - Gemfile
+  - Gemfile-rest-client-1.8.rb
 sudo: false
 cache: bundler
 script: bundle exec rake $TASK
 env:
  - TASK=test
  - TASK=rubocop
+matrix:
+  exclude:
+    # No point running rubocop with old rest-client
+    - gemfile: Gemfile-rest-client-1.8.rb
+      env: TASK=rubocop

Gemfile-rest-client-1.8.rb

@@ -0,0 +1,11 @@
+# For travis to additionally test rest-client 1.x.
+
+source 'https://rubygems.org'
+
+# Specify your gem's dependencies in kubeclient.gemspec
+gemspec
+
+if dependencies.any? # needed for overriding with recent bundler (1.13 ?)
+  dependencies.delete('rest-client')
+  gem 'rest-client', '= 1.8.0'
+end

README.md

@@ -57,6 +57,8 @@ uri = URI::HTTP.build(host: "somehostname", port: 8080)
 client = Kubeclient::Client.new(uri)
 ```
 
+### SSL
+
 It is also possible to use https and configure ssl with:
 
 ```ruby
@@ -94,6 +96,8 @@ client = Kubeclient::Client.new(
 )
 ```
 
+### Authentication
+
 If you are using basic authentication or bearer tokens as described
 [here](https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/authentication.md) then you can specify one
 of the following:
@@ -148,6 +152,8 @@ client = Kubeclient::Client.new(
 
 You can find information about tokens in [this guide](http://kubernetes.io/docs/user-guide/accessing-the-cluster/) and in [this reference](http://kubernetes.io/docs/admin/authentication/).
 
+### Non-blocking IO
+
 You can also use kubeclient with non-blocking sockets such as Celluloid::IO, see [here](https://github.com/httprb/http/wiki/Parallel-requests-with-Celluloid%3A%3AIO)
 for details. For example:
 
@@ -162,7 +168,11 @@ client = Kubeclient::Client.new(
 )
 ```
 
-You can also use kubeclient with an http proxy server such as tinyproxy. It can be entered as a string or a URI object
+This affects only `.watch_*` sockets, not one-off actions like `.get_*`, `.delete_*` etc.
+
+### Proxies
+
+You can also use kubeclient with an http proxy server such as tinyproxy. It can be entered as a string or a URI object.
 For example:
 ```ruby
 proxy_uri = URI::HTTP.build(host: "myproxyhost", port: 8443)
@@ -171,6 +181,28 @@ client = Kubeclient::Client.new(
 )
 ```
 
+
+### Timeouts
+
+Watching never times out.
+
+One-off actions like `.get_*`, `.delete_*` have a configurable timeout:
+```ruby
+timeouts = {
+  open: 10,  # unit is seconds
+  read: nil  # nil means never time out
+}
+client = Kubeclient::Client.new(
+  'https://localhost:8443/api/', timeouts: timeouts
+)
+```
+
+Default timeouts match `Net::HTTP` and `RestClient`, which unfortunately depends on ruby version:
+- open was infinite up to ruby 2.2, 60 seconds in 2.3+.
+- read is 60 seconds.
+
+If you want ruby-independent behavior, always specify `:open`.
+
 ### Discovery
 
 Discovery from the kube-apiserver is done lazily on method calls so it would not change behavior.
@@ -287,7 +319,7 @@ service.metadata = {}
 service.metadata.name = "redis-master"
 service.metadata.namespace = 'staging'
 service.spec = {}
-service.spec.ports = [{ 
+service.spec.ports = [{
   'port' => 6379,
   'targetPort' => 'redis-server'
 }]

lib/kubeclient/common.rb

@@ -26,6 +26,12 @@ module ClientMixin
       ssl_socket_class: nil
     }.freeze
 
+    DEFAULT_TIMEOUTS = {
+      # These do NOT affect watch, watching never times out.
+      open: Net::HTTP.new('127.0.0.1').open_timeout, # depends on ruby version
+      read: Net::HTTP.new('127.0.0.1').read_timeout
+    }.freeze
+
     DEFAULT_HTTP_PROXY_URI = nil
 
     SEARCH_ARGUMENTS = {
@@ -50,6 +56,7 @@ def initialize_client(
       ssl_options: DEFAULT_SSL_OPTIONS,
       auth_options: DEFAULT_AUTH_OPTIONS,
       socket_options: DEFAULT_SOCKET_OPTIONS,
+      timeouts: DEFAULT_TIMEOUTS,
       http_proxy_uri: DEFAULT_HTTP_PROXY_URI
     )
       validate_auth_options(auth_options)
@@ -63,6 +70,9 @@ def initialize_client(
       @ssl_options = ssl_options
       @auth_options = auth_options
       @socket_options = socket_options
+      # Allow passing partial timeouts hash, without unspecified
+      # @timeouts[:foo] == nil resulting in infinite timeout.
+      @timeouts = DEFAULT_TIMEOUTS.merge(timeouts)
       @http_proxy_uri = http_proxy_uri.to_s if http_proxy_uri
 
       if auth_options[:bearer_token]
@@ -216,7 +226,9 @@ def create_rest_client(path = nil)
         ssl_client_key: @ssl_options[:client_key],
         proxy: @http_proxy_uri,
         user: @auth_options[:username],
-        password: @auth_options[:password]
+        password: @auth_options[:password],
+        open_timeout: @timeouts[:open],
+        ClientMixin.restclient_read_timeout_option => @timeouts[:read]
       }
       RestClient::Resource.new(@api_endpoint.merge(path).to_s, options)
     end
@@ -417,6 +429,18 @@ def api
       JSON.parse(response)
     end
 
+    def self.restclient_read_timeout_option
+      @restclient_read_timeout_option ||=
+        # RestClient silently accepts unknown options, so check accessors instead.
+        if RestClient::Resource.instance_methods.include?(:read_timeout) # rest-client 2.0
+          :read_timeout
+        elsif RestClient::Resource.instance_methods.include?(:timeout) # rest-client 1.x
+          :timeout
+        else
+          raise ArgumentError("RestClient doesn't support neither :read_timeout nor :timeout")
+        end
+    end
+
     private
 
     def load_entities

test/test_kubeclient.rb

@@ -116,6 +116,15 @@ def test_api_ssl_failure
     assert_equal(error_message, exception.message)
   end
 
+  def test_api_timeout
+    stub_request(:get, 'http://localhost:8080/api').to_timeout
+
+    client = Kubeclient::Client.new('http://localhost:8080/api/')
+
+    exception = assert_raises(Kubeclient::HttpError) { client.api }
+    assert_match(/(timed out|timeout)/i, exception.message)
+  end
+
   def test_api_valid
     stub_request(:get, 'http://localhost:8080/api')
       .to_return(status: 200, body: open_test_file('versions_list.json'))
@@ -613,6 +622,69 @@ def test_nil_items
     client.get_persistent_volume_claims
   end
 
+  # Timeouts
+
+  def test_timeouts_defaults
+    client = Kubeclient::Client.new(
+      'http://localhost:8080/api/'
+    )
+    rest_client = client.rest_client
+    assert_default_open_timeout(rest_client.open_timeout)
+    assert_equal(60, read_timeout(rest_client))
+  end
+
+  def test_timeouts_open
+    client = Kubeclient::Client.new(
+      'http://localhost:8080/api/',
+      timeouts: { open: 10 }
+    )
+    rest_client = client.rest_client
+    assert_equal(10, rest_client.open_timeout)
+    assert_equal(60, read_timeout(rest_client))
+  end
+
+  def test_timeouts_read
+    client = Kubeclient::Client.new(
+      'http://localhost:8080/api/',
+      timeouts: { read: 300 }
+    )
+    rest_client = client.rest_client
+    assert_default_open_timeout(rest_client.open_timeout)
+    assert_equal(300, read_timeout(rest_client))
+  end
+
+  def test_timeouts_both
+    client = Kubeclient::Client.new(
+      'http://localhost:8080/api/',
+      timeouts: { open: 10, read: 300 }
+    )
+    rest_client = client.rest_client
+    assert_equal(10, rest_client.open_timeout)
+    assert_equal(300, read_timeout(rest_client))
+  end
+
+  def test_timeouts_infinite
+    client = Kubeclient::Client.new(
+      'http://localhost:8080/api/',
+      timeouts: { open: nil, read: nil }
+    )
+    rest_client = client.rest_client
+    assert_nil(rest_client.open_timeout)
+    assert_nil(read_timeout(rest_client))
+  end
+
+  def read_timeout(rest_client)
+    rest_client.send(Kubeclient::ClientMixin.restclient_read_timeout_option)
+  end
+
+  def assert_default_open_timeout(actual)
+    if RUBY_VERSION >= '2.3'
+      assert_equal(60, actual)
+    else
+      assert_nil(actual)
+    end
+  end
+
   private
 
   # dup method creates a shallow copy which is not good in this case