Add timeouts option

Doesn't affect watch, only rest-client based functionality.
ManageIQ · Apr 28, 2017 · baaa92d · baaa92d
1 parent 991871e
commit baaa92d
Show file tree

Hide file tree

Showing 3 changed files with 130 additions and 2 deletions.
diff --git a/README.md b/README.md
@@ -51,6 +51,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
@@ -85,6 +87,8 @@ client = Kubeclient::Client.new 'https://localhost:8443/api/' , 'v1',
                                 ssl_options: ssl_options
 ```
 
+### 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:
@@ -135,6 +139,8 @@ client = Kubeclient::Client.new 'https://localhost:8443/api/' , 'v1',
 
 You can find information about token 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:
 
@@ -148,14 +154,40 @@ client = Kubeclient::Client.new 'https://localhost:8443/api/' , 'v1',
                                 socket_options: socket_options
 ```
 
-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)
 client = Kubeclient::Client.new('https://localhost:8443/api/',
                                 :http_proxy_uri => proxy_uri)
 ```
 
+
+### 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.

diff --git a/lib/kubeclient/common.rb b/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]
@@ -215,7 +225,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
@@ -420,6 +432,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

diff --git a/test/test_kubeclient.rb b/test/test_kubeclient.rb
@@ -103,6 +103,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(KubeException) { 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'))
@@ -627,6 +636,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