From 778e57e13faecad92e48b2bbd2e9aa6181081931 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 3 Oct 2016 14:49:19 -0400 Subject: [PATCH] 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 | 26 +++++++++++++++++++ pkg/client/restclient/config.go | 10 +++++++ .../unversioned/clientcmd/client_config.go | 1 + pkg/client/unversioned/clientcmd/overrides.go | 23 +++++++++++++++- 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 53c57f75402a..e004d62c8bf1 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -1033,6 +1033,32 @@ __EOF__ # Clean up kubectl delete deployment nginx "${kube_flags[@]}" + ################## + # 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=1s) + 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=1s --watch 2>&1) + kube::test::if_has_string "${output_message}" 'Timeout exceeded while reading body' + + ## check argument validation for --request-timeout flag + output_message=$(! kubectl get pod valid-pod --request-timeout=1) + kube::test::if_has_string "${output_message}" 'invalid argument "1"' + ############### # Kubectl get # ############### diff --git a/pkg/client/restclient/config.go b/pkg/client/restclient/config.go index ae1c8e7b77fd..c8f6a0ba4885 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 diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index 8223bad94efa..68f3a2dbfe77 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -108,6 +108,7 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { clientConfig := &restclient.Config{} clientConfig.Host = configClusterInfo.Server + clientConfig.Timeout = config.overrides.Timeout 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..81a79d39ec0f 100644 --- a/pkg/client/unversioned/clientcmd/overrides.go +++ b/pkg/client/unversioned/clientcmd/overrides.go @@ -18,6 +18,7 @@ package clientcmd import ( "strconv" + "time" "github.com/spf13/pflag" @@ -33,6 +34,7 @@ type ConfigOverrides struct { ClusterInfo clientcmdapi.Cluster Context clientcmdapi.Context CurrentContext string + Timeout time.Duration } // ConfigOverrideFlags holds the flag names to be used for binding command line flags. Notice that this structure tightly @@ -42,6 +44,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 @@ -105,6 +108,20 @@ func (f FlagInfo) BindBoolFlag(flags *pflag.FlagSet, target *bool) { } } +// BindDurationFlag binds the flag based on the provided info. If LongName == "", nothing is registered +func (f FlagInfo) BindDurationFlag(flags *pflag.FlagSet, target *time.Duration) { + // you can't register a flag without a long name + if len(f.LongName) > 0 { + // try to parse Default as a Duration. If it fails, default to 0s + durationVal, err := time.ParseDuration(f.Default) + if err != nil { + durationVal = 0 + } + + flags.DurationVarP(target, f.LongName, f.ShortName, durationVal, f.Description) + } +} + const ( FlagClusterName = "cluster" FlagAuthInfoName = "user" @@ -121,6 +138,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 +169,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, "", "", "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 +210,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.BindDurationFlag(flags, &overrides.Timeout) } // BindFlags is a convenience method to bind the specified flags to their associated variables