这是indexloc提供的服务,不要输入任何密码
Skip to content

Make exec timeout configurable #803

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.

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

Merged
merged 7 commits into from
Jan 18, 2020

Conversation

cwdsuzhou
Copy link
Contributor

Make exec timeout configurable

See
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/apis/config/types.go#L165
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/kubelet.go#L2296

kubelet also allows to change default steamIdleTimeout, so I think it's to reasonable to make it configurable

Fix: #791

@cwdsuzhou cwdsuzhou requested a review from cpuguy83 January 15, 2020 00:34
@cwdsuzhou
Copy link
Contributor Author

@cpuguy83 PTAL

@cpuguy83
Copy link
Contributor

Thanks!

One thing I'd like is if this can be changed so we aren't breaking callers of these functions.
Maybe we can have it accept something like a functional options ... func(*Config)

@cwdsuzhou
Copy link
Contributor Author

Thanks!

One thing I'd like is if this can be changed so we aren't breaking callers of these functions.
Maybe we can have it accept something like a functional options ... func(*Config)

I have updated the PR, thanks

@@ -39,7 +39,7 @@ type PodHandlerConfig struct {
}

// PodHandler creates an http handler for interacting with pods/containers.
func PodHandler(p PodHandlerConfig, debug bool) http.Handler {
func PodHandler(p PodHandlerConfig, debug bool, cfg ...StreamTimeOutConfig) http.Handler {
Copy link
Contributor

@cpuguy83 cpuguy83 Jan 16, 2020

Choose a reason for hiding this comment

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

Sorry I was thinking something like functional options so ...func(*Config) And then you can provide a list of functions to set the config... e.g. WithStreamIdleTimeout(dur).

But looking at the underlying code again, I think we could probably just add a stream config to the PodHandlerConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks

Co-Authored-By: Brian Goff <cpuguy83@gmail.com>
@cwdsuzhou
Copy link
Contributor Author

Updated

Copy link
Contributor

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you!

@cpuguy83 cpuguy83 merged commit 0bdf742 into virtual-kubelet:master Jan 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kubectl exec api conn idleTimeout 30s too short.
2 participants