Skip to content

Commit

Permalink
run/refactor: WIP extract attach logic from run
Browse files Browse the repository at this point in the history 
Signed-off-by: Laura Brehm <[email protected]>
  • Loading branch information
@laurazard
laurazard committed Dec 16, 2024
1 parent 1a19197 commit 24c8ee2
Showing 1 changed file with 91 additions and 79 deletions.
170 changes: 91 additions & 79 deletions cli/command/container/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"os"
"strings"
"syscall"

Expand Down Expand Up @@ -116,12 +117,8 @@ func runRun(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, ro
return runContainer(ctx, dockerCli, ropts, copts, containerCfg)
}

//nolint:gocyclo
func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOptions, copts *containerOptions, containerCfg *containerConfig) error {
config := containerCfg.Config
stdout, stderr := dockerCli.Out(), dockerCli.Err()
apiClient := dockerCli.Client()

config.ArgsEscaped = false

if !runOpts.detach {
Expand All @@ -144,123 +141,140 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
return toStatusError(err)
}

apiClient := dockerCli.Client()

// New context here because we don't to cancel waiting on container exit/remove
// when we cancel attach, etc.
statusCtx, cancelStatusCtx := context.WithCancel(context.WithoutCancel(ctx))
defer cancelStatusCtx()
statusChan := waitExitOrRemoved(statusCtx, apiClient, containerID, copts.autoRemove)

var (
waitDisplayID chan struct{}
errCh chan error
)
var waitDisplayID chan struct{}
attach := config.AttachStdin || config.AttachStdout || config.AttachStderr
if !attach {
// Make this asynchronous to allow the client to write to stdin before having to read the ID
waitDisplayID = make(chan struct{})
go func() {
defer close(waitDisplayID)
_, _ = fmt.Fprintln(stdout, containerID)
_, _ = fmt.Fprintln(dockerCli.Out(), containerID)
}()
}

attachCtx, attachCancel := context.WithCancel(context.WithoutCancel(ctx))
defer attachCancel()
var attachWait func(<-chan int, error) error
if attach {
detachKeys := dockerCli.ConfigFile().DetachKeys
if runOpts.detachKeys != "" {
detachKeys = runOpts.detachKeys
}

// ctx should not be cancellable here, as this would kill the stream to the container
// and we want to keep the stream open until the process in the container exits or until
// the user forcefully terminates the CLI.
closeFn, err := attachContainer(attachCtx, dockerCli, containerID, &errCh, config, container.AttachOptions{
Stream: true,
Stdin: config.AttachStdin,
Stdout: config.AttachStdout,
Stderr: config.AttachStderr,
DetachKeys: detachKeys,
})
attachWait, err = setupContainerAttach(ctx, dockerCli, containerID, runOpts, config)
if err != nil {
return err
}
defer closeFn()
}

if runOpts.sigProxy {
sigc := notifyAllSignals()
// since we're explicitly setting up signal handling here, and the daemon will
// get notified independently of the clients ctx cancellation, we use this context
// but without cancellation to avoid ForwardAllSignals from returning
// before all signals are forwarded.
bgCtx := context.WithoutCancel(ctx)
go ForwardAllSignals(bgCtx, apiClient, containerID, sigc)
defer signal.StopCatch(sigc)
}

// start the container
if err := apiClient.ContainerStart(ctx, containerID, container.StartOptions{}); err != nil {
// If we have hijackedIOStreamer, we should notify
// hijackedIOStreamer we are going to exit and wait
// to avoid the terminal are not restored.
err = apiClient.ContainerStart(ctx, containerID, container.StartOptions{})
if err != nil {
if attach {
attachCancel()
<-errCh
attachWait(statusChan, err)
}

if copts.autoRemove {
// wait container to be removed
<-statusChan
}
return toStatusError(err)
}

if attach {
return attachWait(statusChan, nil)
}

// Detached mode: wait for the id to be displayed and return.
if !attach {
// Detached mode
<-waitDisplayID
return nil
<-waitDisplayID
return nil
}

func setupContainerAttach(ctx context.Context, dockerCli command.Cli, containerID string, runOpts *runOptions, config *container.Config) (func(<-chan int, error) error, error) {
detachKeys := dockerCli.ConfigFile().DetachKeys
if runOpts.detachKeys != "" {
detachKeys = runOpts.detachKeys
}

if config.Tty && dockerCli.Out().IsTerminal() {
if err := MonitorTtySize(attachCtx, dockerCli, containerID, false); err != nil {
_, _ = fmt.Fprintln(stderr, "Error monitoring TTY size:", err)
}
// ctx should not be cancellable here, as this would kill the stream to the container
// and we want to keep the stream open until the process in the container exits or until
// the user forcefully terminates the CLI.
attachCtx, attachCancel := context.WithCancel(context.WithoutCancel(ctx))
errCh, closeFn, err := attachContainer(attachCtx, dockerCli, containerID, config, container.AttachOptions{
Stream: true,
Stdin: config.AttachStdin,
Stdout: config.AttachStdout,
Stderr: config.AttachStderr,
DetachKeys: detachKeys,
})
if err != nil {
attachCancel()
return nil, err
}

select {
case err := <-errCh:
if err != nil {
if _, ok := err.(term.EscapeError); ok {
// The user entered the detach escape sequence.
return nil
}
var sigc chan os.Signal
if runOpts.sigProxy {
sigc = notifyAllSignals()
// since we're explicitly setting up signal handling here, and the daemon will
// get notified independently of the clients ctx cancellation, we use this context
// but without cancellation to avoid ForwardAllSignals from returning
// before all signals are forwarded.
bgCtx := context.WithoutCancel(ctx)
go ForwardAllSignals(bgCtx, dockerCli.Client(), containerID, sigc)
}

logrus.Debugf("Error hijack: %s", err)
return err
return func(statusC <-chan int, err error) error {
defer closeFn()
if runOpts.sigProxy {
defer signal.StopCatch(sigc)
}
status := <-statusChan
if status != 0 {
return cli.StatusError{StatusCode: status}

// if the container failed to start, just cancel the streamer
// and wait for the terminal to be restored
if err != nil {
attachCancel()
<-errCh
return nil
}
case status := <-statusChan:
// notify hijackedIOStreamer that we're exiting and wait
// so that the terminal can be restored.
attachCancel()
<-errCh
if status != 0 {
return cli.StatusError{StatusCode: status}

if config.Tty && dockerCli.Out().IsTerminal() {
if err := MonitorTtySize(attachCtx, dockerCli, containerID, false); err != nil {
_, _ = fmt.Fprintln(dockerCli.Err(), "Error monitoring TTY size:", err)
}
}
}

return nil
select {
case err := <-errCh:
if err != nil {
if _, ok := err.(term.EscapeError); ok {
// The user entered the detach escape sequence.
return nil
}

logrus.Debugf("Error hijack: %s", err)
return err
}
status := <-statusC
if status != 0 {
return cli.StatusError{StatusCode: status}
}
case status := <-statusC:
// notify hijackedIOStreamer that we're exiting and wait
// so that the terminal can be restored.
attachCancel()
<-errCh
if status != 0 {
return cli.StatusError{StatusCode: status}
}
}
return nil
}, nil
}

func attachContainer(ctx context.Context, dockerCli command.Cli, containerID string, errCh *chan error, config *container.Config, options container.AttachOptions) (func(), error) {
func attachContainer(ctx context.Context, dockerCli command.Cli, containerID string, config *container.Config, options container.AttachOptions) (chan error, func(), error) {
resp, errAttach := dockerCli.Client().ContainerAttach(ctx, containerID, options)
if errAttach != nil {
return nil, errAttach
return nil, nil, errAttach
}

var (
Expand All @@ -282,8 +296,6 @@ func attachContainer(ctx context.Context, dockerCli command.Cli, containerID str
}

ch := make(chan error, 1)
*errCh = ch

go func() {
ch <- func() error {
streamer := hijackedIOStreamer{
Expand All @@ -302,7 +314,7 @@ func attachContainer(ctx context.Context, dockerCli command.Cli, containerID str
return errAttach
}()
}()
return resp.Close, nil
return ch, resp.Close, nil
}

// withHelp decorates the error with a suggestion to use "--help".
Expand Down

0 comments on commit 24c8ee2

Please sign in to comment.