Merge pull request #47991 from ncdc/fix-initial-exec-terminal-dimensions

Automatic merge from submit-queue

Fix initial exec terminal dimensions

**What this PR does / why we need it**:
Delay attempting to send a terminal resize request to docker until after
the exec has started; otherwise, the initial resize request will fail.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #47990

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

pkg/kubelet/dockershim/exec.go

@@ -135,6 +135,9 @@ func (*NsenterExecHandler) ExecInContainer(client libdocker.Interface, container
 type NativeExecHandler struct{}
 
 func (*NativeExecHandler) ExecInContainer(client libdocker.Interface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize, timeout time.Duration) error {
+	done := make(chan struct{})
+	defer close(done)
+
 	createOpts := dockertypes.ExecConfig{
 		Cmd:          cmd,
 		AttachStdin:  stdin != nil,
@@ -149,9 +152,23 @@ func (*NativeExecHandler) ExecInContainer(client libdocker.Interface, container
 
 	// Have to start this before the call to client.StartExec because client.StartExec is a blocking
 	// call :-( Otherwise, resize events don't get processed and the terminal never resizes.
+	//
+	// We also have to delay attempting to send a terminal resize request to docker until after the
+	// exec has started; otherwise, the initial resize request will fail.
+	execStarted := make(chan struct{})
+	go func() {
+		select {
+		case <-execStarted:
+			// client.StartExec has started the exec, so we can start resizing
+		case <-done:
+			// ExecInContainer has returned, so short-circuit
+			return
+		}
+
 		kubecontainer.HandleResizing(resize, func(size remotecommand.TerminalSize) {
 			client.ResizeExecTTY(execObj.ID, uint(size.Height), uint(size.Width))
 		})
+	}()
 
 	startOpts := dockertypes.ExecStartCheck{Detach: false, Tty: tty}
 	streamOpts := libdocker.StreamOptions{
@@ -159,6 +176,7 @@ func (*NativeExecHandler) ExecInContainer(client libdocker.Interface, container
 		OutputStream: stdout,
 		ErrorStream:  stderr,
 		RawTerminal:  tty,
+		ExecStarted:  execStarted,
 	}
 	err = client.StartExec(execObj.ID, startOpts, streamOpts)
 	if err != nil {

pkg/kubelet/dockershim/libdocker/kube_docker_client.go

@@ -464,6 +464,15 @@ func (d *kubeDockerClient) StartExec(startExec string, opts dockertypes.ExecStar
 		return err
 	}
 	defer resp.Close()
+
+	if sopts.ExecStarted != nil {
+		// Send a message to the channel indicating that the exec has started. This is needed so
+		// interactive execs can handle resizing correctly - the request to resize the TTY has to happen
+		// after the call to d.client.ContainerExecAttach, and because d.holdHijackedConnection below
+		// blocks, we use sopts.ExecStarted to signal the caller that it's ok to resize.
+		sopts.ExecStarted <- struct{}{}
+	}
+
 	return d.holdHijackedConnection(sopts.RawTerminal || opts.Tty, sopts.InputStream, sopts.OutputStream, sopts.ErrorStream, resp)
 }
 
@@ -594,6 +603,7 @@ type StreamOptions struct {
 	InputStream  io.Reader
 	OutputStream io.Writer
 	ErrorStream  io.Writer
+	ExecStarted  chan struct{}
 }
 
 // operationTimeout is the error returned when the docker operations are timeout.