Merge pull request #33958 from juanvallejo/jvallejo/add-global-timeout-flag

Automatic merge from submit-queue

Add global timeout flag

**Release note**:
```release-note
Add a new global option "--request-timeout" to the `kubectl` client
```

UPSTREAM: https://github.com/kubernetes/client-go/pull/10

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.

Adding a global timeout ensures that user-made scripts won't hang for an
indefinite amount of time while performing remote calls (right now, remote
calls are re-tried up to 10 times when each attempt fails, however, there is
no option to set a timeout in order to prevent any of these 10 attempts from
hanging indefinitely).

**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 --request-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 --request-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

@@ -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             #
   #####################################

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,
 	}
 }

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 = ""

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

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

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 = ""

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