Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add global timeout flag #33958

Merged
merged 2 commits into from Oct 15, 2016
Merged

Add global timeout flag #33958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd087ca
Add global timeout flag
juanvallejo Oct 3, 2016
5dad125
bump(k8s.io/client-go): add Timeout field to 1.5 restclient
juanvallejo Oct 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions hack/make-rules/test-cmd.sh
View file
Open in desktop
Expand Up @@ -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 #
#####################################
Expand Down
11 changes: 11 additions & 0 deletions pkg/client/restclient/config.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (
"path"
gruntime "runtime"
"strings"
"time"

"github.com/golang/glog"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -353,5 +363,6 @@ func AnonymousClientConfig(config *Config) *Config {
WrapTransport: config.WrapTransport,
QPS: config.QPS,
Burst: config.Burst,
Timeout: config.Timeout,
}
}
14 changes: 14 additions & 0 deletions pkg/client/unversioned/clientcmd/client_config.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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 = ""
Expand Down
8 changes: 7 additions & 1 deletion pkg/client/unversioned/clientcmd/overrides.go
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -121,6 +123,7 @@ const (
FlagImpersonate = "as"
FlagUsername = "username"
FlagPassword = "password"
FlagTimeout = "request-timeout"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kubernetes/kubectl Any comments on the flag name? --timeout was already take by commands that use reapers for a different meaning.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request-timeout sounds good to me.

)

// RecommendedAuthOverrideFlags is a convenience method to return recommended flag names prefixed with a string of your choosing
Expand Down Expand Up @@ -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."},
}
}

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions staging/src/k8s.io/client-go/1.5/rest/config.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (
"path"
gruntime "runtime"
"strings"
"time"

"github.com/golang/glog"

Expand Down Expand Up @@ -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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about actually setting this field like you do in the core k8s code? I'd expect this patch looks identical to the other config.go file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, will do right now

// Version forces a specific version to be used (if registered)
// Do we need this?
// Version string
Expand Down
14 changes: 14 additions & 0 deletions staging/src/k8s.io/client-go/1.5/tools/clientcmd/client_config.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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 = ""
Expand Down
8 changes: 7 additions & 1 deletion staging/src/k8s.io/client-go/1.5/tools/clientcmd/overrides.go
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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."},
}
}

Expand Down Expand Up @@ -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
Expand Down