Merge pull request #21624 from smarterclayton/debug

Auto commit by PR queue bot

pkg/kubectl/cmd/attach.go

@@ -20,19 +20,18 @@ import (
 	"fmt"
 	"io"
 	"net/url"
-	"os"
-	"os/signal"
-	"syscall"
 
-	"github.com/docker/docker/pkg/term"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
+
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/client/restclient"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/remotecommand"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
+	"k8s.io/kubernetes/pkg/util/interrupt"
+	"k8s.io/kubernetes/pkg/util/term"
 )
 
 const (
@@ -53,6 +52,8 @@ func NewCmdAttach(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer)
 		Out: cmdOut,
 		Err: cmdErr,
 
+		CommandName: "kubectl attach",
+
 		Attach: &DefaultRemoteAttach{},
 	}
 	cmd := &cobra.Command{
@@ -96,11 +97,17 @@ type AttachOptions struct {
 	ContainerName string
 	Stdin         bool
 	TTY           bool
+	CommandName   string
+
+	// InterruptParent, if set, is used to handle interrupts while attached
+	InterruptParent *interrupt.Handler
 
 	In  io.Reader
 	Out io.Writer
 	Err io.Writer
 
+	Pod *api.Pod
+
 	Attach RemoteAttach
 	Client *client.Client
 	Config *restclient.Config
@@ -154,80 +161,65 @@ func (p *AttachOptions) Validate() error {
 
 // Run executes a validated remote execution against a pod.
 func (p *AttachOptions) Run() error {
-	pod, err := p.Client.Pods(p.Namespace).Get(p.PodName)
-	if err != nil {
-		return err
+	if p.Pod == nil {
+		pod, err := p.Client.Pods(p.Namespace).Get(p.PodName)
+		if err != nil {
+			return err
+		}
+		if pod.Status.Phase != api.PodRunning {
+			return fmt.Errorf("pod %s is not running and cannot be attached to; current phase is %s", p.PodName, pod.Status.Phase)
+		}
+		p.Pod = pod
+		// TODO: convert this to a clean "wait" behavior
 	}
+	pod := p.Pod
 
-	if pod.Status.Phase != api.PodRunning {
-		return fmt.Errorf("pod %s is not running and cannot be attached to; current phase is %s", p.PodName, pod.Status.Phase)
-	}
+	// ensure we can recover the terminal while attached
+	t := term.TTY{Parent: p.InterruptParent}
 
-	var stdin io.Reader
+	// check for TTY
 	tty := p.TTY
-
 	containerToAttach := p.GetContainer(pod)
 	if tty && !containerToAttach.TTY {
 		tty = false
-		fmt.Fprintf(p.Err, "Unable to use a TTY - container %s doesn't allocate one\n", containerToAttach.Name)
+		fmt.Fprintf(p.Err, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name)
 	}
-
-	// TODO: refactor with terminal helpers from the edit utility once that is merged
 	if p.Stdin {
-		stdin = p.In
-		if tty {
-			if file, ok := stdin.(*os.File); ok {
-				inFd := file.Fd()
-				if term.IsTerminal(inFd) {
-					oldState, err := term.SetRawTerminal(inFd)
-					if err != nil {
-						glog.Fatal(err)
-					}
-					fmt.Fprintln(p.Out, "\nHit enter for command prompt")
-					// this handles a clean exit, where the command finished
-					defer term.RestoreTerminal(inFd, oldState)
-
-					// SIGINT is handled by term.SetRawTerminal (it runs a goroutine that listens
-					// for SIGINT and restores the terminal before exiting)
-
-					// this handles SIGTERM
-					sigChan := make(chan os.Signal, 1)
-					signal.Notify(sigChan, syscall.SIGTERM)
-					go func() {
-						<-sigChan
-						term.RestoreTerminal(inFd, oldState)
-						os.Exit(0)
-					}()
-				} else {
-					fmt.Fprintln(p.Err, "STDIN is not a terminal")
-				}
-			} else {
-				tty = false
-				fmt.Fprintln(p.Err, "Unable to use a TTY - input is not the right kind of file")
-			}
+		t.In = p.In
+		if tty && !t.IsTerminal() {
+			tty = false
+			fmt.Fprintln(p.Err, "Unable to use a TTY - input is not a terminal or the right kind of file")
 		}
 	}
+	t.Raw = tty
 
-	// TODO: consider abstracting into a client invocation or client helper
-	req := p.Client.RESTClient.Post().
-		Resource("pods").
-		Name(pod.Name).
-		Namespace(pod.Namespace).
-		SubResource("attach")
-	req.VersionedParams(&api.PodAttachOptions{
-		Container: containerToAttach.Name,
-		Stdin:     stdin != nil,
-		Stdout:    p.Out != nil,
-		Stderr:    p.Err != nil,
-		TTY:       tty,
-	}, api.ParameterCodec)
+	fn := func() error {
+		if tty {
+			fmt.Fprintln(p.Out, "\nHit enter for command prompt")
+		}
+		// TODO: consider abstracting into a client invocation or client helper
+		req := p.Client.RESTClient.Post().
+			Resource("pods").
+			Name(pod.Name).
+			Namespace(pod.Namespace).
+			SubResource("attach")
+		req.VersionedParams(&api.PodAttachOptions{
+			Container: containerToAttach.Name,
+			Stdin:     p.In != nil,
+			Stdout:    p.Out != nil,
+			Stderr:    p.Err != nil,
+			TTY:       tty,
+		}, api.ParameterCodec)
 
-	err = p.Attach.Attach("POST", req.URL(), p.Config, stdin, p.Out, p.Err, tty)
-	if err != nil {
+		return p.Attach.Attach("POST", req.URL(), p.Config, p.In, p.Out, p.Err, tty)
+	}
+
+	if err := t.Safe(fn); err != nil {
 		return err
 	}
+
 	if p.Stdin && tty && pod.Spec.RestartPolicy == api.RestartPolicyAlways {
-		fmt.Fprintf(p.Out, "Session ended, resume using 'kubectl attach %s -c %s -i -t' command when the pod is running\n", pod.Name, containerToAttach.Name)
+		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
 }

pkg/kubectl/cmd/attach_test.go

@@ -208,7 +208,7 @@ func TestAttachWarnings(t *testing.T) {
 			pod:         attachPod(),
 			stdin:       true,
 			tty:         true,
-			expectedErr: "Unable to use a TTY - container bar doesn't allocate one",
+			expectedErr: "Unable to use a TTY - container bar did not allocate one",
 		},
 	}
 	for _, test := range tests {

pkg/kubectl/cmd/logs.go

@@ -61,6 +61,7 @@ type LogsOptions struct {
 	ClientMapper resource.ClientMapper
 	Decoder      runtime.Decoder
 
+	Object        runtime.Object
 	LogsForObject func(object, options runtime.Object) (*restclient.Request, error)
 
 	Out io.Writer
@@ -150,14 +151,27 @@ func (o *LogsOptions) Complete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Com
 		logOptions.SinceSeconds = &sec
 	}
 	o.Options = logOptions
-
-	o.Mapper, o.Typer = f.Object()
-	o.Decoder = f.Decoder(true)
-	o.ClientMapper = resource.ClientMapperFunc(f.ClientForMapping)
 	o.LogsForObject = f.LogsForObject
-
+	o.ClientMapper = resource.ClientMapperFunc(f.ClientForMapping)
 	o.Out = out
 
+	mapper, typer := f.Object()
+	decoder := f.Decoder(true)
+	if o.Object == nil {
+		infos, err := resource.NewBuilder(mapper, typer, o.ClientMapper, decoder).
+			NamespaceParam(o.Namespace).DefaultNamespace().
+			ResourceNames("pods", o.ResourceArg).
+			SingleResourceType().
+			Do().Infos()
+		if err != nil {
+			return err
+		}
+		if len(infos) != 1 {
+			return errors.New("expected a resource")
+		}
+		o.Object = infos[0].Object
+	}
+
 	return nil
 }
 
@@ -178,20 +192,7 @@ func (o LogsOptions) Validate() error {
 
 // RunLogs retrieves a pod log
 func (o LogsOptions) RunLogs() (int64, error) {
-	infos, err := resource.NewBuilder(o.Mapper, o.Typer, o.ClientMapper, o.Decoder).
-		NamespaceParam(o.Namespace).DefaultNamespace().
-		ResourceNames("pods", o.ResourceArg).
-		SingleResourceType().
-		Do().Infos()
-	if err != nil {
-		return 0, err
-	}
-	if len(infos) != 1 {
-		return 0, errors.New("expected a resource")
-	}
-	info := infos[0]
-
-	req, err := o.LogsForObject(info.Object, o.Options)
+	req, err := o.LogsForObject(o.Object, o.Options)
 	if err != nil {
 		return 0, err
 	}

pkg/kubectl/cmd/logs_test.go

@@ -130,7 +130,6 @@ func TestValidateLogFlags(t *testing.T) {
 		cmd.Run = func(cmd *cobra.Command, args []string) {
 			o.Complete(f, os.Stdout, cmd, args)
 			out = o.Validate().Error()
-			o.RunLogs()
 		}
 		cmd.Run(cmd, []string{"foo"})
 

pkg/kubectl/cmd/util/editor/editor.go

@@ -23,13 +23,13 @@ import (
 	"math/rand"
 	"os"
 	"os/exec"
-	"os/signal"
 	"path/filepath"
 	"runtime"
 	"strings"
 
-	"github.com/docker/docker/pkg/term"
 	"github.com/golang/glog"
+
+	"k8s.io/kubernetes/pkg/util/term"
 )
 
 const (
@@ -125,7 +125,7 @@ func (e Editor) Launch(path string) error {
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin
 	glog.V(5).Infof("Opening file with editor %v", args)
-	if err := withSafeTTYAndInterrupts(cmd.Run); err != nil {
+	if err := (term.TTY{In: os.Stdin, TryDev: true}).Safe(cmd.Run); err != nil {
 		if err, ok := err.(*exec.Error); ok {
 			if err.Err == exec.ErrNotFound {
 				return fmt.Errorf("unable to launch the editor %q", strings.Join(e.Args, " "))
@@ -160,40 +160,6 @@ func (e Editor) LaunchTempFile(prefix, suffix string, r io.Reader) ([]byte, stri
 	return bytes, path, err
 }
 
-// withSafeTTYAndInterrupts invokes the provided function after the terminal
-// state has been stored, and then on any error or termination attempts to
-// restore the terminal state to its prior behavior. It also eats signals
-// for the duration of the function.
-func withSafeTTYAndInterrupts(fn func() error) error {
-	ch := make(chan os.Signal, 1)
-	signal.Notify(ch, childSignals...)
-	defer signal.Stop(ch)
-
-	inFd := os.Stdin.Fd()
-	if !term.IsTerminal(inFd) {
-		if f, err := os.Open("/dev/tty"); err == nil {
-			defer f.Close()
-			inFd = f.Fd()
-		}
-	}
-
-	if term.IsTerminal(inFd) {
-		state, err := term.SaveState(inFd)
-		if err != nil {
-			return err
-		}
-		go func() {
-			if _, ok := <-ch; !ok {
-				return
-			}
-			term.RestoreTerminal(inFd, state)
-		}()
-		defer term.RestoreTerminal(inFd, state)
-		return fn()
-	}
-	return fn()
-}
-
 func tempFile(prefix, suffix string) (f *os.File, err error) {
 	dir := os.TempDir()
 

pkg/util/interrupt/child.go

@@ -1,7 +1,7 @@
-// +build windows
+// +build !linux
 
 /*
-Copyright 2015 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors All rights reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package editor
+package interrupt
 
 import (
 	"os"

pkg/util/interrupt/child_linux.go

@@ -1,7 +1,7 @@
-// +build !windows
+// +build linux
 
 /*
-Copyright 2015 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors All rights reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package editor
+package interrupt
 
 import (
 	"os"

pkg/util/interrupt/interrupt.go

@@ -0,0 +1,78 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package interrupt
+
+import (
+	"os"
+	"os/signal"
+	"sync"
+)
+
+type Handler struct {
+	notify []func()
+	final  func(os.Signal)
+	once   sync.Once
+}
+
+func Chain(handler *Handler, notify ...func()) *Handler {
+	if handler == nil {
+		return New(nil, notify...)
+	}
+	return New(handler.Signal, append(notify, handler.Close)...)
+}
+
+func New(final func(os.Signal), notify ...func()) *Handler {
+	return &Handler{
+		final:  final,
+		notify: notify,
+	}
+}
+
+func (h *Handler) Close() {
+	h.once.Do(func() {
+		for _, fn := range h.notify {
+			fn()
+		}
+	})
+}
+
+func (h *Handler) Signal(s os.Signal) {
+	h.once.Do(func() {
+		for _, fn := range h.notify {
+			fn()
+		}
+		if h.final == nil {
+			os.Exit(0)
+		}
+		h.final(s)
+	})
+}
+
+func (h *Handler) Run(fn func() error) error {
+	ch := make(chan os.Signal, 1)
+	signal.Notify(ch, childSignals...)
+	defer signal.Stop(ch)
+	go func() {
+		sig, ok := <-ch
+		if !ok {
+			return
+		}
+		h.Signal(sig)
+	}()
+	defer h.Close()
+	return fn()
+}

pkg/util/term/term.go

@@ -0,0 +1,100 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package term
+
+import (
+	"io"
+	"os"
+
+	"github.com/docker/docker/pkg/term"
+	"k8s.io/kubernetes/pkg/util/interrupt"
+)
+
+// SafeFunc is a function to be invoked by TTY.
+type SafeFunc func() error
+
+// TTY helps invoke a function and preserve the state of the terminal, even if the
+// process is terminated during execution.
+type TTY struct {
+	// In is a reader to check for a terminal.
+	In io.Reader
+	// Raw is true if the terminal should be set raw.
+	Raw bool
+	// TryDev indicates the TTY should try to open /dev/tty if the provided input
+	// is not a file descriptor.
+	TryDev bool
+	// Parent is an optional interrupt handler provided to this function - if provided
+	// it will be invoked after the terminal state is restored. If it is not provided,
+	// a signal received during the TTY will result in os.Exit(0) being invoked.
+	Parent *interrupt.Handler
+}
+
+// fd returns a file descriptor for a given object.
+type fd interface {
+	Fd() uintptr
+}
+
+// IsTerminal returns true if the provided input is a terminal. Does not check /dev/tty
+// even if TryDev is set.
+func (t TTY) IsTerminal() bool {
+	return IsTerminal(t.In)
+}
+
+// Safe invokes the provided function and will attempt to ensure that when the
+// function returns (or a termination signal is sent) that the terminal state
+// is reset to the condition it was in prior to the function being invoked. If
+// t.Raw is true the terminal will be put into raw mode prior to calling the function.
+// 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
+
+	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 f, err := os.Open("/dev/tty"); err == nil {
+			defer f.Close()
+			inFd = f.Fd()
+			hasFd = true
+		}
+	}
+	if !hasFd || !term.IsTerminal(inFd) {
+		return fn()
+	}
+
+	var state *term.State
+	var err error
+	if t.Raw {
+		state, err = term.MakeRaw(inFd)
+	} else {
+		state, err = term.SaveState(inFd)
+	}
+	if err != nil {
+		return err
+	}
+	return interrupt.Chain(t.Parent, func() { term.RestoreTerminal(inFd, state) }).Run(fn)
+}
+
+// IsTerminal returns whether the passed io.Reader is a terminal or not
+func IsTerminal(r io.Reader) bool {
+	file, ok := r.(fd)
+	return ok && term.IsTerminal(file.Fd())
+}