diff --git a/cmd/hyperkube/kubectl.go b/cmd/hyperkube/kubectl.go index 7e76bd3fde..068b2781e0 100644 --- a/cmd/hyperkube/kubectl.go +++ b/cmd/hyperkube/kubectl.go @@ -17,15 +17,14 @@ limitations under the License. package main import ( - "github.com/docker/docker/pkg/term" + "os" + "k8s.io/kubernetes/pkg/kubectl/cmd" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" ) func NewKubectlServer() *Server { - // need to use term.StdStreams to get the right IO refs on Windows - stdin, stdout, stderr := term.StdStreams() - cmd := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), stdin, stdout, stderr) + cmd := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr) localFlags := cmd.LocalFlags() localFlags.SetInterspersed(false) diff --git a/cmd/kubectl/app/kubectl.go b/cmd/kubectl/app/kubectl.go index 455ae55139..27cbfd83b5 100644 --- a/cmd/kubectl/app/kubectl.go +++ b/cmd/kubectl/app/kubectl.go @@ -17,7 +17,7 @@ limitations under the License. package app import ( - "github.com/docker/docker/pkg/term" + "os" "k8s.io/kubernetes/pkg/kubectl/cmd" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -28,8 +28,6 @@ WARNING: this logic is duplicated, with minor changes, in cmd/hyperkube/kubectl. Any salient changes here will need to be manually reflected in that file. */ func Run() error { - // need to use term.StdStreams to get the right IO refs on Windows - stdin, stdout, stderr := term.StdStreams() - cmd := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), stdin, stdout, stderr) + cmd := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr) return cmd.Execute() } diff --git a/pkg/kubectl/cmd/attach.go b/pkg/kubectl/cmd/attach.go index 2a2c76ed18..437aa22831 100644 --- a/pkg/kubectl/cmd/attach.go +++ b/pkg/kubectl/cmd/attach.go @@ -32,7 +32,6 @@ import ( cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" remotecommandserver "k8s.io/kubernetes/pkg/kubelet/server/remotecommand" utilerrors "k8s.io/kubernetes/pkg/util/errors" - "k8s.io/kubernetes/pkg/util/interrupt" "k8s.io/kubernetes/pkg/util/term" ) @@ -51,9 +50,11 @@ var ( func NewCmdAttach(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *cobra.Command { options := &AttachOptions{ - In: cmdIn, - Out: cmdOut, - Err: cmdErr, + StreamOptions: StreamOptions{ + In: cmdIn, + Out: cmdOut, + Err: cmdErr, + }, Attach: &DefaultRemoteAttach{}, } @@ -100,19 +101,9 @@ func (*DefaultRemoteAttach) Attach(method string, url *url.URL, config *restclie // AttachOptions declare the arguments accepted by the Exec command type AttachOptions struct { - Namespace string - PodName string - ContainerName string - Stdin bool - TTY bool - CommandName string + StreamOptions - // InterruptParent, if set, is used to handle interrupts while attached - InterruptParent *interrupt.Handler - - In io.Reader - Out io.Writer - Err io.Writer + CommandName string Pod *api.Pod @@ -188,38 +179,30 @@ func (p *AttachOptions) Run() error { } pod := p.Pod - // ensure we can recover the terminal while attached - t := term.TTY{ - Parent: p.InterruptParent, - Out: p.Out, - } - // check for TTY - tty := p.TTY containerToAttach, err := p.containerToAttachTo(pod) if err != nil { return fmt.Errorf("cannot attach to the container: %v", err) } - if tty && !containerToAttach.TTY { - tty = false - fmt.Fprintf(p.Err, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) - } - if p.Stdin { - t.In = p.In - if tty && !t.IsTerminalIn() { - tty = false - fmt.Fprintln(p.Err, "Unable to use a TTY - input is not a terminal or the right kind of file") + if p.TTY && !containerToAttach.TTY { + p.TTY = false + if p.Err != nil { + fmt.Fprintf(p.Err, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) } - } else { - p.In = nil + } else if !p.TTY && containerToAttach.TTY { + // the container was launched with a TTY, so we have to force a TTY here, otherwise you'll get + // an error "Unrecognized input header" + p.TTY = true } - t.Raw = tty + + // ensure we can recover the terminal while attached + t := p.setupTTY() // save p.Err so we can print the command prompt message below stderr := p.Err var sizeQueue term.TerminalSizeQueue - if tty { + if t.Raw { if size := t.GetSize(); size != nil { // fake resizing +1 and then back to normal so that attach-detach-reattach will result in the // screen being redrawn @@ -252,17 +235,17 @@ func (p *AttachOptions) Run() error { Stdin: p.Stdin, Stdout: p.Out != nil, Stderr: p.Err != nil, - TTY: tty, + TTY: t.Raw, }, api.ParameterCodec) - return p.Attach.Attach("POST", req.URL(), p.Config, p.In, p.Out, p.Err, tty, sizeQueue) + return p.Attach.Attach("POST", req.URL(), p.Config, p.In, p.Out, p.Err, t.Raw, sizeQueue) } if err := t.Safe(fn); err != nil { return err } - if p.Stdin && tty && pod.Spec.RestartPolicy == api.RestartPolicyAlways { + if p.Stdin && t.Raw && pod.Spec.RestartPolicy == api.RestartPolicyAlways { fmt.Fprintf(p.Out, "Session ended, resume using '%s %s -c %s -i -t' command when the pod is running\n", p.CommandName, pod.Name, containerToAttach.Name) } return nil diff --git a/pkg/kubectl/cmd/attach_test.go b/pkg/kubectl/cmd/attach_test.go index 19e287e460..fe43e90788 100644 --- a/pkg/kubectl/cmd/attach_test.go +++ b/pkg/kubectl/cmd/attach_test.go @@ -74,21 +74,21 @@ func TestPodAndContainerAttach(t *testing.T) { name: "no container, no flags", }, { - p: &AttachOptions{ContainerName: "bar"}, + p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}}, args: []string{"foo"}, expectedPod: "foo", expectedContainer: "bar", name: "container in flag", }, { - p: &AttachOptions{ContainerName: "initfoo"}, + p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "initfoo"}}, args: []string{"foo"}, expectedPod: "foo", expectedContainer: "initfoo", name: "init container in flag", }, { - p: &AttachOptions{ContainerName: "bar"}, + p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}}, args: []string{"foo", "-c", "wrong"}, expectError: true, name: "non-existing container in flag", @@ -187,11 +187,13 @@ func TestAttach(t *testing.T) { remoteAttach.err = fmt.Errorf("attach error") } params := &AttachOptions{ - ContainerName: test.container, - In: bufIn, - Out: bufOut, - Err: bufErr, - Attach: remoteAttach, + StreamOptions: StreamOptions{ + ContainerName: test.container, + In: bufIn, + Out: bufOut, + Err: bufErr, + }, + Attach: remoteAttach, } cmd := &cobra.Command{} if err := params.Complete(f, cmd, []string{"foo"}); err != nil { @@ -261,13 +263,15 @@ func TestAttachWarnings(t *testing.T) { bufIn := bytes.NewBuffer([]byte{}) ex := &fakeRemoteAttach{} params := &AttachOptions{ - ContainerName: test.container, - In: bufIn, - Out: bufOut, - Err: bufErr, - Stdin: test.stdin, - TTY: test.tty, - Attach: ex, + StreamOptions: StreamOptions{ + ContainerName: test.container, + In: bufIn, + Out: bufOut, + Err: bufErr, + Stdin: test.stdin, + TTY: test.tty, + }, + Attach: ex, } cmd := &cobra.Command{} if err := params.Complete(f, cmd, []string{"foo"}); err != nil { diff --git a/pkg/kubectl/cmd/exec.go b/pkg/kubectl/cmd/exec.go index 697b59595e..e498bcfc08 100644 --- a/pkg/kubectl/cmd/exec.go +++ b/pkg/kubectl/cmd/exec.go @@ -21,6 +21,7 @@ import ( "io" "net/url" + dockerterm "github.com/docker/docker/pkg/term" "github.com/golang/glog" "github.com/renstrom/dedent" "github.com/spf13/cobra" @@ -49,9 +50,11 @@ var ( func NewCmdExec(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *cobra.Command { options := &ExecOptions{ - In: cmdIn, - Out: cmdOut, - Err: cmdErr, + StreamOptions: StreamOptions{ + In: cmdIn, + Out: cmdOut, + Err: cmdErr, + }, Executor: &DefaultRemoteExecutor{}, } @@ -98,21 +101,28 @@ func (*DefaultRemoteExecutor) Execute(method string, url *url.URL, config *restc }) } -// ExecOptions declare the arguments accepted by the Exec command -type ExecOptions struct { +type StreamOptions struct { Namespace string PodName string ContainerName string Stdin bool TTY bool - Command []string - // InterruptParent, if set, is used to handle interrupts while attached InterruptParent *interrupt.Handler + In io.Reader + Out io.Writer + Err io.Writer - In io.Reader - Out io.Writer - Err io.Writer + // for testing + overrideStreams func() (io.ReadCloser, io.Writer, io.Writer) + isTerminalIn func(t term.TTY) bool +} + +// ExecOptions declare the arguments accepted by the Exec command +type ExecOptions struct { + StreamOptions + + Command []string Executor RemoteExecutor Client *client.Client @@ -177,6 +187,58 @@ func (p *ExecOptions) Validate() error { return nil } +func (o *StreamOptions) setupTTY() term.TTY { + t := term.TTY{ + Parent: o.InterruptParent, + Out: o.Out, + } + + if !o.Stdin { + // need to nil out o.In to make sure we don't create a stream for stdin + o.In = nil + o.TTY = false + return t + } + + t.In = o.In + if !o.TTY { + return t + } + + if o.isTerminalIn == nil { + o.isTerminalIn = func(tty term.TTY) bool { + return tty.IsTerminalIn() + } + } + if !o.isTerminalIn(t) { + o.TTY = false + + if o.Err != nil { + fmt.Fprintln(o.Err, "Unable to use a TTY - input is not a terminal or the right kind of file") + } + + return t + } + + // if we get to here, the user wants to attach stdin, wants a TTY, and o.In is a terminal, so we + // can safely set t.Raw to true + t.Raw = true + + if o.overrideStreams == nil { + // use dockerterm.StdStreams() to get the right I/O handles on Windows + o.overrideStreams = dockerterm.StdStreams + } + stdin, stdout, _ := o.overrideStreams() + o.In = stdin + t.In = stdin + if o.Out != nil { + o.Out = stdout + t.Out = stdout + } + + return t +} + // Run executes a validated remote execution against a pod. func (p *ExecOptions) Run() error { pod, err := p.Client.Pods(p.Namespace).Get(p.PodName) @@ -195,26 +257,10 @@ func (p *ExecOptions) Run() error { } // ensure we can recover the terminal while attached - t := term.TTY{ - Parent: p.InterruptParent, - Out: p.Out, - } - - // check for TTY - tty := p.TTY - if p.Stdin { - t.In = p.In - if tty && !t.IsTerminalIn() { - tty = false - fmt.Fprintln(p.Err, "Unable to use a TTY - input is not a terminal or the right kind of file") - } - } else { - p.In = nil - } - t.Raw = tty + t := p.setupTTY() var sizeQueue term.TerminalSizeQueue - if tty { + if t.Raw { // this call spawns a goroutine to monitor/update the terminal size sizeQueue = t.MonitorSize(t.GetSize()) @@ -237,10 +283,10 @@ func (p *ExecOptions) Run() error { Stdin: p.Stdin, Stdout: p.Out != nil, Stderr: p.Err != nil, - TTY: tty, + TTY: t.Raw, }, api.ParameterCodec) - return p.Executor.Execute("POST", req.URL(), p.Config, p.In, p.Out, p.Err, tty, sizeQueue) + return p.Executor.Execute("POST", req.URL(), p.Config, p.In, p.Out, p.Err, t.Raw, sizeQueue) } if err := t.Safe(fn); err != nil { diff --git a/pkg/kubectl/cmd/exec_test.go b/pkg/kubectl/cmd/exec_test.go index 0b9c33d670..3340aef029 100644 --- a/pkg/kubectl/cmd/exec_test.go +++ b/pkg/kubectl/cmd/exec_test.go @@ -20,9 +20,11 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "net/http" "net/url" "reflect" + "strings" "testing" "github.com/spf13/cobra" @@ -65,19 +67,19 @@ func TestPodAndContainer(t *testing.T) { name: "empty", }, { - p: &ExecOptions{PodName: "foo"}, + p: &ExecOptions{StreamOptions: StreamOptions{PodName: "foo"}}, argsLenAtDash: -1, expectError: true, name: "no cmd", }, { - p: &ExecOptions{PodName: "foo", ContainerName: "bar"}, + p: &ExecOptions{StreamOptions: StreamOptions{PodName: "foo", ContainerName: "bar"}}, argsLenAtDash: -1, expectError: true, name: "no cmd, w/ container", }, { - p: &ExecOptions{PodName: "foo"}, + p: &ExecOptions{StreamOptions: StreamOptions{PodName: "foo"}}, args: []string{"cmd"}, argsLenAtDash: -1, expectedPod: "foo", @@ -115,7 +117,7 @@ func TestPodAndContainer(t *testing.T) { name: "cmd, cmd is behind dash", }, { - p: &ExecOptions{ContainerName: "bar"}, + p: &ExecOptions{StreamOptions: StreamOptions{ContainerName: "bar"}}, args: []string{"foo", "cmd"}, argsLenAtDash: -1, expectedPod: "foo", @@ -206,12 +208,14 @@ func TestExec(t *testing.T) { ex.execErr = fmt.Errorf("exec error") } params := &ExecOptions{ - PodName: "foo", - ContainerName: "bar", - In: bufIn, - Out: bufOut, - Err: bufErr, - Executor: ex, + StreamOptions: StreamOptions{ + PodName: "foo", + ContainerName: "bar", + In: bufIn, + Out: bufOut, + Err: bufErr, + }, + Executor: ex, } cmd := &cobra.Command{} args := []string{"test", "command"} @@ -257,3 +261,124 @@ func execPod() *api.Pod { }, } } + +func TestSetupTTY(t *testing.T) { + stderr := &bytes.Buffer{} + + // test 1 - don't attach stdin + o := &StreamOptions{ + // InterruptParent: , + Stdin: false, + In: &bytes.Buffer{}, + Out: &bytes.Buffer{}, + Err: stderr, + TTY: true, + } + + tty := o.setupTTY() + + if o.In != nil { + t.Errorf("don't attach stdin: o.In should be nil") + } + if tty.In != nil { + t.Errorf("don't attach stdin: tty.In should be nil") + } + if o.TTY { + t.Errorf("don't attach stdin: o.TTY should be false") + } + if tty.Raw { + t.Errorf("don't attach stdin: tty.Raw should be false") + } + if len(stderr.String()) > 0 { + t.Errorf("don't attach stdin: stderr wasn't empty: %s", stderr.String()) + } + + // tests from here on attach stdin + // test 2 - don't request a TTY + o.Stdin = true + o.In = &bytes.Buffer{} + o.TTY = false + + tty = o.setupTTY() + + if o.In == nil { + t.Errorf("attach stdin, no TTY: o.In should not be nil") + } + if tty.In != o.In { + t.Errorf("attach stdin, no TTY: tty.In should equal o.In") + } + if o.TTY { + t.Errorf("attach stdin, no TTY: o.TTY should be false") + } + if tty.Raw { + t.Errorf("attach stdin, no TTY: tty.Raw should be false") + } + if len(stderr.String()) > 0 { + t.Errorf("attach stdin, no TTY: stderr wasn't empty: %s", stderr.String()) + } + + // test 3 - request a TTY, but stdin is not a terminal + o.Stdin = true + o.In = &bytes.Buffer{} + o.Err = stderr + o.TTY = true + + tty = o.setupTTY() + + if o.In == nil { + t.Errorf("attach stdin, TTY, not a terminal: o.In should not be nil") + } + if tty.In != o.In { + t.Errorf("attach stdin, TTY, not a terminal: tty.In should equal o.In") + } + if o.TTY { + t.Errorf("attach stdin, TTY, not a terminal: o.TTY should be false") + } + if tty.Raw { + t.Errorf("attach stdin, TTY, not a terminal: tty.Raw should be false") + } + if !strings.Contains(stderr.String(), "input is not a terminal") { + t.Errorf("attach stdin, TTY, not a terminal: expected 'input is not a terminal' to stderr") + } + + // test 4 - request a TTY, stdin is a terminal + o.Stdin = true + o.In = &bytes.Buffer{} + stderr.Reset() + o.TTY = true + + overrideStdin := ioutil.NopCloser(&bytes.Buffer{}) + overrideStdout := &bytes.Buffer{} + overrideStderr := &bytes.Buffer{} + o.overrideStreams = func() (io.ReadCloser, io.Writer, io.Writer) { + return overrideStdin, overrideStdout, overrideStderr + } + + o.isTerminalIn = func(tty term.TTY) bool { + return true + } + + tty = o.setupTTY() + + if o.In != overrideStdin { + t.Errorf("attach stdin, TTY, is a terminal: o.In should equal overrideStdin") + } + if tty.In != o.In { + t.Errorf("attach stdin, TTY, is a terminal: tty.In should equal o.In") + } + if !o.TTY { + t.Errorf("attach stdin, TTY, is a terminal: o.TTY should be true") + } + if !tty.Raw { + t.Errorf("attach stdin, TTY, is a terminal: tty.Raw should be true") + } + if len(stderr.String()) > 0 { + t.Errorf("attach stdin, TTY, is a terminal: stderr wasn't empty: %s", stderr.String()) + } + if o.Out != overrideStdout { + t.Errorf("attach stdin, TTY, is a terminal: o.Out should equal overrideStdout") + } + if tty.Out != o.Out { + t.Errorf("attach stdin, TTY, is a terminal: tty.Out should equal o.Out") + } +} diff --git a/pkg/kubectl/cmd/run.go b/pkg/kubectl/cmd/run.go index 15bbb496c7..f534fa8ada 100644 --- a/pkg/kubectl/cmd/run.go +++ b/pkg/kubectl/cmd/run.go @@ -226,11 +226,13 @@ func Run(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cob if attach { opts := &AttachOptions{ - In: cmdIn, - Out: cmdOut, - Err: cmdErr, - Stdin: interactive, - TTY: tty, + StreamOptions: StreamOptions{ + In: cmdIn, + Out: cmdOut, + Err: cmdErr, + Stdin: interactive, + TTY: tty, + }, CommandName: cmd.Parent().CommandPath() + " attach", diff --git a/pkg/util/term/resize.go b/pkg/util/term/resize.go index 155c840d6a..83b01e5412 100644 --- a/pkg/util/term/resize.go +++ b/pkg/util/term/resize.go @@ -32,10 +32,11 @@ type Size struct { // GetSize returns the current size of the user's terminal. If it isn't a terminal, // nil is returned. func (t TTY) GetSize() *Size { - if !t.IsTerminalOut() { + outFd, isTerminal := term.GetFdInfo(t.Out) + if !isTerminal { return nil } - return GetSize(t.Out.(fd).Fd()) + return GetSize(outFd) } // GetSize returns the current size of the terminal associated with fd. @@ -57,7 +58,8 @@ func SetSize(fd uintptr, size Size) error { // MonitorSize monitors the terminal's size. It returns a TerminalSizeQueue primed with // initialSizes, or nil if there's no TTY present. func (t *TTY) MonitorSize(initialSizes ...*Size) TerminalSizeQueue { - if !t.IsTerminalOut() { + outFd, isTerminal := term.GetFdInfo(t.Out) + if !isTerminal { return nil } @@ -69,7 +71,7 @@ func (t *TTY) MonitorSize(initialSizes ...*Size) TerminalSizeQueue { stopResizing: make(chan struct{}), } - t.sizeQueue.monitorSize(initialSizes...) + t.sizeQueue.monitorSize(outFd, initialSizes...) return t.sizeQueue } @@ -94,7 +96,7 @@ var _ TerminalSizeQueue = &sizeQueue{} // monitorSize primes resizeChan with initialSizes and then monitors for resize events. With each // new event, it sends the current terminal size to resizeChan. -func (s *sizeQueue) monitorSize(initialSizes ...*Size) { +func (s *sizeQueue) monitorSize(outFd uintptr, initialSizes ...*Size) { // send the initial sizes for i := range initialSizes { if initialSizes[i] != nil { @@ -104,7 +106,7 @@ func (s *sizeQueue) monitorSize(initialSizes ...*Size) { resizeEvents := make(chan Size, 1) - monitorResizeEvents(s.t.Out.(fd).Fd(), resizeEvents, s.stopResizing) + monitorResizeEvents(outFd, resizeEvents, s.stopResizing) // listen for resize events in the background go func() { diff --git a/pkg/util/term/resizeevents_windows.go b/pkg/util/term/resizeevents_windows.go index aa6de728b6..e994d28da3 100644 --- a/pkg/util/term/resizeevents_windows.go +++ b/pkg/util/term/resizeevents_windows.go @@ -29,7 +29,11 @@ func monitorResizeEvents(fd uintptr, resizeEvents chan<- Size, stop chan struct{ go func() { defer runtime.HandleCrash() - var lastSize Size + size := GetSize(fd) + if size == nil { + return + } + lastSize := *size for { // see if we need to stop running diff --git a/pkg/util/term/term.go b/pkg/util/term/term.go index dc780878c1..58baee8310 100644 --- a/pkg/util/term/term.go +++ b/pkg/util/term/term.go @@ -52,11 +52,6 @@ type TTY struct { sizeQueue *sizeQueue } -// fd returns a file descriptor for a given object. -type fd interface { - Fd() uintptr -} - // IsTerminalIn returns true if t.In is a terminal. Does not check /dev/tty // even if TryDev is set. func (t TTY) IsTerminalIn() bool { @@ -71,8 +66,8 @@ func (t TTY) IsTerminalOut() bool { // IsTerminal returns whether the passed object is a terminal or not func IsTerminal(i interface{}) bool { - file, ok := i.(fd) - return ok && term.IsTerminal(file.Fd()) + _, terminal := term.GetFdInfo(i) + return terminal } // Safe invokes the provided function and will attempt to ensure that when the @@ -82,22 +77,16 @@ func IsTerminal(i interface{}) bool { // If the input file descriptor is not a TTY and TryDev is true, the /dev/tty file // will be opened (if available). func (t TTY) Safe(fn SafeFunc) error { - in := t.In + inFd, isTerminal := term.GetFdInfo(t.In) - var hasFd bool - var inFd uintptr - if desc, ok := in.(fd); ok && in != nil { - inFd = desc.Fd() - hasFd = true - } - if t.TryDev && (!hasFd || !term.IsTerminal(inFd)) { + if !isTerminal && t.TryDev { if f, err := os.Open("/dev/tty"); err == nil { defer f.Close() inFd = f.Fd() - hasFd = true + isTerminal = term.IsTerminal(inFd) } } - if !hasFd || !term.IsTerminal(inFd) { + if !isTerminal { return fn() }