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
Add timeout argument to ExecInContainer #33366
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ const ( | |
type Executor interface { | ||
// ExecInContainer executes a command in a container in the pod, copying data | ||
// between in/out/err and the container's stdin/stdout/stderr. | ||
ExecInContainer(name string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool, resize <-chan term.Size) error | ||
ExecInContainer(name string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool, resize <-chan term.Size, timeout time.Duration) error | ||
} | ||
|
||
// ServeExec handles requests to execute a command in a container. After | ||
|
@@ -56,7 +56,7 @@ func ServeExec(w http.ResponseWriter, req *http.Request, executor Executor, podN | |
|
||
cmd := req.URL.Query()[api.ExecCommandParamm] | ||
|
||
err := executor.ExecInContainer(podName, uid, container, cmd, ctx.stdinStream, ctx.stdoutStream, ctx.stderrStream, ctx.tty, ctx.resizeChan) | ||
err := executor.ExecInContainer(podName, uid, container, cmd, ctx.stdinStream, ctx.stdoutStream, ctx.stderrStream, ctx.tty, ctx.resizeChan, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you wanted to support it, you would probably want to add it as a new request parameter and have the client send the desired timeout. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll leave that to an independent feature request and keep this focused on the exec probes. |
||
if err != nil { | ||
if exitErr, ok := err.(utilexec.ExitError); ok && exitErr.Exited() { | ||
rc := exitErr.ExitStatus() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @yifan-gu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am OK with this if we have a plan on how to actually implement the timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For rkt it is possible to implement the timeout today -- refer to #27956, in particular 657cb46
Same for the nsenter backend: d82bc85