From fd087caadb7cec7e7d81633a73c8933b94231293 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 3 Oct 2016 14:49:19 -0400 Subject: [PATCH 1/2] Add global timeout flag This patch adds a global timeout flag (viewable with `kubectl -h`) with a default value of `0s` (meaning no timeout). The timeout value is added to the default http client, so that zero values and default behavior are enforced by the client. **Example** ``` $ kubectl get pods # no timeout flag set - default to 0s (which means no timeout) NAME READY STATUS RESTARTS AGE docker-registry-1-h7etw 1/1 Running 1 2h router-1-uv0f9 1/1 Running 1 2h $ kubectl get pods --timeout=0 # zero means no timeout no timeout flag set NAME READY STATUS RESTARTS AGE docker-registry-1-h7etw 1/1 Running 1 2h router-1-uv0f9 1/1 Running 1 2h $kubectl get pods --timeout=1ms Unable to connect to the server: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers) ``` --- hack/make-rules/test-cmd.sh | 33 +++++++++++++++++++ pkg/client/restclient/config.go | 11 +++++++ .../unversioned/clientcmd/client_config.go | 14 ++++++++ pkg/client/unversioned/clientcmd/overrides.go | 8 ++++- 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 6ab1e26edf79..818c1e459bfd 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -1184,6 +1184,39 @@ __EOF__ kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/replicasets 200 OK" + ################## + # Global timeout # + ################## + + ### Test global request timeout option + # Pre-condition: no POD exists + create_and_use_new_namespace + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' + # Command + kubectl create "${kube_flags[@]}" -f test/fixtures/doc-yaml/admin/limitrange/valid-pod.yaml + # Post-condition: valid-pod POD is created + kubectl get "${kube_flags[@]}" pods -o json + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' + + ## check --request-timeout on 'get pod' + output_message=$(kubectl get pod valid-pod --request-timeout=1) + kube::test::if_has_string "${output_message}" 'valid-pod' + + ## check --request-timeout on 'get pod' with --watch + output_message=$(kubectl get pod valid-pod --request-timeout=1 --watch 2>&1) + kube::test::if_has_string "${output_message}" 'Timeout exceeded while reading body' + + ## check --request-timeout value with no time unit + output_message=$(kubectl get pod valid-pod --request-timeout=1 2>&1) + kube::test::if_has_string "${output_message}" 'valid-pod' + + ## check --request-timeout value with invalid time unit + output_message=$(! kubectl get pod valid-pod --request-timeout="1p" 2>&1) + kube::test::if_has_string "${output_message}" 'Invalid value for option' + + # cleanup + kubectl delete pods valid-pod "${kube_flags[@]}" + ##################################### # Third Party Resources # ##################################### diff --git a/pkg/client/restclient/config.go b/pkg/client/restclient/config.go index 358ddac8a093..e739d796afbf 100644 --- a/pkg/client/restclient/config.go +++ b/pkg/client/restclient/config.go @@ -25,6 +25,7 @@ import ( "path" gruntime "runtime" "strings" + "time" "github.com/golang/glog" @@ -109,6 +110,9 @@ type Config struct { // Rate limiter for limiting connections to the master from this client. If present overwrites QPS/Burst RateLimiter flowcontrol.RateLimiter + // The maximum length of time to wait before giving up on a server request. A value of zero means no timeout. + Timeout time.Duration + // Version forces a specific version to be used (if registered) // Do we need this? // Version string @@ -185,6 +189,9 @@ func RESTClientFor(config *Config) (*RESTClient, error) { var httpClient *http.Client if transport != http.DefaultTransport { httpClient = &http.Client{Transport: transport} + if config.Timeout > 0 { + httpClient.Timeout = config.Timeout + } } return NewRESTClient(baseURL, versionedAPIPath, config.ContentConfig, qps, burst, config.RateLimiter, httpClient) @@ -210,6 +217,9 @@ func UnversionedRESTClientFor(config *Config) (*RESTClient, error) { var httpClient *http.Client if transport != http.DefaultTransport { httpClient = &http.Client{Transport: transport} + if config.Timeout > 0 { + httpClient.Timeout = config.Timeout + } } versionConfig := config.ContentConfig @@ -353,5 +363,6 @@ func AnonymousClientConfig(config *Config) *Config { WrapTransport: config.WrapTransport, QPS: config.QPS, Burst: config.Burst, + Timeout: config.Timeout, } } diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index 5eebc02b18db..aa9996d8b854 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -27,6 +27,9 @@ import ( "github.com/golang/glog" "github.com/imdario/mergo" + "strconv" + "time" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/restclient" clientauth "k8s.io/kubernetes/pkg/client/unversioned/auth" @@ -124,6 +127,17 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { clientConfig := &restclient.Config{} clientConfig.Host = configClusterInfo.Server + + if len(config.overrides.Timeout) > 0 { + if i, err := strconv.ParseInt(config.overrides.Timeout, 10, 64); err == nil && i >= 0 { + clientConfig.Timeout = time.Duration(i) * time.Second + } else if requestTimeout, err := time.ParseDuration(config.overrides.Timeout); err == nil { + clientConfig.Timeout = requestTimeout + } else { + return nil, fmt.Errorf("Invalid value for option '--request-timeout'. Value must be a single integer, or an integer followed by a corresponding time unit (e.g. 1s | 2m | 3h)") + } + } + if u, err := url.ParseRequestURI(clientConfig.Host); err == nil && u.Opaque == "" && len(u.Path) > 1 { u.RawQuery = "" u.Fragment = "" diff --git a/pkg/client/unversioned/clientcmd/overrides.go b/pkg/client/unversioned/clientcmd/overrides.go index 9c117ea3548e..626cdeaae42d 100644 --- a/pkg/client/unversioned/clientcmd/overrides.go +++ b/pkg/client/unversioned/clientcmd/overrides.go @@ -33,6 +33,7 @@ type ConfigOverrides struct { ClusterInfo clientcmdapi.Cluster Context clientcmdapi.Context CurrentContext string + Timeout string } // ConfigOverrideFlags holds the flag names to be used for binding command line flags. Notice that this structure tightly @@ -42,6 +43,7 @@ type ConfigOverrideFlags struct { ClusterOverrideFlags ClusterOverrideFlags ContextOverrideFlags ContextOverrideFlags CurrentContext FlagInfo + Timeout FlagInfo } // AuthOverrideFlags holds the flag names to be used for binding command line flags for AuthInfo objects @@ -121,6 +123,7 @@ const ( FlagImpersonate = "as" FlagUsername = "username" FlagPassword = "password" + FlagTimeout = "request-timeout" ) // RecommendedAuthOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing @@ -151,7 +154,9 @@ func RecommendedConfigOverrideFlags(prefix string) ConfigOverrideFlags { AuthOverrideFlags: RecommendedAuthOverrideFlags(prefix), ClusterOverrideFlags: RecommendedClusterOverrideFlags(prefix), ContextOverrideFlags: RecommendedContextOverrideFlags(prefix), - CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, + + CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, + Timeout: FlagInfo{prefix + FlagTimeout, "", "0", "The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests."}, } } @@ -190,6 +195,7 @@ func BindOverrideFlags(overrides *ConfigOverrides, flags *pflag.FlagSet, flagNam BindClusterFlags(&overrides.ClusterInfo, flags, flagNames.ClusterOverrideFlags) BindContextFlags(&overrides.Context, flags, flagNames.ContextOverrideFlags) flagNames.CurrentContext.BindStringFlag(flags, &overrides.CurrentContext) + flagNames.Timeout.BindStringFlag(flags, &overrides.Timeout) } // BindFlags is a convenience method to bind the specified flags to their associated variables From 5dad125d48c21a9a644c5679dd333520296f44e8 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Fri, 7 Oct 2016 19:12:17 -0400 Subject: [PATCH 2/2] bump(k8s.io/client-go): add Timeout field to 1.5 restclient --- staging/src/k8s.io/client-go/1.5/rest/config.go | 4 ++++ .../client-go/1.5/tools/clientcmd/client_config.go | 14 ++++++++++++++ .../client-go/1.5/tools/clientcmd/overrides.go | 8 +++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/client-go/1.5/rest/config.go b/staging/src/k8s.io/client-go/1.5/rest/config.go index 5c5e1e038126..41609617125e 100644 --- a/staging/src/k8s.io/client-go/1.5/rest/config.go +++ b/staging/src/k8s.io/client-go/1.5/rest/config.go @@ -25,6 +25,7 @@ import ( "path" gruntime "runtime" "strings" + "time" "github.com/golang/glog" @@ -109,6 +110,9 @@ type Config struct { // Rate limiter for limiting connections to the master from this client. If present overwrites QPS/Burst RateLimiter flowcontrol.RateLimiter + // The maximum length of time to wait before giving up on a server request. A value of zero means no timeout. + Timeout time.Duration + // Version forces a specific version to be used (if registered) // Do we need this? // Version string diff --git a/staging/src/k8s.io/client-go/1.5/tools/clientcmd/client_config.go b/staging/src/k8s.io/client-go/1.5/tools/clientcmd/client_config.go index a8b217683319..1b30022163bf 100644 --- a/staging/src/k8s.io/client-go/1.5/tools/clientcmd/client_config.go +++ b/staging/src/k8s.io/client-go/1.5/tools/clientcmd/client_config.go @@ -27,6 +27,9 @@ import ( "github.com/golang/glog" "github.com/imdario/mergo" + "strconv" + "time" + "k8s.io/client-go/1.5/pkg/api" "k8s.io/client-go/1.5/rest" clientauth "k8s.io/client-go/1.5/tools/auth" @@ -108,6 +111,17 @@ func (config *DirectClientConfig) ClientConfig() (*rest.Config, error) { clientConfig := &rest.Config{} clientConfig.Host = configClusterInfo.Server + + if len(config.overrides.Timeout) > 0 { + if i, err := strconv.ParseInt(config.overrides.Timeout, 10, 64); err == nil && i >= 0 { + clientConfig.Timeout = time.Duration(i) * time.Second + } else if requestTimeout, err := time.ParseDuration(config.overrides.Timeout); err == nil { + clientConfig.Timeout = requestTimeout + } else { + return nil, fmt.Errorf("Invalid value for option '--request-timeout'. Value must be a single integer, or an integer followed by a corresponding time unit (e.g. 1s | 2m | 3h)") + } + } + if u, err := url.ParseRequestURI(clientConfig.Host); err == nil && u.Opaque == "" && len(u.Path) > 1 { u.RawQuery = "" u.Fragment = "" diff --git a/staging/src/k8s.io/client-go/1.5/tools/clientcmd/overrides.go b/staging/src/k8s.io/client-go/1.5/tools/clientcmd/overrides.go index 304242f606b8..c81a7dd50cd3 100644 --- a/staging/src/k8s.io/client-go/1.5/tools/clientcmd/overrides.go +++ b/staging/src/k8s.io/client-go/1.5/tools/clientcmd/overrides.go @@ -33,6 +33,7 @@ type ConfigOverrides struct { ClusterInfo clientcmdapi.Cluster Context clientcmdapi.Context CurrentContext string + Timeout string } // ConfigOverrideFlags holds the flag names to be used for binding command line flags. Notice that this structure tightly @@ -42,6 +43,7 @@ type ConfigOverrideFlags struct { ClusterOverrideFlags ClusterOverrideFlags ContextOverrideFlags ContextOverrideFlags CurrentContext FlagInfo + Timeout FlagInfo } // AuthOverrideFlags holds the flag names to be used for binding command line flags for AuthInfo objects @@ -121,6 +123,7 @@ const ( FlagImpersonate = "as" FlagUsername = "username" FlagPassword = "password" + FlagTimeout = "request-timeout" ) // RecommendedAuthOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing @@ -151,7 +154,9 @@ func RecommendedConfigOverrideFlags(prefix string) ConfigOverrideFlags { AuthOverrideFlags: RecommendedAuthOverrideFlags(prefix), ClusterOverrideFlags: RecommendedClusterOverrideFlags(prefix), ContextOverrideFlags: RecommendedContextOverrideFlags(prefix), - CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, + + CurrentContext: FlagInfo{prefix + FlagContext, "", "", "The name of the kubeconfig context to use"}, + Timeout: FlagInfo{prefix + FlagTimeout, "", "0", "The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests."}, } } @@ -190,6 +195,7 @@ func BindOverrideFlags(overrides *ConfigOverrides, flags *pflag.FlagSet, flagNam BindClusterFlags(&overrides.ClusterInfo, flags, flagNames.ClusterOverrideFlags) BindContextFlags(&overrides.Context, flags, flagNames.ContextOverrideFlags) flagNames.CurrentContext.BindStringFlag(flags, &overrides.CurrentContext) + flagNames.Timeout.BindStringFlag(flags, &overrides.Timeout) } // BindFlags is a convenience method to bind the specified flags to their associated variables